8000 Improved refinement type and existential type handling · scala/scala@56f23af · GitHub
[go: up one dir, main page]

Skip to content

Commit 56f23af

Browse files
retronymadriaanm
authored andcommitted
Improved refinement type and existential type handling
Lazy base type seq elements are encoded as a refined type with an empty scope and a list of type refs over some common type symbol that will be merged when `BaseTypeSeq#apply` is called. The first change in this commit is to mark the creation and consumption of such elements with calls to `[is]IntersectionTypeForBaseTypeSeq`. They are distinguished by using the actual type symbol rather than a refinement class symbol, which in turn simplifies the code in `BaseTypeSeq#typeSymbol`. I have also made `lub` aware of this encoding: it is now able to "see through" to the parents of such refined types and merge them with other base types of the same class symbol (even other refined types representing lazy BTS elements.) To make this fix work, I also had to fix a bug in LUBs of multiple with existential types. Because of the way the recursion was structured in `mergePrefixAndArgs`, the order of list of types being merged changed behaviour: quantified varialbles of existential types were being rewrapped around the resultting type, but only if we hadn't encountered the first regular `TypeRef`. This can be seen with the following before/after shot: ``` // 2.11.8 scala> val ts = typeOf[Set[Any]] :: typeOf[Set[X] forSome { type X <: Y; type Y <: Int}] :: Nil; def merge(ts: List[Type]) = mergePrefixAndArgs(ts, Variance.Contravariant, lubDepth(ts)); val merged1 = merge(ts); val merged2 = merge(ts.reverse); (ts.forall(_ <:< merged1), ts.forall(_ <:< merged2)) ts: List[$r.intp.global.Type] = List(Set[Any], Set[_ <: Int]) merge: (ts: List[$r.intp.global.Type])$r.intp.global.Type merged1: $r.intp.global.Type = scala.collection.immutable.Set[_ >: Int] merged2: $r.intp.global.Type = scala.collection.immutable.Set[_53] forSome { type X <: Int; type _53 >: X } res0: (Boolean, Boolean) = (false,true) // HEAD ... merged1: $r.intp.global.Type = scala.collection.immutable.Set[_10] forSome { type X <: Int; type _10 >: X } merged2: $r.intp.global.Type = scala.collection.immutable.Set[_11] forSome { type X <: Int; type _11 >: X } res0: (Boolean, Boolean) = (true,true) ``` Furthermore, I have fixed the computation of the base type sequences of existential types over refinement types, in order to maintain the invariant that each slot of the base type sequence of a existential has the same type symbol as that of its underlying type. Before, what I've now called a `RefinementTypeRef` was transformed into a `RefinedType` during rewrapping in the existential, which led to it being wrongly considered as a lazy element of the base type sequence. The first change above should also be sufficient to avoid the bug, but I felt it was worth cleaning up `maybeRewrap` as an extra line of defence. Finally, I have added another special case to `BaseTypeSeq#apply` to be able to lazily compute elements that have been wrapped in an existential. The unit test cases in `TypesTest` rely on these changes. A subsequent commit will build on this foundation to make a fix to `asSeenFrom`.
1 parent a56afcd commit 56f23af

File tree

8 files changed

+211
-103
lines changed

8 files changed

+211
-103
lines changed

src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
142142
elided += r
143143
else
144144
result += r
145+
()
145146
}
146147
result.toList
147148
}

