Skip to content

Commit d60f92c

Browse files
committed
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
1 parent b8ed1ae commit d60f92c

12 files changed

+99
-89
lines changed

base/compiler/abstractinterpretation.jl

+8-12
Original file line numberDiff line numberDiff line change
@@ -778,10 +778,7 @@ function precise_container_type(interp::AbstractInterpreter, @nospecialize(itft)
778778
if isa(tti, DataType) && tti.name === NamedTuple_typename
779779
# A NamedTuple iteration is the same as the iteration of its Tuple parameter:
780780
# compute a new `tti == unwrap_unionall(tti0)` based on that Tuple type
781-
tti = tti.parameters[2]
782-
while isa(tti, TypeVar)
783-
tti = tti.ub
784-
end
781+
tti = unwraptv(tti.parameters[2])
785782
tti0 = rewrap_unionall(tti, tti0)
786783
end
787784
if isa(tti, Union)
@@ -1153,7 +1150,8 @@ function abstract_call_builtin(interp::AbstractInterpreter, f::Builtin, fargs::U
11531150
end
11541151
end
11551152
end
1156-
return isa(rt, TypeVar) ? rt.ub : rt
1153+
@assert !isa(rt, TypeVar) "unhandled TypeVar"
1154+
return rt
11571155
end
11581156

11591157
function abstract_call_unionall(argtypes::Vector{Any})
@@ -1405,7 +1403,7 @@ function sp_type_rewrap(@nospecialize(T), linfo::MethodInstance, isreturn::Bool)
14051403
spsig = linfo.def.sig
14061404
if isa(spsig, UnionAll)
14071405
if !isempty(linfo.sparam_vals)
1408-
sparam_vals = Any[isa(v, Core.TypeofVararg) ? TypeVar(:N, Union{}, Any) :
1406+
sparam_vals = Any[isvarargtype(v) ? TypeVar(:N, Union{}, Any) :
14091407
v for v in linfo.sparam_vals]
14101408
T = ccall(:jl_instantiate_type_in_env, Any, (Any, Any, Ptr{Any}), T, spsig, sparam_vals)
14111409
isref && isreturn && T === Any && return Bottom # catch invalid return Ref{T} where T = Any
@@ -1419,10 +1417,7 @@ function sp_type_rewrap(@nospecialize(T), linfo::MethodInstance, isreturn::Bool)
14191417
end
14201418
end
14211419
end
1422-
while isa(T, TypeVar)
1423-
T = T.ub
1424-
end
1425-
return T
1420+
return unwraptv(T)
14261421
end
14271422

14281423
function abstract_eval_cfunction(interp::AbstractInterpreter, e::Expr, vtypes::VarTable, sv::InferenceState)
@@ -1632,7 +1627,7 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e),
16321627
else
16331628
t = abstract_eval_value_expr(interp, e, vtypes, sv)
16341629
end
1635-
@assert !isa(t, TypeVar)
1630+
@assert !isa(t, TypeVar) "unhandled TypeVar"
16361631
if isa(t, DataType) && isdefined(t, :instance)
16371632
# replace singleton types with their equivalent Const object
16381633
t = Const(t.instance)
@@ -1717,7 +1712,8 @@ function widenreturn(@nospecialize(rt), @nospecialize(bestguess), nslots::Int, s
17171712
fields = copy(rt.fields)
17181713
haveconst = false
17191714
for i in 1:length(fields)
1720-
a = widenreturn(fields[i], bestguess, nslots, slottypes, changes)
1715+
a = fields[i]
1716+
a = isvarargtype(a) ? a : widenreturn(a, bestguess, nslots, slottypes, changes)
17211717
if !haveconst && has_const_info(a)
17221718
# TODO: consider adding && const_prop_profitable(a) here?
17231719
haveconst = true

base/compiler/bootstrap.jl

+1-3
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@ let
3636
# remove any TypeVars from the intersection
3737
typ = Any[m.spec_types.parameters...]
3838
for i = 1:length(typ)
39-
if isa(typ[i], TypeVar)
40-
typ[i] = typ[i].ub
41-
end
39+
typ[i] = unwraptv(typ[i])
4240
end
4341
typeinf_type(interp, m.method, Tuple{typ...}, m.sparams)
4442
end

base/compiler/compiler.jl

+2-1
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

+1-3
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,7 @@ function most_general_argtypes(method::Union{Method, Nothing}, @nospecialize(spe
111111
atyp = unwrapva(atyp)
112112
tail_index -= 1
113113
end
114-
while isa(atyp, TypeVar)
115-
atyp = atyp.ub
116-
end
114+
atyp = unwraptv(atyp)
117115
if isa(atyp, DataType) && isdefined(atyp, :instance)
118116
# replace singleton types with their equivalent Const object
119117
atyp = Const(atyp.instance)

base/compiler/inferencestate.jl

+1-1
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ function sptypes_from_meth_instance(linfo::MethodInstance)
311311
ty = UnionAll(tv, Type{tv})
312312
end
313313
end
314-
elseif isa(v, Core.TypeofVararg)
314+
elseif isvarargtype(v)
315315
ty = Int
316316
else
317317
ty = Const(v)

base/compiler/ssair/inlining.jl

+2-4
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,7 @@ end
785785

786786
function validate_sparams(sparams::SimpleVector)
787787
for i = 1:length(sparams)
788-
(isa(sparams[i], TypeVar) || isa(sparams[i], Core.TypeofVararg)) && return false
788+
(isa(sparams[i], TypeVar) || isvarargtype(sparams[i])) && return false
789789
end
790790
return true
791791
end
@@ -873,9 +873,7 @@ function is_valid_type_for_apply_rewrite(@nospecialize(typ), params::Optimizatio
873873
typ = widenconst(typ)
874874
if isa(typ, DataType) && typ.name === NamedTuple_typename
875875
typ = typ.parameters[2]
876-
while isa(typ, TypeVar)
877-
typ = typ.ub
878-
end
876+
typ = unwraptv(typ)
879877
end
880878
isa(typ, DataType) || return false
881879
if typ.name === Tuple.name

base/compiler/tfuncs.jl

+25-18
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))
@@ -865,10 +870,7 @@ function getfield_tfunc(@nospecialize(s00), @nospecialize(name))
865870
elseif Symbol name
866871
name = Int
867872
end
868-
_ts = s.parameters[2]
869-
while isa(_ts, TypeVar)
870-
_ts = _ts.ub
871-
end
873+
_ts = unwraptv(s.parameters[2])
872874
_ts = rewrap_unionall(_ts, s00)
873875
if !(_ts <: Tuple)
874876
return Any
@@ -1268,7 +1270,7 @@ function apply_type_tfunc(@nospecialize(headtypetype), @nospecialize args...)
12681270
return Any
12691271
end
12701272
if !isempty(args) && isvarargtype(args[end])
1271-
return isvarargtype(headtype) ? Core.TypeofVararg : Type
1273+
return isvarargtype(headtype) ? TypeofVararg : Type
12721274
end
12731275
largs = length(args)
12741276
if headtype === Union
@@ -1329,7 +1331,7 @@ function apply_type_tfunc(@nospecialize(headtypetype), @nospecialize args...)
13291331
canconst &= !has_free_typevars(aip1)
13301332
push!(tparams, aip1)
13311333
elseif isa(ai, Const) && (isa(ai.val, Type) || isa(ai.val, TypeVar) ||
1332-
valid_tparam(ai.val) || (istuple && isa(ai.val, Core.TypeofVararg)))
1334+
valid_tparam(ai.val) || (istuple && isvarargtype(ai.val)))
13331335
push!(tparams, ai.val)
13341336
elseif isa(ai, PartialTypeVar)
13351337
canconst = false
@@ -1395,11 +1397,11 @@ function apply_type_tfunc(@nospecialize(headtypetype), @nospecialize args...)
13951397
catch ex
13961398
# type instantiation might fail if one of the type parameters
13971399
# doesn't match, which could happen if a type estimate is too coarse
1398-
return isvarargtype(headtype) ? Core.TypeofVararg : Type{<:headtype}
1400+
return isvarargtype(headtype) ? TypeofVararg : Type{<:headtype}
13991401
end
14001402
!uncertain && canconst && return Const(appl)
14011403
if isvarargtype(appl)
1402-
return Core.TypeofVararg
1404+
return TypeofVararg
14031405
end
14041406
if istuple
14051407
return Type{<:appl}
@@ -1439,12 +1441,15 @@ function tuple_tfunc(atypes::Vector{Any})
14391441
if has_struct_const_info(x)
14401442
anyinfo = true
14411443
else
1442-
atypes[i] = x = widenconst(x)
1444+
if !isvarargtype(x)
1445+
x = widenconst(x)
1446+
end
1447+
atypes[i] = x
14431448
end
14441449
if isa(x, Const)
14451450
params[i] = typeof(x.val)
14461451
else
1447-
x = widenconst(x)
1452+
x = isvarargtype(x) ? x : widenconst(x)
14481453
if isType(x)
14491454
anyinfo = true
14501455
xparam = x.parameters[1]
@@ -1467,10 +1472,12 @@ end
14671472
function arrayref_tfunc(@nospecialize(boundscheck), @nospecialize(a), @nospecialize i...)
14681473
a = widenconst(a)
14691474
if a <: Array
1470-
if isa(a, DataType) && (isa(a.parameters[1], Type) || isa(a.parameters[1], TypeVar))
1475+
if isa(a, DataType) && begin
1476+
ap1 = a.parameters[1]
1477+
isa(ap1, Type) || isa(ap1, TypeVar)
1478+
end
14711479
# TODO: the TypeVar case should not be needed here
1472-
a = a.parameters[1]
1473-
return isa(a, TypeVar) ? a.ub : a
1480+
return unwraptv(ap1)
14741481
elseif isa(a, UnionAll) && !has_free_typevars(a)
14751482
unw = unwrap_unionall(a)
14761483
if isa(unw, DataType)
@@ -1632,7 +1639,7 @@ function builtin_tfunction(interp::AbstractInterpreter, @nospecialize(f), argtyp
16321639
if length(argtypes) - 1 == tf[2]
16331640
argtypes = argtypes[1:end-1]
16341641
else
1635-
vatype = argtypes[end]::Core.TypeofVararg
1642+
vatype = argtypes[end]::TypeofVararg
16361643
argtypes = argtypes[1:end-1]
16371644
while length(argtypes) < tf[1]
16381645
push!(argtypes, unwrapva(vatype))

base/compiler/typeinfer.jl

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

base/compiler/typelattice.jl

+2-2
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,8 @@ widenconst(c::PartialTypeVar) = TypeVar
283283
widenconst(t::PartialStruct) = t.typ
284284
widenconst(t::PartialOpaque) = t.typ
285285
widenconst(t::Type) = t
286-
widenconst(t::TypeVar) = t
287-
widenconst(t::Core.TypeofVararg) = t
286+
widenconst(t::TypeVar) = error("unhandled TypeVar")
287+
widenconst(t::TypeofVararg) = error("unhandled Vararg")
288288
widenconst(t::LimitedAccuracy) = error("unhandled LimitedAccuracy")
289289

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

base/compiler/typelimits.jl

+42-33
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])
@@ -603,10 +611,11 @@ function tmeet(@nospecialize(v), @nospecialize(t))
603611
@assert widev <: Tuple
604612
new_fields = Vector{Any}(undef, length(v.fields))
605613
for i = 1:length(new_fields)
606-
if isa(v.fields[i], Core.TypeofVararg)
607-
new_fields[i] = v.fields[i]
614+
vfi = v.fields[i]
615+
if isvarargtype(vfi)
616+
new_fields[i] = vfi
608617
else
609-
new_fields[i] = tmeet(v.fields[i], widenconst(getfield_tfunc(t, Const(i))))
618+
new_fields[i] = tmeet(vfi, widenconst(getfield_tfunc(t, Const(i))))
610619
if new_fields[i] === Bottom
611620
return Bottom
612621
end

0 commit comments

Comments
 (0)