Ensure TypeVar definitions accompany method defs moved to top level

c42f · c42f · commit 44544df2a4d7 · 2025-02-06T18:03:10.000+10:00
The `K"method_defs"` needs to group a preamble+sets of methods for a
function together so that typevars can be defined in the preamble and
the whole thing moved to top level as one piece, if necessary, during
closure conversion. Previous refactoring had broken that - reinstated it
here.

There's some repetition of the typevars here for certain cases like
kwcall overloads vs the actual function overloads, vs the separate kw
body function - we can't tell during desugaring whether a function will
become a closure or not so it's possible we might need to run these
TypeVar initializations twice.
diff --git a/JuliaLowering/src/desugaring.jl b/JuliaLowering/src/desugaring.jl
@@ -2369,10 +2369,17 @@ function expand_function_generator(ctx, srcref, callex_srcref, func_name, func_n
         push!(gen_arg_types, @ast ctx gen_arg_names[i] "Any"::K"core")
     end
     # Code generator definition
-    gen_func_method_defs = @ast ctx srcref [K"method_defs"
-        gen_name
-        method_def_expr(ctx, srcref, callex_srcref, nothing, SyntaxList(ctx), gen_arg_names,
-                        gen_arg_types, gen_body, nothing)
+    gen_func_method_defs = @ast ctx srcref [K"block"
+        [K"function_decl" gen_name]
+        [K"scope_block"(scope_type=:hard)
+            [K"method_defs"
+                gen_name
+                [K"block"
+                    method_def_expr(ctx, srcref, callex_srcref, nothing, SyntaxList(ctx),
+                                    gen_arg_names, gen_arg_types, gen_body, nothing)
+                ]
+            ]
+        ]
     ]
 
     # Extract non-generated body
@@ -2414,7 +2421,7 @@ function expand_function_generator(ctx, srcref, callex_srcref, func_name, func_n
         split_generated(ctx, body, false)
     ]
 
-    return gen_name, gen_func_method_defs, nongen_body
+    return gen_func_method_defs, nongen_body
 end
 
 # Generate a method for every number of allowed optional arguments
@@ -2472,8 +2479,8 @@ function scope_nest(ctx, names, values, body)
 end
 
 # Generate body function and `Core.kwcall` overloads for functions taking keywords.
-function keyword_function_defs(ctx, srcref, callex_srcref, name_str,
-                               typevar_names, typevar_stmts, arg_names,
+function keyword_function_defs(ctx, srcref, callex_srcref, name_str, typevar_names,
+                               typevar_stmts, new_typevar_stmts, arg_names,
                                arg_types, has_slurp, first_default, arg_defaults,
                                keywords, body, ret_var)
     mangled_name = let n = isnothing(name_str) ? "_" : name_str
@@ -2700,23 +2707,29 @@ function keyword_function_defs(ctx, srcref, callex_srcref, name_str,
     check_all_typevars_used(body_arg_types, typevar_names, typevar_stmts)
 
     kw_func_method_defs = @ast ctx srcref [K"block"
-        [K"method_defs"
-            body_func_name
-            [K"block"
-                # TODO: nkw
-                method_def_expr(ctx, srcref, callex_srcref, "nothing"::K"core",
-                                typevar_names, body_arg_names, body_arg_types,
-                                [K"block"
-                                    [K"meta" "nkw"::K"Symbol" numchildren(keywords)::K"Integer"]
-                                    body
-                                ],
-                                ret_var)
+        [K"function_decl" body_func_name]
+        [K"scope_block"(scope_type=:hard)
+            [K"method_defs"
+                body_func_name
+                [K"block"
+                    new_typevar_stmts...
+                    method_def_expr(ctx, srcref, callex_srcref, "nothing"::K"core",
+                                    typevar_names, body_arg_names, body_arg_types,
+                                    [K"block"
+                                        [K"meta" "nkw"::K"Symbol" numchildren(keywords)::K"Integer"]
+                                        body
+                                    ],
+                                    ret_var)
+                ]
             ]
         ]
-        [K"method_defs"
-            "nothing"::K"core"
-            [K"block"
-                kwcall_method_defs...
+        [K"scope_block"(scope_type=:hard)
+            [K"method_defs"
+                "nothing"::K"core"
+                [K"block"
+                    new_typevar_stmts...
+                    kwcall_method_defs...
+                ]
             ]
         ]
     ]
@@ -2737,7 +2750,7 @@ function keyword_function_defs(ctx, srcref, callex_srcref, name_str,
         ]
     end
 
-    body_func_name, kw_func_method_defs, body_for_positional_args_only
+    kw_func_method_defs, body_for_positional_args_only
 end
 
 # Check valid identifier/function names
@@ -2936,27 +2949,27 @@ function expand_function_def(ctx, ex, docs, rewrite_call=identity, rewrite_body=
         ]
     end
 
-    gen_func_name = nothing
     gen_func_method_defs = nothing
     if is_generated(body)
-        gen_func_name, gen_func_method_defs, body =
+        gen_func_method_defs, body =
             expand_function_generator(ctx, ex, callex, name, name_str, body, arg_names, typevar_names)
 
     end
 
     if isnothing(keywords)
-        body_func_name, kw_func_method_defs = (nothing, nothing)
-        # NB: This check seems good as it statically catches any useless
-        # typevars which can't be inferred. However it wasn't previously an
-        # error so we might need to reduce it to a warning?
+        kw_func_method_defs = nothing
+        # NB: The following check seems good as it statically catches any useless
+        # static parameters which can't be bound during method invocation.
+        # However it wasn't previously an error so we might need to reduce it
+        # to a warning?
         check_all_typevars_used(arg_types, typevar_names, typevar_stmts)
         main_typevar_names = typevar_names
     else
         # Rewrite `body` here so that the positional-only versions dispatch there.
-        body_func_name, kw_func_method_defs, body =
+        kw_func_method_defs, body =
             keyword_function_defs(ctx, ex, callex, name_str, typevar_names, typevar_stmts,
-                                  arg_names, arg_types, has_slurp, first_default, arg_defaults,
-                                  keywords, body, ret_var)
+                                  new_typevar_stmts, arg_names, arg_types, has_slurp,
+                                  first_default, arg_defaults, keywords, body, ret_var)
         # The main function (but without keywords) needs its typevars trimmed,
         # as some of them may be for the keywords only.
         main_typevar_names = trim_used_typevars(ctx, arg_types, typevar_names, typevar_stmts)
@@ -2993,28 +3006,22 @@ function expand_function_def(ctx, ex, docs, rewrite_call=identity, rewrite_body=
     end
 
     @ast ctx ex [K"block"
-        if !isnothing(gen_func_name)
-            [K"function_decl"(gen_func_name) gen_func_name]
-        end
-        if !isnothing(body_func_name)
-            [K"function_decl"(body_func_name) body_func_name]
-        end
         if !isnothing(bare_func_name)
+            # Need the main function type created here before running any code
+            # in kw_func_method_defs
             [K"function_decl"(bare_func_name) bare_func_name]
         end
+        gen_func_method_defs
+        kw_func_method_defs
         [K"scope_block"(scope_type=:hard)
-            [K"block"
-                new_typevar_stmts...
-                gen_func_method_defs
-                kw_func_method_defs
-                [K"method_defs"
-                    isnothing(bare_func_name) ? "nothing"::K"core" : bare_func_name
-                    [K"block"
-                        if !isnothing(method_table_val)
-                            [K"=" method_table method_table_val]
-                        end
-                        method_stmts...
-                    ]
+            [K"method_defs"
+                isnothing(bare_func_name) ? "nothing"::K"core" : bare_func_name
+                [K"block"
+                    new_typevar_stmts...
+                    if !isnothing(method_table_val)
+                        [K"=" method_table method_table_val]
+                    end
+                    method_stmts...
                 ]
             ]
         ]
diff --git a/JuliaLowering/test/closures_ir.jl b/JuliaLowering/test/closures_ir.jl
@@ -572,12 +572,12 @@ let y = y_init
 end
 #---------------------
 1   TestMod.y_init
-2   (call core.svec :y)
+2   (call core.svec :#f_kw_closure#0)
 3   (call core.svec true)
-4   (call JuliaLowering.eval_closure_type TestMod :##f_kw_closure#0##0 %₂ %₃)
-5   (call core.svec :#f_kw_closure#0)
+4   (call JuliaLowering.eval_closure_type TestMod :#f_kw_closure##0 %₂ %₃)
+5   (call core.svec :y)
 6   (call core.svec true)
-7   (call JuliaLowering.eval_closure_type TestMod :#f_kw_closure##0 %₅ %₆)
+7   (call JuliaLowering.eval_closure_type TestMod :##f_kw_closure#0##0 %₅ %₆)
 8   TestMod.##f_kw_closure#0##0
 9   TestMod.X
 10  TestMod.#f_kw_closure##0
@@ -619,15 +619,15 @@ end
 23  (= slot₂/#f_kw_closure#0 (call core.Box))
 24  slot₁/y
 25  (call core.setfield! %₂₄ :contents %₁)
-26  TestMod.##f_kw_closure#0##0
-27  slot₁/y
+26  TestMod.#f_kw_closure##0
+27  slot₂/#f_kw_closure#0
 28  (new %₂₆ %₂₇)
-29  slot₂/#f_kw_closure#0
-30  (call core.setfield! %₂₉ :contents %₂₈)
-31  TestMod.#f_kw_closure##0
-32  slot₂/#f_kw_closure#0
-33  (new %₃₁ %₃₂)
-34  (= slot₃/f_kw_closure %₃₃)
+29  (= slot₃/f_kw_closure %₂₈)
+30  TestMod.##f_kw_closure#0##0
+31  slot₁/y
+32  (new %₃₀ %₃₁)
+33  slot₂/#f_kw_closure#0
+34  (call core.setfield! %₃₃ :contents %₃₂)
 35  (call core.typeof core.kwcall)
 36  TestMod.#f_kw_closure##0
 37  (call core.svec %₃₅ core.NamedTuple %₃₆)
diff --git a/JuliaLowering/test/functions_ir.jl b/JuliaLowering/test/functions_ir.jl
@@ -955,8 +955,8 @@ function f_kw_simple(a::Int=1, b::Float64=1.0; x::Char='a', y::Bool=true)
     (a, b, x, y)
 end
 #---------------------
-1   (method TestMod.#f_kw_simple#0)
-2   (method TestMod.f_kw_simple)
+1   (method TestMod.f_kw_simple)
+2   (method TestMod.#f_kw_simple#0)
 3   TestMod.#f_kw_simple#0
 4   (call core.Typeof %₃)
 5   TestMod.Char
@@ -1091,8 +1091,8 @@ function f_kw_slurp_simple(; all_kws...)
     all_kws
 end
 #---------------------
-1   (method TestMod.#f_kw_slurp_simple#0)
-2   (method TestMod.f_kw_slurp_simple)
+1   (method TestMod.f_kw_slurp_simple)
+2   (method TestMod.#f_kw_slurp_simple#0)
 3   TestMod.#f_kw_slurp_simple#0
 4   (call core.Typeof %₃)
 5   (call top.pairs core.NamedTuple)
@@ -1143,8 +1143,8 @@ function f_kw_slurp(; x=x_default, non_x_kws...)
     all_kws
 end
 #---------------------
-1   (method TestMod.#f_kw_slurp#0)
-2   (method TestMod.f_kw_slurp)
+1   (method TestMod.f_kw_slurp)
+2   (method TestMod.#f_kw_slurp#0)
 3   TestMod.#f_kw_slurp#0
 4   (call core.Typeof %₃)
 5   (call top.pairs core.NamedTuple)
@@ -1212,8 +1212,8 @@ function f_kw_sparams(x::X; a::A=a_def, b::X=b_def) where {X,A}
     (X,A)
 end
 #---------------------
-1   (method TestMod.#f_kw_sparams#0)
-2   (method TestMod.f_kw_sparams)
+1   (method TestMod.f_kw_sparams)
+2   (method TestMod.#f_kw_sparams#0)
 3   (= slot₂/X (call core.TypeVar :X))
 4   (= slot₁/A (call core.TypeVar :A))
 5   TestMod.#f_kw_sparams#0
@@ -1236,16 +1236,18 @@ end
     3   static_parameter₂
     4   (call core.tuple %₂ %₃)
     5   (return %₄)
-19  (call core.typeof core.kwcall)
-20  TestMod.f_kw_sparams
-21  (call core.Typeof %₂₀)
-22  slot₂/X
-23  (call core.svec %₁₉ core.NamedTuple %₂₁ %₂₂)
-24  slot₂/X
-25  (call core.svec %₂₄)
-26  SourceLocation::1:10
-27  (call core.svec %₂₃ %₂₅ %₂₆)
-28  --- method core.nothing %₂₇
+19  (= slot₄/X (call core.TypeVar :X))
+20  (= slot₃/A (call core.TypeVar :A))
+21  (call core.typeof core.kwcall)
+22  TestMod.f_kw_sparams
+23  (call core.Typeof %₂₂)
+24  slot₄/X
+25  (call core.svec %₂₁ core.NamedTuple %₂₃ %₂₄)
+26  slot₄/X
+27  (call core.svec %₂₆)
+28  SourceLocation::1:10
+29  (call core.svec %₂₅ %₂₇ %₂₈)
+30  --- method core.nothing %₂₉
     slots: [slot₁/#self#(!read) slot₂/kws slot₃/#self# slot₄/x slot₅/kwtmp slot₆/a(!read) slot₇/b(!read)]
     1   (newvar slot₆/a)
     2   (newvar slot₇/b)
@@ -1282,23 +1284,25 @@ end
     33  TestMod.#f_kw_sparams#0
     34  (call %₃₃ %₁₀ %₂₅ slot₃/#self# slot₄/x)
     35  (return %₃₄)
-29  TestMod.f_kw_sparams
-30  (call core.Typeof %₂₉)
-31  slot₂/X
-32  (call core.svec %₃₀ %₃₁)
-33  slot₂/X
-34  (call core.svec %₃₃)
-35  SourceLocation::1:10
-36  (call core.svec %₃₂ %₃₄ %₃₅)
-37  --- method core.nothing %₃₆
+31  (= slot₆/X (call core.TypeVar :X))
+32  (= slot₅/A (call core.TypeVar :A))
+33  TestMod.f_kw_sparams
+34  (call core.Typeof %₃₃)
+35  slot₆/X
+36  (call core.svec %₃₄ %₃₅)
+37  slot₆/X
+38  (call core.svec %₃₇)
+39  SourceLocation::1:10
+40  (call core.svec %₃₆ %₃₈ %₃₉)
+41  --- method core.nothing %₄₀
     slots: [slot₁/#self# slot₂/x]
     1   TestMod.#f_kw_sparams#0
     2   TestMod.a_def
     3   TestMod.b_def
     4   (call %₁ %₂ %₃ slot₁/#self# slot₂/x)
     5   (return %₄)
-38  TestMod.f_kw_sparams
-39  (return %₃₈)
+42  TestMod.f_kw_sparams
+43  (return %₄₂)
 
 ########################################
 # Error: Static parameter which is unused in keyword body arg types
@@ -1358,8 +1362,8 @@ end
     generator_code(x,y)
 end
 #---------------------
-1   (method TestMod.#f_only_generated@generator#0)
-2   (method TestMod.f_only_generated)
+1   (method TestMod.f_only_generated)
+2   (method TestMod.#f_only_generated@generator#0)
 3   TestMod.#f_only_generated@generator#0
 4   (call core.Typeof %₃)
 5   (call core.svec %₄ JuliaLowering.MacroContext core.Any core.Any core.Any)
@@ -1399,8 +1403,8 @@ function f_partially_generated(x, y)
     (nongen_stuff, maybe_gen_stuff)
 end
 #---------------------
-1   (method TestMod.#f_partially_generated@generator#0)
-2   (method TestMod.f_partially_generated)
+1   (method TestMod.f_partially_generated)
+2   (method TestMod.#f_partially_generated@generator#0)
 3   TestMod.#f_partially_generated@generator#0
 4   (call core.Typeof %₃)
 5   (call core.svec %₄ JuliaLowering.MacroContext core.Any core.Any core.Any)
