Skip to content

Restore outer variable bindings after nested block redeclarations - #587

Open
jogibear9988 wants to merge 1 commit into
dadhi:masterfrom
jogibear9988:fix/scoped-variable-bindings
Open

jogibear9988 wants to merge 1 commit into
dadhi:masterfrom
jogibear9988:fix/scoped-variable-bindings

Conversation

@jogibear9988

@jogibear9988 jogibear9988 commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

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.

Validation

Covers the unrelated-intermediate-scope reproduction, sibling redeclarations, and existing direct/distinct bindings under default/no-inlining/debug flags. This fixes ordinary variable bindings. Captured-variable storage is outside this change.

  • The unfixed master regression fails in both APIs; the same fixture passes after the patch on .NET 8, .NET 10 and .NET Framework 4.7.2. Strict FEC compilation is used.
  • Full Release solution build passes with zero warnings/errors.
  • Legacy suites pass: 1,756 on .NET 10 and 1,755 on .NET 8.
  • Framework runner passes 251 TestX + 1,639 legacy cases (en-US culture for existing message assertions).
  • Full modern runners retain only the upstream nullable MultiplyChecked parity failure for (1,1)/(2,1), in both APIs. Unmodified master was rebuilt and checked separately. No assertions were skipped or weakened; legacy suites ran separately because that existing TestX failure gates the normal runner.

Performance checks

Compared with master at f8f2a240abd650dda738162957606aac2659873f using already-working expressions, for both Standard and LightExpression. The broken regression establishes correctness and is not used as a before/after speed baseline. All selected working controls retain identical decoded emitted IL and the same measured compilation/invocation allocation counts.

Measurement setup and full results

Windows ARM64, SDK 10.0.301, .NET 8.0.28/10.0.9, Release. Prebuilt trees; Light conversion outside the timed section; strict CompileFast without System fallback; fixed CPU affinity, tiering disabled, 300 compilation warmups and 100,000 invocation warmups. Two fresh processes with opposite load order, nine alternating samples per side, GC before samples and a 256 MiB no-GC region where available. Table values are geometric means of process medians; compilation uses the longer approximately 100 ms paired-calibration run and invocation uses approximately 24 ms. Framework is correctness-tested, not timed.

Runtime / API / workload Compile ns, master → fix Change Compile B/op, both Invoke ns, master → fix Invoke B/op, both
net10.0 / Standard / add_control 1153.4 → 1132.3 -1.8% 872 1.88 → 1.89 0
net10.0 / Standard / block_local 1635.0 → 1623.2 -0.7% 872 1.83 → 1.82 0
net10.0 / Standard / nested_distinct_blocks 1986.4 → 1908.0 -3.9% 872 1.85 → 1.84 0
net10.0 / Standard / captured_local 4323.7 → 5148.8 +19.1% 3008 18.47 → 20.05 184
net10.0 / Light / add_control 1019.5 → 993.8 -2.5% 872 2.25 → 2.24 0
net10.0 / Light / block_local 1201.0 → 1204.7 +0.3% 872 2.40 → 2.59 0
net10.0 / Light / nested_distinct_blocks 1403.3 → 1398.2 -0.4% 872 2.42 → 2.41 0
net10.0 / Light / captured_local 3450.0 → 3374.2 -2.2% 3008 24.81 → 24.80 184
net8.0 / Standard / add_control 1111.5 → 1058.4 -4.8% 872 1.87 → 1.85 0
net8.0 / Standard / block_local 1351.7 → 1373.0 +1.6% 872 1.83 → 1.85 0
net8.0 / Standard / nested_distinct_blocks 1768.9 → 1719.7 -2.8% 872 3.41 → 1.86 0
net8.0 / Standard / captured_local 3973.8 → 3876.8 -2.4% 3008 19.05 → 20.02 184
net8.0 / Light / add_control 987.0 → 958.8 -2.9% 872 1.85 → 1.90 0
net8.0 / Light / block_local 1196.8 → 1221.9 +2.1% 872 1.90 → 1.86 0
net8.0 / Light / nested_distinct_blocks 1465.4 → 1399.4 -4.5% 872 1.91 → 1.86 0
net8.0 / Light / captured_local 3306.6 → 3266.4 -1.2% 3008 20.36 → 18.89 184

The full short run covers add, an ordinary local block, nested blocks with distinct variables and an ordinary captured local. Captured shadowing is not used as a working performance control. The longer run focuses on this fix's affected working paths plus unchanged controls. Slower observations are included above. Identical-binary master/master compilation variation in the same setup was net10.0: -26.4% to +1.6%; net8.0: -8.9% to +10.0%. These timings do not prove zero overhead for every input/machine, and no speedup claim is made. Runtime timing differences above are observations of identical emitted control IL.

All-process compilation timing ranges

These ranges include the less stable observations, not just the longer-run aggregate in the table. They are individual process-median changes across working controls; they are not speedup claims.

Runtime Short-run range Longer-run range Unchanged master/master longer-run range
net10.0 -10.9% to +5.9% -8.2% to +36.0% -48.5% to +5.4%
net8.0 -6.5% to +16.7% -11.0% to +7.6% -10.7% to +14.8%

The .NET 10 Standard captured-local compile row was slower in the initial longer run (+19.1%). Four fresh-process confirmations ranged from -3.7% to +28.6%; Light ranged from -11.7% to -0.5%. The measurements are not stable enough to guarantee neutral compilation CPU cost for that path. This limitation is retained explicitly. Both outer and returned child delegates have identical decoded IL in Standard/Light on .NET 8/10, and allocations remain unchanged.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant