Skip to content

Commit bbe91db

Browse files
aviateskKristofferC
authored andcommitted
inference: improve TypeVar/Vararg handling
During working on the incoming lattice overhaul, I found it's quite confusing that `TypeVar` and `Vararg` can appear in the same context as valid `Type` objects as well as extended lattice elements. Since it usually needs special cases to operate on `TypeVar` and `Vararg` (e.g. they can not be used in subtyping as an obvious example), I believe it would be great avoid bugs and catch logic errors in the future development if we separate contexts where they can appear from ones where `Type` objects and extended lattice elements are expected. So this commit: - tries to separate their context, e.g. now `TypeVar` and `Vararg` should not be used in `_limit_type_size`, which is supposed to return `Type`, but they should be handled its helper function `__limit_type_size` - makes sure `tfunc`s don't return `TypeVar`s and `TypeVar` never spills into the abstract state - makes sure `widenconst` are not called on `TypeVar` and `Vararg`, and now `widenconst` is ensured to return `Type` always - and does other general refactors (cherry picked from commit d60f92c)
1 parent 9723f82 commit bbe91db

File tree

12 files changed

+99
-89
lines changed

12 files changed

+99
-89
lines changed

base/compiler/abstractinterpretation.jl

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -771,10 +771,7 @@ function precise_container_type(interp::AbstractInterpreter, @nospecialize(itft)
771771
if isa(tti, DataType) && tti.name === NamedTuple_typename
772772
# A NamedTuple iteration is the same as the iteration of its Tuple parameter:
773773
# compute a new `tti == unwrap_unionall(tti0)` based on that Tuple type
774-
tti = tti.parameters[2]
775-
while isa(tti, TypeVar)
776-
tti = tti.ub
777-
end
774+
tti = unwraptv(tti.parameters[2])
778775
tti0 = rewrap_unionall(tti, tti0)
779776
end
780777
if isa(tti, Union)
@@ -1156,7 +1153,8 @@ function abstract_call_builtin(interp::AbstractInterpreter, f::Builtin, fargs::U
11561153
end
11571154
end
11581155
end
1159-
return isa(rt, TypeVar) ? rt.ub : rt
1156+
@assert !isa(rt, TypeVar) "unhandled TypeVar"
1157+
return rt
11601158
end
11611159

11621160
function abstract_call_unionall(argtypes::Vector{Any})
@@ -1420,7 +1418,7 @@ function sp_type_rewrap(@nospecialize(T), linfo::MethodInstance, isreturn::Bool)
14201418
spsig = linfo.def.sig
14211419
if isa(spsig, UnionAll)
14221420
if !isempty(linfo.sparam_vals)
1423-
sparam_vals = Any[isa(v, Core.TypeofVararg) ? TypeVar(:N, Union{}, Any) :
1421+
sparam_vals = Any[isvarargtype(v) ? TypeVar(:N, Union{}, Any) :
14241422
v for v in linfo.sparam_vals]
14251423
T = ccall(:jl_instantiate_type_in_env, Any, (Any, Any, Ptr{Any}), T, spsig, sparam_vals)
14261424
isref && isreturn && T === Any && return Bottom # catch invalid return Ref{T} where T = Any
@@ -1434,10 +1432,7 @@ function sp_type_rewrap(@nospecialize(T), linfo::MethodInstance, isreturn::Bool)
14341432
end
14351433
end
14361434
end
1437-
while isa(T, TypeVar)
1438-
T = T.ub
1439-
end
1440-
return T
1435+
return unwraptv(T)
14411436
end
14421437

14431438
function abstract_eval_cfunction(interp::AbstractInterpreter, e::Expr, vtypes::VarTable, sv::InferenceState)
@@ -1651,7 +1646,7 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e),
16511646
else
16521647
t = abstract_eval_value_expr(interp, e, vtypes, sv)
16531648
end
1654-
@assert !isa(t, TypeVar)
1649+
@assert !isa(t, TypeVar) "unhandled TypeVar"
16551650
if isa(t, DataType) && isdefined(t, :instance)
16561651
# replace singleton types with their equivalent Const object
16571652
t = Const(t.instance)
@@ -1736,7 +1731,8 @@ function widenreturn(@nospecialize(rt), @nospecialize(bestguess), nslots::Int, s
17361731
fields = copy(rt.fields)
17371732
haveconst = false
17381733
for i in 1:length(fields)
1739-
a = widenreturn(fields[i], bestguess, nslots, slottypes, changes)
1734+
a = fields[i]
1735+
a = isvarargtype(a) ? a : widenreturn(a, bestguess, nslots, slottypes, changes)
17401736
if !haveconst && has_const_info(a)
17411737
# TODO: consider adding && const_prop_profitable(a) here?
17421738
haveconst = true

base/compiler/bootstrap.jl

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,7 @@ let fs = Any[typeinf_ext, typeinf, typeinf_edge, pure_eval_call, run_passes],
2525
# remove any TypeVars from the intersection
2626
typ = Any[m.spec_types.parameters...]
2727
for i = 1:length(typ)
28-
if isa(typ[i], TypeVar)
29-
typ[i] = typ[i].ub
30-
end
28+
typ[i] = unwraptv(typ[i])
3129
end
3230
typeinf_type(interp, m.method, Tuple{typ...}, m.sparams)
3331
end

base/compiler/compiler.jl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ using Core.Intrinsics, Core.IR
66

77
import Core: print, println, show, write, unsafe_write, stdout, stderr,
88
_apply_iterate, svec, apply_type, Builtin, IntrinsicFunction,
9-
MethodInstance, CodeInstance, MethodMatch, PartialOpaque
9+
MethodInstance, CodeInstance, MethodMatch, PartialOpaque,
10+
TypeofVararg
1011

1112
const getproperty = Core.getfield
1213
const setproperty! = Core.setfield!

base/compiler/inferenceresult.jl

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,7 @@ function most_general_argtypes(method::Union{Method, Nothing}, @nospecialize(spe
113113
atyp = unwrapva(atyp)
114114
tail_index -= 1
115115
end
116-
while isa(atyp, TypeVar)
117-
atyp = atyp.ub
118-
end
116+
atyp = unwraptv(atyp)
119117
if isa(atyp, DataType) && isdefined(atyp, :instance)
120118
# replace singleton types with their equivalent Const object
121119
atyp = Const(atyp.instance)

base/compiler/inferencestate.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ function sptypes_from_meth_instance(linfo::MethodInstance)
297297
ty = UnionAll(tv, Type{tv})
298298
end
299299
end
300-
elseif isa(v, Core.TypeofVararg)
300+
elseif isvarargtype(v)
301301
ty = Int
302302
else
303303
ty = Const(v)

base/compiler/ssair/inlining.jl

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,7 @@ end
808808

809809
function validate_sparams(sparams::SimpleVector)
810810
for i = 1:length(sparams)
811-
(isa(sparams[i], TypeVar) || isa(sparams[i], Core.TypeofVararg)) && return false
811+
(isa(sparams[i], TypeVar) || isvarargtype(sparams[i])) && return false
812812
end
813813
return true
814814
end
@@ -937,9 +937,7 @@ function is_valid_type_for_apply_rewrite(@nospecialize(typ), params::Optimizatio
937937
typ = widenconst(typ)
938938
if isa(typ, DataType) && typ.name === NamedTuple_typename
939939
typ = typ.parameters[2]
940-
while isa(typ, TypeVar)
941-
typ = typ.ub
942-
end
940+
typ = unwraptv(typ)
943941
end
944942
isa(typ, DataType) || return false
945943
if typ.name === Tuple.name

base/compiler/tfuncs.jl

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -549,11 +549,9 @@ function typeof_tfunc(@nospecialize(t))
549549
return Type{<:t}
550550
end
551551
elseif isa(t, Union)
552-
a = widenconst(typeof_tfunc(t.a))
553-
b = widenconst(typeof_tfunc(t.b))
552+
a = widenconst(_typeof_tfunc(t.a))
553+
b = widenconst(_typeof_tfunc(t.b))
554554
return Union{a, b}
555-
elseif isa(t, TypeVar) && !(Any === t.ub)
556-
return typeof_tfunc(t.ub)
557555
elseif isa(t, UnionAll)
558556
u = unwrap_unionall(t)
559557
if isa(u, DataType) && !isabstracttype(u)
@@ -570,6 +568,13 @@ function typeof_tfunc(@nospecialize(t))
570568
end
571569
return DataType # typeof(anything)::DataType
572570
end
571+
# helper function of `typeof_tfunc`, which accepts `TypeVar`
572+
function _typeof_tfunc(@nospecialize(t))
573+
if isa(t, TypeVar)
574+
return t.ub !== Any ? _typeof_tfunc(t.ub) : DataType
575+
end
576+
return typeof_tfunc(t)
577+
end
573578
add_tfunc(typeof, 1, 1, typeof_tfunc, 1)
574579

575580
function typeassert_tfunc(@nospecialize(v), @nospecialize(t))
@@ -864,10 +869,7 @@ function getfield_tfunc(@nospecialize(s00), @nospecialize(name))
864869
elseif Symbol name
865870
name = Int
866871
end
867-
_ts = s.parameters[2]
868-
while isa(_ts, TypeVar)
869-
_ts = _ts.ub
870-
end
872+
_ts = unwraptv(s.parameters[2])
871873
_ts = rewrap_unionall(_ts, s00)
872874
if !(_ts <: Tuple)
873875
return Any
@@ -1266,7 +1268,7 @@ function apply_type_tfunc(@nospecialize(headtypetype), @nospecialize args...)
12661268
return Any
12671269
end
12681270
if !isempty(args) && isvarargtype(args[end])
1269-
return isvarargtype(headtype) ? Core.TypeofVararg : Type
1271+
return isvarargtype(headtype) ? TypeofVararg : Type
12701272
end
12711273
largs = length(args)
12721274
if headtype === Union
@@ -1327,7 +1329,7 @@ function apply_type_tfunc(@nospecialize(headtypetype), @nospecialize args...)
13271329
canconst &= !has_free_typevars(aip1)
13281330
push!(tparams, aip1)
13291331
elseif isa(ai, Const) && (isa(ai.val, Type) || isa(ai.val, TypeVar) ||
1330-
valid_tparam(ai.val) || (istuple && isa(ai.val, Core.TypeofVararg)))
1332+
valid_tparam(ai.val) || (istuple && isvarargtype(ai.val)))
13311333
push!(tparams, ai.val)
13321334
elseif isa(ai, PartialTypeVar)
13331335
canconst = false
@@ -1393,11 +1395,11 @@ function apply_type_tfunc(@nospecialize(headtypetype), @nospecialize args...)
13931395
catch ex
13941396
# type instantiation might fail if one of the type parameters
13951397
# doesn't match, which could happen if a type estimate is too coarse
1396-
return isvarargtype(headtype) ? Core.TypeofVararg : Type{<:headtype}
1398+
return isvarargtype(headtype) ? TypeofVararg : Type{<:headtype}
13971399
end
13981400
!uncertain && canconst && return Const(appl)
13991401
if isvarargtype(appl)
1400-
return Core.TypeofVararg
1402+
return TypeofVararg
14011403
end
14021404
if istuple
14031405
return Type{<:appl}
@@ -1437,12 +1439,15 @@ function tuple_tfunc(atypes::Vector{Any})
14371439
if has_struct_const_info(x)
14381440
anyinfo = true
14391441
else
1440-
atypes[i] = x = widenconst(x)
1442+
if !isvarargtype(x)
1443+
x = widenconst(x)
1444+
end
1445+
atypes[i] = x
14411446
end
14421447
if isa(x, Const)
14431448
params[i] = typeof(x.val)
14441449
else
1445-
x = widenconst(x)
1450+
x = isvarargtype(x) ? x : widenconst(x)
14461451
if isType(x)
14471452
anyinfo = true
14481453
xparam = x.parameters[1]
@@ -1465,10 +1470,12 @@ end
14651470
function arrayref_tfunc(@nospecialize(boundscheck), @nospecialize(a), @nospecialize i...)
14661471
a = widenconst(a)
14671472
if a <: Array
1468-
if isa(a, DataType) && (isa(a.parameters[1], Type) || isa(a.parameters[1], TypeVar))
1473+
if isa(a, DataType) && begin
1474+
ap1 = a.parameters[1]
1475+
isa(ap1, Type) || isa(ap1, TypeVar)
1476+
end
14691477
# TODO: the TypeVar case should not be needed here
1470-
a = a.parameters[1]
1471-
return isa(a, TypeVar) ? a.ub : a
1478+
return unwraptv(ap1)
14721479
elseif isa(a, UnionAll) && !has_free_typevars(a)
14731480
unw = unwrap_unionall(a)
14741481
if isa(unw, DataType)
@@ -1627,7 +1634,7 @@ function builtin_tfunction(interp::AbstractInterpreter, @nospecialize(f), argtyp
16271634
if length(argtypes) - 1 == tf[2]
16281635
argtypes = argtypes[1:end-1]
16291636
else
1630-
vatype = argtypes[end]::Core.TypeofVararg
1637+
vatype = argtypes[end]::TypeofVararg
16311638
argtypes = argtypes[1:end-1]
16321639
while length(argtypes) < tf[1]
16331640
push!(argtypes, unwrapva(vatype))

base/compiler/typeinfer.jl

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -970,11 +970,7 @@ function _return_type(interp::AbstractInterpreter, @nospecialize(f), @nospeciali
970970
rt = Union{}
971971
if isa(f, Builtin)
972972
rt = builtin_tfunction(interp, f, Any[t.parameters...], nothing)
973-
if isa(rt, TypeVar)
974-
rt = rt.ub
975-
else
976-
rt = widenconst(rt)
977-
end
973+
rt = widenconst(rt)
978974
else
979975
for match in _methods(f, t, -1, get_world_counter(interp))::Vector
980976
match = match::Core.MethodMatch

base/compiler/typelattice.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,8 +293,8 @@ widenconst(c::PartialTypeVar) = TypeVar
293293
widenconst(t::PartialStruct) = t.typ
294294
widenconst(t::PartialOpaque) = t.typ
295295
widenconst(t::Type) = t
296-
widenconst(t::TypeVar) = t
297-
widenconst(t::Core.TypeofVararg) = t
296+
widenconst(t::TypeVar) = error("unhandled TypeVar")
297+
widenconst(t::TypeofVararg) = error("unhandled Vararg")
298298
widenconst(t::LimitedAccuracy) = error("unhandled LimitedAccuracy")
299299

300300
issubstate(a::VarState, b::VarState) = (a.typ b.typ && a.undef <= b.undef)

base/compiler/typelimits.jl

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ function is_derived_type(@nospecialize(t), @nospecialize(c), mindepth::Int)
4646
# see if it is derived from the body
4747
# also handle the var here, since this construct bounds the mindepth to the smallest possible value
4848
return is_derived_type(t, c.var.ub, mindepth) || is_derived_type(t, c.body, mindepth)
49-
elseif isa(c, Core.TypeofVararg)
49+
elseif isvarargtype(c)
5050
return is_derived_type(t, unwrapva(c), mindepth)
5151
elseif isa(c, DataType)
5252
if mindepth > 0
@@ -79,6 +79,7 @@ end
7979
# The goal of this function is to return a type of greater "size" and less "complexity" than
8080
# both `t` or `c` over the lattice defined by `sources`, `depth`, and `allowed_tuplelen`.
8181
function _limit_type_size(@nospecialize(t), @nospecialize(c), sources::SimpleVector, depth::Int, allowed_tuplelen::Int)
82+
@assert isa(t, Type) && isa(c, Type) "unhandled TypeVar / Vararg"
8283
if t === c
8384
return t # quick egal test
8485
elseif t === Union{}
@@ -98,40 +99,22 @@ function _limit_type_size(@nospecialize(t), @nospecialize(c), sources::SimpleVec
9899
# first attempt to turn `c` into a type that contributes meaningful information
99100
# by peeling off meaningless non-matching wrappers of comparison one at a time
100101
# then unwrap `t`
101-
if isa(c, TypeVar)
102-
if isa(t, TypeVar) && t.ub === c.ub && (t.lb === Union{} || t.lb === c.lb)
103-
return t # it's ok to change the name, or widen `lb` to Union{}, so we can handle this immediately here
104-
end
105-
return _limit_type_size(t, c.ub, sources, depth, allowed_tuplelen)
106-
end
102+
# NOTE that `TypeVar` / `Vararg` are handled separately to catch the logic errors
107103
if isa(c, UnionAll)
108-
return _limit_type_size(t, c.body, sources, depth, allowed_tuplelen)
104+
return __limit_type_size(t, c.body, sources, depth, allowed_tuplelen)::Type
109105
end
110106
if isa(t, UnionAll)
111-
tbody = _limit_type_size(t.body, c, sources, depth, allowed_tuplelen)
107+
tbody = __limit_type_size(t.body, c, sources, depth, allowed_tuplelen)
112108
tbody === t.body && return t
113-
return UnionAll(t.var, tbody)
114-
elseif isa(t, TypeVar)
115-
# don't have a matching TypeVar in comparison, so we keep just the upper bound
116-
return _limit_type_size(t.ub, c, sources, depth, allowed_tuplelen)
109+
return UnionAll(t.var, tbody)::Type
117110
elseif isa(t, Union)
118111
if isa(c, Union)
119-
a = _limit_type_size(t.a, c.a, sources, depth, allowed_tuplelen)
120-
b = _limit_type_size(t.b, c.b, sources, depth, allowed_tuplelen)
112+
a = __limit_type_size(t.a, c.a, sources, depth, allowed_tuplelen)
113+
b = __limit_type_size(t.b, c.b, sources, depth, allowed_tuplelen)
121114
return Union{a, b}
122115
end
123-
elseif isa(t, Core.TypeofVararg)
124-
isa(c, Core.TypeofVararg) || return Vararg
125-
VaT = _limit_type_size(unwrapva(t), unwrapva(c), sources, depth + 1, 0)
126-
if isdefined(t, :N) && (isa(t.N, TypeVar) || (isdefined(c, :N) && t.N === c.N))
127-
return Vararg{VaT, t.N}
128-
end
129-
return Vararg{VaT}
130116
elseif isa(t, DataType)
131-
if isa(c, Core.TypeofVararg)
132-
# Tuple{Vararg{T}} --> Tuple{T} is OK
133-
return _limit_type_size(t, unwrapva(c), sources, depth, 0)
134-
elseif isType(t) # allow taking typeof as Type{...}, but ensure it doesn't start nesting
117+
if isType(t) # allow taking typeof as Type{...}, but ensure it doesn't start nesting
135118
tt = unwrap_unionall(t.parameters[1])
136119
(!isa(tt, DataType) || isType(tt)) && (depth += 1)
137120
is_derived_type_from_any(tt, sources, depth) && return t
@@ -161,7 +144,7 @@ function _limit_type_size(@nospecialize(t), @nospecialize(c), sources::SimpleVec
161144
else
162145
cPi = Any
163146
end
164-
Q[i] = _limit_type_size(Q[i], cPi, sources, depth + 1, 0)
147+
Q[i] = __limit_type_size(Q[i], cPi, sources, depth + 1, 0)
165148
end
166149
return Tuple{Q...}
167150
end
@@ -182,6 +165,31 @@ function _limit_type_size(@nospecialize(t), @nospecialize(c), sources::SimpleVec
182165
return Any
183166
end
184167

168+
# helper function of `_limit_type_size`, which has the right to take and return `TypeVar` / `Vararg`
169+
function __limit_type_size(@nospecialize(t), @nospecialize(c), sources::SimpleVector, depth::Int, allowed_tuplelen::Int)
170+
if isa(c, TypeVar)
171+
if isa(t, TypeVar) && t.ub === c.ub && (t.lb === Union{} || t.lb === c.lb)
172+
return t # it's ok to change the name, or widen `lb` to Union{}, so we can handle this immediately here
173+
end
174+
return __limit_type_size(t, c.ub, sources, depth, allowed_tuplelen)
175+
elseif isa(t, TypeVar)
176+
# don't have a matching TypeVar in comparison, so we keep just the upper bound
177+
return __limit_type_size(t.ub, c, sources, depth, allowed_tuplelen)
178+
elseif isvarargtype(t)
179+
isvarargtype(c) || return Vararg
180+
VaT = __limit_type_size(unwrapva(t), unwrapva(c), sources, depth + 1, 0)
181+
if isdefined(t, :N) && (isa(t.N, TypeVar) || (isdefined(c, :N) && t.N === c.N))
182+
return Vararg{VaT, t.N}
183+
end
184+
return Vararg{VaT}
185+
elseif isvarargtype(c)
186+
# Tuple{Vararg{T}} --> Tuple{T} is OK
187+
return __limit_type_size(t, unwrapva(c), sources, depth, 0)
188+
else
189+
return _limit_type_size(t, c, sources, depth, allowed_tuplelen)
190+
end
191+
end
192+
185193
function type_more_complex(@nospecialize(t), @nospecialize(c), sources::SimpleVector, depth::Int, tupledepth::Int, allowed_tuplelen::Int)
186194
# detect cases where the comparison is trivial
187195
if t === c
@@ -225,13 +233,13 @@ function type_more_complex(@nospecialize(t), @nospecialize(c), sources::SimpleVe
225233
return t !== 1 && !(0 <= t < c) # alternatively, could use !(abs(t) <= abs(c) || abs(t) < n) for some n
226234
end
227235
# base case for data types
228-
if isa(t, Core.TypeofVararg)
229-
if isa(c, Core.TypeofVararg)
236+
if isvarargtype(t)
237+
if isvarargtype(c)
230238
return type_more_complex(unwrapva(t), unwrapva(c), sources, depth + 1, tupledepth, 0)
231239
end
232240
elseif isa(t, DataType)
233241
tP = t.parameters
234-
if isa(c, Core.TypeofVararg)
242+
if isvarargtype(c)
235243
return type_more_complex(t, unwrapva(c), sources, depth, tupledepth, 0)
236244
elseif isType(t) # allow taking typeof any source type anywhere as Type{...}, as long as it isn't nesting Type{Type{...}}
237245
tt = unwrap_unionall(t.parameters[1])
@@ -604,10 +612,11 @@ function tmeet(@nospecialize(v), @nospecialize(t))
604612
@assert widev <: Tuple
605613
new_fields = Vector{Any}(undef, length(v.fields))
606614
for i = 1:length(new_fields)
607-
if isa(v.fields[i], Core.TypeofVararg)
608-
new_fields[i] = v.fields[i]
615+
vfi = v.fields[i]
616+
if isvarargtype(vfi)
617+
new_fields[i] = vfi
609618
else
610-
new_fields[i] = tmeet(v.fields[i], widenconst(getfield_tfunc(t, Const(i))))
619+
new_fields[i] = tmeet(vfi, widenconst(getfield_tfunc(t, Const(i))))
611620
if new_fields[i] === Bottom
612621
return Bottom
613622
end

0 commit comments

Comments
 (0)