[JuliaLowering] Fix is_ambiguous_local in permeable scopes#61473
Open
[JuliaLowering] Fix is_ambiguous_local in permeable scopes#61473
is_ambiguous_local in permeable scopes#61473Conversation
aviatesk
force-pushed
the
avi/JL-fix-soft-scope
branch
2 times, most recently
from
728f90c to
8948d4d
Compare
aviatesk
changed the title
is_ambiguous_local for toplevel-assigned globals in begin blocksis_ambiguous_local in permeable scopes
The `b.kind === :global` fallback in `enter_scope!` was creating a plain local without setting `is_ambiguous_local` when a toplevel global was reassigned from within a permeable scope (`for`/`while`/`try`). This affected two cases: 1. When a pre-existing global (defined in the module before lowering) is reassigned in a `while` loop where the condition also references the variable — e.g. `while x < 5; x += 1; end`. The condition's read resolves `x` as `:global` via `is_defined_and_owned_global`, then the body's assignment enters the `b.kind === :global` path instead of the `b === nothing` path. 2. When a toplevel assignment precedes a permeable scope within the same toplevel thunk — e.g. `begin; x = 0; for ...; x = 1; end; end`. The toplevel `x = 0` declares `x` as `:global`, so the inner assignment also enters `b.kind === :global`. The fix sets `is_ambiguous_local` based solely on `scope.is_permeable`: any assignment to a toplevel global from within a permeable scope is ambiguous regardless of how the global was resolved.
aviatesk
force-pushed
the
avi/JL-fix-soft-scope
branch
from
8948d4d to
64986de
Compare
mlechu
reviewed
Apr 2, 2026
| est = JuliaLowering.expr_to_est(ex) | ||
| ctx1, ex1 = JuliaLowering.expand_forms_1(mod, est, false, world) | ||
| ctx2, ex2 = JuliaLowering.expand_forms_2(ctx1, ex1) | ||
| ctx3, _ex3 = JuliaLowering.resolve_scopes(ctx2, ex2; soft_scope) |
Member
There was a problem hiding this comment.
Suggested change
| ctx3, _ex3 = JuliaLowering.resolve_scopes(ctx2, ex2; soft_scope) | |
| ctx3, _ex3 = JuliaLowering.resolve_scopes(ctx2, ex2) |
Assuming from #61476
mlechu
reviewed
Apr 2, 2026
| return ctx3.bindings.info | ||
| end | ||
|
|
||
| # When a toplevel assignment precedes a permeable scope (for/while/try) within |
Member
There was a problem hiding this comment.
I don't think "precedes" is necessary for this to happen; an assignment "below" the ambiguous local's scope should be the same. Maybe worth writing a test for.
mlechu
reviewed
Apr 2, 2026
|
|
||
| # Block containing global assignment | ||
| let bindings = resolve_and_get_bindings(Module(), quote | ||
| x = 0 |
Member
There was a problem hiding this comment.
Noting this doesn't match flisp when this is global x = 0, which somehow makes the local unambiguous to flisp, but still ambiguous to us. I don't think it's worth changing unless we can describe the behaviour we want, but if you would write a comment or a test_broken, that would be helpful.
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
2 participants
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.
The
b.kind === :globalfallback inenter_scope!was creatinga plain local without setting
is_ambiguous_localwhen atoplevel global was reassigned from within a permeable scope
(
for/while/try). This affected two cases:When a pre-existing global (defined in the module before
lowering) is reassigned in a
whileloop where thecondition also references the variable — e.g.
while x < 5; x += 1; end. The condition's read resolvesxas:globalviais_defined_and_owned_global, thenthe body's assignment enters the
b.kind === :globalpathinstead of the
b === nothingpath.When a toplevel assignment precedes a permeable scope within
the same toplevel thunk — e.g.
begin; x = 0; for ...; x = 1; end; end. The toplevelx = 0declaresxas:global,so the inner assignment also enters
b.kind === :global.The fix sets
is_ambiguous_localbased solely onscope.is_permeable: any assignment to a toplevel global fromwithin a permeable scope is ambiguous regardless of how the
global was resolved.