fonsp / Pluto.jl

🎈 Simple reactive notebooks for Julia
https://plutojl.org/
MIT License
5k stars 296 forks source link

Method table test failures on Julia nightly #2840

Closed fonsp closed 7 months ago

fonsp commented 8 months ago

It looks like we have a new issue with method tables on Julia nightly. See the last CI runs on https://github.com/fonsp/Pluto.jl/pull/2815

Here is a MWE that probably fails on nightly:


####################
# This file has two parts:
# 1. the functions to remove a method
# 2. the tests

####################
# 1. the functions to remove a method
####################

if VERSION < v"1.7.0-0"
    @eval macro atomic(ex)
        esc(ex)
    end
end

const alive_world_val = methods(Base.sqrt).ms[1].deleted_world # typemax(UInt) in Julia v1.3, Int(-1) in Julia 1.0

const here = @__FILE__
isfromhere(method::Method) = here == String(method.file)

"""
Delete all methods of `f` that were defined in this notebook, and leave the ones defined in other packages, base, etc. ✂

Return whether the function has any methods left after deletion.
"""
function delete_toplevel_methods(f::Function)::Bool
    # we can delete methods of functions!
    # instead of deleting all methods, we only delete methods that were defined in this notebook. This is necessary when the notebook code extends a function from remote code
    methods_table = typeof(f).name.mt
    deleted_sigs = Set{Type}()
    Base.visit(methods_table) do method # iterates through all methods of `f`, including overridden ones
        if isfromhere(method) && method.deleted_world == alive_world_val
            Base.delete_method(method)
            delete_method_doc(method)
            push!(deleted_sigs, method.sig)
        end
    end

    # if `f` is an extension to an external function, and we defined a method that overrides a method, for example,
    # we define `Base.isodd(n::Integer) = rand(Bool)`, which overrides the existing method `Base.isodd(n::Integer)`
    # calling `Base.delete_method` on this method won't bring back the old method, because our new method still exists in the method table, and it has a world age which is newer than the original. (our method has a deleted_world value set, which disables it)
    #
    # To solve this, we iterate again, and _re-enable any methods that were hidden in this way_, by adding them again to the method table with an even newer `primary_world`.
    if !isempty(deleted_sigs)
        to_insert = Method[]
        Base.visit(methods_table) do method
            if !isfromhere(method) && method.sig ∈ deleted_sigs
                push!(to_insert, method)
            end
        end
        # separate loop to avoid visiting the recently added method
        for method in Iterators.reverse(to_insert)
            if VERSION >= v"1.11.0-0"
                @atomic method.primary_world = one(typeof(alive_world_val)) # `1` will tell Julia to increment the world counter and set it as this function's world
                @atomic method.deleted_world = alive_world_val # set the `deleted_world` property back to the 'alive' value (for Julia v1.6 and up)
            else
                method.primary_world = one(typeof(alive_world_val))
                method.deleted_world = alive_world_val
            end
            ccall(:jl_method_table_insert, Cvoid, (Any, Any, Ptr{Cvoid}), methods_table, method, C_NULL) # i dont like doing this either!
        end
    end
    return !isempty(methods(f).ms)
end

"""
    delete_method_doc(m::Method)

Tries to delete the documentation for this method, this is used when methods are removed.
"""
function delete_method_doc(m::Method)
    binding = Docs.Binding(m.module, m.name)
    meta = Docs.meta(m.module)
    if haskey(meta, binding)
        method_sig = Tuple{m.sig.parameters[2:end]...}
        multidoc = meta[binding]
        filter!(multidoc.order) do msig
            if method_sig == msig
                pop!(multidoc.docs, msig)
                false
            else
                true
            end
        end
    end
end

####################
# 2. the tests
####################

using Test

@test tan(missing) === missing
@test tan(pi) == 0

Core.eval(Main, quote
Base.tan(::Missing) = 32
end)

@test tan(missing) == 32
@test tan(pi) == 0

delete_toplevel_methods(tan)

@test tan(missing) === missing
@test tan(pi) == 0

It works on Julia 1.6, Julia 1.10, and this older Julia nightly that I have installed

➜  ~ julianightly
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.11.0-DEV.1495 (2024-02-07)
 _/ |\__'_|_|_|\__'_|  |  Commit bead1d32f1a (26 days old master)
|__/                   |

I'm on a hotspot so I can't download the latest Julia nightly at the moment. But this should help debug the issue!

fonsp commented 8 months ago

On Julia nightly (today) it fails:

fons@georgettarm:~$ julia-b50344fa0a/bin/julia ./method\ tables.jl 
Error During Test at /home/fons/method tables.jl:125
  Test threw exception
  Expression: tan(missing) === missing
  MethodError: tan(::Missing) is ambiguous.

ERROR: LoadError: There was an error during testing
in expression starting at /home/fons/method tables.jl:125

Looks like the original method is still there somehow...

fonsp commented 8 months ago

It's probably happening since https://github.com/JuliaLang/julia/pull/53415 , which says it will make things easier for us, interesting!