Skip to content

Commit

Permalink
Give MethodMatch its own type (#36702)
Browse files Browse the repository at this point in the history
Every time I see any code dealing with the svecs we get back from
the method match code, I think that it shouldn't really be an svec:
1. It's always the same length
2. Basically every use of it typeasserts the field type
3. Every use of it needs the same magic numbers to access the fields.

All put together this should just be a type. This updates all
the uses accordingly, but adds fallback getindex/iterate defintions
for external users that expect this to be a SimpleVector (in
deprecated.jl - hopefully by 2.0 all external users will upgraded).
  • Loading branch information
Keno committed Jul 20, 2020
1 parent 8f8604b commit 2cca0a4
Show file tree
Hide file tree
Showing 22 changed files with 215 additions and 155 deletions.
25 changes: 12 additions & 13 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
end
push!(infos, MethodMatchInfo(xapplicable, ambig))
append!(applicable, xapplicable)
thisfullmatch = _any(match->match[4], xapplicable)
thisfullmatch = _any(match->(match::MethodMatch).fully_covers, xapplicable)
found = false
for (i, mt′) in enumerate(mts)
if mt′ === mt
Expand Down Expand Up @@ -95,7 +95,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
return CallMeta(Any, false)
end
push!(mts, mt)
push!(fullmatch, _any(match->match[4], applicable))
push!(fullmatch, _any(match->(match::MethodMatch).fully_covers, applicable))
info = MethodMatchInfo(applicable, ambig)
end
update_valid_age!(min_valid[1], max_valid[1], sv)
Expand All @@ -109,17 +109,17 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
istoplevel = sv.linfo.def isa Module
multiple_matches = napplicable > 1

if f !== nothing && napplicable == 1 && is_method_pure(applicable[1][3], applicable[1][1], applicable[1][2])
if f !== nothing && napplicable == 1 && is_method_pure(applicable[1]::MethodMatch)
val = pure_eval_call(f, argtypes)
if val !== false
return CallMeta(val, info)
end
end

for i in 1:napplicable
match = applicable[i]::SimpleVector
method = match[3]::Method
sig = match[1]
match = applicable[i]::MethodMatch
method = match.method
sig = match.spec_types
if istoplevel && !isdispatchtuple(sig)
# only infer concrete call sites in top-level expressions
add_remark!(interp, sv, "Refusing to infer non-concrete call site in top-level expression")
Expand All @@ -143,7 +143,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
this_rt === Any && break
end
else
this_rt, edgecycle1, edge = abstract_call_method(interp, method, sig, match[2]::SimpleVector, multiple_matches, sv)
this_rt, edgecycle1, edge = abstract_call_method(interp, method, sig, match.sparams, multiple_matches, sv)
edgecycle |= edgecycle1::Bool
if edge !== nothing
push!(edges, edge)
Expand All @@ -167,7 +167,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
# if there's a possibility we could constant-propagate a better result
# (hopefully without doing too much work), try to do that now
# TODO: it feels like this could be better integrated into abstract_call_method / typeinf_edge
const_rettype = abstract_call_method_with_const_args(interp, rettype, f, argtypes, applicable[nonbot]::SimpleVector, sv, edgecycle)
const_rettype = abstract_call_method_with_const_args(interp, rettype, f, argtypes, applicable[nonbot]::MethodMatch, sv, edgecycle)
if const_rettype rettype
# use the better result, if it's a refinement of rettype
rettype = const_rettype
Expand Down Expand Up @@ -214,8 +214,8 @@ function const_prop_profitable(@nospecialize(arg))
return false
end

function abstract_call_method_with_const_args(interp::AbstractInterpreter, @nospecialize(rettype), @nospecialize(f), argtypes::Vector{Any}, match::SimpleVector, sv::InferenceState, edgecycle::Bool)
method = match[3]::Method
function abstract_call_method_with_const_args(interp::AbstractInterpreter, @nospecialize(rettype), @nospecialize(f), argtypes::Vector{Any}, match::MethodMatch, sv::InferenceState, edgecycle::Bool)
method = match.method
nargs::Int = method.nargs
method.isva && (nargs -= 1)
length(argtypes) >= nargs || return Any
Expand Down Expand Up @@ -261,9 +261,7 @@ function abstract_call_method_with_const_args(interp::AbstractInterpreter, @nosp
if istopfunction(f, :getproperty) || istopfunction(f, :setproperty!)
force_inference = true
end
sig = match[1]
sparams = match[2]::SimpleVector
mi = specialize_method(method, sig, sparams, !force_inference)
mi = specialize_method(match, !force_inference)
mi === nothing && return Any
mi = mi::MethodInstance
# decide if it's likely to be worthwhile
Expand Down Expand Up @@ -743,6 +741,7 @@ function is_method_pure(method::Method, @nospecialize(sig), sparams::SimpleVecto
end
return method.pure
end
is_method_pure(match::MethodMatch) = is_method_pure(match.method, match.spec_types, match.sparams)

function pure_eval_call(@nospecialize(f), argtypes::Vector{Any})
for i = 2:length(argtypes)
Expand Down
4 changes: 2 additions & 2 deletions base/compiler/bootstrap.jl
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ let fs = Any[typeinf_ext, typeinf, typeinf_edge, pure_eval_call, run_passes],
for f in fs
for m in _methods_by_ftype(Tuple{typeof(f), Vararg{Any}}, 10, typemax(UInt))
# remove any TypeVars from the intersection
typ = Any[m[1].parameters...]
typ = Any[m.spec_types.parameters...]
for i = 1:length(typ)
if isa(typ[i], TypeVar)
typ[i] = typ[i].ub
end
end
typeinf_type(interp, m[3], Tuple{typ...}, m[2])
typeinf_type(interp, m.method, Tuple{typ...}, m.sparams)
end
end
end
3 changes: 2 additions & 1 deletion base/compiler/compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ getfield(getfield(Main, :Core), :eval)(getfield(Main, :Core), :(baremodule Compi
using Core.Intrinsics, Core.IR

import Core: print, println, show, write, unsafe_write, stdout, stderr,
_apply, _apply_iterate, svec, apply_type, Builtin, IntrinsicFunction, MethodInstance, CodeInstance
_apply, _apply_iterate, svec, apply_type, Builtin, IntrinsicFunction,
MethodInstance, CodeInstance, MethodMatch

const getproperty = Core.getfield
const setproperty! = Core.setfield!
Expand Down
8 changes: 4 additions & 4 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1080,15 +1080,15 @@ function analyze_single_call!(ir::IRCode, todo::Vector{Any}, idx::Int, @nospecia
continue
elseif length(meth) == 1 && only_method !== false
if only_method === nothing
only_method = meth[1][3]
elseif only_method !== meth[1][3]
only_method = meth[1].method
elseif only_method !== meth[1].method
only_method = false
end
else
only_method = false
end
for match in meth::Vector{Any}
(metharg, methsp, method) = (match[1]::Type, match[2]::SimpleVector, match[3]::Method)
(metharg, methsp, method) = (match.spec_types, match.sparams, match.method)
signature_union = Union{signature_union, metharg}
if !isdispatchtuple(metharg)
fully_covered = false
Expand Down Expand Up @@ -1119,7 +1119,7 @@ function analyze_single_call!(ir::IRCode, todo::Vector{Any}, idx::Int, @nospecia
sig.atype, method.sig)::SimpleVector
else
@assert length(meth) == 1
(metharg, methsp, method) = (meth[1][1]::Type, meth[1][2]::SimpleVector, meth[1][3]::Method)
(metharg, methsp, method) = (meth[1].spec_types, meth[1].sparams, meth[1].method)
end
fully_covered = true
case = analyze_method!(idx, sig, metharg, methsp, method,
Expand Down
4 changes: 2 additions & 2 deletions base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -683,8 +683,8 @@ function _return_type(interp::AbstractInterpreter, @nospecialize(f), @nospeciali
rt = widenconst(rt)
end
else
for m in _methods(f, t, -1, get_world_counter(interp))
ty = typeinf_type(interp, m[3], m[1], m[2])
for match in _methods(f, t, -1, get_world_counter(interp))
ty = typeinf_type(interp, match.method, match.spec_types, match.sparams)
ty === nothing && return Any
rt = tmerge(rt, ty)
rt === Any && break
Expand Down
4 changes: 4 additions & 0 deletions base/compiler/utilities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ function specialize_method(method::Method, @nospecialize(atypes), sparams::Simpl
return ccall(:jl_specializations_get_linfo, Ref{MethodInstance}, (Any, Any, Any), method, atypes, sparams)
end

function specialize_method(match::MethodMatch, preexisting::Bool=false)
return specialize_method(match.method, match.spec_types, match.sparams, preexisting)
end

# This function is used for computing alternate limit heuristics
function method_for_inference_heuristics(method::Method, @nospecialize(sig), sparams::SimpleVector)
if isdefined(method, :generator) && method.generator.expand_early && may_invoke_generator(method, sig, sparams)
Expand Down
12 changes: 12 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -208,3 +208,15 @@ macro get!(h, key0, default)
end

# END 1.5 deprecations

# BEGIN 1.6 deprecations

# These changed from SimpleVector to `MethodMatch`. These definitions emulate
# being a SimpleVector to ease transition for packages that make explicit
# use of (internal) APIs that return raw method matches.
iterate(match::Core.MethodMatch, field::Int=1) =
field > nfields(match) ? nothing : (getfield(match, field), field+1)
getindex(match::Core.MethodMatch, field::Int) =
getfield(match, field)

# END 1.6 deprecations
33 changes: 17 additions & 16 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -893,7 +893,7 @@ function methods(@nospecialize(f), @nospecialize(t),
end
t = to_tuple_type(t)
world = typemax(UInt)
MethodList(Method[m[3] for m in _methods(f, t, -1, world) if mod === nothing || m[3].module in mod],
MethodList(Method[m.method for m in _methods(f, t, -1, world) if mod === nothing || m.method.module in mod],
typeof(f).name.mt)
end

Expand All @@ -907,7 +907,7 @@ function methods_including_ambiguous(@nospecialize(f), @nospecialize(t))
has_ambig = Int32[0]
ms = _methods_by_ftype(tt, -1, world, true, min, max, has_ambig)
ms === false && return false
return MethodList(Method[m[3] for m in ms], typeof(f).name.mt)
return MethodList(Method[m.method for m in ms], typeof(f).name.mt)
end

function methods(@nospecialize(f),
Expand Down Expand Up @@ -979,9 +979,9 @@ const _uncompressed_ast = _uncompressed_ir
function method_instances(@nospecialize(f), @nospecialize(t), world::UInt = typemax(UInt))
tt = signature_type(f, t)
results = Core.MethodInstance[]
for method_data in _methods_by_ftype(tt, -1, world)
mtypes, msp, m = method_data
instance = ccall(:jl_specializations_get_linfo, Ref{MethodInstance}, (Any, Any, Any), m, mtypes, msp)
for match in _methods_by_ftype(tt, -1, world)
instance = ccall(:jl_specializations_get_linfo, Ref{MethodInstance},
(Any, Any, Any), match.method, match.spec_types, match.sparams)
push!(results, instance)
end
return results
Expand Down Expand Up @@ -1142,14 +1142,14 @@ function code_typed_by_type(@nospecialize(tt::Type);
throw(ArgumentError("'debuginfo' must be either :source or :none"))
end
tt = to_tuple_type(tt)
meths = _methods_by_ftype(tt, -1, world)
if meths === false
matches = _methods_by_ftype(tt, -1, world)
if matches === false
error("signature does not correspond to a generic function")
end
asts = []
for x in meths
meth = func_for_method_checked(x[3], tt, x[2])
(code, ty) = Core.Compiler.typeinf_code(interp, meth, x[1], x[2], optimize)
for match in matches
meth = func_for_method_checked(match.method, tt, match.sparams)
(code, ty) = Core.Compiler.typeinf_code(interp, meth, match.spec_types, match.sparams, optimize)
code === nothing && error("inference not successful") # inference disabled?
debuginfo === :none && remove_linenums!(code)
push!(asts, code => ty)
Expand All @@ -1165,9 +1165,9 @@ function return_types(@nospecialize(f), @nospecialize(types=Tuple), interp=Core.
types = to_tuple_type(types)
rt = []
world = get_world_counter()
for x in _methods(f, types, -1, world)
meth = func_for_method_checked(x[3], types, x[2])
ty = Core.Compiler.typeinf_type(interp, meth, x[1], x[2])
for match in _methods(f, types, -1, world)
meth = func_for_method_checked(match.method, types, match.sparams)
ty = Core.Compiler.typeinf_type(interp, meth, match.spec_types, match.sparams)
ty === nothing && error("inference not successful") # inference disabled?
push!(rt, ty)
end
Expand Down Expand Up @@ -1359,11 +1359,12 @@ function isambiguous(m1::Method, m2::Method; ambiguous_bottom::Bool=false)
if !ambiguous_bottom
has_bottom_parameter(ti) && return false
end
ml = _methods_by_ftype(ti, -1, typemax(UInt))
for m in ml
matches = _methods_by_ftype(ti, -1, typemax(UInt))
for match in matches
m = match.method
m === m1 && continue
m === m2 && continue
if ti <: m[3].sig && morespecific(m[3].sig, m1.sig) && morespecific(m[3].sig, m2.sig)
if ti <: m.sig && morespecific(m.sig, m1.sig) && morespecific(m.sig, m2.sig)
return false
end
end
Expand Down
1 change: 1 addition & 0 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -1583,6 +1583,7 @@ void jl_init_primitives(void) JL_GC_DISABLED
add_builtin("Argument", (jl_value_t*)jl_argument_type);
add_builtin("Const", (jl_value_t*)jl_const_type);
add_builtin("PartialStruct", (jl_value_t*)jl_partial_struct_type);
add_builtin("MethodMatch", (jl_value_t*)jl_method_match_type);
add_builtin("IntrinsicFunction", (jl_value_t*)jl_intrinsic_type);
add_builtin("Function", (jl_value_t*)jl_function_type);
add_builtin("Builtin", (jl_value_t*)jl_builtin_type);
Expand Down
1 change: 1 addition & 0 deletions src/clangsa/GCChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ bool GCChecker::isGCTrackedType(QualType QT) {
Name.endswith_lower("jl_excstack_t") ||
Name.endswith_lower("jl_task_t") ||
Name.endswith_lower("jl_uniontype_t") ||
Name.endswith_lower("jl_method_match_t") ||
// Probably not technically true for these, but let's allow
// it
Name.endswith_lower("typemap_intersection_env") ||
Expand Down
6 changes: 4 additions & 2 deletions src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,8 @@ static void jl_collect_backedges(jl_array_t *s, jl_array_t *t)
}
size_t k;
for (k = 0; k < jl_array_len(matches); k++) {
jl_array_ptr_set(matches, k, jl_svecref(jl_array_ptr_ref(matches, k), 2));
jl_method_match_t *match = (jl_method_match_t *)jl_array_ptr_ref(matches, k);
jl_array_ptr_set(matches, k, match->method);
}
jl_array_ptr_1d_push(t, callee);
jl_array_ptr_1d_push(t, matches);
Expand Down Expand Up @@ -1837,7 +1838,8 @@ static void jl_verify_edges(jl_array_t *targets, jl_array_t **pvalids)
else {
size_t j, k, l = jl_array_len(expected);
for (k = 0; k < jl_array_len(matches); k++) {
jl_method_t *m = (jl_method_t*)jl_svecref(jl_array_ptr_ref(matches, k), 2);
jl_method_match_t *match = jl_array_ptr_ref(matches, k);
jl_method_t *m = match->method;
for (j = 0; j < l; j++) {
if (m == (jl_method_t*)jl_array_ptr_ref(expected, j))
break;
Expand Down
Loading

0 comments on commit 2cca0a4

Please sign in to comment.