Desugaring of remaining let syntax forms

c42f · c42f · commit e5dd569e2517 · 2025-01-30T22:26:48.000+10:00
`let` was done very early in prototyping so only understood a very basic
left hand side.  Fix this to now support the remaining fancy syntax of
type declarations, tuple unpacking and function definitions.

There's a couple of variations in the lowering here vs the flisp
lowering which I think are bug fixes:

    let f() = y
        local y   # &lt;- f does not capture this `y`
    end

    let x::T = rhs
        local T   # x is of type `T` from outside the `let`, not this one.
    end

Also some cleanup
* Remove local_def in favour of using `local` and a separate
  `always_defined` intermediate form
* Some cleanup in linearization.
diff --git a/JuliaLowering/src/ast.jl b/JuliaLowering/src/ast.jl
@@ -594,10 +594,45 @@ function is_simple_atom(ctx, ex)
     is_literal(k) || k == K"Symbol" || k == K"Value" || is_ssa(ctx, ex) || is_core_nothing(ex)
 end
 
+function is_identifier_like(ex)
+    k = kind(ex)
+    k == K"Identifier" || k == K"BindingId" || k == K"Placeholder"
+end
+
 function decl_var(ex)
     kind(ex) == K"::" ? ex[1] : ex
 end
 
+# Given the signature of a `function`, return the symbol that will ultimately
+# be assigned to in local/global scope, if any.
+function assigned_function_name(ex)
+    while kind(ex) == K"where"
+        # f() where T
+        ex = ex[1]
+    end
+    if kind(ex) == K"::" && numchildren(ex) == 2
+        # f()::T
+        ex = ex[1]
+    end
+    if kind(ex) != K"call"
+        throw(LoweringError(ex, "Expected call syntax in function signature"))
+    end
+    ex = ex[1]
+    if kind(ex) == K"curly"
+        # f{T}()
+        ex = ex[1]
+    end
+    if kind(ex) == K"::" || kind(ex) == K"."
+        # (obj::CallableType)(args)
+        # A.b.c(args)
+        nothing
+    elseif is_identifier_like(ex)
+        ex
+    else
+        throw(LoweringError(ex, "Unexpected name in function signature"))
+    end
+end
+
 # Remove empty parameters block, eg, in the arg list of `f(x, y;)`
 function remove_empty_parameters(args)
     i = length(args)
diff --git a/JuliaLowering/src/desugaring.jl b/JuliaLowering/src/desugaring.jl
@@ -20,11 +20,6 @@ end
 
 #-------------------------------------------------------------------------------
 
-function is_identifier_like(ex)
-    k = kind(ex)
-    k == K"Identifier" || k == K"BindingId" || k == K"Placeholder"
-end
-
 # Return true when `x` and `y` are "the same identifier", but also works with
 # bindings (and hence ssa vars). See also `is_identifier_like()`
 function is_same_identifier_like(ex::SyntaxTree, y::SyntaxTree)
@@ -1394,22 +1389,71 @@ function expand_let(ctx, ex)
         elseif kb == K"=" && numchildren(binding) == 2
             lhs = binding[1]
             rhs = binding[2]
