make ml_matches return nothing when there are too many matches

aviatesk · aviatesk · commit 04f6c8f16269 · 2022-11-29T08:11:41.000+09:00
Continuation from <#44448 (comment)>. 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.
diff --git 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
@@ -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
@@ -57,38 +57,38 @@ Overlays another method table view with an additional local fast path cache that
 can respond to repeated, identical queries faster than the original method table.
 """
 struct CachedMethodTable{T} <: MethodTableView
-    cache::IdDict{MethodMatchKey, Union{Missing,MethodMatchResult}}
+    cache::IdDict{MethodMatchKey, Union{Nothing,MethodMatchResult}}
     table::T
 end
-CachedMethodTable(table::T) where T = CachedMethodTable{T}(IdDict{MethodMatchKey, Union{Missing,MethodMatchResult}}(), table)
+CachedMethodTable(table::T) where T = CachedMethodTable{T}(IdDict{MethodMatchKey, Union{Nothing,MethodMatchResult}}(), table)
 
 """
     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
         return MethodMatchResult(result, true)
     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
@@ -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
@@ -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
@@ -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
@@ -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))

Original file line number	Diff line number	Diff line change
`@@ -1102,7 +1102,7 @@ static jl_method_instance_t *cache_method(`
`1102`	`1102`	`size_t max_valid2 = ~(size_t)0;`
`1103`	`1103`	`temp = ml_matches(mt, compilationsig, MAX_UNSPECIALIZED_CONFLICTS, 1, 1, world, 0, &min_valid2, &max_valid2, NULL);`
`1104`	`1104`	`int guards = 0;`
`1105`		`- if (temp == jl_false) {`
	`1105`	`+ if (temp == jl_nothing) {`
`1106`	`1106`	`cache_with_orig = 1;`
`1107`	`1107`	`}`
`1108`	`1108`	`else {`
`@@ -2304,7 +2304,7 @@ jl_method_instance_t jl_get_specialization1(jl_tupletype_t types JL_PROPAGATES`
`2304`	`2304`	`*min_valid = min_valid2;`
`2305`	`2305`	`if (*max_valid > max_valid2)`
`2306`	`2306`	`*max_valid = max_valid2;`
`2307`		`- if (matches == jl_false \|\| jl_array_len(matches) != 1 \|\| ambig)`
	`2307`	`+ if (matches == jl_nothing \|\| jl_array_len(matches) != 1 \|\| ambig)`
`2308`	`2308`	`return NULL;`
`2309`	`2309`	`JL_GC_PUSH1(&matches);`
`2310`	`2310`	`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`
`2716`	`2716`	`if (mt == jl_nothing)`
`2717`	`2717`	`mt = NULL;`
`2718`	`2718`	`jl_value_t matches = ml_matches((jl_methtable_t)mt, (jl_tupletype_t*)types, 1, 0, 0, world, 1, min_valid, max_valid, NULL);`
`2719`		`- if (matches == jl_false \|\| jl_array_len(matches) != 1)`
	`2719`	`+ if (matches == jl_nothing \|\| jl_array_len(matches) != 1)`
`2720`	`2720`	`return NULL;`
`2721`	`2721`	`jl_method_match_t matc = (jl_method_match_t)jl_array_ptr_ref(matches, 0);`
`2722`	`2722`	`return matc;`
`@@ -3016,14 +3016,14 @@ static jl_value_t ml_matches(jl_methtable_t mt,`
`3016`	`3016`	`}`
`3017`	`3017`	`if (!jl_typemap_intersection_visitor(jl_atomic_load_relaxed(&mt->defs), 0, &env.match)) {`
`3018`	`3018`	`JL_GC_POP();`
`3019`		`- return jl_false;`
	`3019`	`+ return jl_nothing;`
`3020`	`3020`	`}`
`3021`	`3021`	`}`
`3022`	`3022`	`else {`
`3023`	`3023`	`// else: scan everything`
`3024`	`3024`	`if (!jl_foreach_reachable_mtable(ml_mtable_visitor, &env.match)) {`
`3025`	`3025`	`JL_GC_POP();`
`3026`		`- return jl_false;`
	`3026`	`+ return jl_nothing;`
`3027`	`3027`	`}`
`3028`	`3028`	`}`
`3029`	`3029`	`*min_valid = env.min_valid;`
`@@ -3105,7 +3105,7 @@ static jl_value_t ml_matches(jl_methtable_t mt,`
`3105`	`3105`	`}`
`3106`	`3106`	`else if (lim == 1) {`
`3107`	`3107`	`JL_GC_POP();`
`3108`		`- return jl_false;`
	`3108`	`+ return jl_nothing;`
`3109`	`3109`	`}`
`3110`	`3110`	`}`
`3111`	`3111`	`else {`
`@@ -3249,7 +3249,7 @@ static jl_value_t ml_matches(jl_methtable_t mt,`
`3249`	`3249`	`ndisjoint += 1;`
`3250`	`3250`	`if (ndisjoint > lim) {`
`3251`	`3251`	`JL_GC_POP();`
`3252`		`- return jl_false;`
	`3252`	`+ return jl_nothing;`
`3253`	`3253`	`}`
`3254`	`3254`	`}`
`3255`	`3255`	`}`
`@@ -3396,7 +3396,7 @@ static jl_value_t ml_matches(jl_methtable_t mt,`
`3396`	`3396`	`*ambig = has_ambiguity;`
`3397`	`3397`	`JL_GC_POP();`
`3398`	`3398`	`if (lim >= 0 && len > lim)`
`3399`		`- return jl_false;`
	`3399`	`+ return jl_nothing;`
`3400`	`3400`	`return env.t;`
`3401`	`3401`	`}`
`3402`	`3402`