LoweredCodeUtils does not compose with CassetteOverlay

[Keno]

┌ Error: Failed to revise /home/keno/.julia/dev/InterfaceSpecs/src/engines/property.jl
│   exception =
│    KeyError: key :PropCheckOverlay not found
│    Stacktrace:
│     [1] getindex
│       @ Base ./dict.jl:504 [inlined]
│     [2] identify_framemethod_calls(frame::JuliaInterpreter.Frame)
│       @ LoweredCodeUtils ~/.julia/packages/LoweredCodeUtils/30gbF/src/signatures.jl:168
└ @ Revise ~/.julia/packages/Revise/7HQ7u/src/packagedef.jl:725

while trying to use revise.

@MethodTable PropCheckOverlay

@overlay PropCheckOverlay function forall(prop, T)
    instances = try
        nonoverlay(testinstances, T)
    catch err
        @error "Error during test instance creation" err=err
        rethrow(err)
    end
    for x in instances
        prop(x)()
    end
    return
end

[timholy]

Here's why this happens: typically there's a "registration" of the :method (the Expr(:method, :f)) before defining it:

julia> Meta.lower(Main, :(f() = nothing))
:($(Expr(:thunk, CodeInfo(
    @ none within `top-level scope`
1 ─      $(Expr(:thunk, CodeInfo(
    @ none within `top-level scope`
1 ─     return $(Expr(:method, :f))
)))
│        $(Expr(:method, :f))
│   %3 = Core.Typeof(f)
│   %4 = Core.svec(%3)
│   %5 = Core.svec()
│   %6 = Core.svec(%4, %5, $(QuoteNode(:(#= REPL[1]:1 =#))))
│        $(Expr(:method, :f, :(%6), CodeInfo(
    @ REPL[1]:1 within `none`
1 ─     return nothing
)))
└──      return f
))))

But @overlay isn't doing that:

julia> Meta.lower(Main, :(Base.Experimental.@overlay SomeMT f() = nothing))
:($(Expr(:thunk, CodeInfo(
    @ none within `top-level scope`
1 ─ %1 = Core.Typeof(f)
│   %2 = Core.svec(%1)
│   %3 = Core.svec()
│   %4 = Core.svec(%2, %3, $(QuoteNode(:(#= REPL[4]:1 =#))))
│        $(Expr(:method, :SomeMT, :(%4), CodeInfo(
    @ REPL[4]:1 within `none`
1 ─     return nothing
)))
└──      return nothing
))))

I wonder if the actual bug is in @overlay rather than here? Or is it basically because of the split between @MethodTable and @overlay?

[timholy]

There isn't a full reproducer here, but one option might be to try this diff:

$ git diff
diff --git a/src/signatures.jl b/src/signatures.jl
index 4ae33d5..3318b75 100644
--- a/src/signatures.jl
+++ b/src/signatures.jl
@@ -165,7 +165,7 @@ function identify_framemethod_calls(frame)
             key = stmt.args[1]
             key = normalize_defsig(key, frame)
             if key isa Symbol
-                mi = methodinfos[key]
+                mi = get(methodinfos, key, MethodInfo(1))
                 mi.stop = i
             elseif key isa Expr   # this is a module-scoped call. We don't have to worry about these because they are named
                 continue

and see how far that gets you.

[Keno]

In the variant of :method with more than one arg, the fname is ignored semantically, so it was re-used for the MethodTable argument. If LoweredCodeUtils is making use of it for something other than the mt, it's probably doing something incorrect:

https://github.com/JuliaLang/julia/blob/master/src/interpreter.c#L100-L112

[timholy]

That's essentially what this package is doing, too:

LoweredCodeUtils.jl/src/signatures.jl

Lines 154 to 172 in ef12495

	elseif ismethod1(stmt)
	key = stmt.args[1]
	key = normalize_defsig(key, frame)
	key = key::Symbol
	mi = get(methodinfos, key, nothing)
	if mi === nothing
	methodinfos[key] = MethodInfo(i)
	else
	mi.stop == -1 && (mi.start = i) # advance the statement # unless we've seen the method3
	end
	elseif ismethod3(stmt)
	key = stmt.args[1]
	key = normalize_defsig(key, frame)
	if key isa Symbol
	mi = methodinfos[key]
	mi.stop = i
	elseif key isa Expr # this is a module-scoped call. We don't have to worry about these because they are named
	continue
	end

But it expects to see a 1-arg Expr(:method) first, followed by a 3-arg Expr(:method). That's where the get option above might be the right fix, but I can't really test because I don't have a way of reproducing this yet.

If you can provide a recipe for reproducing it, I can fix it.