src/reflect/scala/reflect/internal/BaseTypeSeqs.scala

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -56,42 +56,44 @@ trait BaseTypeSeqs {
5656
if(pending contains i) {
5757
pending.clear()
5858
throw CyclicInheritance
59-
} else
60-
elems(i) match {
61-
case rtp @ RefinedType(variants, decls) =>
62-
// can't assert decls.isEmpty; see t0764
63-
//if (!decls.isEmpty) abort("computing closure of "+this+":"+this.isInstanceOf[RefinedType]+"/"+closureCache(j))
64-
//Console.println("compute closure of "+this+" => glb("+variants+")")
65-
pending += i
66-
try {
67-
mergePrefixAndArgs(variants, Variance.Contravariant, lubDepth(variants)) match {
68-
case NoType => typeError("no common type instance of base types "+(variants mkString ", and ")+" exists.")
69-
case tp0 =>
70-
pending(i) = false
71-
elems(i) = tp0
72-
tp0
73-
}
74-
}
75-
catch {
76-
case CyclicInheritance =>
77-
typeError(
78-
"computing the common type instance of base types "+(variants mkString ", and ")+" leads to a cycle.")
59+
} else {
60+
def computeLazyType(rtp: RefinedType): Type = {
61+
if (!isIntersectionTypeForLazyBaseType(rtp))
62+
abort("unexpected RefinedType in base type seq, lazy BTS elements should be created via intersectionTypeForLazyBaseType: " + rtp)
63+
val variants = rtp.parents
64+
// can't assert decls.isEmpty; see t0764
65+
//if (!decls.isEmpty) abort("computing closure of "+this+":"+this.isInstanceOf[RefinedType]+"/"+closureCache(j))
66+
//Console.println("compute closure of "+this+" => glb("+variants+")")
67+
pending += i
68+
try {
69+
mergePrefixAndArgs(variants, Variance.Contravariant, lubDepth(variants)) match {
70+
case NoType => typeError("no common type instance of base types " + (variants mkString ", and ") + " exists.")
71+
case tp0 =>
72+
pending(i) = false
73+
elems(i) = tp0
74+
tp0
7975
}
76+
}
77+
catch {
78+
case CyclicInheritance =>
79+
typeError(
80+
"computing the common type instance of base types " + (variants mkString ", and ") + " leads to a cycle.")
81+
}
82+
}
83+
elems(i) match {
84+
case rtp@RefinedType(variants, decls) =>
85+
computeLazyType(rtp)
86+
case et @ ExistentialType(quantified, rtp: RefinedType) =>
87+
existentialAbstraction(quantified, computeLazyType(rtp))
8088
case tp =>
8189
tp
8290
}
91+
}
8392

8493
def rawElem(i: Int) = elems(i)
8594

86-
/** The type symbol of the type at i'th position in this sequence;
87-
* no evaluation needed.
88-
*/
89-
def typeSymbol(i: Int): Symbol = {
90-
elems(i) match {
91-
case RefinedType(v :: vs, _) => v.typeSymbol
92-
case tp => tp.typeSymbol
93-
}
94-
}
95+
/** The type symbol of the type at i'th position in this sequence */
96+
def typeSymbol(i: Int): Symbol = elems(i).typeSymbol
9597

9698
/** Return all evaluated types in this sequence as a list */
9799
def toList: List[Type] = elems.toList
@@ -215,7 +217,7 @@ trait BaseTypeSeqs {
215217
}
216218
i += 1
217219
}
218-
buf += intersectionType(minTypes)
220+
buf += intersectionTypeForLazyBaseType(minTypes) // TODO this reverses the order. Does this matter? Or should this be minTypes.reverse?
219221
btsSize += 1
220222
}
221223
}

src/reflect/scala/reflect/internal/Types.scala

