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
  • Loading branch information
vtjnash authored Jan 21, 2025
1 parent 323ca86 commit eb9f24c
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 18 deletions.
4 changes: 3 additions & 1 deletion Compiler/src/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ function find_union_split_method_matches(interp::AbstractInterpreter, argtypes::
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 @@ -614,7 +615,7 @@ function abstract_call_method(interp::AbstractInterpreter,
sigtuple = unwrap_unionall(sig)
sigtuple isa DataType ||
return Future(MethodCallResult(Any, Any, Effects(), nothing, false, false))
all(@nospecialize(x) -> valid_as_lattice(unwrapva(x), true), sigtuple.parameters) ||
all(@nospecialize(x) -> isvarargtype(x) || valid_as_lattice(x, true), sigtuple.parameters) ||
return Future(MethodCallResult(Union{}, Any, EFFECTS_THROWS, nothing, false, false)) # catch bad type intersections early

if is_nospecializeinfer(method)
Expand Down Expand Up @@ -2840,6 +2841,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 Future(CallMeta(Union{}, Union{}, EFFECTS_THROWS, NoCallInfo())) # accidentally unreachable
return abstract_call_gf_by_type(interp, nothing, arginfo, si, atype, sv, max_methods)::Future
end

Expand Down
1 change: 1 addition & 0 deletions Compiler/src/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1399,6 +1399,7 @@ function handle_call!(todo::Vector{Pair{Int,Any}},
cases === nothing && return nothing
cases, handled_all_cases, fully_covered, joint_effects = cases
atype = argtypes_to_type(sig.argtypes)
atype === Union{} && return nothing # accidentally actually unreachable
handle_cases!(todo, ir, idx, stmt, atype, cases, handled_all_cases, fully_covered, joint_effects)
end

Expand Down
36 changes: 20 additions & 16 deletions Compiler/src/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3016,24 +3016,28 @@ function abstract_applicable(interp::AbstractInterpreter, argtypes::Vector{Any},
isvarargtype(argtypes[2]) && return Future(CallMeta(Bool, ArgumentError, EFFECTS_THROWS, NoCallInfo()))
argtypes = argtypes[2:end]
atype = argtypes_to_type(argtypes)
matches = find_method_matches(interp, argtypes, atype; max_methods)
info = NoCallInfo()
if isa(matches, FailedMethodMatch)
rt = Bool # too many matches to analyze
if atype === Union{}
rt = Union{} # accidentally unreachable code
else
(; valid_worlds, applicable) = matches
update_valid_age!(sv, valid_worlds)
napplicable = length(applicable)
if napplicable == 0
rt = Const(false) # never any matches
elseif !fully_covering(matches) || any_ambig(matches)
# Account for the fact that we may encounter a MethodError with a non-covered or ambiguous signature.
rt = Bool
matches = find_method_matches(interp, argtypes, atype; max_methods)
info = NoCallInfo()
if isa(matches, FailedMethodMatch)
rt = Bool # too many matches to analyze
else
rt = Const(true) # has applicable matches
end
if rt !== Bool
info = VirtualMethodMatchInfo(matches.info)
(; valid_worlds, applicable) = matches
update_valid_age!(sv, valid_worlds)
napplicable = length(applicable)
if napplicable == 0
rt = Const(false) # never any matches
elseif !fully_covering(matches) || any_ambig(matches)
# Account for the fact that we may encounter a MethodError with a non-covered or ambiguous signature.
rt = Bool
else
rt = Const(true) # has applicable matches
end
if rt !== Bool
info = VirtualMethodMatchInfo(matches.info)
end
end
end
return Future(CallMeta(rt, Union{}, EFFECTS_TOTAL, info))
Expand Down
7 changes: 6 additions & 1 deletion Compiler/src/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 Compiler/test/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6182,3 +6182,9 @@ end <: Any
end
return out
end == Union{Float64,DomainError}

# issue #56628
@test Compiler.argtypes_to_type(Any[ Int, UnitRange{Int}, Vararg{Pair{Any, Union{}}} ]) === Tuple{Int, UnitRange{Int}}
@test Compiler.argtypes_to_type(Any[ Int, UnitRange{Int}, Vararg{Pair{Any, Union{}}}, Float64 ]) === Tuple{Int, UnitRange{Int}, Float64}
@test Compiler.argtypes_to_type(Any[ Int, UnitRange{Int}, Vararg{Pair{Any, Union{}}}, Float64, Memory{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 eb9f24c

Please sign in to comment.