-            if is_sym_decl(lhs)
+            kl = kind(lhs)
+            if kl == K"Identifier" || kl == K"BindingId"
                 blk = @ast ctx binding [K"block"
                     tmp := rhs
                     [K"scope_block"(ex, scope_type=scope_type)
-                        # TODO: Use single child for scope_block?
-                        [K"local_def"(lhs) lhs] # TODO: Use K"local" with attr?
-                        [K"="(rhs)
-                            decl_var(lhs)
-                            tmp
-                        ]
+                        [K"local"(lhs) lhs]
+                        [K"always_defined" lhs]
+                        [K"="(binding) lhs tmp]
+                        blk
+                    ]
+                ]
+            elseif kl == K"::"
+                var = lhs[1]
+                if !(kind(var) in KSet"Identifier BindingId")
+                    throw(LoweringError(var, "Invalid assignment location in let syntax"))
+                end
+                blk = @ast ctx binding [K"block"
+                    tmp := rhs
+                    type := lhs[2]
+                    [K"scope_block"(ex, scope_type=scope_type)
+                        [K"local"(lhs) [K"::" var type]]
+                        [K"always_defined" var]
+                        [K"="(binding) var tmp]
+                        blk
+                    ]
+                ]
+            elseif kind(lhs) == K"tuple"
+                lhs_locals = SyntaxList(ctx)
+                foreach_lhs_var(lhs) do var
+                    push!(lhs_locals, @ast ctx var [K"local" var])
+                    push!(lhs_locals, @ast ctx var [K"always_defined" var])
+                end
+                blk = @ast ctx binding [K"block"
+                    tmp := rhs
+                    [K"scope_block"(ex, scope_type=scope_type)
+                        lhs_locals...
+                        [K"="(binding) lhs tmp]
                         blk
                     ]
                 ]
             else
-                TODO("Functions and multiple assignment")
+                throw(LoweringError(lhs, "Invalid assignment location in let syntax"))
             end
+        elseif kind(binding) == K"function"
+            sig = binding[1]
+            func_name = assigned_function_name(sig)
+            if isnothing(func_name)
+                # Some valid function syntaxes define methods on existing types and
+                # don't really make sense with let:
+                #    let A.f() = 1 ... end
+                #    let (obj::Callable)() = 1 ... end
+                throw(LoweringError(sig, "Function signature does not define a local function name"))
+            end
+            blk = @ast ctx binding [K"block"
+                [K"scope_block"(ex, scope_type=scope_type)
+                    [K"local"(func_name) func_name]
+                    [K"always_defined" func_name]
+                    binding
+                    [K"scope_block"(ex, scope_type=scope_type)
+                        # The inside of the block is isolated from the closure,
+                        # which itself can only capture values from the outside.
+                        blk
+                    ]
+                ]
+            ]
         else
             throw(LoweringError(binding, "Invalid binding in let"))
             continue
@@ -1801,10 +1845,14 @@ end
 
 function foreach_lhs_var(f::Function, ex)
     k = kind(ex)
-    if k == K"Identifier"
+    if k == K"Identifier" || k == K"BindingId"
         f(ex)
     elseif k == K"Placeholder"
         # Ignored
+    elseif k == K"tuple"
+        for e in children(ex)
+            foreach_lhs_var(f, e)
+        end
     else
         TODO(ex, "LHS vars")
     end