Lines changed: 94 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,10 @@ trait Types
174174
if (newtp eq underlying) this
175175
// BoundedWildcardTypes reach here during erroneous compilation: neg/t6258
176176
// Higher-kinded exclusion is because [x]CC[x] compares =:= to CC: pos/t3800
177+
// Avoid reusing the existing Wrapped(RefinedType) when we've be asked to wrap an =:= RefinementTypeRef, the
178+
// distinction is important in base type sequences.
177179
// Otherwise, if newtp =:= underlying, don't rewrap it.
178-
else if (!newtp.isWildcard && !newtp.isHigherKinded && (newtp =:= underlying)) this
180+
else if (!newtp.isWildcard && !newtp.isHigherKinded && !newtp.isInstanceOf[RefinementTypeRef] && (newtp =:= underlying)) this
179181
else rewrap(newtp)
180182
)
181183
protected def rewrap(newtp: Type): Type
@@ -1589,7 +1591,6 @@ trait Types
15891591
*/
15901592
case class RefinedType(override val parents: List[Type],
15911593
override val decls: Scope) extends CompoundType with RefinedTypeApi {
1592-
15931594
override def isHigherKinded = (
15941595
parents.nonEmpty &&
15951596
(parents forall typeIsHigherKinded) &&
@@ -2704,6 +2705,7 @@ trait Types
27042705
def isRepresentableWithWildcards = {
27052706
val qset = quantified.toSet
27062707
underlying match {
2708+
case _: RefinementTypeRef => false
27072709
case TypeRef(pre, sym, args) =>
27082710
def isQuantified(tpe: Type): Boolean = {
27092711
(tpe exists (t => qset contains t.typeSymbol)) ||
@@ -3521,7 +3523,9 @@ trait Types
35213523
if ((parents eq original.parents) && (decls eq original.decls)) original
35223524
else {
35233525
val owner = original.typeSymbol.owner
3524-
val result = refinedType(parents, owner)
3526+
val result =
3527+
if (isIntersectionTypeForLazyBaseType(original)) intersectionTypeForLazyBaseType(parents)
3528+
else refinedType(parents, owner)
35253529
val syms1 = decls.toList
35263530
for (sym <- syms1)
35273531
result.decls.enter(sym.cloneSymbol(result.typeSymbol))
@@ -3596,6 +3600,14 @@ trait Types
35963600
case tp :: Nil => tp
35973601
case _ => refinedType(tps, commonOwner(tps))
35983602
}
3603+
def intersectionTypeForLazyBaseType(tps: List[Type]) = tps match {
3604+
case tp :: Nil => tp
3605+
case _ => RefinedType(tps, newScope, tps.head.typeSymbolDirect)
3606+
}
3607+
def isIntersectionTypeForLazyBaseType(tp: RefinedType) = tp.parents match {
3608+
case head :: _ => tp.typeSymbolDirect eq head.typeSymbolDirect
3609+
case _ => false
3610+
}
35993611

36003612
/**** This implementation to merge parents was checked in in commented-out
36013613
form and has languished unaltered for five years. I think we should
@@ -4409,83 +4421,93 @@ trait Types
44094421
* Return `x` if the computation succeeds with result `x`.
44104422
* Return `NoType` if the computation fails.
44114423
*/
4412-
def mergePrefixAndArgs(tps: List[Type], variance: Variance, depth: Depth): Type = tps match {
4413-
case tp :: Nil => tp
4414-
case TypeRef(_, sym, _) :: rest =>
4415-
val pres = tps map (_.prefix) // prefix normalizes automatically
4424+
def mergePrefixAndArgs(tps0: List[Type], variance: Variance, depth: Depth): Type = {
4425+
var tparams = mutable.ListBuffer[Symbol]()
4426+
val tps = tps0.flatMap {
4427+
case rt: RefinedType if isIntersectionTypeForLazyBaseType(rt) => rt.parents
4428+
case ExistentialType(qs, underlying) =>
4429+
tparams ++= qs
4430+
underlying match {
4431+
case rt: RefinedType if isIntersectionTypeForLazyBaseType(rt) => rt.parents
4432+
case tp => tp :: Nil
4433+
}
4434+
case tp => tp :: Nil
4435+
}
4436+
4437+
val merged = tps match {
4438+
case tp :: Nil => tp
4439+
case TypeRef(_, sym, _) :: rest =>
4440+
val pres = tps map (_.prefix) // prefix normalizes automatically
44164441
val pre = if (variance.isPositive) lub(pres, depth) else glb(pres, depth)
4417-
val argss = tps map (_.normalize.typeArgs) // symbol equality (of the tp in tps) was checked using typeSymbol, which normalizes, so should normalize before retrieving arguments
4442+
val argss = tps map (_.normalize.typeArgs) // symbol equality (of the tp in tps) was checked using typeSymbol, which normalizes, so should normalize before retrieving arguments
44184443
val capturedParams = new ListBuffer[Symbol]
4419-
try {
4420-
if (sym == ArrayClass && phase.erasedTypes) {
4421-
// special treatment for lubs of array types after erasure:
4422-
// if argss contain one value type and some other type, the lub is Object
4423-
// if argss contain several reference types, the lub is an array over lub of argtypes
4424-
if (argss exists typeListIsEmpty) {
4425-
NoType // something is wrong: an array without a type arg.
4426-
}
4427-
else {
4428-
val args = argss map (_.head)
4429-
if (args.tail forall (_ =:= args.head)) typeRef(pre, sym, List(args.head))
4430-
else if (args exists (arg => isPrimitiveValueClass(arg.typeSymbol))) ObjectTpe
4431-
else typeRef(pre, sym, List(lub(args)))
4444+
try {
4445+
if (sym == ArrayClass && phase.erasedTypes) {
4446+
// special treatment for lubs of array types after erasure:
4447+
// if argss contain one value type and some other type, the lub is Object
4448+
// if argss contain several reference types, the lub is an array over lub of argtypes
4449+
if (argss exists typeListIsEmpty) {
4450+
NoType // something is wrong: an array without a type arg.
4451+
}
4452+
else {
4453+
val args = argss map (_.head)
4454+
if (args.tail forall (_ =:= args.head)) typeRef(pre, sym, List(args.head))
4455+
else if (args exists (arg => isPrimitiveValueClass(arg.typeSymbol))) ObjectTpe
4456+
else typeRef(pre, sym, List(lub(args)))
4457+
}
44324458
}
4433-
}
4434-
else transposeSafe(argss) match {
4435-
case None =>
4436-
// transpose freaked out because of irregular argss
4437-
// catching just in case (shouldn't happen, but also doesn't cost us)
4438-
// [JZ] It happens: see SI-5683.
4439-
debuglog(s"transposed irregular matrix!? tps=$tps argss=$argss")
4440-
NoType
4441-
case Some(argsst) =>
4442-
val args = map2(sym.typeParams, argsst) { (tparam, as0) =>
4443-
val as = as0.distinct
4444-
if (as.size == 1) as.head
4445-
else if (depth.isZero) {
4446-
log("Giving up merging args: can't unify %s under %s".format(as.mkString(", "), tparam.fullLocationString))
4447-
// Don't return "Any" (or "Nothing") when we have to give up due to
4448-
// recursion depth. Return NoType, which prevents us from poisoning
4449-
// lublist's results. It can recognize the recursion and deal with it, but
4450-
// only if we aren't returning invalid types.
4451-
NoType
4452-
}
4453-
else {
4454-
if (tparam.variance == variance) lub(as, depth.decr)
4455-
else if (tparam.variance == variance.flip) glb(as, depth.decr)
4459+
else transposeSafe(argss) match {
4460+
case None =>
4461+
// transpose freaked out because of irregular argss
4462+
// catching just in case (shouldn't happen, but also doesn't cost us)
4463+
// [JZ] It happens: see SI-5683.
4464+
debuglog(s"transposed irregular matrix!? tps=$tps argss=$argss")
4465+
NoType
4466+
case Some(argsst) =>
4467+
val args = map2(sym.typeParams, argsst) { (tparam, as0) =>
4468+
val as = as0.distinct
4469+
if (as.size == 1) as.head
4470+
else if (depth.isZero) {
4471+
log("Giving up merging args: can't unify %s under %s".format(as.mkString(", "), tparam.fullLocationString))
4472+
// Don't return "Any" (or "Nothing") when we have to give up due to
4473+
// recursion depth. Return NoType, which prevents us from poisoning
4474+
// lublist's results. It can recognize the recursion and deal with it, but
4475+
// only if we aren't returning invalid types.
4476+
NoType
4477+
}
44564478
else {
4457-
val l = lub(as, depth.decr)
4458-
val g = glb(as, depth.decr)
4459-
if (l <:< g) l
4460-
else { // Martin: I removed this, because incomplete. Not sure there is a good way to fix it. For the moment we
4461-
// just err on the conservative side, i.e. with a bound that is too high.
4462-
// if(!(tparam.info.bounds contains tparam)) //@M can't deal with f-bounds, see #2251
4463-
4464-
val qvar = commonOwner(as) freshExistential "" setInfo TypeBounds(g, l)
4465-
capturedParams += qvar
4466-
qvar.tpe
4479+
if (tparam.variance == variance) lub(as, depth.decr)
4480+
else if (tparam.variance == variance.flip) glb(as, depth.decr)
4481+
else {
4482+
val l = lub(as, depth.decr)
4483+
val g = glb(as, depth.decr)
4484+
if (l <:< g) l
4485+
else { // Martin: I removed this, because incomplete. Not sure there is a good way to fix it. For the moment we
4486+
// just err on the conservative side, i.e. with a bound that is too high.
4487+
// if(!(tparam.info.bounds contains tparam)) //@M can't deal with f-bounds, see #2251
4488+
4489+
val qvar = commonOwner(as) freshExistential "" setInfo TypeBounds(g, l)
4490+
capturedParams += qvar
4491+
qvar.tpe
4492+
}
44674493
}
44684494
}
44694495
}
4470-
}
4471-
if (args contains NoType) NoType
4472-
else existentialAbstraction(capturedParams.toList, typeRef(pre, sym, args))
4496+
if (args contains NoType) NoType
4497+
else existentialAbstraction(capturedParams.toList, typeRef(pre, sym, args))
4498+
}
4499+
} catch {
4500+
case ex: MalformedType => NoType
44734501
}
4474-
} catch {
4475-
case ex: MalformedType => NoType
4476-
}
4477-
case SingleType(_, sym) :: rest =>
4478-
val pres = tps map (_.prefix)
4479-
val pre = if (variance.isPositive) lub(pres, depth) else glb(pres, depth)
4480-
try singleType(pre, sym)
4481-
catch { case ex: MalformedType => NoType }
4482-
case ExistentialType(tparams, quantified) :: rest =>
4483-
mergePrefixAndArgs(quantified :: rest, variance, depth) match {
4484-
case NoType => NoType
4485-
case tpe => existentialAbstraction(tparams, tpe)
4486-
}
4487-
case _ =>
4488-
abort(s"mergePrefixAndArgs($tps, $variance, $depth): unsupported tps")
4502+
case SingleType(_, sym) :: rest =>
4503+
val pres = tps map (_.prefix)
4504+
val pre = if (variance.isPositive) lub(pres, depth) else glb(pres, depth)
4505+
try singleType(pre, sym)
4506+
catch { case ex: MalformedType => NoType }
4507+
case _ =>
4508+
abort(s"mergePrefixAndArgs($tps, $variance, $depth): unsupported tps")
4509+
}
4510+
existentialAbstraction(tparams.toList, merged)
44894511
}
44904512

44914513
def addMember(thistp: Type, tp: Type, sym: Symbol): Unit = addMember(thistp, tp, sym, AnyDepth)

src/reflect/scala/reflect/internal/tpe/GlbLubs.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ private[internal] trait GlbLubs {
118118
// ts0 is the 1-dimensional frontier of symbols cutting through 2-dimensional tsBts.
119119
// Invariant: all symbols "under" (closer to the first row) the frontier
120120
// are smaller (according to _.isLess) than the ones "on and beyond" the frontier
121-
val ts0 = tsBts map (_.head)
121+
val ts0 = tsBts map (_.head)
122122

123123
// Is the frontier made up of types with the same symbol?
124124
val isUniformFrontier = (ts0: @unchecked) match {

test/files/neg/lub-from-hell-2.check

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
lub-from-hell-2.scala:3: error: type arguments [Any,Iterable[Any] with Int => Any with scala.collection.generic.Subtractable[Any,Iterable[Any] with Int => Any with scala.collection.generic.Subtractable[Any,Iterable[Any] with Int => Any]{def seq: Iterable[Any] with Int => Any}]{def seq: Iterable[Any] with Int => Any{def seq: Iterable[Any] with Int => Any}}] do not conform to trait Subtractable's type parameter bounds [A,+Repr <: scala.collection.generic.Subtractable[A,Repr]]
2+
def foo(a: Boolean, b: collection.mutable.Set[Any], c: collection.mutable.ListBuffer[Any]) = if (a) b else c
3+
^
4+
lub-from-hell-2.scala:4: error: type arguments [Any,scala.collection.mutable.Iterable[Any] with scala.collection.mutable.Cloneable[scala.collection.mutable.Iterable[Any] with scala.collection.mutable.Cloneable[scala.collection.mutable.Iterable[Any] with Cloneable with Int => Any] with Int => Any{def seq: scala.collection.mutable.Iterable[Any] with Cloneable with Int => Any}] with scala.collection.generic.Growable[Any] with Int => Any with scala.collection.generic.Shrinkable[Any] with scala.collection.generic.Subtractable[Any,Iterable[Any] with Int => Any with scala.collection.generic.Subtractable[Any,Iterable[Any] with Int => Any]{def seq: Iterable[Any] with Int => Any}] with scala.collection.script.Scriptable[Any]] do not conform to trait Subtractable's type parameter bounds [A,+Repr <: scala.collection.generic.Subtractable[A,Repr]]
5+
def bar(a: Boolean, b: scala.collection.mutable.SetLike[Any,scala.collection.mutable.Set[Any]], c: scala.collection.mutable.Buffer[Any]) = if (a) b else c
6+
^
7+
two errors found

test/files/neg/lub-from-hell-2.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
class Test {
2+
trait Tree
3+
def foo(a: Boolean, b: collection.mutable.Set[Any], c: collection.mutable.ListBuffer[Any]) = if (a) b else c
4+
def bar(a: Boolean, b: scala.collection.mutable.SetLike[Any,scala.collection.mutable.Set[Any]], c: scala.collection.mutable.Buffer[Any]) = if (a) b else c
5+
// bar produces an ill-bounded LUB in 2.11.8. After this commit, which fixes a bug in existential+refinement lubs, foo also fails.
6+
}

test/files/pos/lub-from-hell.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
class Test {
2+
trait Tree
3+
def foo(b: Boolean, buf: collection.mutable.ArrayBuffer[Any], acc: StringBuilder) = if (b) buf else acc
4+
5+
// def bar(b: Boolean,
6+
// buf: scala.collection.IndexedSeqLike[Any,Cloneable with Mutable with Equals],
7+
// acc: scala.collection.IndexedSeqLike[_18,scala.collection.mutable.IndexedSeq[_18]] with scala.collection.IndexedSeqLike[_18,IndexedSeq[_18]] forSome { type _18 >: String with Char }
8+
// ) = if (b) buf else acc
9+
10+
11+
}

0 commit comments

Comments
 (0)
0