Repository navigation
Restore outer variable bindings after nested block redeclarations - #587
Open
jogibear9988 wants to merge 1 commit into
Open
jogibear9988 wants to merge 1 commit into
jogibear9988 wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
Performance checks
Compared with master at
f8f2a240abd650dda738162957606aac2659873fusing 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.
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.
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.