From c6e48b0573572cf685bf7b2a521384ce46b5ef7b 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 | 16 +++++++--------- base/reflection.jl | 2 +- src/dump.c | 4 ++-- src/gf.c | 16 ++++++++-------- stdlib/REPL/src/REPLCompletions.jl | 3 +-- 7 files changed, 23 insertions(+), 25 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 0fb85c4d00d1a2..d95692599f5f00 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -294,7 +294,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 @@ -333,7 +333,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 4b79cd57f9d11d..77b36cb9c7f71b 100644 --- a/base/compiler/bootstrap.jl +++ b/base/compiler/bootstrap.jl @@ -36,8 +36,9 @@ let interp = NativeInterpreter() 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 93ea00da4986e7..af72c6e7cd6979 100644 --- a/base/compiler/methodtable.jl +++ b/base/compiler/methodtable.jl @@ -64,23 +64,23 @@ CachedMethodTable(table::T) where T = CachedMethodTable{T}(IdDict{MethodMatchKey """ findall(sig::Type, view::MethodTableView; limit::Int=-1) -> - MethodMatchResult(matches::MethodLookupResult, overlayed::Bool) or missing + MethodMatchResult(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. Note that the default setting `limit=-1` does not limit the number of applicable methods. `overlayed` indicates if any of the matching methods comes from an overlayed method table. """ function findall(@nospecialize(sig::Type), table::InternalMethodTable; limit::Int=-1) result = _findall(sig, nothing, table.world, limit) - result === missing && return missing + result === nothing && return nothing return MethodMatchResult(result, false) end function findall(@nospecialize(sig::Type), table::OverlayMethodTable; limit::Int=-1) 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 @@ -88,7 +88,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 MethodMatchResult( MethodLookupResult( @@ -105,10 +105,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 function findall(@nospecialize(sig::Type), table::CachedMethodTable; limit::Int=-1) diff --git a/base/reflection.jl b/base/reflection.jl index de0296447be581..fe4de742b0f555 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -950,7 +950,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 96c875c4ec7f59..125f3faca36877 100644 --- a/src/dump.c +++ b/src/dump.c @@ -1398,7 +1398,7 @@ static void jl_collect_edges(jl_array_t *edges, jl_array_t *ext_targets) int ambig = 0; matches = jl_matching_methods((jl_tupletype_t*)sig, jl_nothing, -1, 0, world, &min_valid, &max_valid, &ambig); - if (matches == jl_false) { + if (matches == jl_nothing) { callee_ids = NULL; // invalid break; } @@ -2408,7 +2408,7 @@ static jl_array_t *jl_verify_edges(jl_array_t *targets) // TODO: possibly need to included ambiguities too (for the optimizer correctness)? matches = jl_matching_methods((jl_tupletype_t*)sig, jl_nothing, -1, 0, world, &min_valid, &max_valid, &ambig); - if (matches == jl_false) { + if (matches == jl_nothing) { valid = 0; } else { diff --git a/src/gf.c b/src/gf.c index 0e98f2a140d4a1..3be1457afe2d66 100644 --- a/src/gf.c +++ b/src/gf.c @@ -1102,7 +1102,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 { @@ -2304,7 +2304,7 @@ jl_method_instance_t *jl_get_specialization1(jl_tupletype_t *types JL_PROPAGATES *min_valid = min_valid2; if (*max_valid > max_valid2) *max_valid = max_valid2; - if (matches == jl_false || jl_array_len(matches) != 1 || ambig) + if (matches == jl_nothing || jl_array_len(matches) != 1 || ambig) return NULL; JL_GC_PUSH1(&matches); jl_method_match_t *match = (jl_method_match_t*)jl_array_ptr_ref(matches, 0); @@ -2716,7 +2716,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; @@ -3016,14 +3016,14 @@ static jl_value_t *ml_matches(jl_methtable_t *mt, } if (!jl_typemap_intersection_visitor(jl_atomic_load_relaxed(&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; @@ -3105,7 +3105,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 { @@ -3249,7 +3249,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; } } } @@ -3396,7 +3396,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 2ce7a6151b351b..ff7b6114d51033 100644 --- a/stdlib/REPL/src/REPLCompletions.jl +++ b/stdlib/REPL/src/REPLCompletions.jl @@ -656,10 +656,9 @@ 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, use SHIFT-TAB to show )")) 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))