Skip to content

Commit

Permalink
make ml_matches return nothing when there are too many matches
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aviatesk committed Nov 28, 2022
1 parent 1badb9d commit c6e48b0
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 25 deletions.
4 changes: 2 additions & 2 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
3 changes: 2 additions & 1 deletion base/compiler/bootstrap.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
16 changes: 7 additions & 9 deletions base/compiler/methodtable.jl
Original file line number Diff line number Diff line change
Expand Up @@ -64,31 +64,31 @@ 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
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(
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 {
Expand Down
16 changes: 8 additions & 8 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
}
}
Expand Down Expand Up @@ -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;
}

Expand Down
3 changes: 1 addition & 2 deletions stdlib/REPL/src/REPLCompletions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down

0 comments on commit c6e48b0

Please sign in to comment.