Skip to content

Commit c88e6e4

Browse files
committed
Fix scala#6635: Improve subtype tests for aliases and singleton types
1. backtrack in more cases when we we fail after dealiasing 2. fall back to atoms comparisons when comparing two singleton types The widened criterion for (1) is still provisional. We need to come up with a criterion that is sound and has a low risk of leading to combinatorial explosion when comparing two long alias chains.
1 parent 91e5f35 commit c88e6e4

File tree

2 files changed

+125
-61
lines changed

2 files changed

+125
-61
lines changed

compiler/src/dotty/tools/dotc/core/TypeComparer.scala

Lines changed: 79 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -231,56 +231,69 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
231231
}
232232
}
233233

234+
def singleBounds(tp: Type): List[Type] = tp.widenExpr.dealias match
235+
case tp1: SingletonType => tp1 :: Nil
236+
case AndType(tp11, tp12) => singleBounds(tp1) ::: singleBounds(tp2)
237+
case _ => Nil
238+
239+
def isSingletonAlias(tp: Type): Boolean = tp match
240+
case tp: TermRef => singleBounds(tp).nonEmpty || isSingletonAlias(tp.prefix)
241+
case _ => false
242+
243+
// THIS IS STILL PROVISIONAL
244+
def canDropAlias(tp: NamedType): Boolean = !isSingletonAlias(tp.prefix)
245+
234246
def firstTry: Boolean = tp2 match {
235247
case tp2: NamedType =>
236-
def compareNamed(tp1: Type, tp2: NamedType): Boolean = {
248+
def compareNamed(tp1: Type, tp2: NamedType): Boolean =
237249
implicit val ctx: Context = this.ctx
238-
tp2.info match {
250+
val info2 = tp2.info
251+
info2 match
239252
case info2: TypeAlias =>
240-
recur(tp1, info2.alias)
241-
case _ => tp1 match {
242-
case tp1: NamedType =>
243-
tp1.info match {
244-
case info1: TypeAlias =>
245-
if (recur(info1.alias, tp2)) return true
246-
if (tp1.prefix.isStable) return false
247-
// If tp1.prefix is stable, the alias does contain all information about the original ref, so
248-
// there's no need to try something else. (This is important for performance).
249-
// To see why we cannot in general stop here, consider:
250-
//
251-
// trait C { type A }
252-
// trait D { type A = String }
253-
// (C & D)#A <: C#A
254-
//
255-
// Following the alias leads to the judgment `String <: C#A` which is false.
256-
// However the original judgment should be true.
257-
case _ =>
258-
}
259-
val sym2 = tp2.symbol
260-
var sym1 = tp1.symbol
261-
if (sym1.is(ModuleClass) && sym2.is(ModuleVal))
262-
// For convenience we want X$ <:< X.type
263-
// This is safe because X$ self-type is X.type
264-
sym1 = sym1.companionModule
265-
if ((sym1 ne NoSymbol) && (sym1 eq sym2))
266-
ctx.erasedTypes ||
267-
sym1.isStaticOwner ||
268-
isSubType(tp1.prefix, tp2.prefix) ||
269-
thirdTryNamed(tp2)
270-
else
271-
( (tp1.name eq tp2.name)
272-
&& tp1.isMemberRef
273-
&& tp2.isMemberRef
274-
&& isSubType(tp1.prefix, tp2.prefix)
275-
&& tp1.signature == tp2.signature
276-
&& !(sym1.isClass && sym2.isClass) // class types don't subtype each other
277-
) ||
278-
thirdTryNamed(tp2)
279-
case _ =>
280-
secondTry
281-
}
282-
}
283-
}
253+
if recur(tp1, info2.alias) then return true
254+
if canDropAlias(tp2) then return false
255+
case _ =>
256+
tp1 match
257+
case tp1: NamedType =>
258+
tp1.info match {
259+
case info1: TypeAlias =>
260+
if recur(info1.alias, tp2) then return true
261+
if tp1.prefix.isStable && canDropAlias(tp1) then return false
262+
// If tp1.prefix is stable, the alias does contain all information about the original ref, so
263+
// there's no need to try something else. (This is important for performance).
264+
// To see why we cannot in general stop here, consider:
265+
//
266+
// trait C { type A }
267+
// trait D { type A = String }
268+
// (C & D)#A <: C#A
269+
//
270+
// Following the alias leads to the judgment `String <: C#A` which is false.
271+
// However the original judgment should be true.
272+
case _ =>
273+
}
274+
val sym2 = tp2.symbol
275+
var sym1 = tp1.symbol
276+
if (sym1.is(ModuleClass) && sym2.is(ModuleVal))
277+
// For convenience we want X$ <:< X.type
278+
// This is safe because X$ self-type is X.type
279+
sym1 = sym1.companionModule
280+
if ((sym1 ne NoSymbol) && (sym1 eq sym2))
281+
ctx.erasedTypes ||
282+
sym1.isStaticOwner ||
283+
isSubType(tp1.prefix, tp2.prefix) ||
284+
thirdTryNamed(tp2)
285+
else
286+
( (tp1.name eq tp2.name)
287+
&& tp1.isMemberRef
288+
&& tp2.isMemberRef
289+
&& isSubType(tp1.prefix, tp2.prefix)
290+
&& tp1.signature == tp2.signature
291+
&& !(sym1.isClass && sym2.isClass) // class types don't subtype each other
292+
) ||
293+
thirdTryNamed(tp2)
294+
case _ =>
295+
secondTry
296+
end compareNamed
284297
compareNamed(tp1, tp2)
285298
case tp2: ProtoType =>
286299
isMatchedByProto(tp2, tp1)
@@ -753,20 +766,12 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
753766
case tp1 @ AppliedType(tycon1, args1) =>
754767
compareAppliedType1(tp1, tycon1, args1)
755768
case tp1: SingletonType =>
756-
/** if `tp2 == p.type` and `p: q.type` then try `tp1 <:< q.type` as a last effort.*/
757-
def comparePaths = tp2 match {
769+
def comparePaths = tp2 match
758770
case tp2: TermRef =>
759-
tp2.info.widenExpr.dealias match {
760-
case tp2i: SingletonType =>
761-
recur(tp1, tp2i)
762-
// see z1720.scala for a case where this can arise even in typer.
763-
// Also, i1753.scala, to show why the dealias above is necessary.
764-
case _ => false
765-
}
771+
compareAtoms(tp1, tp2, knownSingletons = true).getOrElse(false)
766772
case _ =>
767773
false
768-
}
769-
isNewSubType(tp1.underlying.widenExpr) || comparePaths
774+
comparePaths || isNewSubType(tp1.underlying.widenExpr)
770775
case tp1: RefinedType =>
771776
isNewSubType(tp1.parent)
772777
case tp1: RecType =>
@@ -1174,8 +1179,18 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
11741179

11751180
/** If both `tp1` and `tp2` have atoms information, compare the atoms
11761181
* in a Some, otherwise None.
1182+
* @param knownSingletons If true, we are coming from a comparison of two singleton types
1183+
* This influences the comparison as shown below:
1184+
*
1185+
* Say you have singleton types p.type and q.type the atoms of p.type are `{p.type}..{p.type}`,
1186+
* and the atoms of `q.type` are `{}..{p.type}`. Normally the atom comparison between p's
1187+
* atoms and q's atoms gives false. But in this case we know that `q.type` is an alias of `p.type`
1188+
* so we are still allowed to conclude that `p.type <:< q.type`. A situation where this happens
1189+
* is in i6635.scala. Here,
1190+
*
1191+
* p: A, q: B & p.type and we want to conclude that p.type <: q.type.
11771192
*/
1178-
def compareAtoms(tp1: Type, tp2: Type): Option[Boolean] =
1193+
def compareAtoms(tp1: Type, tp2: Type, knownSingletons: Boolean = false): Option[Boolean] =
11791194

11801195
/** Check whether we can compare the given set of atoms with another to determine
11811196
* a subtype test between OrTypes. There is one situation where this is not
@@ -1209,9 +1224,12 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
12091224
case Atoms.Range(lo2, hi2) if canCompareAtoms && canCompare(hi2) =>
12101225
tp1.atoms match
12111226
case Atoms.Range(lo1, hi1) =>
1212-
if hi1.subsetOf(lo2) then Some(verified(true))
1213-
else if !lo1.subsetOf(hi2) then Some(verified(false))
1214-
else None
1227+
if hi1.subsetOf(lo2) || knownSingletons && hi2.size == 1 && hi1 == hi2 then
1228+
Some(verified(true))
1229+
else if !lo1.subsetOf(hi2) then
1230+
Some(verified(false))
1231+
else
1232+
None
12151233
case _ => Some(verified(recur(tp1, NothingType)))
12161234
case _ => None
12171235

tests/pos/i6635.scala

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
object Test {
2+
abstract class ExprBase { s =>
3+
type A
4+
}
5+
6+
abstract class Lit extends ExprBase { s =>
7+
type A = Int
8+
val n: A
9+
}
10+
11+
abstract class LitU extends ExprBase { s =>
12+
type A <: Int
13+
val n: A
14+
}
15+
16+
abstract class LitL extends ExprBase { s =>
17+
type A <: Int
18+
val n: A
19+
}
20+
21+
def castTest1(e1: ExprBase)(e2: e1.type)(x: e1.A): e2.A = x
22+
def castTest2(e1: ExprBase { type A = Int })(e2: e1.type)(x: e1.A): e2.A = x
23+
def castTest3(e1: ExprBase)(e2: ExprBase with e1.type)(x: e2.A): e1.A = x
24+
25+
def castTest4(e1: ExprBase { type A = Int })(e2: ExprBase with e1.type)(x: e2.A): e1.A = x
26+
27+
def castTest5a(e1: ExprBase)(e2: LitU with e1.type)(x: e2.A): e1.A = x
28+
def castTest5b(e1: ExprBase)(e2: LitL with e1.type)(x: e2.A): e1.A = x
29+
30+
//fail:
31+
def castTestFail1(e1: ExprBase)(e2: Lit with e1.type)(x: e2.A): e1.A = x // this is like castTest5a/b, but with Lit instead of LitU/LitL
32+
// the other direction never works:
33+
def castTestFail2a(e1: ExprBase)(e2: Lit with e1.type)(x: e1.A): e2.A = x
34+
def castTestFail2b(e1: ExprBase)(e2: LitL with e1.type)(x: e1.A): e2.A = x
35+
def castTestFail2c(e1: ExprBase)(e2: LitU with e1.type)(x: e1.A): e2.A = x
36+
37+
// the problem isn't about order of intersections.
38+
def castTestFail2bFlip(e1: ExprBase)(e2: e1.type with LitL)(x: e1.A): e2.A = x
39+
def castTestFail2cFlip(e1: ExprBase)(e2: e1.type with LitU)(x: e1.A): e2.A = x
40+
41+
def castTestFail3(e1: ExprBase)(e2: Lit with e1.type)(x: e1.A): e2.A = {
42+
val y: e1.type with e2.type = e2
43+
val z = x: y.A
44+
z
45+
}
46+
}

0 commit comments

Comments
 (0)