Skip to content

Make consts, functions distinct from assignments in scope resolution #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+110 −31
Open

Make consts, functions distinct from assignments in scope resolution #115

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c01c3b
Make consts, functions distinct from assingments in scope resolution
mlechu Oct 29, 2025
9955abe
Apply suggestions from code review
mlechu Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 45 additions & 12 deletions src/scope_analysis.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
#
Expand Down Expand Up @@ -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`.
Expand Down
38 changes: 19 additions & 19 deletions test/closures_ir.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -38,7 +38,7 @@ end
9 (call %₁ %₈ slot₂/y)
10 (return %₉)
19 latestworld
20 slot₁/f
20 slot₂/f
21 (return %₂₀)

########################################
Expand Down Expand Up @@ -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)
Expand All @@ -92,7 +92,7 @@ end
3 (call core.setfield! %₂ :contents %₁)
4 (return %₁)
19 latestworld
20 slot₁/f
20 slot₂/f
21 (return %₂₀)

########################################
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 %₁₃)
Expand Down
46 changes: 46 additions & 0 deletions test/scopes.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Owner

@c42f c42f Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the macro itself defines a function I would think it "should" be local for the same reason that assignments are local by default. For example:

macro define_func(outer_def)
    quote
        function should_be_local()
        end
        $outer_def
    end
end

@define_func function should_be_global() end

So the non-esc version seems like it should be local for the same reasons (ie, not exposing the macro implementation details as global symbols). The same reasoning applies to const.

I'm guessing you found examples where doing the logical thing breaks existing code? It would help if you could list a couple just so I can come to terms with doing the "wrong" thing in the name of compatibility 😅

Copy link
Owner

@c42f c42f Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A possibly-appealing idea would be to use different rules for this, based on whether symbols arise from an old or new-style macro. I think we could do this by recording that information in the scope layer in the same way that we currently record is_macro_expansion?

That way, we don't break old macros but we don't commit to the old and arguably-buggy rules?

Copy link
Collaborator Author

@mlechu mlechu Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example where this came up was fixed by something else (#114), so I've just removed the macro behaviour change for now in the interest of keeping this PR small.

We'll probably need to implement flisp hygiene exemptions later, though. If you thought this was bad, I think explicitly-declared globals (including abstract/primitive IIRC) unconditionally holepunch their way to the base scope layer.

I'm OK with doing this based on old/new macro status.

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
Loading