From 9e14841d7276abc8f88bb77022aca5ce37c74c5e Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Sun, 6 Mar 2022 13:37:21 +0900 Subject: [PATCH] make `ml_matches` return `nothing` when there are too many matches Continuation from . Previously `ml_matches` (and its dependent utilities like `_methods_by_ftype`) returned `false` for cases when there are too many matching method, but its consumer like `findall(::Type, ::InternalMethodTable)` usually handles such case as `missing`. This commit does a refactor so that they all use the more consistent value `nothing` for representing that case. --- base/compiler/abstractinterpretation.jl | 4 ++-- base/compiler/bootstrap.jl | 3 ++- base/compiler/methodtable.jl | 14 ++++++-------- base/reflection.jl | 2 +- src/dump.c | 4 ++-- src/gf.c | 14 +++++++------- stdlib/REPL/src/REPLCompletions.jl | 4 ++-- 7 files changed, 22 insertions(+), 23 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 5dc75685b6257..5d12d46a6d0eb 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -287,7 +287,7 @@ function find_matching_methods(argtypes::Vector{Any}, @nospecialize(atype), meth mt === nothing && return FailedMethodMatch("Could not identify method table for call") mt = mt::Core.MethodTable result = findall(sig_n, method_table; limit = max_methods) - if result === missing + if result === nothing return FailedMethodMatch("For one of the union split cases, too many methods matched") end matches, overlayedᵢ = result @@ -326,7 +326,7 @@ function find_matching_methods(argtypes::Vector{Any}, @nospecialize(atype), meth end mt = mt::Core.MethodTable result = findall(atype, method_table; limit = max_methods) - if result === missing + if result === nothing # this means too many methods matched # (assume this will always be true, so we don't compute / update valid age in this case) return FailedMethodMatch("Too many methods matched") diff --git a/base/compiler/bootstrap.jl b/base/compiler/bootstrap.jl index f335cf31a8467..502a5ec1ed53b 100644 --- a/base/compiler/bootstrap.jl +++ b/base/compiler/bootstrap.jl @@ -38,8 +38,9 @@ let else tt = Tuple{typeof(f), Vararg{Any}} end - for m in _methods_by_ftype(tt, 10, typemax(UInt)) + for m in _methods_by_ftype(tt, 10, typemax(UInt))::Vector # remove any TypeVars from the intersection + m = m::MethodMatch typ = Any[m.spec_types.parameters...] for i = 1:length(typ) typ[i] = unwraptv(typ[i]) diff --git a/base/compiler/methodtable.jl b/base/compiler/methodtable.jl index da493cf9a9ef5..91e0e8d48b7ed 100644 --- a/base/compiler/methodtable.jl +++ b/base/compiler/methodtable.jl @@ -41,11 +41,11 @@ getindex(result::MethodLookupResult, idx::Int) = getindex(result.matches, idx):: """ findall(sig::Type, view::MethodTableView; limit::Int=typemax(Int)) -> - (matches::MethodLookupResult, overlayed::Bool) or missing + (matches::MethodLookupResult, overlayed::Bool) or nothing Find all methods in the given method table `view` that are applicable to the given signature `sig`. If no applicable methods are found, an empty result is returned. -If the number of applicable methods exceeded the specified limit, `missing` is returned. +If the number of applicable methods exceeded the specified limit, `nothing` is returned. `overlayed` indicates if any matching method is defined in an overlayed method table. """ function findall(@nospecialize(sig::Type), table::InternalMethodTable; limit::Int=Int(typemax(Int32))) @@ -56,7 +56,7 @@ end function findall(@nospecialize(sig::Type), table::OverlayMethodTable; limit::Int=Int(typemax(Int32))) result = _findall(sig, table.mt, table.world, limit) - result === missing && return missing + result === nothing && return nothing nr = length(result) if nr ≥ 1 && result[nr].fully_covers # no need to fall back to the internal method table @@ -64,7 +64,7 @@ function findall(@nospecialize(sig::Type), table::OverlayMethodTable; limit::Int end # fall back to the internal method table fallback_result = _findall(sig, nothing, table.world, limit) - fallback_result === missing && return missing + fallback_result === nothing && return nothing # merge the fallback match results with the internal method table return MethodLookupResult( vcat(result.matches, fallback_result.matches), @@ -79,10 +79,8 @@ function _findall(@nospecialize(sig::Type), mt::Union{Nothing,Core.MethodTable}, _max_val = RefValue{UInt}(typemax(UInt)) _ambig = RefValue{Int32}(0) ms = _methods_by_ftype(sig, mt, limit, world, false, _min_val, _max_val, _ambig) - if ms === false - return missing - end - return MethodLookupResult(ms::Vector{Any}, WorldRange(_min_val[], _max_val[]), _ambig[] != 0) + isa(ms, Vector) || return nothing + return MethodLookupResult(ms, WorldRange(_min_val[], _max_val[]), _ambig[] != 0) end """ diff --git a/base/reflection.jl b/base/reflection.jl index 484dc8b586664..0446da674aa6e 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -942,7 +942,7 @@ function _methods_by_ftype(@nospecialize(t), mt::Union{Core.MethodTable, Nothing return _methods_by_ftype(t, mt, lim, world, false, RefValue{UInt}(typemin(UInt)), RefValue{UInt}(typemax(UInt)), Ptr{Int32}(C_NULL)) end function _methods_by_ftype(@nospecialize(t), mt::Union{Core.MethodTable, Nothing}, lim::Int, world::UInt, ambig::Bool, min::Ref{UInt}, max::Ref{UInt}, has_ambig::Ref{Int32}) - return ccall(:jl_matching_methods, Any, (Any, Any, Cint, Cint, UInt, Ptr{UInt}, Ptr{UInt}, Ptr{Int32}), t, mt, lim, ambig, world, min, max, has_ambig)::Union{Array{Any,1}, Bool} + return ccall(:jl_matching_methods, Any, (Any, Any, Cint, Cint, UInt, Ptr{UInt}, Ptr{UInt}, Ptr{Int32}), t, mt, lim, ambig, world, min, max, has_ambig)::Union{Vector{Any},Nothing} end # high-level, more convenient method lookup functions diff --git a/src/dump.c b/src/dump.c index accc3bfb4916f..4fc80faf02702 100644 --- a/src/dump.c +++ b/src/dump.c @@ -1258,7 +1258,7 @@ static void jl_collect_backedges( /* edges */ jl_array_t *s, /* ext_targets */ j size_t max_valid = ~(size_t)0; int ambig = 0; jl_value_t *matches = jl_matching_methods((jl_tupletype_t*)sig, jl_nothing, -1, 0, jl_atomic_load_acquire(&jl_world_counter), &min_valid, &max_valid, &ambig); - if (matches == jl_false) { + if (matches == jl_nothing) { valid = 0; break; } @@ -2339,7 +2339,7 @@ static void jl_verify_edges(jl_array_t *targets, jl_array_t **pvalids) int ambig = 0; // TODO: possibly need to included ambiguities too (for the optimizer correctness)? jl_value_t *matches = jl_matching_methods((jl_tupletype_t*)sig, jl_nothing, -1, 0, jl_atomic_load_acquire(&jl_world_counter), &min_valid, &max_valid, &ambig); - if (matches == jl_false || jl_array_len(matches) != jl_array_len(expected)) { + if (matches == jl_nothing || jl_array_len(matches) != jl_array_len(expected)) { valid = 0; } else { diff --git a/src/gf.c b/src/gf.c index f6643e014d785..674a0ab7218ac 100644 --- a/src/gf.c +++ b/src/gf.c @@ -1089,7 +1089,7 @@ static jl_method_instance_t *cache_method( size_t max_valid2 = ~(size_t)0; temp = ml_matches(mt, compilationsig, MAX_UNSPECIALIZED_CONFLICTS, 1, 1, world, 0, &min_valid2, &max_valid2, NULL); int guards = 0; - if (temp == jl_false) { + if (temp == jl_nothing) { cache_with_orig = 1; } else { @@ -2559,7 +2559,7 @@ static jl_method_match_t *_gf_invoke_lookup(jl_value_t *types JL_PROPAGATES_ROOT if (mt == jl_nothing) mt = NULL; jl_value_t *matches = ml_matches((jl_methtable_t*)mt, (jl_tupletype_t*)types, 1, 0, 0, world, 1, min_valid, max_valid, NULL); - if (matches == jl_false || jl_array_len(matches) != 1) + if (matches == jl_nothing || jl_array_len(matches) != 1) return NULL; jl_method_match_t *matc = (jl_method_match_t*)jl_array_ptr_ref(matches, 0); return matc; @@ -2887,14 +2887,14 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, } if (!jl_typemap_intersection_visitor(mt->defs, 0, &env.match)) { JL_GC_POP(); - return jl_false; + return jl_nothing; } } else { // else: scan everything if (!jl_foreach_reachable_mtable(ml_mtable_visitor, &env.match)) { JL_GC_POP(); - return jl_false; + return jl_nothing; } } *min_valid = env.min_valid; @@ -2976,7 +2976,7 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, } else if (lim == 1) { JL_GC_POP(); - return jl_false; + return jl_nothing; } } else { @@ -3120,7 +3120,7 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, ndisjoint += 1; if (ndisjoint > lim) { JL_GC_POP(); - return jl_false; + return jl_nothing; } } } @@ -3271,7 +3271,7 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, *ambig = has_ambiguity; JL_GC_POP(); if (lim >= 0 && len > lim) - return jl_false; + return jl_nothing; return env.t; } diff --git a/stdlib/REPL/src/REPLCompletions.jl b/stdlib/REPL/src/REPLCompletions.jl index 162d1184d18c3..bf15b03fb3315 100644 --- a/stdlib/REPL/src/REPLCompletions.jl +++ b/stdlib/REPL/src/REPLCompletions.jl @@ -613,10 +613,10 @@ function complete_methods!(out::Vector{Completion}, @nospecialize(funct), args_e t_in = Tuple{funct, args_ex...} m = Base._methods_by_ftype(t_in, nothing, max_method_completions, Base.get_world_counter(), #=ambig=# true, Ref(typemin(UInt)), Ref(typemax(UInt)), Ptr{Int32}(C_NULL)) - if m === false + if !isa(m, Vector) push!(out, TextCompletion(sprint(Base.show_signature_function, funct) * "( too many methods to show )")) + return end - m isa Vector || return for match in m # TODO: if kwargs_ex, filter out methods without kwargs? push!(out, MethodCompletion(match.spec_types, match.method))