timholy
Revising methods with external method tables fails
The error is from JuliaInterpreter:
┌ Error: Failed to revise /home/tim/Julia/pkg/CUDA/src/device/intrinsics/dynamic_parallelism.jl
│ exception =
│ TypeError: in typeassert, expected Union{Nothing, Bool, Symbol}, got a value of type Core.MethodTable
│ Stacktrace:
│ [1] identify_framemethod_calls(frame::JuliaInterpreter.Frame)
│ @ LoweredCodeUtils ~/Julia/depot/packages/LoweredCodeUtils/jZY56/src/signatures.jl:176
└ @ Revise ~/Julia/depot/packages/Revise/OgnOk/src/packagedef.jl:709
Some info from around the error:
msrc = CodeInfo(
@ /home/tim/Julia/pkg/CUDA/src/device/intrinsics/dynamic_parallelism.jl:25 within `none`
1 ─ res = cudaDeviceSynchronize()
│ @ /home/tim/Julia/pkg/CUDA/src/device/intrinsics/dynamic_parallelism.jl:26 within `none`
│ %2 = res != CUDA.cudaSuccess
└── goto #3 if not %2
@ /home/tim/Julia/pkg/CUDA/src/device/intrinsics/dynamic_parallelism.jl:27 within `none`
2 ─ %4 = CUDA.CuDeviceError(res)
│ %5 = CUDA.throw_device_cuerror(%4)
└── return %5
3 ─ return nothing
)
stmt = :($(Expr(:method, Core.MethodTable(...), %J4, CodeInfo(...))))
key = # 167 methods:
[1] atan(x::Float32, y::Float32) in CUDA at /home/tim/Julia/pkg/CUDA/src/device/intrinsics/math.jl:70
...
[167] mul24(x::Int32, y::Int32) in CUDA at /home/tim/Julia/pkg/CUDA/src/device/intrinsics/math.jl:329
key is CUDA.jl's external method table with all device-specific functions.
This is on Julia 1.7, with Revise 3.1.19 and JuliaInterpreter 0.8.21.
As I'm sure you can imagine, a reproducer would be lovely. What version of LoweredCodeUtils are you on? I assume that line is https://github.com/JuliaDebug/LoweredCodeUtils.jl/blob/ff8998f6151bce37bbf6a82ce72622e692fd8df3/src/signatures.jl#L175. Obviously I can extend the Union to include Core.MethodTable, but it would be nice to have confidence that everything else is right.
Yeah, sorry, was going to put up some additional details, but wanted to make a note as soon as I ran into the issue :slightly_smiling_face:
I was using LoweredCodeUtils v2.1.2, so yeah that was the line.
The basic reproducer is as follows:
Base.Experimental.@MethodTable(method_table)
foo() = 1
Base.Experimental.@overlay method_table foo() = 2
bar() = foo()
This fails to Revise.includet, but with a different failure:
KeyError: key :method_table not found
Stacktrace:
[1] getindex(h::Dict{Symbol, LoweredCodeUtils.MethodInfo}, key::Symbol)
@ Base ./dict.jl:481
[2] identify_framemethod_calls(frame::JuliaInterpreter.Frame)
@ LoweredCodeUtils ~/.julia/packages/LoweredCodeUtils/jZY56/src/signatures.jl:168
[3] rename_framemethods!
@ ~/.julia/packages/LoweredCodeUtils/jZY56/src/signatures.jl:307 [inlined]
[4] 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.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}})
@ Revise ~/.julia/packages/Revise/OgnOk/src/lowered.jl:191
[5] #eval_with_signatures#92
@ ~/.julia/packages/Revise/OgnOk/src/packagedef.jl:463 [inlined]
[6] instantiate_sigs!(modexsigs::OrderedCollections.OrderedDict{Module, OrderedCollections.OrderedDict{Revise.RelocatableExpr, Union{Nothing, Vector{Any}}}}; mode::Symbol, kwargs::Base.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}})
@ Revise ~/.julia/packages/Revise/OgnOk/src/packagedef.jl:471
[7] track(mod::Module, file::String; mode::Symbol, kwargs::Base.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}})
@ Revise ~/.julia/packages/Revise/OgnOk/src/packagedef.jl:899
[8] includet(mod::Module, file::String)
@ Revise ~/.julia/packages/Revise/OgnOk/src/packagedef.jl:994
[9] includet(file::String)
@ Revise ~/.julia/packages/Revise/OgnOk/src/packagedef.jl:1016
The reason that the error is slightly different, is that in CUDA.jl/GPUCompiler.jl we use a wrapper macro that interpolates the method_table in the AST, so it isn't a Symbol but a MethodTable directly:
Base.Experimental.@MethodTable(method_table)
foo() = 1
macro override(ex)
quote
Base.Experimental.@overlay $method_table $ex
end
end
@override foo() = 2
bar() = foo()
This yields the error reported above.
Does it work if you make it a GlobalRef instead? I can't find the reference now, but @aviatesk recently pointed out an older quote of Jeff Bezanson's effectively saying "we allow this, but it's perhaps a mis-feature, because Exprs should be close to the source code itself."
It will still throw a Revise error (GlobalRef is not currently in that Union), but that's very fixable.
A GlobalRef works for Julia, but triggers another error in the Revise stack:
macro override(ex)
quote
Base.Experimental.@overlay $(GlobalRef(@__MODULE__, :method_table)) $ex
end
end
MethodError: no method matching nameof(::Core.MethodTable)
Closest candidates are:
nameof(::DataType) at reflection.jl:223
nameof(::UnionAll) at reflection.jl:224
nameof(::Module) at reflection.jl:16
...
Stacktrace:
[1] normalize_defsig(def::Any, frame::JuliaInterpreter.Frame)
@ LoweredCodeUtils ~/.julia/packages/LoweredCodeUtils/jZY56/src/signatures.jl:217
[2] identify_framemethod_calls(frame::JuliaInterpreter.Frame)
@ LoweredCodeUtils ~/.julia/packages/LoweredCodeUtils/jZY56/src/signatures.jl:166
[3] rename_framemethods!
@ ~/.julia/packages/LoweredCodeUtils/jZY56/src/signatures.jl:307 [inlined]
[4] 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.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}})
@ Revise ~/.julia/packages/Revise/OgnOk/src/lowered.jl:191
[5] #eval_with_signatures#92
@ ~/.julia/packages/Revise/OgnOk/src/packagedef.jl:463 [inlined]
[6] instantiate_sigs!(modexsigs::OrderedCollections.OrderedDict{Module, OrderedCollections.OrderedDict{Revise.RelocatableExpr, Union{Nothing, Vector{Any}}}}; mode::Symbol, kwargs::Base.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}})
@ Revise ~/.julia/packages/Revise/OgnOk/src/packagedef.jl:471
[7] track(mod::Module, file::String; mode::Symbol, kwargs::Base.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}})
@ Revise ~/.julia/packages/Revise/OgnOk/src/packagedef.jl:899
[8] includet(mod::Module, file::String)
@ Revise ~/.julia/packages/Revise/OgnOk/src/packagedef.jl:994
[9] includet(file::String)
@ Revise ~/.julia/packages/Revise/OgnOk/src/packagedef.jl:1016
But why a GlobalRef? Wouldn't source code typically contain a Symbol or Expr? Changing the method_table to an Expr seems 'compatible' (i.e. it doesn't error) with the current logic in LoweredCodeUtils:
macro override(ex)
quote
Base.Experimental.@overlay $(@__MODULE__).method_table $ex
end
end
Similarly, without the wrapper macro, making sure the method table is an Expr (by qualifying the module) gets past Revise.includet:
Base.Experimental.@MethodTable(method_table)
foo() = 1
Base.Experimental.@overlay Main.method_table foo() = 2
So I guess this only needs a fix for the method_table::Symbol case, and if we want to, for when the method_table is actually a full-blown Core.MethodTable.
I quoted 'compatible' above, because even though Revise doesn't choke on these sources anymore, it incorrectly redefines methods. I guess that's the second part of this issue.
Expected behavior:
julia> Base.Experimental.@MethodTable(method_table)
julia> foo() = 1
julia> Base.Experimental.@overlay Main.method_table foo() = 2
julia> foo()
1
julia> Base.Experimental.@overlay Main.method_table foo() = 3
julia> foo()
1
Whereas putting this in a file and having Revise track it yields the following:
julia> using Revise
julia> Revise.includet("wip.jl")
julia> foo()
1
# change @overlay foo()=2 to =3
julia> Revise.revise()
julia> foo()
3
Yes, this is still relevant. The PR just made it so that Revise doesn't error out, but it doesn't correctly handle overlay method tables. Let me demonstrate:
Base.Experimental.@MethodTable(method_table)
foo() = 1
Base.Experimental.@overlay Main.method_table foo() = 2
function main()
println("Method in main method table:")
methods = Base._methods_by_ftype(Tuple{typeof(foo)}, nothing, 1, Base.get_world_counter())
display(Base.uncompressed_ir(methods[1].method))
println("Method in overlay method table:")
methods = Base._methods_by_ftype(Tuple{typeof(foo)}, method_table, 1, Base.get_world_counter())
display(Base.uncompressed_ir(methods[1].method))
return
end
julia> using Revise
julia> Revise.includet("...")
julia> main()
Method in main method table:
CodeInfo(
@ /tmp/wip.jl:3 within `foo`
1 ─ return 1
)
Method in overlay method table:
CodeInfo(
@ /tmp/wip.jl:4 within `#foo`
1 ─ return 2
)
julia> # redefine `@overlay foo() = 2` to `@overlay foo() = 3`
julia> main()
Method in main method table:
CodeInfo(
@ /tmp/wip.jl:4 within `foo`
1 ─ return 3
)
Method in overlay method table:
CodeInfo(
@ /tmp/wip.jl:4 within `#foo`
1 ─ return 2
)
i.e. revising the overlaid method results in the main method table method being redefined, instead of the one on the overlay method table.