diff --git a/src/FastExpressionCompiler/FastExpressionCompiler.cs b/src/FastExpressionCompiler/FastExpressionCompiler.cs index feb94970..b0e95741 100644 --- a/src/FastExpressionCompiler/FastExpressionCompiler.cs +++ b/src/FastExpressionCompiler/FastExpressionCompiler.cs @@ -5729,8 +5729,9 @@ public static bool TryEmitMemberGet(MemberExpression expr, ILGenerator il, ref C // we don't need to duplicate the instance if we are working with the field address to save to it directly, // so the field address should be Dupped instead for loading and then storing by-ref for assignment (see below). // (don't forget to Pop if the assignment should be skipped for nullable with `null` value) - if ((parent & ParentFlags.DupIt) != 0 & - (!isByAddress | (parent & ParentFlags.AssignmentLeftValue) == 0)) + var dupIt = (parent & ParentFlags.DupIt) != 0; + var dupAddress = dupIt && isByAddress && (parent & ParentFlags.AssignmentLeftValue) != 0; + if (dupIt && !dupAddress) il.Demit(OpCodes.Dup); context.LastEmitIsAddress = isByAddress; @@ -5743,7 +5744,7 @@ public static bool TryEmitMemberGet(MemberExpression expr, ILGenerator il, ref C else { il.Demit(OpCodes.Ldflda, field); - if ((parent & ParentFlags.AssignmentLeftValue) != 0) + if (dupAddress) il.Demit(OpCodes.Dup); } } diff --git a/test/FastExpressionCompiler.TestsRunner.Net472/Program.cs b/test/FastExpressionCompiler.TestsRunner.Net472/Program.cs index 34674fd9..7bdfc33b 100644 --- a/test/FastExpressionCompiler.TestsRunner.Net472/Program.cs +++ b/test/FastExpressionCompiler.TestsRunner.Net472/Program.cs @@ -34,6 +34,7 @@ static void RunLightExpressionTests(object state) var justLightTestsStopwatch = Stopwatch.StartNew(); var t = (LightExpression.TestRun)state; + t.Run(new LightExpression.UnitTests.NestedStructFieldAddressRegressionTests()); t.Run(new LightExpression.IssueTests.Issue183_NullableDecimal()); t.Run(new LightExpression.IssueTests.Issue398_Optimize_Switch_with_OpCodes_Switch()); t.Run(new LightExpression.IssueTests.Issue431_Add_structural_equality_comparison_to_LightExpression()); @@ -59,6 +60,7 @@ static void RunLightExpressionTests(object state) var fecTestsStopwatch = Stopwatch.StartNew(); var fecTests = new TestRun(); + fecTests.Run(new NestedStructFieldAddressRegressionTests()); fecTests.Run(new Issue183_NullableDecimal()); fecTests.Run(new Issue468_Optimize_the_delegate_access_to_the_Closure_object_for_the_modern_NET()); fecTests.Run(new Issue472_TryInterpret_and_Reduce_primitive_arithmetic_and_logical_expressions_during_the_compilation()); diff --git a/test/FastExpressionCompiler.TestsRunner/Program.cs b/test/FastExpressionCompiler.TestsRunner/Program.cs index 036aee7c..ca07a0ca 100644 --- a/test/FastExpressionCompiler.TestsRunner/Program.cs +++ b/test/FastExpressionCompiler.TestsRunner/Program.cs @@ -64,6 +64,7 @@ public static void Main(string[] args) { var ltStopwatch = Stopwatch.StartNew(); #if NET8_0_OR_GREATER + lt.Run(new LightExpression.UnitTests.NestedStructFieldAddressRegressionTests()); lt.Run(new LightExpression.IssueTests.Issue490_Regression_in_compiling_lambdas_with_ref_struct_parameters()); #endif @@ -85,6 +86,7 @@ public static void Main(string[] args) }); var stStopwatch = Stopwatch.StartNew(); + st.Run(new NestedStructFieldAddressRegressionTests()); st.Run(new Issue499_InvalidProgramException_for_Sorting_and_comparison_function()); st.Run(new Issue561_NonBooleanEqualityOperator()); st.Run(new ReductionProgressRegressionTests()); diff --git a/test/FastExpressionCompiler.UnitTests/NestedStructFieldAddressRegressionTests.cs b/test/FastExpressionCompiler.UnitTests/NestedStructFieldAddressRegressionTests.cs new file mode 100644 index 00000000..fd228f8b --- /dev/null +++ b/test/FastExpressionCompiler.UnitTests/NestedStructFieldAddressRegressionTests.cs @@ -0,0 +1,131 @@ +using System; +using System.Runtime.CompilerServices; +using S = System.Linq.Expressions.Expression; +#if LIGHT_EXPRESSION +namespace FastExpressionCompiler.LightExpression.UnitTests; +#else +namespace FastExpressionCompiler.UnitTests; +#endif + +public struct NestedStructFieldAddressRegressionTests : ITestX +{ + public struct Counter { public int Value; } + private static int OwnerReads; + private static int RightReads; + public static StrongBox Owner(StrongBox box) { ++OwnerReads; return box; } + public static int Right(int value) { ++RightReads; return value; } + public static void Increment(ref int value) => ++value; + + private static readonly CompilerFlags[] Flags = + { + CompilerFlags.Default, + CompilerFlags.NoInvocationLambdaInlining, + CompilerFlags.EnableDelegateDebugInfo | CompilerFlags.ThrowOnNotSupportedExpression + }; + + public void Run(TestRun t) + { + Nested_struct_field_assignment_uses_one_owner_address(t); + Compound_and_increment_results_preserve_the_final_address(t); + Nested_struct_field_ref_argument_mutates_original_storage(t); + } + + private static System.Linq.Expressions.MemberExpression Field(System.Linq.Expressions.Expression box) => + S.Field(S.Field(box, nameof(StrongBox.Value)), nameof(Counter.Value)); + + private static System.Linq.Expressions.MethodCallExpression OwnerCall(System.Linq.Expressions.Expression box) => + S.Call(typeof(NestedStructFieldAddressRegressionTests).GetMethod(nameof(Owner)), box); + + public void Nested_struct_field_assignment_uses_one_owner_address(TestContext t) + { + var box = S.Parameter(typeof(StrongBox)); + var value = S.Parameter(typeof(int)); + var source = S.Lambda, int, int>>(S.Assign(Field(OwnerCall(box)), + S.Call(typeof(NestedStructFieldAddressRegressionTests).GetMethod(nameof(Right)), value)), box, value); + Verify(t, source, 5, 12, 12, 12, 1); + + // A void delegate cannot leave an extra intermediate owner address at ret. + var action = S.Lambda, int>>(source.Body, box, value); + var expected = new StrongBox(new Counter { Value = 5 }); + OwnerReads = RightReads = 0; action.Compile()(expected, 17); + t.AreEqual(17, expected.Value.Value); t.AreEqual(1, OwnerReads); t.AreEqual(1, RightReads); + foreach (var flags in Flags) + { + var fast = action.FromSysExpression().CompileFast(true, flags); + if (!t.IsNotNull(fast)) continue; + var actual = new StrongBox(new Counter { Value = 5 }); + OwnerReads = RightReads = 0; fast(actual, 17); + t.AreEqual(17, actual.Value.Value); t.AreEqual(1, OwnerReads); t.AreEqual(1, RightReads); + } + } + + public void Compound_and_increment_results_preserve_the_final_address(TestContext t) + { + var box = S.Parameter(typeof(StrongBox)); + var value = S.Parameter(typeof(int)); + var field = Field(OwnerCall(box)); + var right = S.Call(typeof(NestedStructFieldAddressRegressionTests).GetMethod(nameof(Right)), value); + // System's compound-expression reduction mutates a copy of this nested struct. + // Keep that measured control, and prove writable storage with an explicit + // once-only owner/value expansion using ordinary field assignment. + Verify(t, S.Lambda, int, int>>(S.AddAssign(field, right), box, value), + 5, 7, 12, 12, 1, CompoundControl(box, value, add: true, post: false)); + Verify(t, S.Lambda, int, int>>(S.PostIncrementAssign(field), box, value), + 5, 0, 5, 6, 0, CompoundControl(box, value, add: false, post: true)); + Verify(t, S.Lambda, int, int>>(S.PreIncrementAssign(field), box, value), + 5, 0, 6, 6, 0, CompoundControl(box, value, add: false, post: false)); + } + + private static System.Linq.Expressions.Expression, int, int>> CompoundControl( + System.Linq.Expressions.ParameterExpression box, System.Linq.Expressions.ParameterExpression value, bool add, bool post) + { + var owner = S.Variable(typeof(StrongBox)); + var previous = S.Variable(typeof(int)); + System.Linq.Expressions.Expression increment = add + ? S.Call(typeof(NestedStructFieldAddressRegressionTests).GetMethod(nameof(Right)), value) + : S.Constant(1); + return S.Lambda, int, int>>(S.Block(new[] { owner, previous }, + S.Assign(owner, OwnerCall(box)), + S.Assign(previous, Field(owner)), + S.Assign(Field(owner), S.Add(previous, increment)), + post ? (System.Linq.Expressions.Expression)previous : Field(owner)), box, value); + } + + public void Nested_struct_field_ref_argument_mutates_original_storage(TestContext t) + { + var box = S.Parameter(typeof(StrongBox)); + var value = S.Parameter(typeof(int)); + var source = S.Lambda, int, int>>(S.Block( + S.Call(typeof(NestedStructFieldAddressRegressionTests).GetMethod(nameof(Increment)), Field(OwnerCall(box))), + Field(box)), box, value); + Verify(t, source, 5, 0, 6, 6, 0); + } + + private static void Verify(TestContext t, + System.Linq.Expressions.Expression, int, int>> source, + int initial, int argument, int result, int stored, int rightReads, + System.Linq.Expressions.Expression, int, int>> systemBehaviorControl = null) + { + var expected = new StrongBox(new Counter { Value = initial }); + OwnerReads = RightReads = 0; + t.AreEqual(result, source.Compile()(expected, argument)); + t.AreEqual(systemBehaviorControl == null ? stored : initial, expected.Value.Value); + t.AreEqual(1, OwnerReads); t.AreEqual(rightReads, RightReads); + if (systemBehaviorControl != null) + { + expected.Value = new Counter { Value = initial }; + OwnerReads = RightReads = 0; + t.AreEqual(result, systemBehaviorControl.Compile()(expected, argument)); + t.AreEqual(stored, expected.Value.Value); t.AreEqual(1, OwnerReads); t.AreEqual(rightReads, RightReads); + } + foreach (var flags in Flags) + { + var fast = source.FromSysExpression().CompileFast(true, flags); + if (!t.IsNotNull(fast)) continue; + var actual = new StrongBox(new Counter { Value = initial }); + OwnerReads = RightReads = 0; + t.AreEqual(result, fast(actual, argument)); + t.AreEqual(stored, actual.Value.Value); t.AreEqual(1, OwnerReads); t.AreEqual(rightReads, RightReads); + } + } +}