Skip to content

Commit

Permalink
inference: ensure inferring reachable code methods (#57088)
Browse files Browse the repository at this point in the history
PR #51317 was a bit over-eager about inferring inferring unreachable
code methods. Filter out the Vararg case, since that can be handled by
simply removing it instead of discarding the whole call.

Fixes #56628

(cherry picked from commit eb9f24c)
  • Loading branch information
vtjnash committed Feb 19, 2025
1 parent 1895479 commit 4226f1e
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 4 deletions.
4 changes: 3 additions & 1 deletion base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ function find_matching_methods(𝕃::AbstractLattice,
for i in 1:length(split_argtypes)
arg_n = split_argtypes[i]::Vector{Any}
sig_n = argtypes_to_type(arg_n)
sig_n === Bottom && continue
mt = ccall(:jl_method_table_for, Any, (Any,), sig_n)
mt === nothing && return FailedMethodMatch("Could not identify method table for call")
mt = mt::MethodTable
Expand Down Expand Up @@ -508,7 +509,7 @@ function abstract_call_method(interp::AbstractInterpreter,
sigtuple = unwrap_unionall(sig)
sigtuple isa DataType ||
return MethodCallResult(Any, false, false, nothing, Effects())
all(@nospecialize(x) -> valid_as_lattice(unwrapva(x), true), sigtuple.parameters) ||
all(@nospecialize(x) -> isvarargtype(x) || valid_as_lattice(x, true), sigtuple.parameters) ||
return MethodCallResult(Union{}, false, false, nothing, EFFECTS_THROWS) # catch bad type intersections early

if is_nospecializeinfer(method)
Expand Down Expand Up @@ -2166,6 +2167,7 @@ function abstract_call_unknown(interp::AbstractInterpreter, @nospecialize(ft),
end
# non-constant function, but the number of arguments is known and the `f` is not a builtin or intrinsic
atype = argtypes_to_type(arginfo.argtypes)
atype === Bottom && return CallMeta(Union{}, Union{}, EFFECTS_THROWS, NoCallInfo()) # accidentally unreachable
return abstract_call_gf_by_type(interp, nothing, arginfo, si, atype, sv, max_methods)
end

Expand Down
5 changes: 3 additions & 2 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1450,8 +1450,9 @@ function handle_call!(todo::Vector{Pair{Int,Any}},
cases = compute_inlining_cases(info, flag, sig, state)
cases === nothing && return nothing
cases, all_covered, joint_effects = cases
handle_cases!(todo, ir, idx, stmt, argtypes_to_type(sig.argtypes), cases,
all_covered, joint_effects)
atype = argtypes_to_type(sig.argtypes)
atype === Union{} && return nothing # accidentally actually unreachable
handle_cases!(todo, ir, idx, stmt, atype, cases, all_covered, joint_effects)
end

function handle_match!(cases::Vector{InliningCase},
Expand Down
4 changes: 4 additions & 0 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2697,6 +2697,10 @@ function abstract_applicable(interp::AbstractInterpreter, argtypes::Vector{Any},
isvarargtype(argtypes[2]) && return CallMeta(Bool, EFFECTS_UNKNOWN, NoCallInfo())
argtypes = argtypes[2:end]
atype = argtypes_to_type(argtypes)
if atype === Union{}
rt = Union{} # accidentally unreachable code
return CallMeta(rt, EFFECTS_TOTAL, NoCallInfo())
end
matches = find_matching_methods(typeinf_lattice(interp), argtypes, atype, method_table(interp),
InferenceParams(interp).max_union_splitting, max_methods)
if isa(matches, FailedMethodMatch)
Expand Down
7 changes: 6 additions & 1 deletion base/compiler/typeutils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ has_extended_info(@nospecialize x) = (!isa(x, Type) && !isvarargtype(x)) || isTy
# certain combinations of `a` and `b` where one/both isa/are `Union`/`UnionAll` type(s)s.
isnotbrokensubtype(@nospecialize(a), @nospecialize(b)) = (!iskindtype(b) || !isType(a) || hasuniquerep(a.parameters[1]) || b <: a)

argtypes_to_type(argtypes::Array{Any,1}) = Tuple{anymap(@nospecialize(a) -> isvarargtype(a) ? a : widenconst(a), argtypes)...}
function argtypes_to_type(argtypes::Array{Any,1})
argtypes = anymap(@nospecialize(a) -> isvarargtype(a) ? a : widenconst(a), argtypes)
filter!(@nospecialize(x) -> !isvarargtype(x) || valid_as_lattice(unwrapva(x), true), argtypes)
all(@nospecialize(x) -> isvarargtype(x) || valid_as_lattice(x, true), argtypes) || return Bottom
return Tuple{argtypes...}
end

function isknownlength(t::DataType)
isvatuple(t) || return true
Expand Down
6 changes: 6 additions & 0 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5193,3 +5193,9 @@ end
@test only(Base.return_types((x,f) -> getfield(x, f), (An51317, Symbol))) === Int
@test only(Base.return_types(x -> getfield(x, :b), (A51317,))) === Union{}
@test only(Base.return_types(x -> getfield(x, :b), (An51317,))) === Union{}

# issue #56628
@test Core.Compiler.argtypes_to_type(Any[ Int, UnitRange{Int}, Vararg{Pair{Any, Union{}}} ]) === Tuple{Int, UnitRange{Int}}
@test Core.Compiler.argtypes_to_type(Any[ Int, UnitRange{Int}, Vararg{Pair{Any, Union{}}}, Float64 ]) === Tuple{Int, UnitRange{Int}, Float64}
@test Core.Compiler.argtypes_to_type(Any[ Int, UnitRange{Int}, Vararg{Pair{Any, Union{}}}, Float64, Tuple{2} ]) === Union{}
@test Base.return_types(Tuple{Tuple{Int, Vararg{Pair{Any, Union{}}}}},) do x; Returns(true)(x...); end |> only === Bool

0 comments on commit 4226f1e

Please sign in to comment.