Skip to content

Commit

Permalink
Optimize capture evaluation during expr comparison
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
stakx committed May 16, 2022
1 parent 3535d7a commit 819f315
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 11 deletions.
34 changes: 28 additions & 6 deletions src/Moq/ExpressionComparer.cs
Expand Up @@ -14,13 +14,11 @@ internal sealed class ExpressionComparer : IEqualityComparer<Expression>
{
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)
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/Moq/Match.cs
Expand Up @@ -217,7 +217,7 @@ public bool Equals(Match<T> 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
{
Expand Down
2 changes: 1 addition & 1 deletion src/Moq/Matchers/ExpressionMatcher.cs
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/Moq/MethodExpectation.cs
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Moq/StubbedPropertiesSetup.cs
Expand Up @@ -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()
Expand Down

0 comments on commit 819f315

Please sign in to comment.