Functions defined in type definitions break Revise

[thofma]

shell> cat test.jl
mutable struct A
  x::Int

  A(x) = f(x)

  function f(x)
    x^2
  end
end

This works fine with include:

julia> include("test.jl")

julia> A(2)
4

But Revise is choking on this:

julia> Revise.includet("test.jl")
ERROR: UndefVarError: f not defined
top-level scope at /home/bla/test.jl:1
in expression starting at /home/bla/test.jl:1

This is julia 1.5 and master branch of Revise.

[timholy]

It's a LoweredCodeUtils issue (different org so I can't transfer this issue). In case I don't get to it soon, here's progress so far:

 1 f 1 ─       global A
 2 f │         const A
 3 f │   %3  = Core.svec()
 4 f │   %4  = Core.svec(:x)
 5 f │         _1 = Core._structtype(Main, :A, %3, %4, true, 1)
 6 f │         Core._setsuper!(_1, $(QuoteNode(Any)))
 7 f │   %7  = $(Expr(:isdefined, :A))
 8 f └──       goto #6 if not %7
 9 f 2 ─ %9  = A
10 f │   %10 = Core._equiv_typedef(%9, _1)
11 f └──       goto #4 if not %10
12 f 3 ─       _1 = %9
13 f └──       goto #5
14 f 4 ─       A = _1
15 f 5 ┄       goto #7
16 f 6 ─       A = _1
17 f 7 ┄ %17 = _1
18 f │   %18 = Core.svec(Int)
19 f │         Core._typebody!(%17, %18)
20 f │         _2 = ($(QuoteNode(Core.Box)))()
21 f │         global A
22 t │         $(Expr(:method, :A))
23 t │   %23 = Core.Typeof(A)
24 t │   %24 = Core.svec(%23, $(QuoteNode(Any)))
25 t │   %25 = Core.svec()
26 t │   %26 = Core.svec(%24, %25, $(QuoteNode(:(#= /tmp/revise/test.jl:4 =#))))
27 t │   %27 = $(Expr(:copyast, :($(QuoteNode(Any[Symbol("#self#"), :x])))))
28 t │   %28 = $(Expr(:copyast, :($(QuoteNode(Any[Any[Any[Symbol("#self#"), :(Core.Any), 0], Any[:x, :(Core.Any), 8], Any[Symbol("#55#f"), :Any, 32]], Any[], 9, Any[]])))))
29 t │   %29 = ($(QuoteNode(QuoteNode)))(_2)
30 t │   %30 = $(Expr(:copyast, :($(QuoteNode(:(Core.isdefined(%1, :contents)))))))
31 t │   %31 = $(Expr(:copyast, :($(QuoteNode(:(goto %5 if not %2))))))
32 t │   %32 = $(Expr(:copyast, :($(QuoteNode(:(goto %7))))))
33 t │   %33 = $(Expr(:copyast, :($(QuoteNode(Core.NewvarNode(:(_3)))))))
34 t │   %34 = $(Expr(:copyast, :($(QuoteNode(:(_3))))))
35 t │   %35 = $(Expr(:copyast, :($(QuoteNode(:(Core.getfield(%1, :contents)))))))
36 t │   %36 = $(Expr(:copyast, :($(QuoteNode(:((%7)(_2)))))))
37 t │   %37 = $(Expr(:copyast, :($(QuoteNode(:(return %8))))))
38 t │   %38 = Core._expr(:block, %29, %30, %31, %32, %33, %34, %35, %36, %37)
39 t │   %39 = $(Expr(:copyast, :($(QuoteNode(Any[1, 1, 1, 1, 1, 1, 1, 1, 1])))))
40 t │   %40 = $(Expr(:copyast, :($(QuoteNode(Any[Core.LineInfoNode(Main, :none, Symbol("/tmp/revise/test.jl"), 4, 0)])))))
41 t │   %41 = Core._expr(:lambda, %27, %28, %38, %39, %40)
42 t │         $(Expr(:method, :A, :(%26), :(%41)))
43 t │         $(Expr(:thunk, CodeInfo(
    @ none within `top-level scope'
1 ─      global var"#f#6"
│        const var"#f#6"
│   %3 = Core._structtype(Main, Symbol("#f#6"), Core.svec(), Core.svec(), false, 0)
│        var"#f#6" = %3
│        Core._setsuper!(var"#f#6", Core.Function)
│        Core._typebody!(var"#f#6", Core.svec())
└──      return nothing
)))
44 t │   %44 = Core.svec(var"#f#6", $(QuoteNode(Any)))
45 t │   %45 = Core.svec()
46 t │   %46 = Core.svec(%44, %45, $(QuoteNode(:(#= /tmp/revise/test.jl:6 =#))))
47 t │         $(Expr(:method, false, :(%46), CodeInfo(
    @ /tmp/revise/test.jl:7 within `none'
1 ─ %1 = Core.apply_type(Base.Val, 2)
│   %2 = (%1)()
│   %3 = Base.literal_pow(^, x, %2)
└──      return %3
)))
48 f │   %48 = %new(var"#f#6")
49 f │         Core.setfield!(_2, :contents, %48)
50 f └──       return nothing
pc = 22
stmt = :($(Expr(:method, :A)))
Main, pc = 22
node = :($(Expr(:method, :A)))
Main, pc = 23
node = :(($(QuoteNode(Core.Typeof)))(A))
Main
  Typeof(x) in Core at boot.jl:242, pc = 1
node = nothing
Main
  Typeof(x) in Core at boot.jl:242, pc = 2
node = :(($(QuoteNode(isa)))(_J2, $(QuoteNode(Type))))
Main
  Typeof(x) in Core at boot.jl:242, pc = 3
node = :(goto %6 if not %J2)
Main
  Typeof(x) in Core at boot.jl:242, pc = 4
node = :(($(QuoteNode(Core.apply_type)))($(QuoteNode(Type)), _J2))
Main
  Typeof(x) in Core at boot.jl:242, pc = 5
node = :(return %J4)
Main, pc = 24
node = :(($(QuoteNode(Core.svec)))(%J23, $(QuoteNode(Any))))
Main, pc = 25
node = :(($(QuoteNode(Core.svec)))())
Main, pc = 26
node = :(($(QuoteNode(Core.svec)))(%J24, %J25, $(QuoteNode(:(#= /tmp/revise/test.jl:4 =#)))))
Main, pc = 27
node = :($(Expr(:copyast, :($(QuoteNode(Any[Symbol("#self#"), :x]))))))
Main, pc = 28
node = :($(Expr(:copyast, :($(QuoteNode(Any[Any[Any[Symbol("#self#"), :(Core.Any), 0], Any[:x, :(Core.Any), 8], Any[Symbol("#55#f"), :Any, 32]], Any[], 9, Any[]]))))))
Main, pc = 29
node = :(($(QuoteNode(QuoteNode)))(_J2))
slot = _J2
ERROR: UndefVarError: f not defined
Stacktrace:
 [1] includet(mod::Module, file::String)
   @ Revise ~/.julia/dev/Revise/src/packagedef.jl:1004
 [2] includet(file::String)
   @ Revise ~/.julia/dev/Revise/src/packagedef.jl:1009
 [3] top-level scope
   @ REPL[1]:1

caused by: UndefVarError: f not defined
Stacktrace:
  [1] lookup_var(frame::JuliaInterpreter.Frame, slot::JuliaInterpreter.SlotNumber)
    @ JuliaInterpreter ~/.julia/dev/JuliaInterpreter/src/interpret.jl:10
  [2] collect_args(frame::JuliaInterpreter.Frame, call_expr::Expr; isfc::Bool)
    @ JuliaInterpreter ~/.julia/dev/JuliaInterpreter/src/interpret.jl:61
  [3] collect_args
    @ ~/.julia/dev/JuliaInterpreter/src/interpret.jl:135 [inlined]
  [4] evaluate_call_recurse!(recurse::Any, frame::JuliaInterpreter.Frame, call_expr::Expr; enter_generated::Bool)
    @ JuliaInterpreter ~/.julia/dev/JuliaInterpreter/src/interpret.jl:208
  [5] evaluate_call_recurse!
    @ ~/.julia/dev/JuliaInterpreter/src/interpret.jl:202 [inlined]
  [6] eval_rhs(recurse::Any, frame::JuliaInterpreter.Frame, node::Expr)
    @ JuliaInterpreter ~/.julia/dev/JuliaInterpreter/src/interpret.jl:396
  [7] step_expr!(recurse::Any, frame::JuliaInterpreter.Frame, node::Any, istoplevel::Bool)
    @ JuliaInterpreter ~/.julia/dev/JuliaInterpreter/src/interpret.jl:534
  [8] signature(recurse::Any, frame::JuliaInterpreter.Frame, stmt::Any, pc::Int64)
    @ LoweredCodeUtils ~/.julia/packages/LoweredCodeUtils/s03Ba/src/signatures.jl:51
  [9] methoddef!(recurse::Any, signatures::Vector{Any}, frame::JuliaInterpreter.Frame, stmt::Any, pc::Int64; define::Bool)
    @ LoweredCodeUtils ~/.julia/packages/LoweredCodeUtils/s03Ba/src/signatures.jl:477
 [10] methods_by_execution!(recurse::Any, methodinfo::Revise.CodeTrackingMethodInfo, docexprs::Dict{Module, Vector{Expr}}, frame::JuliaInterpreter.Frame, isrequired::Vector{Bool}; mode::Symbol, skip_include::Bool)
    @ Revise ~/.julia/dev/Revise/src/lowered.jl:285
 [11] methods_by_execution!(recurse::Any, methodinfo::Revise.CodeTrackingMethodInfo, docexprs::Dict{Module, Vector{Expr}}, mod::Module, ex::Expr; mode::Symbol, disablebp::Bool, always_rethrow::Bool, kwargs::Base.Iterators.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}})
    @ Revise ~/.julia/dev/Revise/src/lowered.jl:222
 [12] #eval_with_signatures#92
    @ ~/.julia/dev/Revise/src/packagedef.jl:463 [inlined]
 [13] instantiate_sigs!(modexsigs::OrderedCollections.OrderedDict{Module, OrderedCollections.OrderedDict{Revise.RelocatableExpr, Union{Nothing, Vector{Any}}}}; mode::Symbol, kwargs::Base.Iterators.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}})
    @ Revise ~/.julia/dev/Revise/src/packagedef.jl:471
 [14] track(mod::Module, file::String; mode::Symbol, kwargs::Base.Iterators.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}})
    @ Revise ~/.julia/dev/Revise/src/packagedef.jl:889
 [15] includet(mod::Module, file::String)
    @ Revise ~/.julia/dev/Revise/src/packagedef.jl:987
 [16] includet(file::String)
    @ Revise ~/.julia/dev/Revise/src/packagedef.jl:1009
 [17] top-level scope
    @ REPL[1]:1

In the dependency resolution, %29 uses _2 which is set on %20 but that is marked f.

[thofma]

I can move it over to LoweredCodeUtils.jl if you would prefer that.

[timholy]

No need, I'll have to run the tests here to be sure about the fix. Commenting out the kwarg in

Revise.jl/src/lowered.jl

Line 110 in 903863b

lines_required!(isrequired, src, edges; exclude_named_typedefs=mode===:sigs)

fixes your test case but breaks one of the signature tests. (Might be good to see if that change fixes the case you actually care about, though.)

[thofma]

With

    lines_required!(isrequired, src, edges)

the example in OP works.

For the actual case, Revise is not able to track changes. If I change something, I get

┌ Error: Failed to revise /home/bla/blub.jl
│   exception =
│    invalid redefinition of constant MyType

(Although the change was not in the type defnition.)

Do you want an MWE?

[timholy]

This is going to be a bit trickier than I hoped, and might only be fixable on certain Julia versions. See JuliaLang/julia#39778.

[thofma]

Thanks for looking into this.

[timholy]

Hah! There's an incredibly simple workaround: define f before the A constructor. (Also, your constructor should be A(x) = new(f(x)).)

But this still needs to be fixed, so don't close it.

[thofma]

Ah, very nice, I did not try that!

[thofma]

Thanks for looking at this. Much appreciated.

[timholy]

I'm always glad for bug reports. The goal is that there should be no negatives to using Revise. Achieving that is probably impossible, but I think it's important to get close. So keep those bug reports coming!