From d1a878b178a4108477df774c22d5ea59b7b8a234 Mon Sep 17 00:00:00 2001 From: jogibear9988 Date: Wed, 4 Nov 2026 23:35:18 +0100 Subject: [PATCH] fix: restore variable bindings by their declaring block depth Reusing a ParameterExpression in an inner block through an unrelated intermediate scope can leave the inner binding active after that block. Master returns 2 where System returns the outer value 1. Pop a binding only when its encoded declaring block index matches the block being left. Binding-stack length is not lexical depth. This changes one existing condition, with no new traversal, context fields, allocations or closure layout. Focused Standard/Light regressions pass on .NET 8, .NET 10 and .NET Framework 4.7.2. Full solution and legacy suites pass. Modern full runners retain only the reproduced upstream nullable MultiplyChecked parity failure. Working benchmark controls retain allocation counts and emitted IL. --- .../FastExpressionCompiler.cs | 2 +- .../Program.cs | 2 + .../Program.cs | 2 + .../BlockVariableBindingRegressionTests.cs | 82 +++++++++++++++++++ 4 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 test/FastExpressionCompiler.UnitTests/BlockVariableBindingRegressionTests.cs diff --git a/src/FastExpressionCompiler/FastExpressionCompiler.cs b/src/FastExpressionCompiler/FastExpressionCompiler.cs index b0e95741..71856a67 100644 --- a/src/FastExpressionCompiler/FastExpressionCompiler.cs +++ b/src/FastExpressionCompiler/FastExpressionCompiler.cs @@ -853,7 +853,7 @@ public void PopBlock() for (var i = 0; i < varCount; ++i) { ref var varBlocks = ref _varInBlock.Map.GetSurePresentEntryRef(i); - if (varBlocks.Value.Count == _blockCount) + if (varBlocks.Value.Count != 0 && (varBlocks.Value.GetLastSurePresentRef() >>> 16) == _blockCount - 1) varBlocks.Value.RemoveLastSurePresent(); } --_blockCount; diff --git a/test/FastExpressionCompiler.TestsRunner.Net472/Program.cs b/test/FastExpressionCompiler.TestsRunner.Net472/Program.cs index 7bdfc33b..1bb41586 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.BlockVariableBindingRegressionTests()); t.Run(new LightExpression.UnitTests.NestedStructFieldAddressRegressionTests()); t.Run(new LightExpression.IssueTests.Issue183_NullableDecimal()); t.Run(new LightExpression.IssueTests.Issue398_Optimize_Switch_with_OpCodes_Switch()); @@ -60,6 +61,7 @@ static void RunLightExpressionTests(object state) var fecTestsStopwatch = Stopwatch.StartNew(); var fecTests = new TestRun(); + fecTests.Run(new BlockVariableBindingRegressionTests()); 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()); diff --git a/test/FastExpressionCompiler.TestsRunner/Program.cs b/test/FastExpressionCompiler.TestsRunner/Program.cs index ca07a0ca..6fb4c573 100644 --- a/test/FastExpressionCompiler.TestsRunner/Program.cs +++ b/test/FastExpressionCompiler.TestsRunner/Program.cs @@ -63,6 +63,7 @@ public static void Main(string[] args) var ltTask = Task.Run(() => { var ltStopwatch = Stopwatch.StartNew(); + lt.Run(new LightExpression.UnitTests.BlockVariableBindingRegressionTests()); #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()); @@ -86,6 +87,7 @@ public static void Main(string[] args) }); var stStopwatch = Stopwatch.StartNew(); + st.Run(new BlockVariableBindingRegressionTests()); st.Run(new NestedStructFieldAddressRegressionTests()); st.Run(new Issue499_InvalidProgramException_for_Sorting_and_comparison_function()); st.Run(new Issue561_NonBooleanEqualityOperator()); diff --git a/test/FastExpressionCompiler.UnitTests/BlockVariableBindingRegressionTests.cs b/test/FastExpressionCompiler.UnitTests/BlockVariableBindingRegressionTests.cs new file mode 100644 index 00000000..6d163303 --- /dev/null +++ b/test/FastExpressionCompiler.UnitTests/BlockVariableBindingRegressionTests.cs @@ -0,0 +1,82 @@ +using System; +using System.Collections.Generic; +using S = System.Linq.Expressions.Expression; +#if LIGHT_EXPRESSION +namespace FastExpressionCompiler.LightExpression.UnitTests; +#else +namespace FastExpressionCompiler.UnitTests; +#endif + +public struct BlockVariableBindingRegressionTests : ITestX +{ + private static readonly CompilerFlags[] Flags = + { + CompilerFlags.Default, CompilerFlags.NoInvocationLambdaInlining, + CompilerFlags.EnableDelegateDebugInfo | CompilerFlags.ThrowOnNotSupportedExpression + }; + + public void Run(TestRun t) + { + Leaving_nested_binding_restores_outer_value(t); + Sibling_redeclarations_restore_outer_value(t); + Direct_nested_and_distinct_bindings_keep_their_values(t); + } + + public void Leaving_nested_binding_restores_outer_value(TestContext t) + { + var value = S.Variable(typeof(int), "value"); + var middle = S.Variable(typeof(int), "middle"); + var tree = S.Lambda>(S.Block(new[] { value }, + S.Assign(value, S.Constant(1)), + S.Block(new[] { middle }, S.Assign(middle, S.Constant(0)), + S.Block(new[] { value }, S.Assign(value, S.Constant(2))), value))); + var expected = tree.Compile()(); + t.AreEqual(1, expected); + foreach (var flags in Flags) + { + var fast = tree.FromSysExpression().CompileFast(true, flags); + if (t.IsNotNull(fast)) t.AreEqual(expected, fast()); + } + } + + public void Sibling_redeclarations_restore_outer_value(TestContext t) + { + var value = S.Variable(typeof(int), "value"); + var sum = S.Variable(typeof(int), "sum"); + var statements = new List + { + S.Assign(value, S.Constant(1)), S.Assign(sum, S.Constant(0)) + }; + for (var i = 0; i < 3; ++i) + { + var middle = S.Variable(typeof(int), "middle"); + statements.Add(S.Block(new[] { middle }, S.Assign(middle, S.Constant(0)), + S.Block(new[] { value }, S.Assign(value, S.Constant(i + 10))), S.AddAssign(sum, value))); + } + statements.Add(sum); + var tree = S.Lambda>(S.Block(new[] { value, sum }, statements)); + var expected = tree.Compile()(); + t.AreEqual(3, expected); + foreach (var flags in Flags) + { + var fast = tree.FromSysExpression().CompileFast(true, flags); + if (t.IsNotNull(fast)) t.AreEqual(expected, fast()); + } + } + + public void Direct_nested_and_distinct_bindings_keep_their_values(TestContext t) + { + var value = S.Variable(typeof(int), "value"); + var other = S.Variable(typeof(int), "other"); + var tree = S.Lambda>(S.Block(new[] { value }, S.Assign(value, S.Constant(1)), + S.Block(new[] { value }, S.Assign(value, S.Constant(2))), + S.Block(new[] { other }, S.Assign(other, S.Constant(3))), value)); + var expected = tree.Compile()(); + t.AreEqual(1, expected); + foreach (var flags in Flags) + { + var fast = tree.FromSysExpression().CompileFast(true, flags); + if (t.IsNotNull(fast)) t.AreEqual(expected, fast()); + } + } +}