UndefVarError with closures defined as named functions

[andyferris]

I have run into an unexpected problem with (a more complicated version of) the following code:

julia> function f(x::Bool)
           if x
               function g(y)
                   return y
               end
               return g
           else
               function g(y)
                   return y + 1
               end
               return g
           end
       end
f (generic function with 1 method)

julia> f(true)
(::var"#g#7") (generic function with 1 method)

julia> f(false)
ERROR: UndefVarError: g not defined
Stacktrace:
 [1] f(::Bool) at ./REPL[1]:11
 [2] top-level scope at REPL[3]:1

I had assumed that closure notation function f(x) ... end was intended to be equivalent to f = (x) -> ... (well, the former may allow you to attach an addition method to the function or closure, also). Lowering has generated the following code:

julia> @code_lowered f(true)
CodeInfo(
1 ─     Core.NewvarNode(:(g))
└──     goto #3 if not x
2 ─     g = %new(Main.:(var"#g#7"))
└──     return g
3 ─     return g
)

It seems to have removed the second closure assignment entirely. I expected it to behave like the following assignment form (even with "long" function notation):

julia> function f(x::Bool)
           if x
               g = function (y)
                   return y
               end
               return g
           else
               g = function (y)
                   return y + 1
               end
               return g
           end
       end
f (generic function with 1 method)

julia> @code_lowered f(true)
CodeInfo(
1 ─     Core.NewvarNode(:(#8))
│       Core.NewvarNode(:(#9))
│       Core.NewvarNode(:(g))
└──     goto #3 if not x
2 ─     #8 = %new(Main.:(var"#8#10"))
│       g = #8
└──     return g
3 ─     #9 = %new(Main.:(var"#9#11"))
│       g = #9
└──     return g
)

[andyferris]

In the second example I note it has created entirely seperate closures, too. At top-level I can do things like

f(x::Missing) = missing
if y
    f(x::Number) = x + 1
else
    f(x::Number) = x
end

and this would end up with one function f with two methods attached.

Inside a function the equivalent would be:

function g(y::Bool)
    f(x::Missing) = missing
    if y
        f(x::Number) = x + 1
    else
        f(x::Number) = x
    end
    return f
end

I'm guessing it's not trivial to ask lowering to do what top-level did (basically, this might be to create two seperate closures with two methods each, and return one or the other depending on the branch).

[andyferris]

So here's an example of the above. incrementer is intended to return a function that always increases it's input by abs(z), but instead of manipulating z it attempts to manipulate the methods.

julia> function incrementer(z::Number)
           g(x::Missing) = missing
           if z > 0 
               g(x::Number) = x + z
           else
               g(x::Number) = x - z
           end
           return g
       end
incrementer (generic function with 1 method)

julia> incrementer(1)
(::var"#g#14"{Int64}) (generic function with 2 methods)

julia> incrementer(-1)
(::var"#g#14"{Int64}) (generic function with 2 methods)

julia> incrementer(-1)(0)
1

julia> incrementer(1)(0)
-1

Again I have no idea if this is even possible to "fix". Perhaps the fix to this issue is to have some kind of lowering error in these cases? Or at least a method replacement warning?

[vtjnash]

As a closure, it is equivalent to:

julia> begin
            struct G <: Function end
            (::G)(x::Missing) = missing
            (::G)(x::Number) = x + z
            (::G)(x::Number) = x - z
           end

julia> function incrementer(z::Number)
           g = G()
           if z > 0 
               g = G()
           else
               g = G()
           end
           return g
       end
end

[andyferris]

Yes, that basically makes sense. I assume lowering basically strips out all the methods it finds and attaches it to the closure. Two questions, though:

• Would I expect to see a method replacement warning for G? (Do we still do those?)
• Given that behavior, is it worth considering the binding issue for g in the second branch in the OP? For example creating two seperate g nodes (and closures) in that particular case? (Perhaps even for all variables defined inside if branches but not assigned or referenced anywhere outside?)

[c42f]

The UndefVarError in the original example seems weird and shouldn't be a thing. I think that's just a bug.

Other than that, I agree it would be nice to have an error when closure methods are repeated, I can't think of a reason that would be a good idea. Unfortunately that can't always be done in lowering — duplicate methods could occur if the user wrote different names for the same types — so if we do it as part of lowering we will miss some cases.

[andyferris]

so if we do it as part of lowering we will miss some cases

Sure. In @vtjnash's pseudo-lowering I'd expect any duplicate method warning would happen when those methods are processed for G rather than by the closure lowering pass itself.

[KristofferC]

Dup of #15602?

[andyferris]

Yes, the OP is precisely #15602 (comment) and the rest of the discussion here mirrors there.

Thanks @KristofferC for pointing me in the right direction.