@@ -3091,7 +3139,8 @@ function expand_abstract_or_primitive_type(ctx, ex)
     @ast ctx ex [K"block"
         [K"scope_block"(scope_type=:hard)
             [K"block"
-                [K"local_def" name]
+                [K"local" name]
+                [K"always_defined" name]
                 typevar_stmts...
                 [K"="
                     newtype_var
@@ -3624,7 +3673,8 @@ function expand_struct_def(ctx, ex, docs)
             [K"block"
                 [K"global" global_struct_name]
                 [K"const" global_struct_name]
-                [K"local_def" struct_name]
+                [K"local" struct_name]
+                [K"always_defined" struct_name]
                 typevar_stmts...
                 [K"="
                     newtype_var
diff --git a/JuliaLowering/src/kinds.jl b/JuliaLowering/src/kinds.jl
@@ -21,6 +21,7 @@ function _register_kinds()
             "meta"
             # TODO: Use `meta` for inbounds and loopinfo etc?
             "inbounds"
+            "boundscheck"
             "inline"
             "noinline"
             "loopinfo"
@@ -30,6 +31,11 @@ function _register_kinds()
             "opaque_closure"
             # Test whether a variable is defined
             "isdefined"
+            # [K"throw_undef_if_not" var cond]
+            # This form is used internally in Core.Compiler but might be
+            # emitted by packages such as Diffractor. In principle it needs to
+            # be passed through lowering in a similar way to `isdefined`
+            "throw_undef_if_not"
             # named labels for `@label` and `@goto`
             "symbolic_label"
             # Goto named label
@@ -53,8 +59,11 @@ function _register_kinds()
             # Various heads harvested from flisp lowering.
             # (TODO: May or may not need all these - assess later)
             "break_block"
+            # Like block, but introduces a lexical scope; used during scope resolution.
             "scope_block"
-            "local_def" # TODO: Replace with K"local" plus BindingFlags attribute?
+            # [K"always_defined" x] is an assertion that variable `x` is assigned before use
+            # ('local-def in flisp implementation is K"local" plus K"always_defined"
+            "always_defined"
             "_while"
             "_do_while"
             "_typevars" # used for supplying already-allocated `TypeVar`s to `where`
diff --git a/JuliaLowering/src/linear_ir.jl b/JuliaLowering/src/linear_ir.jl
@@ -268,17 +268,18 @@ function _actually_return(ctx, ex)
 end
 
 function emit_return(ctx, srcref, ex)
+    # todo: Mark implicit returns
     if isnothing(ex)
         return
     elseif isempty(ctx.handler_token_stack)
         _actually_return(ctx, ex)
         return
     end
-    # FIXME: What's this !is_ssa(ctx, ex) here about?
+    # TODO: What's this !is_ssa(ctx, ex) here about?
     x = if is_simple_atom(ctx, ex) && !(is_ssa(ctx, ex) && !isempty(ctx.finally_handlers))
         ex
     elseif !isempty(ctx.finally_handlers)
-        # TODO: Why does flisp lowering create a mutable variable here even
+        # todo: Why does flisp lowering create a mutable variable here even
         # though we don't mutate it?
         # tmp = ssavar(ctx, srcref, "returnval_via_finally") # <- can we use this?
         tmp = new_local_binding(ctx, srcref, "returnval_via_finally")
@@ -293,8 +294,6 @@ function emit_return(ctx, srcref, ex)
         emit(ctx, @ast ctx srcref [K"leave" ctx.handler_token_stack...])
         _actually_return(ctx, x)
     end
-    # Should we return `x` here? The flisp code does, but that doesn't seem
-    # useful as any returned value cannot be used?
     return nothing
 end
 
@@ -563,12 +562,7 @@ function compile(ctx::LinearIRContext, ex, needs_value, in_tail_pos)
     k = kind(ex)
     if k == K"BindingId" || is_literal(k) || k == K"quote" || k == K"inert" ||
             k == K"top" || k == K"core" || k == K"Value" || k == K"Symbol" ||
-            k == K"Placeholder" || k == K"SourceLocation"
-        # TODO: other kinds: copyast $ globalref thismodule cdecl stdcall fastcall thiscall llvmcall
-        if needs_value && k == K"Placeholder"
-            # TODO: ensure outterref, globalref work here
-            throw(LoweringError(ex, "all-underscore identifiers are write-only and their values cannot be used in expressions"))
-        end
+            k == K"SourceLocation"
         if in_tail_pos
             emit_return(ctx, ex)
         elseif needs_value
@@ -579,6 +573,14 @@ function compile(ctx::LinearIRContext, ex, needs_value, in_tail_pos)
             end
             nothing
         end
+    elseif k == K"Placeholder"
+        if needs_value
+            throw(LoweringError(ex, "all-underscore identifiers are write-only and their values cannot be used in expressions"))
+        end
+        nothing
+    elseif k == K"TOMBSTONE"
+        @chk !needs_value (ex,"TOMBSTONE encountered in value position")
+        nothing
     elseif k == K"call" || k == K"new" || k == K"splatnew" || k == K"foreigncall" ||
             k == K"new_opaque_closure"
         # TODO k ∈ cfunction cglobal
@@ -704,9 +706,6 @@ function compile(ctx::LinearIRContext, ex, needs_value, in_tail_pos)
         else
             nothing
         end
-    elseif k == K"TOMBSTONE"
-        @chk !needs_value (ex,"TOMBSTONE encountered in value position")
-        nothing
     elseif k == K"if" || k == K"elseif"
         @chk numchildren(ex) <= 3
         has_else = numchildren(ex) > 2
@@ -828,7 +827,8 @@ function compile(ctx::LinearIRContext, ex, needs_value, in_tail_pos)
         end
         emit(ctx, ex)
         nothing
-    elseif k == K"isdefined" || k == K"captured_local" # TODO || k == K"throw_undef_if_not" (See upstream #53875)
+    elseif k == K"isdefined" || k == K"captured_local" || k == K"throw_undef_if_not" ||
+            k == K"boundscheck"
         if in_tail_pos
             emit_return(ctx, ex)
         elseif needs_value
diff --git a/JuliaLowering/src/scope_analysis.jl b/JuliaLowering/src/scope_analysis.jl
@@ -36,7 +36,7 @@ function _find_scope_vars!(ctx, assignments, locals, destructured_args, globals,
     elseif is_leaf(ex) || is_quoted(k) ||
             k in KSet"scope_block lambda module toplevel"
         return
-    elseif k == K"local" || k == K"local_def"
+    elseif k == K"local"
         if getmeta(ex, :is_destructured_arg, false)
             push!(destructured_args, ex[1])
         else
@@ -433,7 +433,7 @@ function _resolve_scopes(ctx, ex::SyntaxTree)
             end
         end
         ex_out
-    elseif k == K"local_def"
+    elseif k == K"always_defined"
         id = lookup_var(ctx, NameKey(ex[1]))
         update_binding!(ctx, id; is_always_defined=true)
         makeleaf(ctx, ex, K"TOMBSTONE")
diff --git a/JuliaLowering/test/branching.jl b/JuliaLowering/test/branching.jl
@@ -9,18 +9,6 @@ Base.eval(test_mod, quote
     using JuliaSyntax
 end)
 
-Base.eval(test_mod, quote
-    function var"@label"(__context__::JuliaLowering.MacroContext, ex)
-        @chk kind(ex) == JuliaSyntax.K"Identifier"
-        @ast __context__ ex ex=>JuliaSyntax.K"symbolic_label"
-    end
-
-    function var"@goto"(__context__::JuliaLowering.MacroContext, ex)
-        @chk kind(ex) == JuliaSyntax.K"Identifier"
-        @ast __context__ ex ex=>JuliaSyntax.K"symbolic_goto"
-    end
-end)
-
 #-------------------------------------------------------------------------------
 @testset "Tail position" begin
 
diff --git a/JuliaLowering/test/scopes.jl b/JuliaLowering/test/scopes.jl
@@ -17,13 +17,47 @@ end
 """) == (1, 2)
 
 JuliaLowering.include_string(test_mod, """
-    x = 101
-    y = 202
+x = 101
+y = 202
 """)
 @test test_mod.x == 101
 @test test_mod.y == 202
 @test JuliaLowering.include_string(test_mod, "x + y") == 303
 
+@test JuliaLowering.include_string(test_mod, """
+begin
+    local x = 1
+    local x = 2
+    let (x,y) = (:x,:y)
+        (y,x)
+    end
+end
+""") === (:y,:x)
+
+# Types on left hand side of type decls refer to the outer scope
+# (In the flisp implementation they refer to the inner scope, but this seems
+# like a bug.)
+@test JuliaLowering.include_string(test_mod, """
+let x::Int = 10.0
+    local Int = Float64
+    x
+end
+""") === 10
+
+# Closures in let syntax can only capture values from the outside
+# (In the flisp implementation it captures from inner scope, but this is
+# inconsistent with let assignment where the rhs refers to the outer scope and
+# thus seems like a bug.)
+@test JuliaLowering.include_string(test_mod, """
+begin
+    local y = :outer_y
+    let f() = y
+        local y = :inner_y
+        f()
+    end
+end
+""") === :outer_y
+
 # wrap expression in scope block of `scope_type`
 function wrapscope(ex, scope_type)
     g = JuliaLowering.ensure_attributes(ex._graph, scope_type=Symbol)
diff --git a/JuliaLowering/test/scopes_ir.jl b/JuliaLowering/test/scopes_ir.jl
