Fix lambda parameter scope restoration to prevent memory leak (#1560)#1864
Draft
arnaudgelas wants to merge 1 commit intoinformalsystems:mainfrom
Draft
Fix lambda parameter scope restoration to prevent memory leak (#1560)#1864arnaudgelas wants to merge 1 commit intoinformalsystems:mainfrom
arnaudgelas wants to merge 1 commit intoinformalsystems:mainfrom
Conversation
…alsystems#1560) This commit implements proper scope restoration for lambda parameters, addressing FIXME informalsystems#1560 and preventing scope pollution in nested lambdas. ## Problem The previous implementation did not restore lambda parameter registers after executing the lambda body. This caused: 1. Parameter scope pollution: Nested lambda calls would overwrite parent scope parameter values without restoration 2. Memory leak: Previous parameter values were not restored, accumulating state 3. Incorrect semantics: Lambda parameter bindings should be local to the lambda Example issue: ``` let f = \ x => x + 1 let g = \ x => f(x) g(5) // This would corrupt x in g's scope ``` ## Solution The fix implements a save-restore pattern for lambda parameter registers: 1. BEFORE executing lambda: Save the current value of each parameter register 2. DURING execution: Overwrite registers with new parameter values 3. AFTER execution: Restore each register to its previous value This ensures lambda parameters are truly local and don't pollute outer scopes. ## Implementation Details - Save previous values before parameter binding (lines 482-487) - Execute body with new parameter values (line 489) - Restore previous values after execution completes (lines 491-496) - Error handling: Restoration happens whether execution succeeds or fails The fix maintains the Either monad pattern by restoring values even if the body execution returns an error. ## TypeScript Equivalence The TypeScript evaluator does not have this issue due to its function closure model. This Rust fix brings semantic parity with the intended behavior: lambda parameters should be locally scoped and not affect outer bindings. ## Testing This change affects only the Rust evaluator's lambda execution path. Existing tests in simulator_tests.rs should still pass as they don't exercise nested lambda parameter shadowing at the register level. Fixes: informalsystems#1560 Related: fix-semantic-divergence branch Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
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.
This commit implements proper scope restoration for lambda parameters, addressing FIXME #1560 and preventing scope pollution in nested lambdas.
Problem
The previous implementation did not restore lambda parameter registers after executing the lambda body. This caused:
Example issue:
Solution
The fix implements a save-restore pattern for lambda parameter registers:
This ensures lambda parameters are truly local and don't pollute outer scopes.
Implementation Details
The fix maintains the Either monad pattern by restoring values even if the body execution returns an error.
TypeScript Equivalence
The TypeScript evaluator does not have this issue due to its function closure model. This Rust fix brings semantic parity with the intended behavior: lambda parameters should be locally scoped and not affect outer bindings.
Testing
This change affects only the Rust evaluator's lambda execution path. Existing tests in simulator_tests.rs should still pass as they don't exercise nested lambda parameter shadowing at the register level.
Fixes: #1560
Related: fix-semantic-divergence branch
(including screenshots is helpful)
CHANGELOG.mdfor any new functionality