From 819f3153f1b7cfa8a5334e5e3199b336baac748d Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Mon, 16 May 2022 08:51:41 +0200 Subject: [PATCH] Optimize capture evaluation during expr comparison After the last commit, every expression comparison would perform two tree traversals: one for the potential capture evaluation rewrite, and another for the actual comparison. This commit changes `Expression- Comparer` back to a single-traversal algorithm. --- src/Moq/ExpressionComparer.cs | 34 ++++++++++++++++++++++----- src/Moq/Match.cs | 2 +- src/Moq/Matchers/ExpressionMatcher.cs | 2 +- src/Moq/MethodExpectation.cs | 4 ++-- src/Moq/StubbedPropertiesSetup.cs | 2 +- 5 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/Moq/ExpressionComparer.cs b/src/Moq/ExpressionComparer.cs index 40250c58c..304bb4125 100644 --- a/src/Moq/ExpressionComparer.cs +++ b/src/Moq/ExpressionComparer.cs @@ -14,13 +14,11 @@ internal sealed class ExpressionComparer : IEqualityComparer { public static readonly ExpressionComparer Default = new ExpressionComparer(); - private ExpressionComparer() - { - } + [ThreadStatic] + private static int quoteDepth = 0; - public bool EqualsAfterCapturesEvaluated(Expression x, Expression y) + private ExpressionComparer() { - return this.Equals(x?.Apply(EvaluateCaptures.Rewriter), y?.Apply(EvaluateCaptures.Rewriter)); } public bool Equals(Expression x, Expression y) @@ -35,17 +33,41 @@ public bool Equals(Expression x, Expression y) return false; } + // Before actually comparing two nodes, make sure that captured variables have been + // evaluated to their current values (as we don't want to compare their identities). + // But do so only for the main expression; leave quoted (nested) expressions unchanged. + + if (x is MemberExpression && ExpressionComparer.quoteDepth == 0) + { + x = x.Apply(EvaluateCaptures.Rewriter); + } + + if (y is MemberExpression && ExpressionComparer.quoteDepth == 0) + { + y = y.Apply(EvaluateCaptures.Rewriter); + } + if (x.NodeType == y.NodeType) { switch (x.NodeType) { + case ExpressionType.Quote: + ExpressionComparer.quoteDepth++; + try + { + return this.EqualsUnary((UnaryExpression)x, (UnaryExpression)y); + } + finally + { + ExpressionComparer.quoteDepth--; + } + case ExpressionType.Negate: case ExpressionType.NegateChecked: case ExpressionType.Not: case ExpressionType.Convert: case ExpressionType.ConvertChecked: case ExpressionType.ArrayLength: - case ExpressionType.Quote: case ExpressionType.TypeAs: case ExpressionType.UnaryPlus: return this.EqualsUnary((UnaryExpression)x, (UnaryExpression)y); diff --git a/src/Moq/Match.cs b/src/Moq/Match.cs index 5776e36db..479ffa248 100644 --- a/src/Moq/Match.cs +++ b/src/Moq/Match.cs @@ -217,7 +217,7 @@ public bool Equals(Match other) } else if (!(this.RenderExpression is MethodCallExpression ce && ce.Method.DeclaringType == typeof(Match))) { - return ExpressionComparer.Default.EqualsAfterCapturesEvaluated(this.RenderExpression, other.RenderExpression); + return ExpressionComparer.Default.Equals(this.RenderExpression, other.RenderExpression); } else { diff --git a/src/Moq/Matchers/ExpressionMatcher.cs b/src/Moq/Matchers/ExpressionMatcher.cs index ecfed098b..feefc2181 100644 --- a/src/Moq/Matchers/ExpressionMatcher.cs +++ b/src/Moq/Matchers/ExpressionMatcher.cs @@ -19,7 +19,7 @@ public ExpressionMatcher(Expression expression) public bool Matches(object argument, Type parameterType) { return argument is Expression valueExpression - && ExpressionComparer.Default.EqualsAfterCapturesEvaluated(this.expression, valueExpression); + && ExpressionComparer.Default.Equals(this.expression, valueExpression); } public void SetupEvaluatedSuccessfully(object argument, Type parameterType) diff --git a/src/Moq/MethodExpectation.cs b/src/Moq/MethodExpectation.cs index a89a73ae4..7473c2135 100644 --- a/src/Moq/MethodExpectation.cs +++ b/src/Moq/MethodExpectation.cs @@ -240,7 +240,7 @@ public override bool Equals(Expectation obj) { for (int j = 0, nj = e1.Expressions.Count; j < nj; ++j) { - if (!ExpressionComparer.Default.EqualsAfterCapturesEvaluated(e1.Expressions[j], e2.Expressions[j])) + if (!ExpressionComparer.Default.Equals(e1.Expressions[j], e2.Expressions[j])) { return false; } @@ -250,7 +250,7 @@ public override bool Equals(Expectation obj) } } - if (!ExpressionComparer.Default.EqualsAfterCapturesEvaluated(this.partiallyEvaluatedArguments[i], other.partiallyEvaluatedArguments[i])) + if (!ExpressionComparer.Default.Equals(this.partiallyEvaluatedArguments[i], other.partiallyEvaluatedArguments[i])) { return false; } diff --git a/src/Moq/StubbedPropertiesSetup.cs b/src/Moq/StubbedPropertiesSetup.cs index 949d20443..c44cdd3a7 100644 --- a/src/Moq/StubbedPropertiesSetup.cs +++ b/src/Moq/StubbedPropertiesSetup.cs @@ -90,7 +90,7 @@ public PropertyAccessorExpectation(Mock mock) public override bool Equals(Expectation other) { - return other is PropertyAccessorExpectation pae && ExpressionComparer.Default.EqualsAfterCapturesEvaluated(this.expression, pae.expression); + return other is PropertyAccessorExpectation pae && ExpressionComparer.Default.Equals(this.expression, pae.expression); } public override int GetHashCode()