Support external method tables in method evaluation (#680)

* Support evaluation of overlayed methods

* Add comment

* Bump version

* Support more expression types defining the external method table

* Attempt a few fixes

* Fix 1.12/nightly failures that were present before this PR

* Adjust to JuliaLang/julia#58076

* `jl_method_def` actually returned a `Method` already

* Export `extract_method_table`, add option to disallow `eval`

* Quickly try a nightly fix

* Remove unnecessary dependency on Tensors for testing

The test that used it doesn't actually need it to
test what was intended

* Wrap evaluation in try/catch

* Use isdefinedglobal

* Adjust broken test to have it throw consistently

Otherwise, it would fail at the first evaluation and
succeed on subsequent ones.

* Update comment

* Replace `Core.eval` with `getglobal`

* Update src/interpret.jl

Co-authored-by: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com>

---------

Co-authored-by: Cédric Belmant <cedric.belmant@juliahub.com>
Co-authored-by: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com>

Author: serenity4

Project.toml

@@ -1,6 +1,6 @@
 name = "JuliaInterpreter"
 uuid = "aa1ae85d-cabe-5617-a682-6adf51b2e16a"
-version = "0.9.44"
+version = "0.9.45"
 
 [deps]
 CodeTracking = "da1fd8a2-8d9e-5ec2-8556-3022fb5608a2"
@@ -27,8 +27,7 @@ Mmap = "a63ad114-7e13-5084-954f-fe012c677804"
 PyCall = "438e738f-606a-5dbb-bf0a-cddfbfd45ab0"
 SHA = "ea8e919c-243c-51af-8825-aaa63cd721ce"
 SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf"
-Tensors = "48a634ad-e948-5137-8d70-aa71f2a747f4"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [targets]
-test = ["CassetteOverlay", "DataFrames", "Dates", "DeepDiffs", "Distributed", "FunctionWrappers", "HTTP", "LinearAlgebra", "Logging", "LoopVectorization", "Mmap", "PyCall", "SHA", "SparseArrays", "Tensors", "Test"]
+test = ["CassetteOverlay", "DataFrames", "Dates", "DeepDiffs", "Distributed", "FunctionWrappers", "HTTP", "LinearAlgebra", "Logging", "LoopVectorization", "Mmap", "PyCall", "SHA", "SparseArrays", "Test"]

src/interpret.jl

@@ -313,6 +313,8 @@ evaluate_call!(frame::Frame, call_expr::Expr; kwargs...) = evaluate_call!(finish
 
 # The following come up only when evaluating toplevel code
 function evaluate_methoddef(frame::Frame, node::Expr)
+    mt = extract_method_table(frame, node)
+    mt !== nothing && return evaluate_overlayed_methoddef(frame, node, mt)
     f = node.args[1]
     if f isa Symbol || f isa GlobalRef
         mod = f isa Symbol ? moduleof(frame) : f.mod
@@ -332,9 +334,34 @@ function evaluate_methoddef(frame::Frame, node::Expr)
     length(node.args) == 1 && return f
     sig = @lookup(frame, node.args[2])::SimpleVector
     body = @lookup(frame, node.args[3])::Union{CodeInfo, Expr}
-    # branching on https://github.com/JuliaLang/julia/pull/41137
-    ccall(:jl_method_def, Cvoid, (Any, Ptr{Cvoid}, Any, Any), sig, C_NULL, body, moduleof(frame)::Module)
-    return f
+    method = ccall(:jl_method_def, Any, (Any, Ptr{Cvoid}, Any, Any), sig, C_NULL, body, moduleof(frame)::Module)::Method
+    return method
+end
+
+function evaluate_overlayed_methoddef(frame::Frame, node::Expr, mt::MethodTable)
+    # Overlaying an empty function such as `function f end` is not legal, and `f` must
+    # already be defined so we don't need to do as much work as in `evaluate_methoddef`.
+    sig = @lookup(frame, node.args[2])::SimpleVector
+    body = @lookup(frame, node.args[3])::Union{CodeInfo, Expr}
+    method = ccall(:jl_method_def, Any, (Any, Any, Any, Any), sig, mt, body, moduleof(frame)::Module)::Method
+    return method
+end
+
+function extract_method_table(frame::Frame, node::Expr; eval = true)
+    isexpr(node, :method, 3) || return nothing
+    arg = node.args[1]
+    isa(arg, MethodTable) && return arg
+    if !isa(arg, Symbol) && !isa(arg, GlobalRef)
+        eval || return nothing
+        value = try Core.eval(moduleof(frame), arg) catch _ nothing end
+        isa(value, MethodTable) && return value
+        return nothing
+    end
+    mod, name = isa(arg, Symbol) ? (moduleof(frame), arg) : (arg.mod, arg.name)
+    @invokelatest(isdefinedglobal(mod, name)) || return nothing
+    value = @invokelatest getglobal(mod, name)
+    isa(value, MethodTable) && return value
+    return nothing
 end
 
 function do_assignment!(frame::Frame, @nospecialize(lhs), @nospecialize(rhs))
@@ -505,7 +532,7 @@ function step_expr!(@nospecialize(recurse), frame::Frame, @nospecialize(node), i
                 # (https://github.com/JuliaDebug/JuliaInterpreter.jl/issues/591)
             elseif istoplevel
                 if node.head === :method && length(node.args) > 1
-                    evaluate_methoddef(frame, node)
+                    rhs = evaluate_methoddef(frame, node)
                 elseif node.head === :module
                     error("this should have been handled by split_expressions")
                 elseif node.head === :using || node.head === :import || node.head === :export

src/packagedef.jl

@@ -1,7 +1,7 @@
 using Base.Meta
 import Base: +, -, convert, isless, get_world_counter, mapany, ntupleany, invokelatest
 using Core: CodeInfo, SimpleVector, LineInfoNode, GotoNode, GotoIfNot, ReturnNode,
-            GeneratedFunctionStub, MethodInstance, NewvarNode, TypeName
+            GeneratedFunctionStub, MethodInstance, MethodTable, NewvarNode, TypeName
 
 using UUIDs
 using Random

src/utils.jl

@@ -26,16 +26,17 @@ function to_function(@nospecialize(x))
 end
 
 """
-    method = whichtt(tt)
+    method = whichtt(tt, mt = nothing)
 
 Like `which` except it operates on the complete tuple-type `tt`,
 and doesn't throw when there is no matching method.
 """
-function whichtt(@nospecialize(tt))
+function whichtt(@nospecialize(tt), mt::Union{Nothing, MethodTable} = nothing)
     # TODO: provide explicit control over world age? In case we ever need to call "old" methods.
     # branch on https://github.com/JuliaLang/julia/pull/44515
-    # for now, actual code execution doesn't ever need to consider overlayed method table
-    match, _ = Core.Compiler._findsup(tt, nothing, get_world_counter())
+    # for now, code execution doesn't have the capability to use an overlayed method table,
+    # which is meant to be addressed in https://github.com/JuliaDebug/JuliaInterpreter.jl/pull/682.
+    match, _ = Core.Compiler._findsup(tt, mt, get_world_counter())
     match === nothing && return nothing
     return match.method
 end

test/interpret.jl

@@ -414,11 +414,16 @@ end
     f_gf(x) = false ? some_undef_var_zzzzzzz : x
     @test @interpret f_gf(2) == 2
 
-    function g_gf()
-        eval(:(z = 2))
-        return z
+    gvar = gensym()
+    @eval function g_gf()
+        @eval $gvar = 2
+        return $gvar
+    end
+    if VERSION ≥ v"1.12-"
+        @test_throws "`$gvar` not defined" @interpret(g_gf() == 2)
+    else
+        @test @interpret(g_gf() == 2)
     end
-    @test @interpret g_gf() == 2
 
     global q_gf = 0
     function h_gf()
@@ -462,13 +467,13 @@ file, line = JuliaInterpreter.whereis(fr)
 @test line == (@__LINE__() - 4)
 
 # Test path to files in stdlib
-fr = JuliaInterpreter.enter_call(Test.eval, 1)
+fr = JuliaInterpreter.enter_call(rand)
 file, line = JuliaInterpreter.whereis(fr)
 @test isfile(file)
 @static if VERSION < v"1.12.0-DEV.173"
 @test isfile(JuliaInterpreter.getfile(fr.framecode.src.linetable[1]))
 end
-@test occursin(contractuser(Sys.STDLIB), repr(fr))
+@test occursin(joinpath(contractuser(Sys.STDLIB), "Random"), repr(fr))
 
 # Test undef sparam (https://github.com/JuliaDebug/JuliaInterpreter.jl/issues/165)
 function foo(x::T) where {T <: AbstractString, S <: AbstractString}
@@ -626,8 +631,7 @@ end
 
 # parametric llvmcall (issues #112 and #288)
 module VecTest
-    using Tensors
-    Vec{N,T} = NTuple{N,VecElement{T}}
+    const Vec{N,T} = NTuple{N,VecElement{T}}
     # The following test mimic SIMD.jl
     const _llvmtypes = Dict{DataType, String}(
         Float64 => "double",
@@ -647,16 +651,16 @@ module VecTest
             Core.getfield(Base, :llvmcall)($exp, Vec{$N, $T}, Tuple{Vec{$N, $T}, Vec{$N, $T}}, x, y)
         end
     end
-    f() = 1.0 * one(Tensor{2,3})
+    f(a) = vecadd(a, a)
 end
 let
     # NOTE we need to make sure this code block is compiled, since vecadd is generated function,
     # but currently `@interpret` doesn't handle a call to generated functions very well
     Base.Experimental.@force_compile
     a = (VecElement{Float64}(1.0), VecElement{Float64}(2.0))
     @test @interpret(VecTest.vecadd(a, a)) == VecTest.vecadd(a, a)
+    @test @interpret(VecTest.f(a)) == VecTest.f(a)
 end
-@test @interpret(VecTest.f()) == [1 0 0; 0 1 0; 0 0 1]
 
 # Test exception type for undefined variables
 f_undefvar() = s = s + 1

test/toplevel.jl

@@ -606,3 +606,30 @@ end
     end
     @test length(modexs) == 2     # FIXME don't use index in tests
 end
+
+@testset "External method tables" begin
+    ex = quote
+        external_foo() = 1
+        Base.Experimental.@MethodTable method_table
+    end
+    frame = Frame(Toplevel, ex)
+    JuliaInterpreter.finish!(frame, true)
+
+    nmethods_in_overlay() = length(Base.MethodList(Toplevel.method_table).ms)
+    @test nmethods_in_overlay() == 0
+
+    ex = :(Base.Experimental.@overlay method_table external_foo() = 2)
+    frame = Frame(Toplevel, ex)
+    JuliaInterpreter.finish!(frame, true)
+    @test nmethods_in_overlay() == 1
+
+    ex = :(Base.Experimental.@overlay $(Toplevel.method_table) external_foo(x; y = 3) = 3 + y)
+    frame = Frame(Toplevel, ex)
+    JuliaInterpreter.finish!(frame, true)
+    @test nmethods_in_overlay() == 3 # `external_foo(x)` and `kwcall` methods were added
+
+    ex = :(Base.Experimental.@overlay getproperty(@__MODULE__, :method_table) external_foo(x::Int) = 4)
+    frame = Frame(Toplevel, ex)
+    JuliaInterpreter.finish!(frame, true)
+    @test nmethods_in_overlay() == 4
+end