Skip to content
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.

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

Local variable inside closure signature gets hoisted to wrong scope #56711

Closed
Keno opened this issue Nov 29, 2024 · 0 comments · Fixed by #56712
Closed

Local variable inside closure signature gets hoisted to wrong scope #56711

Keno opened this issue Nov 29, 2024 · 0 comments · Fixed by #56712
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage)

Comments

@Keno
Copy link
Member

Keno commented Nov 29, 2024

julia> function fs()
                  f(lhs::Integer) = 1
                  f(lhs::Integer, rhs::(local x=Integer; x)) = 2
                  return f
              end
fs (generic function with 1 method)

julia> x
Integer

This is supposed to end up as a local of the toplevel thunk, but it becomes a global instead. It's fine if there's only one method definition of the closure:

julia> function fs()
                  f(lhs::Integer, rhs::(local x=Integer; x)) = 2
                  return f
              end
fs (generic function with 1 method)

julia> x
ERROR: UndefVarError: `x` not defined in `Main`
Suggestion: check for spelling errors or missing imports.
@Keno Keno added the compiler:lowering Syntax lowering (compiler front end, 2nd stage) label Nov 29, 2024
Keno added a commit that referenced this issue Nov 29, 2024
When we declare inner methods, e.g. the `f` in

```
function fs()
   f(lhs::Integer) = 1
   f(lhs::Integer, rhs::(local x=Integer; x)) = 2
   return f
end
```

we must hoist the definition of the (appropriately mangled) generic
function `f` to top-level, including all variables that were used
in the signature definition of `f`. This situation is a bit
unique in the language because it uses inner function scope, but gets
executed in toplevel scope. For example, you're not allowed to use a local
of the inner function in the signature definition:

```
julia> function fs()
          local x=Integer
          f(lhs::Integer, rhs::x) = 2
          return f
       end
ERROR: syntax: local variable x cannot be used in closure declaration
Stacktrace:
 [1] top-level scope
   @ REPL[3]:1
```

In particular, the restriction is signature-local:
```
julia> function fs()
          f(rhs::(local x=Integer; x)) = 1
          f(lhs::Integer, rhs::x) = 2
          return f
       end
ERROR: syntax: local variable x cannot be used in closure declaration
Stacktrace:
 [1] top-level scope
   @ REPL[4]:1
```

There's a special intermediate form `moved-local` that gets generated
for this definition. In c6c3d72, this form
stopped getting generated for certain inner methods. I suspect this
happened because of the incorrect assumption that the set of moved
locals is being computed over all signatures, rather than being a
per-signature property.

The result of all of this was that this is one of the few places
where lowering still generated a symbol as the lhs of an assignment
for a global (instead of globalref), because the code that generates
the assignment assumes it's a local, but the later pass doesn't know this.
Because we still retain the code for this from before we started using globalref
consistently, this wasn't generally causing a problems, except possibly leaking
a global (or potentially assigning to a global when this wasn't intended).
However, in follow on work, I want to make use of knowing whether the LHS is a
global or local in lowering, so this was causing me trouble.

Fix all of this by putting back the `moved-local` where it was dropped.

Fixes #56711
@Keno Keno closed this as completed in f1b0b01 Dec 2, 2024
stevengj pushed a commit that referenced this issue Jan 2, 2025
When we declare inner methods, e.g. the `f` in

```
function fs()
   f(lhs::Integer) = 1
   f(lhs::Integer, rhs::(local x=Integer; x)) = 2
   return f
end
```

we must hoist the definition of the (appropriately mangled) generic
function `f` to top-level, including all variables that were used in the
signature definition of `f`. This situation is a bit unique in the
language because it uses inner function scope, but gets executed in
toplevel scope. For example, you're not allowed to use a local of the
inner function in the signature definition:

```
julia> function fs()
          local x=Integer
          f(lhs::Integer, rhs::x) = 2
          return f
       end
ERROR: syntax: local variable x cannot be used in closure declaration
Stacktrace:
 [1] top-level scope
   @ REPL[3]:1
```

In particular, the restriction is signature-local:
```
julia> function fs()
          f(rhs::(local x=Integer; x)) = 1
          f(lhs::Integer, rhs::x) = 2
          return f
       end
ERROR: syntax: local variable x cannot be used in closure declaration
Stacktrace:
 [1] top-level scope
   @ REPL[4]:1
```

There's a special intermediate form `moved-local` that gets generated
for this definition. In c6c3d72, this
form stopped getting generated for certain inner methods. I suspect this
happened because of the incorrect assumption that the set of moved
locals is being computed over all signatures, rather than being a
per-signature property.

The result of all of this was that this is one of the few places where
lowering still generated a symbol as the lhs of an assignment for a
global (instead of globalref), because the code that generates the
assignment assumes it's a local, but the later pass doesn't know this.
Because we still retain the code for this from before we started using
globalref consistently, this wasn't generally causing a problems, except
possibly leaking a global (or potentially assigning to a global when
this wasn't intended). However, in follow on work, I want to make use of
knowing whether the LHS is a global or local in lowering, so this was
causing me trouble.

Fix all of this by putting back the `moved-local` where it was dropped.

Fixes #56711
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant