Fix closure type naming to use list of nested functions

This greatly improves the naming stability over using a global gensym(),
allowing our closure tests to be stable.

This follow the naming scheme from #53719
with a fixes for closures nested in non-function scopes.

Author: c42f

JuliaLowering/src/closure_conversion.jl

@@ -54,7 +54,7 @@ function get_box_contents(ctx::ClosureConversionCtx, var, box_ex)
         # to fold away the extraneous null check
         #
         # TODO: Ideally the runtime would rely on provenance info for
-        # this error and we can remove isdefined check.
+        # this error and we can remove the isdefined check.
         [K"if" [K"call"
                 "isdefined"::K"core"
                 box
@@ -230,8 +230,21 @@ function closure_type_fields(ctx, srcref, closure_binds)
     return field_syms, field_orig_bindings, field_name_inds
 end
 
+function closure_name(mod, name_stack)
+    basename = "#$(join(name_stack, "#"))##"
+    i = 0
+    while true
+        name = "$basename$i"
+        if reserve_module_binding(mod, Symbol(name))
+            return name
+        end
+        i += 1
+    end
+end
+
 # Return a thunk which creates a new type for a closure with `field_syms` named
-# fields. The new type will be named `name_str`, which must be unique.
+# fields. The new type will be named `name_str` which must be an unassigned
+# name in the module.
 function type_for_closure(ctx::ClosureConversionCtx, srcref, name_str, field_syms)
     # New closure types always belong to the module we're expanding into - they
     # need to be serialized there during precompile.
@@ -329,10 +342,10 @@ function _convert_closures(ctx::ClosureConversionCtx, ex)
             closure_info = get(ctx.closure_infos, func_name_id, nothing)
             needs_def = isnothing(closure_info)
             if needs_def
-                # TODO: Names for closures without relying on gensym
-                name_str = string(gensym("closure"))
+                closure_binds = ctx.closure_bindings[func_name_id]
                 field_syms, field_orig_bindings, field_name_inds =
-                    closure_type_fields(ctx, ex, ctx.closure_bindings[func_name_id])
+                    closure_type_fields(ctx, ex, closure_binds)
+                name_str = closure_name(ctx.mod, closure_binds.name_stack)
                 closure_type_def, closure_type =
                     type_for_closure(ctx, ex, name_str, field_syms)
                 push!(ctx.toplevel_stmts, closure_type_def)

JuliaLowering/src/runtime.jl

@@ -221,8 +221,8 @@ end
 # Get the binding for `name` if one is already resolved in module `mod`. Note
 # that we cannot use `isdefined(::Module, ::Symbol)` here, because that causes
 # binding resolution which is a massive side effect we must avoid in lowering.
-function _get_module_binding(mod, name)
-    b = @ccall jl_get_module_binding(mod::Module, name::Symbol, 0::Cint)::Ptr{Core.Binding}
+function _get_module_binding(mod, name; create=false)
+    b = @ccall jl_get_module_binding(mod::Module, name::Symbol, create::Cint)::Ptr{Core.Binding}
     b == C_NULL ? nothing : unsafe_pointer_to_objref(b)
 end
 
@@ -245,6 +245,20 @@ function is_defined_nothrow_global(mod, name)
     isdefined(b.owner, :value)
 end
 
+# "Reserve" a binding: create the binding if it doesn't exist but do not assign
+# to it.
+function reserve_module_binding(mod, name)
+    # TODO: Fix the race condition here: We should really hold the Module's
+    # binding lock during this test-and-set type operation. But the binding
+    # lock is only accessible from C. See also the C code in
+    # `fl_module_unique_name`.
+    if _get_module_binding(mod, name; create=false) === nothing
+        _get_module_binding(mod, name; create=true) !== nothing
+    else
+        return false
+    end
+end
+
 #-------------------------------------------------------------------------------
 # The following are versions of macros from Base which act as "standard syntax
 # extensions" with special semantics known to lowering.

JuliaLowering/src/scope_analysis.jl

@@ -136,10 +136,11 @@ function update_lambda_binding!(ctx::AbstractLoweringContext, id; kws...)
 end
 
 struct ClosureBindings
-    lambdas::Vector{LambdaBindings}
+    name_stack::Vector{String}      # Names of functions the closure is nested within
+    lambdas::Vector{LambdaBindings} # Bindings for each method of the closure
 end
 
-ClosureBindings() = ClosureBindings(Vector{LambdaBindings}())
+ClosureBindings(name_stack) = ClosureBindings(name_stack, Vector{LambdaBindings}())
 
 struct ScopeInfo
     # True if scope is the global top level scope
@@ -173,7 +174,8 @@ struct ScopeResolutionContext{GraphType} <: AbstractLoweringContext
     # Variables which were implicitly global due to being assigned to in top
     # level code
     implicit_toplevel_globals::Set{NameKey}
-    # 
+    # Collection of information about each closure, principally which methods
+    # are part of the closure (and hence captures).
     closure_bindings::Dict{IdTag,ClosureBindings}
 end
 
@@ -554,7 +556,13 @@ function _resolve_scopes(ctx, ex::SyntaxTree)
                 func_name_id = func_name.var_id
                 if lookup_binding(ctx, func_name_id).kind == :local
                     cbinds = get!(ctx.closure_bindings, func_name_id) do
-                        ClosureBindings()
+                        name_stack = Vector{String}()
+                        for fname in ctx.method_def_stack
+                            if kind(fname) == K"BindingId"
+                                push!(name_stack, lookup_binding(ctx, fname).name)
+                            end
+                        end
+                        ClosureBindings(name_stack)
                     end
                     push!(cbinds.lambdas, lambda_bindings)
                 end

JuliaLowering/test/closures_ir.jl

@@ -7,21 +7,21 @@ let
     end
 end
 #---------------------
-1   (newvar slot₁)
-2   (= slot₂ (call core.Box))
+1   (= slot₂ (call core.Box))
+2   (newvar slot₁)
 3   --- thunk
-    1   (global TestMod.##closure#277)
+    1   (global TestMod.#f##0)
     2   (call core.svec)
     3   (call core.svec :x)
     4   (call core.svec)
-    5   (call core._structtype TestMod :##closure#277 %₂ %₃ %₄ false 1)
+    5   (call core._structtype TestMod :#f##0 %₂ %₃ %₄ false 1)
     6   (call core._setsuper! %₅ core.Function)
-    7   (const TestMod.##closure#277)
-    8   (= TestMod.##closure#277 %₅)
+    7   (const TestMod.#f##0)
+    8   (= TestMod.#f##0 %₅)
     9   (call core.svec core.Box)
     10  (call core._typebody! %₅ %₉)
     11  (return core.nothing)
-4   TestMod.##closure#277
+4   TestMod.#f##0
 5   (call core.svec %₄ core.Any)
 6   (call core.svec)
 7   (call core.svec %₅ %₆ :($(QuoteNode(:(#= line 3 =#)))))
@@ -39,7 +39,7 @@ end
 9   1
 10  slot₂/x
 11  (call core.setfield! %₁₀ :contents %₉)
-12  TestMod.##closure#277
+12  TestMod.#f##0
 13  slot₂/f
 14  (= slot₁/f (new %₁₂ %₁₃))
 15  slot₁/f
@@ -55,21 +55,21 @@ let
     end
 end
 #---------------------
-1   (newvar slot₁)
-2   (= slot₂ (call core.Box))
+1   (= slot₂ (call core.Box))
+2   (newvar slot₁)
 3   --- thunk
-    1   (global TestMod.##closure#278)
+    1   (global TestMod.#f##1)
     2   (call core.svec)
     3   (call core.svec :x)
     4   (call core.svec)
-    5   (call core._structtype TestMod :##closure#278 %₂ %₃ %₄ false 1)
+    5   (call core._structtype TestMod :#f##1 %₂ %₃ %₄ false 1)
     6   (call core._setsuper! %₅ core.Function)
-    7   (const TestMod.##closure#278)
-    8   (= TestMod.##closure#278 %₅)
+    7   (const TestMod.#f##1)
+    8   (= TestMod.#f##1 %₅)
     9   (call core.svec core.Box)
     10  (call core._typebody! %₅ %₉)
     11  (return core.nothing)
-4   TestMod.##closure#278
+4   TestMod.#f##1
 5   (call core.svec %₄ core.Any)
 6   (call core.svec)
 7   (call core.svec %₅ %₆ :($(QuoteNode(:(#= line 3 =#)))))
@@ -81,9 +81,88 @@ end
 9   1
 10  slot₂/x
 11  (call core.setfield! %₁₀ :contents %₉)
-12  TestMod.##closure#278
+12  TestMod.#f##1
 13  slot₂/f
 14  (= slot₁/f (new %₁₂ %₁₃))
 15  slot₁/f
 16  slot₁/f
 17  (return %₁₆)
+
+########################################
+# Function where arguments are captured into a closure
+function f(x)
+    function g()
+        x = 10
+    end
+    g()
+    x
+end
+#---------------------
+1   (method TestMod.f)
+2   --- thunk
+    1   (global TestMod.#f#g##0)
+    2   (call core.svec)
+    3   (call core.svec :x)
+    4   (call core.svec)
+    5   (call core._structtype TestMod :#f#g##0 %₂ %₃ %₄ false 1)
+    6   (call core._setsuper! %₅ core.Function)
+    7   (const TestMod.#f#g##0)
+    8   (= TestMod.#f#g##0 %₅)
+    9   (call core.svec core.Box)
+    10  (call core._typebody! %₅ %₉)
+    11  (return core.nothing)
+3   TestMod.#f#g##0
+4   (call core.svec %₃)
+5   (call core.svec)
+6   (call core.svec %₄ %₅ :($(QuoteNode(:(#= line 2 =#)))))
+7   --- method core.nothing %₆
+    1   10
+    2   (call core.getfield slot₁/x :x)
+    3   (call core.setfield! %₂ :contents %₁)
+    4   (return %₁)
+8   TestMod.f
+9   (call core.Typeof %₈)
+10  (call core.svec %₉ core.Any)
+11  (call core.svec)
+12  (call core.svec %₁₀ %₁₁ :($(QuoteNode(:(#= line 1 =#)))))
+13  --- method core.nothing %₁₂
+    1   (= slot₂/x (call core.Box slot₂/x))
+    2   slot₂/x
+    3   (call core.isdefined %₂ :contents)
+    4   (gotoifnot %₃ label₆)
+    5   (goto label₈)
+    6   (newvar slot₅/x)
+    7   slot₅/x
+    8   (call core.getfield %₂ :contents)
+    9   (call core.Box %₈)
+    10  (call core.setfield! slot₂/x :contents %₉)
+    11  (newvar slot₃)
+    12  TestMod.#f#g##0
+    13  slot₂/g
+    14  (call core.isdefined %₁₃ :contents)
+    15  (gotoifnot %₁₄ label₁₇)
+    16  (goto label₁₉)
+    17  (newvar slot₆/x)
+    18  slot₆/x
+    19  (call core.getfield %₁₃ :contents)
+    20  (= slot₃/g (new %₁₂ %₁₉))
+    21  slot₃/g
+    22  slot₃/g
+    23  slot₃/g
+    24  (call %₂₃)
+    25  slot₂/x
+    26  (call core.isdefined %₂₅ :contents)
+    27  (gotoifnot %₂₆ label₂₉)
+    28  (goto label₃₁)
+    29  (newvar slot₇/x)
+    30  slot₇/x
+    31  (call core.getfield %₂₅ :contents)
+    32  (call core.isdefined %₃₁ :contents)
+    33  (gotoifnot %₃₂ label₃₅)
+    34  (goto label₃₇)
+    35  (newvar slot₄/x)
+    36  slot₄/x
+    37  (call core.getfield %₃₁ :contents)
+    38  (return %₃₇)
+14  (return %₁₂)
+