Skip to content

Commit

Permalink
further improve clarity of MethodError printing
Browse files Browse the repository at this point in the history
Distinguish some of the cases of manually thrown MethodError by looking
for a method in the specified world. Also refactor some of the kwcall
handling to be closer to supporting `invoke`d calls (although it does
not guaranteed to have a value for `f`, which is often required later).

Fixes #36182
  • Loading branch information
vtjnash committed Feb 3, 2024
1 parent fc06291 commit abcb61e
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 17 deletions.
38 changes: 26 additions & 12 deletions base/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ function show_convert_error(io::IO, ex::MethodError, arg_types_param)
end

function showerror(io::IO, ex::MethodError)
@nospecialize io
# ex.args is a tuple type if it was thrown from `invoke` and is
# a tuple of the arguments otherwise.
is_arg_types = !isa(ex.args, Tuple)
Expand All @@ -258,15 +259,22 @@ function showerror(io::IO, ex::MethodError)
print(io, "MethodError: ")
ft = typeof(f)
f_is_function = false
kwargs = ()
if f === Core.kwcall && !is_arg_types
f = (ex.args::Tuple)[2]
ft = typeof(f)
kwargs = []
if f === Core.kwcall && length(arg_types_param) >= 2 && arg_types_param[1] <: NamedTuple && !is_arg_types
# if this is a kwcall, reformat it as a call with kwargs
# TODO: handle !is_arg_types here (aka invoke with kwargs), which needs a value for `f`
local kwt
let args = ex.args::Tuple
f = args[2]
ft = typeof(f)
kwt = typeof(args[1])
ex = MethodError(f, args[3:end], ex.world)
end
arg_types_param = arg_types_param[3:end]
san_arg_types_param = san_arg_types_param[3:end]
kwargs = pairs(ex.args[1])
ex = MethodError(f, ex.args[3:end::Int], ex.world)
arg_types = Tuple{arg_types_param...}
keys = kwt.parameters[1]::Tuple
kwargs = Any[(keys[i], fieldtype(kwt, i)) for i in 1:length(keys)]
arg_types = rewrap_unionall(Tuple{arg_types_param...}, arg_types)
end
if f === Base.convert && length(arg_types_param) == 2 && !is_arg_types
f_is_function = true
Expand All @@ -286,8 +294,8 @@ function showerror(io::IO, ex::MethodError)
end
buf = IOBuffer()
iob = IOContext(buf, io) # for type abbreviation as in #49795; some, like `convert(T, x)`, should not abbreviate
show_signature_function(iob, isa(f, Type) ? Type{f} : typeof(f))
show_tuple_as_call(iob, :function, arg_types; hasfirst=false, kwargs = !isempty(kwargs) ? Any[(k, typeof(v)) for (k, v) in kwargs] : nothing)
show_signature_function(iob, Core.Typeof(f))
show_tuple_as_call(iob, :function, arg_types; hasfirst=false, kwargs = isempty(kwargs) ? nothing : kwargs)
str = String(take!(buf))
str = type_limited_string_from_context(io, str)
print(io, str)
Expand Down Expand Up @@ -318,8 +326,13 @@ function showerror(io::IO, ex::MethodError)
end
end
end
if (ex.world != typemax(UInt) && hasmethod(f, arg_types) &&
!hasmethod(f, arg_types, world = ex.world))
if ex.world == typemax(UInt) || hasmethod(f, arg_types, world=ex.world)
if ex.world == typemax(UInt) || isempty(kwargs)
print(io, "\nThis error has been manually thrown, explicitly, so the method may exist but be intentionally marked as unimplemented.")
else
print(io, "\nThis method may not support any kwargs.")
end
elseif hasmethod(f, arg_types) && !hasmethod(f, arg_types, world=ex.world)
curworld = get_world_counter()
print(io, "\nThe applicable method may be too new: running in world age $(ex.world), while current world is $(curworld).")
elseif f isa Function
Expand Down Expand Up @@ -398,7 +411,8 @@ stacktrace_expand_basepaths()::Bool = Base.get_bool_env("JULIA_STACKTRACE_EXPAND
stacktrace_contract_userdir()::Bool = Base.get_bool_env("JULIA_STACKTRACE_CONTRACT_HOMEDIR", true) === true
stacktrace_linebreaks()::Bool = Base.get_bool_env("JULIA_STACKTRACE_LINEBREAKS", false) === true

function show_method_candidates(io::IO, ex::MethodError, @nospecialize kwargs=())
function show_method_candidates(io::IO, ex::MethodError, kwargs=[])
@nospecialize io
is_arg_types = !isa(ex.args, Tuple)
arg_types = is_arg_types ? ex.args : typesof(ex.args...)
arg_types_param = Any[(unwrap_unionall(arg_types)::DataType).parameters...]
Expand Down
29 changes: 24 additions & 5 deletions test/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -586,12 +586,12 @@ module EnclosingModule
abstract type AbstractTypeNoConstructors end
end
let
method_error = MethodError(EnclosingModule.AbstractTypeNoConstructors, ())
method_error = MethodError(EnclosingModule.AbstractTypeNoConstructors, (), Base.get_world_counter())

# Test that it shows a special message when no constructors have been defined by the user.
@test sprint(showerror, method_error) ==
@test startswith(sprint(showerror, method_error),
"""MethodError: no constructors have been defined for $(EnclosingModule.AbstractTypeNoConstructors)
The type `$(EnclosingModule.AbstractTypeNoConstructors)` exists, but no method is defined for this combination of argument types when trying to construct it."""
The type `$(EnclosingModule.AbstractTypeNoConstructors)` exists, but no method is defined for this combination of argument types when trying to construct it.""")

# Does it go back to previous behaviour when there *is* at least
# one constructor defined?
Expand Down Expand Up @@ -650,6 +650,24 @@ end
@test startswith(str, "MethodError: no method matching f21006(::Tuple{})")
@test !occursin("The applicable method may be too new", str)
end

str = sprint(Base.showerror, MethodError(+, (1.0, 2.0)))
@test startswith(str, "MethodError: no method matching +(::Float64, ::Float64)")
@test occursin("This error has been manually thrown, explicitly", str)

str = sprint(Base.showerror, MethodError(+, (1.0, 2.0), Base.get_world_counter()))
@test startswith(str, "MethodError: no method matching +(::Float64, ::Float64)")
@test occursin("This error has been manually thrown, explicitly", str)

str = sprint(Base.showerror, MethodError(Core.kwcall, ((; a=3.0), +, 1.0, 2.0)))
@test startswith(str, "MethodError: no method matching +(::Float64, ::Float64; a::Float64)")
@test occursin("This error has been manually thrown, explicitly", str)

str = sprint(Base.showerror, MethodError(Core.kwcall, ((; a=3.0), +, 1.0, 2.0), Base.get_world_counter()))
@test startswith(str, "MethodError: no method matching +(::Float64, ::Float64; a::Float64)")
@test occursin("This method may not support any kwargs", str)

@test_throws "MethodError: no method matching kwcall()" Core.kwcall()
end

# Issue #50200
Expand All @@ -658,8 +676,9 @@ using Base.Experimental: @opaque
test_no_error(f) = @test f() === nothing
function test_worldage_error(f)
ex = try; f(); error("Should not have been reached") catch ex; ex; end
@test occursin("The applicable method may be too new", sprint(Base.showerror, ex))
@test !occursin("!Matched::", sprint(Base.showerror, ex))
strex = sprint(Base.showerror, ex)
@test occursin("The applicable method may be too new", strex)
@test !occursin("!Matched::", sprint(Base.showerror, strex))
end

global callback50200
Expand Down

0 comments on commit abcb61e

Please sign in to comment.