Skip to content

Commit 5473b42

Browse files
dadhidadhi
authored
Merge pull request #562 from jogibear9988/test/issue-561-non-bool-equality
Fix non-Boolean comparison operator results (#561)
2 parents c00fe0a + 3a0576b commit 5473b42

3 files changed

Lines changed: 329 additions & 0 deletions

File tree

‎src/FastExpressionCompiler/FastExpressionCompiler.cs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2011,6 +2011,9 @@ public static bool TryEmit(Expression expr, ILGenerator il, ref CompilerContext
20112011
case ExpressionType.Equal:
20122012
case ExpressionType.NotEqual:
20132013
{
2014+
var comparison = (BinaryExpression)expr;
2015+
if (comparison.Method != null && !comparison.IsLifted)
2016+
return TryEmitComparisonMethod(comparison, il, ref context, parent);
20142017
if (exprType.IsPrimitive && Interpreter.TryInterpretBool(out var boolResult, expr, context.CompilerFlags))
20152018
{
20162019
if ((parent & ParentFlags.IgnoreResult) == 0)
@@ -5521,6 +5524,47 @@ private static MethodInfo FindBinaryOperandMethod(
55215524
return null;
55225525
}
55235526

5527+
private static bool TryEmitComparisonMethod(BinaryExpression expr, ILGenerator il, ref CompilerContext context, ParentFlags parent)
5528+
{
5529+
var method = expr.Method;
5530+
var parameters = method.GetParameters();
5531+
if (!TryEmit(expr.Left, il, ref context, ParentFlags.Call, parameters[0].ParameterType.IsByRef ? 0 : -1))
5532+
return false;
5533+
5534+
// Keep the evaluation stack empty while a nontrivial right operand
5535+
// may emit exception regions or jumps, just as for call arguments.
5536+
var right = expr.Right;
5537+
var spillLeft = !right.IsParamOrConstantOrDefault();
5538+
#if LIGHT_EXPRESSION
5539+
// A conversion chain may contain a custom IL emitter.
5540+
for (var operand = right; !spillLeft;)
5541+
{
5542+
if (operand.IsIntrinsic)
5543+
{
5544+
spillLeft = true;
5545+
break;
5546+
}
5547+
if (operand is not UnaryExpression convert || convert.NodeType != ExpressionType.Convert)
5548+
break;
5549+
operand = convert.Operand;
5550+
}
5551+
#endif
5552+
var leftVar = spillLeft ? EmitStoreLocalVariable(il, parameters[0].ParameterType) : -1;
5553+
5554+
if (!TryEmit(expr.Right, il, ref context, ParentFlags.Call, parameters[1].ParameterType.IsByRef ? 1 : -1))
5555+
return false;
5556+
if (leftVar != -1)
5557+
{
5558+
var rightVar = EmitStoreLocalVariable(il, parameters[1].ParameterType);
5559+
EmitLoadLocalVariable(il, leftVar);
5560+
EmitLoadLocalVariable(il, rightVar);
5561+
}
5562+
if (!EmitMethodCall(il, method))
5563+
return false;
5564+
context.LastEmitIsAddress = false;
5565+
return il.EmitPopIfIgnoreResult(parent);
5566+
}
5567+
55245568
private static bool TryEmitComparison(
55255569
Expression left, Expression right, Type exprType, ExpressionType nodeType,
55265570
ILGenerator il, ref CompilerContext context, ParentFlags parent)
@@ -7310,6 +7354,8 @@ public static bool TryFindSwitchBranch(SwitchExpression switchExpr, CompilerFlag
73107354
/// <summary>Tries to interpret the expression of the Primitive type of Constant, Convert, Logical, Comparison, Arithmetic.</summary>
73117355
internal static bool TryInterpretBool(ref bool resultBool, Expression expr, ExpressionType nodeType)
73127356
{
7357+
if (expr is BinaryExpression binary && binary.Method != null)
7358+
return false;
73137359
// what operations are supported, to get the boolean result?
73147360
// yes: not, logical, comparison, default
73157361
// not: negate, arithmetic, convert to bool

‎test/FastExpressionCompiler.IssueTests/Issue561_NonBooleanEqualityOperator.cs‎

Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
// The safety guard uses the dynamic-method decoder supported on modern .NET.
2+
// The fatal repro itself is also exercised in a separate child-process harness.
3+
#if NET8_0_OR_GREATER
4+
using System;
5+
using System.Linq;
6+
using System.Linq.Expressions;
7+
using System.Reflection;
8+
9+
#if LIGHT_EXPRESSION
10+
using FastExpressionCompiler.LightExpression.ILDecoder;
11+
using FastExpressionCompiler.LightExpression.ImTools;
12+
using static FastExpressionCompiler.LightExpression.ExpressionCompiler;
13+
namespace FastExpressionCompiler.LightExpression.IssueTests;
14+
#else
15+
using FastExpressionCompiler.ILDecoder;
16+
namespace FastExpressionCompiler.IssueTests;
17+
#endif
18+
19+
public struct Issue561_NonBooleanEqualityOperator : ITestX
20+
{
21+
public void Run(TestRun t)
22+
{
23+
Equal_returns_the_interface_from_the_selected_operator(t);
24+
NotEqual_returns_the_interface_from_the_selected_operator(t);
25+
Reversed_equal_returns_the_interface_from_the_selected_operator(t);
26+
Reversed_not_equal_returns_the_interface_from_the_selected_operator(t);
27+
Ignored_result_still_calls_the_operator(t);
28+
Operands_execute_once_in_order_with_nested_try(t);
29+
Non_lifted_nullable_parameters_reach_the_selected_method(t);
30+
Selected_methods_keep_non_boolean_value_results(t);
31+
Boolean_method_is_not_replaced_by_constant_comparison(t);
32+
Converted_simple_right_operands_reach_the_selected_method(t);
33+
#if LIGHT_EXPRESSION
34+
Converted_intrinsic_right_operand_can_emit_exception_regions(t);
35+
#endif
36+
}
37+
38+
public interface ICustomObject { void Ping(); }
39+
40+
private static string Trace;
41+
42+
public sealed class CustomObject : ICustomObject
43+
{
44+
public static int OperatorCalls;
45+
public static int PingCalls;
46+
47+
public static ICustomObject operator ==(double left, CustomObject right) => Result();
48+
public static ICustomObject operator !=(double left, CustomObject right) => Result();
49+
public static ICustomObject operator ==(CustomObject left, double right) => Result();
50+
public static ICustomObject operator !=(CustomObject left, double right) => Result();
51+
private static ICustomObject Result() { ++OperatorCalls; Trace += "O"; return new CustomObject(); }
52+
public void Ping() { ++PingCalls; }
53+
public override bool Equals(object obj) => ReferenceEquals(this, obj);
54+
public override int GetHashCode() => base.GetHashCode();
55+
}
56+
57+
public void Equal_returns_the_interface_from_the_selected_operator(TestContext t) =>
58+
Check(t, () => 1 == new CustomObject());
59+
60+
public void NotEqual_returns_the_interface_from_the_selected_operator(TestContext t) =>
61+
Check(t, () => 1 != new CustomObject());
62+
63+
public void Reversed_equal_returns_the_interface_from_the_selected_operator(TestContext t) =>
64+
Check(t, () => new CustomObject() == 1);
65+
66+
public void Reversed_not_equal_returns_the_interface_from_the_selected_operator(TestContext t) =>
67+
Check(t, () => new CustomObject() != 1);
68+
69+
private static void Check(TestContext t, System.Linq.Expressions.Expression<Func<ICustomObject>> expression)
70+
{
71+
var binary = (System.Linq.Expressions.BinaryExpression)expression.Body;
72+
t.AreEqual(typeof(ICustomObject), binary.Type);
73+
t.IsNotNull(binary.Method);
74+
t.IsTrue(!binary.IsLifted);
75+
76+
CustomObject.OperatorCalls = CustomObject.PingCalls = 0;
77+
var normal = expression.Compile()();
78+
t.AreEqual(1, CustomObject.OperatorCalls);
79+
normal.Ping();
80+
t.AreEqual(1, CustomObject.PingCalls);
81+
82+
foreach (var flags in new[] { CompilerFlags.Default, CompilerFlags.NoInvocationLambdaInlining,
83+
CompilerFlags.EnableDelegateDebugInfo })
84+
{
85+
var fast = Compile(expression, flags);
86+
if (!t.IsNotNull(fast)) return;
87+
if (!CheckSelectedCall(t, fast, binary.Method)) return;
88+
89+
CustomObject.OperatorCalls = CustomObject.PingCalls = 0;
90+
var result = fast();
91+
t.AreEqual(1, CustomObject.OperatorCalls);
92+
if (!t.IsNotNull(result)) return;
93+
t.IsTrue(result is CustomObject);
94+
result.Ping();
95+
t.AreEqual(1, CustomObject.PingCalls);
96+
}
97+
}
98+
99+
private static TDelegate Compile<TDelegate>(System.Linq.Expressions.Expression<TDelegate> expression,
100+
CompilerFlags flags = CompilerFlags.Default) where TDelegate : Delegate =>
101+
#if LIGHT_EXPRESSION
102+
expression.ToLightExpression().CompileFast(true, flags);
103+
#else
104+
expression.CompileFast(true, flags);
105+
#endif
106+
107+
private static bool CheckSelectedCall(TestContext t, Delegate fast, MethodInfo method)
108+
{
109+
// On affected compilers the returned reference is corrupt. Check the
110+
// selected call before invocation so this regression fails without
111+
// taking down the whole runner. This does not assert an IL layout.
112+
var instructions = fast.TryGetDebugInfo()?.ILInstructions ?? fast.Method.ReadAllInstructions();
113+
if (instructions.OfType<InlineMethodInstruction>().Any(i => i.Method.Equals(method))) return true;
114+
t.Fail("Issue561: selected operator call is missing; corrupt-reference delegate was not invoked.");
115+
return false;
116+
}
117+
118+
public void Ignored_result_still_calls_the_operator(TestContext t)
119+
{
120+
System.Linq.Expressions.Expression<Func<ICustomObject>> original = () => 1 == new CustomObject();
121+
var method = ((System.Linq.Expressions.BinaryExpression)original.Body).Method;
122+
var expression = System.Linq.Expressions.Expression.Lambda<Action>(
123+
System.Linq.Expressions.Expression.Block(original.Body, System.Linq.Expressions.Expression.Empty()));
124+
var fast = Compile(expression);
125+
if (!t.IsNotNull(fast) || !CheckSelectedCall(t, fast, method)) return;
126+
CustomObject.OperatorCalls = 0;
127+
fast();
128+
t.AreEqual(1, CustomObject.OperatorCalls);
129+
}
130+
131+
public static double ReadLeft() { Trace += "L"; return 1; }
132+
public static int ReadArgument() { Trace += "A"; return 2; }
133+
public static void FinishArgument() { Trace += "F"; }
134+
public static CustomObject ReadRight(int value) { Trace += "R"; return new CustomObject(); }
135+
136+
public void Operands_execute_once_in_order_with_nested_try(TestContext t)
137+
{
138+
var owner = typeof(Issue561_NonBooleanEqualityOperator);
139+
var left = System.Linq.Expressions.Expression.Call(owner.GetMethod(nameof(ReadLeft)));
140+
var right = System.Linq.Expressions.Expression.Call(owner.GetMethod(nameof(ReadRight)),
141+
System.Linq.Expressions.Expression.TryFinally(
142+
System.Linq.Expressions.Expression.Call(owner.GetMethod(nameof(ReadArgument))),
143+
System.Linq.Expressions.Expression.Call(owner.GetMethod(nameof(FinishArgument)))));
144+
var body = System.Linq.Expressions.Expression.Equal(left, right);
145+
var expression = System.Linq.Expressions.Expression.Lambda<Func<ICustomObject>>(body);
146+
var system = expression.Compile();
147+
Trace = ""; system().Ping();
148+
var expected = Trace;
149+
t.AreEqual("LAFRO", expected);
150+
var fast = Compile(expression);
151+
if (!t.IsNotNull(fast) || !CheckSelectedCall(t, fast, body.Method)) return;
152+
Trace = ""; CustomObject.OperatorCalls = CustomObject.PingCalls = 0;
153+
fast().Ping();
154+
t.AreEqual(expected, Trace);
155+
t.AreEqual(1, CustomObject.OperatorCalls);
156+
t.AreEqual(1, CustomObject.PingCalls);
157+
}
158+
159+
public static ICustomObject CompareNullable(double? left, CustomObject right)
160+
{
161+
Trace += left.HasValue ? "value" : "null";
162+
++CustomObject.OperatorCalls;
163+
return new CustomObject();
164+
}
165+
166+
public void Non_lifted_nullable_parameters_reach_the_selected_method(TestContext t)
167+
{
168+
var left = System.Linq.Expressions.Expression.Parameter(typeof(double?));
169+
var body = System.Linq.Expressions.Expression.Equal(left,
170+
System.Linq.Expressions.Expression.New(typeof(CustomObject)), false,
171+
typeof(Issue561_NonBooleanEqualityOperator).GetMethod(nameof(CompareNullable)));
172+
t.IsTrue(!body.IsLifted);
173+
var expression = System.Linq.Expressions.Expression.Lambda<Func<double?, ICustomObject>>(body, left);
174+
var system = expression.Compile();
175+
var fast = Compile(expression);
176+
if (!t.IsNotNull(fast) || !CheckSelectedCall(t, fast, body.Method)) return;
177+
foreach (var value in new double?[] { null, 2 })
178+
{
179+
Trace = ""; system(value).Ping(); var expected = Trace;
180+
Trace = ""; CustomObject.OperatorCalls = CustomObject.PingCalls = 0;
181+
fast(value).Ping();
182+
t.AreEqual(expected, Trace);
183+
t.AreEqual(1, CustomObject.OperatorCalls);
184+
t.AreEqual(1, CustomObject.PingCalls);
185+
}
186+
}
187+
188+
public static int CompareValue(int left, int right) { ++CustomObject.OperatorCalls; return left * 10 + right; }
189+
public static bool CompareBoolean(int left, int right) { ++CustomObject.OperatorCalls; return left != right; }
190+
191+
public void Selected_methods_keep_non_boolean_value_results(TestContext t)
192+
{
193+
var left = System.Linq.Expressions.Expression.Parameter(typeof(int));
194+
var method = typeof(Issue561_NonBooleanEqualityOperator).GetMethod(nameof(CompareValue));
195+
foreach (var kind in new[] { ExpressionType.Equal, ExpressionType.NotEqual, ExpressionType.LessThan,
196+
ExpressionType.LessThanOrEqual, ExpressionType.GreaterThan, ExpressionType.GreaterThanOrEqual })
197+
{
198+
var body = System.Linq.Expressions.Expression.MakeBinary(kind, left,
199+
System.Linq.Expressions.Expression.Constant(2), false, method);
200+
var expression = System.Linq.Expressions.Expression.Lambda<Func<int, int>>(body, left);
201+
var fast = Compile(expression);
202+
if (!t.IsNotNull(fast) || !CheckSelectedCall(t, fast, method)) return;
203+
var expected = expression.Compile()(3);
204+
CustomObject.OperatorCalls = 0;
205+
t.AreEqual(expected, fast(3));
206+
t.AreEqual(1, CustomObject.OperatorCalls);
207+
}
208+
}
209+
210+
public void Boolean_method_is_not_replaced_by_constant_comparison(TestContext t)
211+
{
212+
var method = typeof(Issue561_NonBooleanEqualityOperator).GetMethod(nameof(CompareBoolean));
213+
var body = System.Linq.Expressions.Expression.Equal(System.Linq.Expressions.Expression.Constant(1),
214+
System.Linq.Expressions.Expression.Constant(2), false, method);
215+
var expression = System.Linq.Expressions.Expression.Lambda<Func<bool>>(body);
216+
var fast = Compile(expression);
217+
if (!t.IsNotNull(fast) || !CheckSelectedCall(t, fast, method)) return;
218+
var expected = expression.Compile()();
219+
CustomObject.OperatorCalls = 0;
220+
t.AreEqual(expected, fast());
221+
t.AreEqual(1, CustomObject.OperatorCalls);
222+
}
223+
public void Converted_simple_right_operands_reach_the_selected_method(TestContext t)
224+
{
225+
var parameter = System.Linq.Expressions.Expression.Parameter(typeof(int));
226+
var method = typeof(Issue561_NonBooleanEqualityOperator).GetMethod(nameof(CompareValue));
227+
foreach (var right in new System.Linq.Expressions.Expression[] { parameter,
228+
System.Linq.Expressions.Expression.Constant(2), System.Linq.Expressions.Expression.Default(typeof(int)) })
229+
{
230+
var converted = System.Linq.Expressions.Expression.Convert(
231+
System.Linq.Expressions.Expression.Convert(right, typeof(long)), typeof(int));
232+
var body = System.Linq.Expressions.Expression.Equal(
233+
System.Linq.Expressions.Expression.Constant(3), converted, false, method);
234+
var expression = System.Linq.Expressions.Expression.Lambda<Func<int, int>>(body, parameter);
235+
var fast = Compile(expression);
236+
if (!t.IsNotNull(fast) || !CheckSelectedCall(t, fast, method)) return;
237+
var expected = expression.Compile()(4);
238+
CustomObject.OperatorCalls = 0;
239+
t.AreEqual(expected, fast(4));
240+
t.AreEqual(1, CustomObject.OperatorCalls);
241+
}
242+
}
243+
244+
#if LIGHT_EXPRESSION
245+
private sealed class TryFinallyConversion : UnaryExpression
246+
{
247+
private readonly Expression _body;
248+
public TryFinallyConversion(Expression operand, Expression body) : base(operand) => _body = body;
249+
public override ExpressionType NodeType => ExpressionType.Convert;
250+
public override bool IsIntrinsic => true;
251+
public override Result TryCollectInfo(ref CompilerContext context, NestedLambdaInfo nestedLambda,
252+
ref SmallList<NestedLambdaInfo> rootNestedLambdas) =>
253+
ExpressionCompiler.TryCollectInfo(ref context, _body, nestedLambda, ref rootNestedLambdas);
254+
public override bool TryEmit(ref CompilerContext context, System.Reflection.Emit.ILGenerator il,
255+
ParentFlags parent, int byRefIndex = -1) =>
256+
EmittingVisitor.TryEmit(_body, il, ref context, parent, byRefIndex);
257+
}
258+
259+
public void Converted_intrinsic_right_operand_can_emit_exception_regions(TestContext t)
260+
{
261+
var owner = typeof(Issue561_NonBooleanEqualityOperator);
262+
var parameter = Expression.Parameter(typeof(int));
263+
var read = Expression.Call(owner.GetMethod(nameof(ReadArgument)));
264+
var intrinsic = new TryFinallyConversion(parameter,
265+
Expression.TryFinally(read, Expression.Call(owner.GetMethod(nameof(FinishArgument)))));
266+
// The intrinsic is hidden inside two ordinary conversion wrappers.
267+
var right = Expression.Convert(Expression.Convert(intrinsic, typeof(long)), typeof(int));
268+
var method = owner.GetMethod(nameof(CompareValue));
269+
var body = Expression.Equal(read, right, false, method);
270+
var fast = Expression.Lambda<Func<int, int>>(body, parameter).CompileFast(true);
271+
if (!t.IsNotNull(fast) || !CheckSelectedCall(t, fast, method)) return;
272+
Trace = "";
273+
CustomObject.OperatorCalls = 0;
274+
t.AreEqual(22, fast(4));
275+
t.AreEqual("AAF", Trace);
276+
t.AreEqual(1, CustomObject.OperatorCalls);
277+
}
278+
#endif
279+
280+
}
281+
#endif

‎test/FastExpressionCompiler.TestsRunner/Program.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public static void Main()
5353
var st = new TestRun(TestFlags.RethrowException);
5454

5555
st.Run(new Issue499_InvalidProgramException_for_Sorting_and_comparison_function());
56+
st.Run(new Issue561_NonBooleanEqualityOperator());
5657
st.Run(new Issue498_InvalidProgramException_when_using_loop());
5758
st.Run(new Issue495_Incomplete_pattern_detection_for_NotSupported_1007_Return_goto_from_TryCatch_with_Assign_generates_invalid_IL());
5859
st.Run(new Issue480_CLR_detected_an_invalid_program_exception());
@@ -75,6 +76,7 @@ public static void Main()
7576
lt.Run(new LightExpression.IssueTests.Issue473_InvalidProgramException_when_using_Expression_Condition_with_converted_decimal_expression());
7677
lt.Run(new LightExpression.IssueTests.Issue476_System_ExecutionEngineException_with_nullables_on_repeated_calls_to_ConcurrentDictionary());
7778
lt.Run(new LightExpression.IssueTests.Issue499_InvalidProgramException_for_Sorting_and_comparison_function());
79+
lt.Run(new LightExpression.IssueTests.Issue561_NonBooleanEqualityOperator());
7880

7981
RunAllTests();
8082
}

0 commit comments

Comments
 (0)