From 8c01c3b8e68ae2274022fee49bf04451546bbcb2 Mon Sep 17 00:00:00 2001 From: Em Chu Date: Wed, 29 Oct 2025 12:50:49 -0700 Subject: [PATCH 1/2] Make consts, functions distinct from assingments in scope resolution --- src/scope_analysis.jl | 57 ++++++++++++++++++++++++++++++++++--------- test/closures_ir.jl | 38 ++++++++++++++--------------- test/scopes.jl | 46 ++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 31 deletions(-) diff --git a/src/scope_analysis.jl b/src/scope_analysis.jl index ce3f0fba..97a904ac 100644 --- a/src/scope_analysis.jl +++ b/src/scope_analysis.jl @@ -20,7 +20,8 @@ end #------------------------------------------------------------------------------- _insert_if_not_present!(dict, key, val) = get!(dict, key, val) -function _find_scope_vars!(ctx, assignments, locals, destructured_args, globals, used_names, used_bindings, ex) +function _find_scope_vars!(ctx, assignments, locals, destructured_args, globals, + consts, used_names, used_bindings, ex, in_toplevel_thunk) k = kind(ex) if k == K"Identifier" _insert_if_not_present!(used_names, NameKey(ex), ex) @@ -43,19 +44,28 @@ function _find_scope_vars!(ctx, assignments, locals, destructured_args, globals, # like v = val, except that if `v` turns out global(either implicitly or # by explicit `global`), it gains an implicit `const` _insert_if_not_present!(assignments, NameKey(ex[1]), ex) - elseif k == K"=" || k == K"constdecl" + elseif k == K"constdecl" + if !(kind(ex[1]) in KSet"BindingId globalref Value Placeholder") + _insert_if_not_present!(consts, NameKey(ex[1]), ex[1]) + end + if numchildren(ex) == 2 + _find_scope_vars!( + ctx, assignments, locals, destructured_args, globals, + consts, used_names, used_bindings, ex[2], in_toplevel_thunk) + end + elseif k == K"=" v = decl_var(ex[1]) if !(kind(v) in KSet"BindingId globalref Value Placeholder") _insert_if_not_present!(assignments, NameKey(v), v) end - if k != K"constdecl" || numchildren(ex) == 2 - _find_scope_vars!(ctx, assignments, locals, destructured_args, globals, used_names, used_bindings, ex[2]) - end + _find_scope_vars!( + ctx, assignments, locals, destructured_args, globals, + consts, used_names, used_bindings, ex[2], in_toplevel_thunk) elseif k == K"function_decl" v = ex[1] kv = kind(v) if kv == K"Identifier" - _insert_if_not_present!(assignments, NameKey(v), v) + _insert_if_not_present!(consts, NameKey(v), v) elseif kv == K"BindingId" binfo = lookup_binding(ctx, v) if !binfo.is_ssa && binfo.kind != :global @@ -66,7 +76,9 @@ function _find_scope_vars!(ctx, assignments, locals, destructured_args, globals, end else for e in children(ex) - _find_scope_vars!(ctx, assignments, locals, destructured_args, globals, used_names, used_bindings, e) + _find_scope_vars!( + ctx, assignments, locals, destructured_args, globals, + consts, used_names, used_bindings, e, in_toplevel_thunk) end end end @@ -75,26 +87,31 @@ end # into sets by type of usage. # # NB: This only works properly after desugaring -function find_scope_vars(ctx, ex) +function find_scope_vars(ctx, ex, in_toplevel_thunk) ExT = typeof(ex) assignments = Dict{NameKey,ExT}() locals = Dict{NameKey,ExT}() destructured_args = Vector{ExT}() globals = Dict{NameKey,ExT}() + consts = Dict{NameKey,ExT}() used_names = Dict{NameKey,ExT}() used_bindings = Set{IdTag}() for e in children(ex) - _find_scope_vars!(ctx, assignments, locals, destructured_args, globals, used_names, used_bindings, e) + _find_scope_vars!( + ctx, assignments, locals, destructured_args, globals, consts, + used_names, used_bindings, e, in_toplevel_thunk) end # Sort by key so that id generation is deterministic assignments = sort!(collect(pairs(assignments)), by=first) locals = sort!(collect(pairs(locals)), by=first) globals = sort!(collect(pairs(globals)), by=first) + consts = sort!(collect(pairs(consts)), by=first) used_names = sort!(collect(pairs(used_names)), by=first) used_bindings = sort!(collect(used_bindings)) - return assignments, locals, destructured_args, globals, used_names, used_bindings + # @info find_scope_vars ex in_toplevel_thunk assignments locals destructured_args globals consts used_names used_bindings + return assignments, locals, destructured_args, globals, consts, used_names, used_bindings end struct ScopeInfo @@ -213,8 +230,8 @@ function analyze_scope(ctx, ex, scope_type, is_toplevel_global_scope=false, in_toplevel_thunk = is_toplevel_global_scope || (!is_outer_lambda_scope && parentscope.in_toplevel_thunk) - assignments, locals, destructured_args, globals, - used_names, used_bindings = find_scope_vars(ctx, ex) + assignments, locals, destructured_args, globals, consts, + used_names, used_bindings = find_scope_vars(ctx, ex, in_toplevel_thunk) # Construct a mapping from identifiers to bindings # @@ -324,6 +341,22 @@ function analyze_scope(ctx, ex, scope_type, is_toplevel_global_scope=false, end end + # Constants and function decls are like assignments, but don't become local + # in soft scopes or macro expansions + for (varkey,e) in consts + vk = haskey(var_ids, varkey) ? + lookup_binding(ctx, var_ids[varkey]).kind : + var_kind(ctx, varkey, true) + if vk === :static_parameter + throw(LoweringError(e, "local variable name `$(varkey.name)` conflicts with a static parameter")) + elseif vk === nothing && (is_toplevel_global_scope || is_soft_scope) + var_ids[varkey] = init_binding(ctx, e, varkey, :global) + else + var_ids[varkey] = init_binding(ctx, e, varkey, something(vk, :local)) + end + end + + #-------------------------------------------------- # At this point we've discovered all the bindings defined in this scope and # added them to `var_ids`. diff --git a/test/closures_ir.jl b/test/closures_ir.jl index 0916e313..f6b729f7 100644 --- a/test/closures_ir.jl +++ b/test/closures_ir.jl @@ -8,18 +8,18 @@ let end end #--------------------- -1 (= slot₂/x (call core.Box)) +1 (= slot₁/x (call core.Box)) 2 1 -3 slot₂/x +3 slot₁/x 4 (call core.setfield! %₃ :contents %₂) 5 (call core.svec :x) 6 (call core.svec true) 7 (call JuliaLowering.eval_closure_type TestMod :#f##0 %₅ %₆) 8 latestworld 9 TestMod.#f##0 -10 slot₂/x +10 slot₁/x 11 (new %₉ %₁₀) -12 (= slot₁/f %₁₁) +12 (= slot₂/f %₁₁) 13 TestMod.#f##0 14 (call core.svec %₁₃ core.Any) 15 (call core.svec) @@ -38,7 +38,7 @@ end 9 (call %₁ %₈ slot₂/y) 10 (return %₉) 19 latestworld -20 slot₁/f +20 slot₂/f 21 (return %₂₀) ######################################## @@ -68,18 +68,18 @@ let end end #--------------------- -1 (= slot₂/x (call core.Box)) +1 (= slot₁/x (call core.Box)) 2 1 -3 slot₂/x +3 slot₁/x 4 (call core.setfield! %₃ :contents %₂) 5 (call core.svec :x) 6 (call core.svec true) 7 (call JuliaLowering.eval_closure_type TestMod :#f##1 %₅ %₆) 8 latestworld 9 TestMod.#f##1 -10 slot₂/x +10 slot₁/x 11 (new %₉ %₁₀) -12 (= slot₁/f %₁₁) +12 (= slot₂/f %₁₁) 13 TestMod.#f##1 14 (call core.svec %₁₃ core.Any) 15 (call core.svec) @@ -92,7 +92,7 @@ end 3 (call core.setfield! %₂ :contents %₁) 4 (return %₁) 19 latestworld -20 slot₁/f +20 slot₂/f 21 (return %₂₀) ######################################## @@ -182,14 +182,14 @@ end 18 SourceLocation::1:10 19 (call core.svec %₁₆ %₁₇ %₁₈) 20 --- method core.nothing %₁₉ - slots: [slot₁/#self#(!read) slot₂/x slot₃/g slot₄/z(!read)] + slots: [slot₁/#self#(!read) slot₂/x slot₃/z(!read) slot₄/g] 1 TestMod.#f#g##1 2 (call core.typeof slot₂/x) 3 (call core.apply_type %₁ %₂) 4 (new %₃ slot₂/x) - 5 (= slot₃/g %₄) + 5 (= slot₄/g %₄) 6 slot₂/x - 7 (= slot₄/z %₆) + 7 (= slot₃/z %₆) 8 (return %₆) 21 latestworld 22 TestMod.f @@ -289,18 +289,18 @@ end 18 SourceLocation::1:10 19 (call core.svec %₁₆ %₁₇ %₁₈) 20 --- method core.nothing %₁₉ - slots: [slot₁/#self#(!read) slot₂/x slot₃/g slot₄/y] - 1 (= slot₄/y (call core.Box)) + slots: [slot₁/#self#(!read) slot₂/x slot₃/y slot₄/g] + 1 (= slot₃/y (call core.Box)) 2 TestMod.#f#g##3 3 (call core.typeof slot₂/x) 4 (call core.apply_type %₂ %₃) - 5 slot₄/y + 5 slot₃/y 6 (new %₄ slot₂/x %₅) - 7 (= slot₃/g %₆) + 7 (= slot₄/g %₆) 8 2 - 9 slot₄/y + 9 slot₃/y 10 (call core.setfield! %₉ :contents %₈) - 11 slot₄/y + 11 slot₃/y 12 (call core.isdefined %₁₁ :contents) 13 (call core.tuple %₁₂ true) 14 (return %₁₃) diff --git a/test/scopes.jl b/test/scopes.jl index e327343e..fc109dfd 100644 --- a/test/scopes.jl +++ b/test/scopes.jl @@ -77,4 +77,50 @@ JuliaLowering.eval(test_mod, wrapscope(assign_z_2, :hard)) JuliaLowering.eval(test_mod, wrapscope(wrapscope(assign_z_2, :neutral), :soft)) @test test_mod.z == 2 +@testset "top-level function defs aren't local to soft scopes" begin + def = parsestmt(SyntaxTree, "function f_softscope_test(); 1; end", filename="foo.jl") + JuliaLowering.eval(test_mod, wrapscope(def, :hard)) + @test !isdefined(test_mod, :f_softscope_test) + JuliaLowering.eval(test_mod, wrapscope(def, :neutral)) + @test !isdefined(test_mod, :f_softscope_test) + JuliaLowering.eval(test_mod, wrapscope(def, :soft)) + @test isdefined(test_mod, :f_softscope_test) + + JuliaLowering.include_string(test_mod, """ + for i in 1 + fdecl_in_loop() = 1 + end + """) + @test !isdefined(test_mod, :fdecl_in_loop) +end + +@testset "constdecls OK in soft scopes" begin + def = parsestmt(SyntaxTree, "const c_softscope_test = 1", filename="foo.jl") + @test_throws LoweringError JuliaLowering.eval(test_mod, wrapscope(def, :hard)) + @test_throws LoweringError JuliaLowering.eval(test_mod, wrapscope(def, :neutral)) + JuliaLowering.eval(test_mod, wrapscope(def, :soft)) + @test isdefined(test_mod, :c_softscope_test) +end + +@eval test_mod module macro_mod + macro m(x); x; end + macro mesc(x); esc(x); end +end + +@testset "top-level function defs aren't local in macro expansions" begin + JuliaLowering.include_string(test_mod, "macro_mod.@m function f_nonlocal_1(); 1; end") + @test isdefined(test_mod.macro_mod, :f_nonlocal_1) + JuliaLowering.include_string(test_mod, "macro_mod.@mesc function f_nonlocal_2(); 1; end") + @test isdefined(test_mod, :f_nonlocal_2) + # Note: for the particular case of an unescaped top-level const declaration, + # flisp makes it macro-local (i.e. mangles the name), but this isn't the + # case for other globals (top-level functions decls, global decls, types) so + # it might be a bug. + JuliaLowering.include_string(test_mod, "macro_mod.@m const c_nonlocal_1 = 1") + @test isdefined(test_mod.macro_mod, :c_nonlocal_1) + # Our behaviour is the same when the const is escaped + JuliaLowering.include_string(test_mod, "macro_mod.@mesc const c_nonlocal_2 = 1") + @test isdefined(test_mod, :c_nonlocal_2) +end + end From 9955abe754405f7f0979cb330860f5fa4d8249ee Mon Sep 17 00:00:00 2001 From: Em Chu Date: Thu, 30 Oct 2025 13:02:31 -0700 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Claire Foster --- src/scope_analysis.jl | 10 +++++----- test/scopes.jl | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/scope_analysis.jl b/src/scope_analysis.jl index 97a904ac..26737050 100644 --- a/src/scope_analysis.jl +++ b/src/scope_analysis.jl @@ -106,11 +106,10 @@ function find_scope_vars(ctx, ex, in_toplevel_thunk) assignments = sort!(collect(pairs(assignments)), by=first) locals = sort!(collect(pairs(locals)), by=first) globals = sort!(collect(pairs(globals)), by=first) - consts = sort!(collect(pairs(consts)), by=first) + consts = sort!(collect(pairs(consts)), by=first) used_names = sort!(collect(pairs(used_names)), by=first) used_bindings = sort!(collect(used_bindings)) - # @info find_scope_vars ex in_toplevel_thunk assignments locals destructured_args globals consts used_names used_bindings return assignments, locals, destructured_args, globals, consts, used_names, used_bindings end @@ -341,15 +340,16 @@ function analyze_scope(ctx, ex, scope_type, is_toplevel_global_scope=false, end end - # Constants and function decls are like assignments, but don't become local - # in soft scopes or macro expansions + # Constants and function decls are like assignments, except when + # (in_soft_scope && not_in_macro_expansion), where they are globals. for (varkey,e) in consts vk = haskey(var_ids, varkey) ? lookup_binding(ctx, var_ids[varkey]).kind : var_kind(ctx, varkey, true) if vk === :static_parameter throw(LoweringError(e, "local variable name `$(varkey.name)` conflicts with a static parameter")) - elseif vk === nothing && (is_toplevel_global_scope || is_soft_scope) + elseif (vk === nothing && (is_toplevel_global_scope || is_soft_scope) && + !(ctx.scope_layers[varkey.layer].is_macro_expansion)) var_ids[varkey] = init_binding(ctx, e, varkey, :global) else var_ids[varkey] = init_binding(ctx, e, varkey, something(vk, :local)) diff --git a/test/scopes.jl b/test/scopes.jl index fc109dfd..c4f90d8d 100644 --- a/test/scopes.jl +++ b/test/scopes.jl @@ -107,20 +107,20 @@ end macro mesc(x); esc(x); end end -@testset "top-level function defs aren't local in macro expansions" begin - JuliaLowering.include_string(test_mod, "macro_mod.@m function f_nonlocal_1(); 1; end") - @test isdefined(test_mod.macro_mod, :f_nonlocal_1) +# Difference from flisp, where top-level functions are unmangled and declared in +# the expansion module (not the calling one) +@testset "functions and consts are local to macro expansions" begin + JuliaLowering.include_string(test_mod, "macro_mod.@m function f_local_1(); 1; end") + @test !isdefined(test_mod.macro_mod, :f_local_1) JuliaLowering.include_string(test_mod, "macro_mod.@mesc function f_nonlocal_2(); 1; end") @test isdefined(test_mod, :f_nonlocal_2) - # Note: for the particular case of an unescaped top-level const declaration, - # flisp makes it macro-local (i.e. mangles the name), but this isn't the - # case for other globals (top-level functions decls, global decls, types) so - # it might be a bug. - JuliaLowering.include_string(test_mod, "macro_mod.@m const c_nonlocal_1 = 1") - @test isdefined(test_mod.macro_mod, :c_nonlocal_1) - # Our behaviour is the same when the const is escaped + # An unescaped const is local to a macro expansion + @test_throws LoweringError JuliaLowering.include_string(test_mod, "macro_mod.@m const c_local_1 = 1") + # The const may be escaped into test_mod JuliaLowering.include_string(test_mod, "macro_mod.@mesc const c_nonlocal_2 = 1") @test isdefined(test_mod, :c_nonlocal_2) + JuliaLowering.include_string(test_mod, "macro_mod.@mesc const c_nonlocal_3 = 1"; expr_compat_mode=true) + @test isdefined(test_mod, :c_nonlocal_3) end end