Skip to content

Commit

Permalink
typelimits: Prevent accidental infinite growth in size limiting (#48421)
Browse files Browse the repository at this point in the history 
* typelimits: Prevent accidental infinite growth in size limiting

There's a few related issues here:
1. Type size limiting depended on the identity of contained TypeVars.
   E.g. `Base.rewrap_unionall(Foo{Base.unwrap_unionall(A)}, A)`, would
   not be limited while the equivalent with typevars substituted would be.
   This is obviously undesirable because the identity of typevars is an
   implementation detail.
2. We were forcing `tupledepth` to 1 in the typevar case to allow replacing
   typevars by something concrete. However, this also allowed typevars being
   replaced by something derived from `sources`, which could be huge and itself
   contain TypeVars, allowing infinite growth.
3. There was a type query that was run on types containg free typevars. This
   would have asserted in a debug build and gave wrong answers in a release build.

Fix all three issues, which together addresses an inference non-termination seen in #48329.

* wip: fix `apply_type_tfunc` accuracy (#48329)

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
  • Loading branch information
@Keno @aviatesk
Keno and aviatesk authored Feb 5, 2023
1 parent 07c4244 commit 51e3bc3
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 33 deletions.
36 changes: 16 additions & 20 deletions base/compiler/tfuncs.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1757,34 +1757,30 @@ const _tvarnames = Symbol[:_A, :_B, :_C, :_D, :_E, :_F, :_G, :_H, :_I, :_J, :_K,
push!(tparams, ai.tv)
else
uncertain = true
# These blocks improve type info but make compilation a bit slower.
# XXX
#unw = unwrap_unionall(ai)
#isT = isType(unw)
#if isT && isa(ai,UnionAll) && contains_is(outervars, ai.var)
# ai = rename_unionall(ai)
# unw = unwrap_unionall(ai)
#end
unw = unwrap_unionall(ai)
isT = isType(unw)
if isT && isa(ai,UnionAll) && contains_is(outervars, ai.var)
ai = rename_unionall(ai)
unw = unwrap_unionall(ai)
end
ai_w = widenconst(ai)
ub = ai_w isa Type && ai_w <: Type ? instanceof_tfunc(ai)[1] : Any
if istuple
# in the last parameter of a Tuple type, if the upper bound is Any
# then this could be a Vararg type.
if i == largs && ub === Any
push!(tparams, Vararg)
# XXX
#elseif isT
# push!(tparams, rewrap_unionall(unw.parameters[1], ai))
elseif isT
push!(tparams, rewrap_unionall(unw.parameters[1], ai))
else
push!(tparams, Any)
end
# XXX
#elseif isT
# push!(tparams, unw.parameters[1])
# while isa(ai, UnionAll)
# push!(outervars, ai.var)
# ai = ai.body
# end
elseif isT
push!(tparams, unw.parameters[1])
while isa(ai, UnionAll)
push!(outervars, ai.var)
ai = ai.body
end
else
# Is this the second parameter to a NamedTuple?
if isa(uw, DataType) && uw.name === _NAMEDTUPLE_NAME && isa(ua, UnionAll) && uw.parameters[2] === ua.var
Expand Down Expand Up @@ -2278,7 +2274,7 @@ function builtin_effects(𝕃::AbstractLattice, @nospecialize(f::Builtin), argin
else
effect_free = ALWAYS_FALSE
end
nothrow = (!(!isempty(argtypes) && isvarargtype(argtypes[end])) && builtin_nothrow(𝕃, f, argtypes, rt))
nothrow = (isempty(argtypes) || !isvarargtype(argtypes[end])) && builtin_nothrow(𝕃, f, argtypes, rt)
if contains_is(_INACCESSIBLEMEM_BUILTINS, f)
inaccessiblememonly = ALWAYS_TRUE
elseif contains_is(_ARGMEM_BUILTINS, f)
Expand Down Expand Up @@ -2460,7 +2456,7 @@ function intrinsic_effects(f::IntrinsicFunction, argtypes::Vector{Any})

consistent = contains_is(_INCONSISTENT_INTRINSICS, f) ? ALWAYS_FALSE : ALWAYS_TRUE
effect_free = !(f === Intrinsics.pointerset) ? ALWAYS_TRUE : ALWAYS_FALSE
nothrow = (!(!isempty(argtypes) && isvarargtype(argtypes[end])) && intrinsic_nothrow(f, argtypes))
nothrow = (isempty(argtypes) || !isvarargtype(argtypes[end])) && intrinsic_nothrow(f, argtypes)

return Effects(EFFECTS_TOTAL; consistent, effect_free, nothrow)
end
Expand Down
16 changes: 8 additions & 8 deletions base/compiler/typelimits.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ end
# try to find `type` somewhere in `comparison` type
# at a minimum nesting depth of `mindepth`
function is_derived_type(@nospecialize(t), @nospecialize(c), mindepth::Int)
if has_free_typevars(t) || has_free_typevars(c)
# Don't allow finding types with free typevars. These strongly depend
# on identity and we do not make any effort to make sure this returns
# sensible results in that case.
return false
end
if t === c
return mindepth <= 1
end
Expand Down Expand Up @@ -87,10 +93,7 @@ function _limit_type_size(@nospecialize(t), @nospecialize(c), sources::SimpleVec
return t # fast path: unparameterized are always simple
else
ut = unwrap_unionall(t)
if isa(ut, DataType) && isa(c, Type) && c !== Union{} && c <: t
# TODO: need to check that the UnionAll bounds on t are limited enough too
return t # t is already wider than the comparison in the type lattice
elseif is_derived_type_from_any(ut, sources, depth)
if is_derived_type_from_any(ut, sources, depth)
return t # t isn't something new
end
end
Expand Down Expand Up @@ -208,9 +211,6 @@ function type_more_complex(@nospecialize(t), @nospecialize(c), sources::SimpleVe
return false # Bottom is as simple as they come
elseif isa(t, DataType) && isempty(t.parameters)
return false # fastpath: unparameterized types are always finite
elseif tupledepth > 0 && isa(unwrap_unionall(t), DataType) && isa(c, Type) && c !== Union{} && c <: t
# TODO: need to check that the UnionAll bounds on t are limited enough too
return false # t is already wider than the comparison in the type lattice
elseif tupledepth > 0 && is_derived_type_from_any(unwrap_unionall(t), sources, depth)
return false # t isn't something new
end
Expand All @@ -227,7 +227,7 @@ function type_more_complex(@nospecialize(t), @nospecialize(c), sources::SimpleVe
end
# rules for various comparison types
if isa(c, TypeVar)
tupledepth = 1 # allow replacing a TypeVar with a concrete value (since we know the UnionAll must be in covariant position)
tupledepth = 1
if isa(t, TypeVar)
return !(t.lb === Union{} || t.lb === c.lb) || # simplify lb towards Union{}
type_more_complex(t.ub, c.ub, sources, depth + 1, tupledepth, 0)
Expand Down
27 changes: 22 additions & 5 deletions test/compiler/inference.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ let comparison = Tuple{X, X} where X<:Tuple
@test Core.Compiler.limit_type_size(sig, comparison, comparison, 100, 100) == Tuple{Tuple, Tuple}
@test Core.Compiler.limit_type_size(sig, ref, comparison, 100, 100) == Tuple{Any, Any}
@test Core.Compiler.limit_type_size(Tuple{sig}, Tuple{ref}, comparison, 100, 100) == Tuple{Tuple{Any, Any}}
@test Core.Compiler.limit_type_size(sig, ref, Tuple{comparison}, 100, 100) == Tuple{Tuple{X, X} where X<:Tuple, Tuple{X, X} where X<:Tuple}
@test Core.Compiler.limit_type_size(ref, sig, Union{}, 100, 100) == ref
end

Expand All @@ -51,6 +50,13 @@ let va = ccall(:jl_type_intersection_with_env, Any, (Any, Any), Tuple{Tuple}, Tu
@test Core.Compiler.__limit_type_size(Tuple, va, Core.svec(va, Union{}), 2, 2) === Tuple
end

mutable struct TS14009{T}; end
let A = TS14009{TS14009{TS14009{TS14009{TS14009{T}}}}} where {T},
B = Base.rewrap_unionall(TS14009{Base.unwrap_unionall(A)}, A)

@test Core.Compiler.Compiler.limit_type_size(B, A, A, 2, 2) == TS14009
end

# issue #42835
@test !Core.Compiler.type_more_complex(Int, Any, Core.svec(), 1, 1, 1)
@test !Core.Compiler.type_more_complex(Int, Type{Int}, Core.svec(), 1, 1, 1)
Expand Down Expand Up @@ -81,7 +87,7 @@ end
@test !Core.Compiler.type_more_complex(Type{1}, Type{2}, Core.svec(), 1, 1, 1)
@test Core.Compiler.type_more_complex(Type{Union{Float32,Float64}}, Union{Float32,Float64}, Core.svec(Union{Float32,Float64}), 1, 1, 1)
@test !Core.Compiler.type_more_complex(Type{Union{Float32,Float64}}, Union{Float32,Float64}, Core.svec(Union{Float32,Float64}), 0, 1, 1)
@test_broken Core.Compiler.type_more_complex(Type{<:Union{Float32,Float64}}, Type{Union{Float32,Float64}}, Core.svec(Union{Float32,Float64}), 1, 1, 1)
@test Core.Compiler.type_more_complex(Type{<:Union{Float32,Float64}}, Type{Union{Float32,Float64}}, Core.svec(Union{Float32,Float64}), 1, 1, 1)
@test Core.Compiler.type_more_complex(Type{<:Union{Float32,Float64}}, Any, Core.svec(Union{Float32,Float64}), 1, 1, 1)


Expand Down Expand Up @@ -2741,6 +2747,17 @@ end |> only === Int
@test only(Base.return_types(Core.apply_type, Tuple{Any})) == Any
@test only(Base.return_types(Core.apply_type, Tuple{Any,Any})) == Any

# `apply_type_tfunc` accuracy for constrained type construction
# https://github.com/JuliaLang/julia/issues/47089
import Core: Const
import Core.Compiler: apply_type_tfunc
struct Issue47089{A,B} end
let 𝕃 = Core.Compiler.fallback_lattice
A = Type{<:Integer}
@test apply_type_tfunc(𝕃, Const(Issue47089), A, A) <: (Type{Issue47089{A,B}} where {A<:Integer, B<:Integer})
end
@test only(Base.return_types(keys, (Dict{String},))) == Base.KeySet{String, T} where T<:(Dict{String})

# PR 27351, make sure optimized type intersection for method invalidation handles typevars

abstract type AbstractT27351 end
Expand Down Expand Up @@ -3690,10 +3707,10 @@ Base.iterate(::Itr41839_3 , i) = i < 16 ? (i, i + 1) : nothing

# issue #32699
f32699(a) = (id = a[1],).id
@test Base.return_types(f32699, (Vector{Union{Int,Missing}},)) == Any[Union{Int,Missing}]
@test only(Base.return_types(f32699, (Vector{Union{Int,Missing}},))) == Union{Int,Missing}
g32699(a) = Tuple{a}
@test Base.return_types(g32699, (Type{<:Integer},))[1] == Type{<:Tuple{Any}}
@test Base.return_types(g32699, (Type,))[1] == Type{<:Tuple}
@test only(Base.return_types(g32699, (Type{<:Integer},))) <: Type{<:Tuple{Any}}
@test only(Base.return_types(g32699, (Type,))) <: Type{<:Tuple}

# Inference precision of union-split calls
function f_apply_union_split(fs, x)
Expand Down

0 comments on commit 51e3bc3

Please sign in to comment.