Skip to content

Commit 5222c11

Browse files
committed
Fixes in TypeComparer for RefinedTypes.
The previous scheme did not propagate bounds correctly. More generally, given a comparison T { X <: A } <: U { X <: B } it would errenously decompose this to T <: U, A <: B But we really need to check whether the total constraint for X in T { X <: A } subsumes the total constraint for X in T { X <: B } The new scheme propagates only if the binding in the lower type is an alias. E.g. T { X = A } <: Y { X <: B } decomposes to T { A = A } <: U, A <: B The change uncovered another bug, where in the slow path we too a member relative to a refined type; We need to "narrow" the type to a RefinedThis instead. (See use of "narrow" in TypeComparer). That change uncovered a third bug concerning the underlying type of a RefinedThis. The last bug was fixed in a previous commit (84f32cd). Two tests (1048, 1843) which were pos tests for scalac but failed compling in dotc have changed their status and location. They typecheck now, but fail later. They have been moved to pending. There's a lot of diagnostic code in TypeComparer to figure out the various problems. I left it in to be able to come back to the commit in case there are more problems. The checks and diagnostics will be removed in a subsequent commit.
1 parent 84f32cd commit 5222c11

File tree

8 files changed

+88
-70
lines changed

8 files changed

+88
-70
lines changed

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

Lines changed: 66 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,11 @@ class TypeComparer(initctx: Context) extends DotClass {
307307
}
308308
}
309309

310+
private def narrowRefined(tp: Type): Type = tp match {
311+
case tp: RefinedType => RefinedThis(tp)
312+
case _ => tp
313+
}
314+
310315
/** If the prefix of a named type is `this` (i.e. an instance of type
311316
* `ThisType` or `RefinedThis`), and there is a refinement type R that
312317
* "refines" (transitively contains as its parent) a class reference
@@ -650,52 +655,71 @@ class TypeComparer(initctx: Context) extends DotClass {
650655
}
651656
compareNamed
652657
case tp2 @ RefinedType(parent2, name2) =>
658+
def qualifies(m: SingleDenotation) = isSubType(m.info, tp2.refinedInfo)
659+
def memberMatches(mbr: Denotation): Boolean = mbr match { // inlined hasAltWith for performance
660+
case mbr: SingleDenotation => qualifies(mbr)
661+
case _ => mbr hasAltWith qualifies
662+
}
663+
def compareRefinedSlow: Boolean = {
664+
def hasMatchingMember(name: Name): Boolean = /*>|>*/ ctx.traceIndented(s"hasMatchingMember($name) ${tp1.member(name).info.show}", subtyping) /*<|<*/ {
665+
val tp1r = rebaseQual(tp1, name)
666+
(memberMatches(narrowRefined(tp1r) member name)
667+
||
668+
{ // special case for situations like:
669+
// foo <: C { type T = foo.T }
670+
tp2.refinedInfo match {
671+
case TypeBounds(lo, hi) if lo eq hi =>
672+
!ctx.phase.erasedTypes && (tp1r select name) =:= lo
673+
case _ => false
674+
}
675+
})
676+
}
677+
val matchesParent = {
678+
val saved = pendingRefinedBases
679+
try {
680+
addPendingName(name2, tp2, tp2)
681+
isSubType(tp1, parent2)
682+
} finally pendingRefinedBases = saved
683+
}
684+
(matchesParent && (
685+
name2 == nme.WILDCARD
686+
|| hasMatchingMember(name2)
687+
|| fourthTry(tp1, tp2))
688+
|| needsEtaLift(tp1, tp2) && tp1.testLifted(tp2.typeParams, isSubType(_, tp2)))
689+
}
653690
def compareRefined: Boolean = tp1.widen match {
654691
case tp1 @ RefinedType(parent1, name1) if name1 == name2 && name1.isTypeName =>
655-
// optimized case; all info on tp1.name1 is in refinement tp1.refinedInfo.
656-
isSubType(normalizedInfo(tp1), tp2.refinedInfo) && {
657-
val saved = pendingRefinedBases
658-
try {
659-
addPendingName(name1, tp1, tp2)
660-
isSubType(parent1, parent2)
661-
}
662-
finally pendingRefinedBases = saved
692+
normalizedInfo(tp1) match {
693+
case bounds1 @ TypeBounds(lo1, hi1) =>
694+
val fastResult = isSubType(bounds1, tp2.refinedInfo) && {
695+
val saved = pendingRefinedBases
696+
try {
697+
addPendingName(name1, tp1, tp2)
698+
isSubType(parent1, parent2)
699+
} finally pendingRefinedBases = saved
700+
}
701+
if (lo1 eq hi1) fastResult
702+
else {
703+
val slowResult = compareRefinedSlow
704+
if (fastResult != slowResult) {
705+
println(i"!!! difference for $tp1 <: $tp2, fast = $fastResult, ${memberMatches(tp1.member(name1))}, slow = $slowResult ${lo1.getClass} ${hi1.getClass}")
706+
println(TypeComparer.explained(implicit ctx => tp1 <:< parent2))
707+
val tp1r = rebaseQual(tp1, name1)
708+
println(s"rebased = $tp1r / ${narrowRefined(tp1r)}")
709+
val mbr = narrowRefined(tp1r) member name1
710+
println(i"member = $mbr : ${mbr.info} / ${mbr.getClass}")
711+
val mbr2 = (tp1r) member name1
712+
println(i"member = $mbr2 : ${mbr2.info} / ${mbr2.getClass}")
713+
println(TypeComparer.explained(implicit ctx => mbr.info <:< tp2.refinedInfo))
714+
compareRefinedSlow
715+
}
716+
slowResult
717+
}
718+
case _ =>
719+
compareRefinedSlow
663720
}
664721
case _ =>
665-
def qualifies(m: SingleDenotation) = isSubType(m.info, tp2.refinedInfo)
666-
def memberMatches(mbr: Denotation): Boolean = mbr match { // inlined hasAltWith for performance
667-
case mbr: SingleDenotation => qualifies(mbr)
668-
case _ => mbr hasAltWith qualifies
669-
}
670-
def hasMatchingMember(name: Name): Boolean = /*>|>*/ ctx.traceIndented(s"hasMatchingMember($name) ${tp1.member(name).info.show}", subtyping) /*<|<*/ {
671-
val tp1r = rebaseQual(tp1, name)
672-
( memberMatches(tp1r member name)
673-
||
674-
{ // special case for situations like:
675-
// foo <: C { type T = foo.T }
676-
tp2.refinedInfo match {
677-
case TypeBounds(lo, hi) if lo eq hi =>
678-
!ctx.phase.erasedTypes && (tp1r select name) =:= lo
679-
case _ => false
680-
}
681-
}
682-
)
683-
}
684-
val matchesParent = {
685-
val saved = pendingRefinedBases
686-
try {
687-
addPendingName(name2, tp2, tp2)
688-
isSubType(tp1, parent2)
689-
}
690-
finally pendingRefinedBases = saved
691-
}
692-
( matchesParent && (
693-
name2 == nme.WILDCARD
694-
|| hasMatchingMember(name2)
695-
|| fourthTry(tp1, tp2)
696-
)
697-
|| needsEtaLift(tp1, tp2) && tp1.testLifted(tp2.typeParams, isSubType(_, tp2))
698-
)
722+
compareRefinedSlow
699723
}
700724
compareRefined
701725
case OrType(tp21, tp22) =>

src/dotty/tools/dotc/core/TypeOps.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ trait TypeOps { this: Context =>
1212

1313
final def asSeenFrom(tp: Type, pre: Type, cls: Symbol, theMap: AsSeenFromMap): Type = {
1414

15-
def toPrefix(pre: Type, cls: Symbol, thiscls: ClassSymbol): Type = /*>|>*/ ctx.debugTraceIndented(s"toPrefix($pre, $cls, $thiscls)") /*<|<*/ {
15+
def toPrefix(pre: Type, cls: Symbol, thiscls: ClassSymbol): Type = /*>|>*/ ctx.conditionalTraceIndented(TypeOps.track, s"toPrefix($pre, $cls, $thiscls)") /*<|<*/ {
1616
if ((pre eq NoType) || (pre eq NoPrefix) || (cls is PackageClass))
1717
tp
1818
else if (thiscls.derivesFrom(cls) && pre.baseTypeRef(thiscls).exists)
@@ -24,7 +24,7 @@ trait TypeOps { this: Context =>
2424
toPrefix(pre.baseTypeRef(cls).normalizedPrefix, cls.owner, thiscls)
2525
}
2626

27-
/*>|>*/ ctx.conditionalTraceIndented(TypeOps.track , s"asSeen ${tp.show} from (${pre.show}, ${cls.show})", show = true) /*<|<*/ { // !!! DEBUG
27+
/*>|>*/ ctx.conditionalTraceIndented(TypeOps.track, s"asSeen ${tp.show} from (${pre.show}, ${cls.show})", show = true) /*<|<*/ { // !!! DEBUG
2828
tp match {
2929
case tp: NamedType =>
3030
val sym = tp.symbol

src/dotty/tools/dotc/core/Types.scala

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -710,8 +710,11 @@ object Types {
710710
case _ => Nil
711711
}
712712

713-
/** A prefix-less termRef to a new skolem symbol that has the given type as info */
714-
def narrow(implicit ctx: Context): TermRef = TermRef(NoPrefix, ctx.newSkolem(this))
713+
/** A prefix-less refined this or a termRef to a new skolem symbol
714+
* that has the given type as info
715+
*/
716+
def narrow(implicit ctx: Context): TermRef =
717+
TermRef(NoPrefix, ctx.newSkolem(this))
715718

716719
// ----- Normalizing typerefs over refined types ----------------------------
717720

test/dotc/tests.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,7 @@ class tests extends CompilerTest {
9595
@Test def neg_tailcall = compileFile(negDir, "tailcall/tailrec", xerrors = 7)
9696
@Test def neg_tailcall2 = compileFile(negDir, "tailcall/tailrec-2", xerrors = 2)
9797
@Test def neg_tailcall3 = compileFile(negDir, "tailcall/tailrec-3", xerrors = 2)
98-
@Test def neg_t1048 = compileFile(negDir, "t1048", xerrors = 1)
9998
@Test def nef_t1279a = compileFile(negDir, "t1279a", xerrors = 1)
100-
@Test def neg_t1843 = compileFile(negDir, "t1843", xerrors = 1)
10199
@Test def neg_t1843_variances = compileFile(negDir, "t1843-variances", xerrors = 1)
102100
@Test def neg_t2660_ambi = compileFile(negDir, "t2660", xerrors = 2)
103101
@Test def neg_t2994 = compileFile(negDir, "t2994", xerrors = 2)
@@ -107,6 +105,7 @@ class tests extends CompilerTest {
107105
@Test def neg_typetest = compileFile(negDir, "typetest", xerrors = 1)
108106
@Test def neg_t1569_failedAvoid = compileFile(negDir, "t1569-failedAvoid", xerrors = 1)
109107
@Test def neg_cycles = compileFile(negDir, "cycles", xerrors = 6)
108+
@Test def neg_boundspropagation = compileFile(negDir, "boundspropagation", xerrors = 3)
110109

111110
@Test def dotc = compileDir(dotcDir + "tools/dotc", twice)(allowDeepSubtypes)
112111
@Test def dotc_ast = compileDir(dotcDir + "tools/dotc/ast", twice)

tests/neg/boundspropagation.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,17 @@ object test3 {
2424
case y: Tree[_] => y
2525
}
2626
}
27+
28+
// Example contributed by Jason. I believe this should not typecheck,
29+
// even though scalac does typecheck it.
30+
object test4 {
31+
class Base {
32+
type N
33+
34+
class Tree[-S, -T >: Option[S]]
35+
36+
def g(x: Any): Tree[_, _ <: Option[N]] = x match {
37+
case y: Tree[_, _] => y
38+
}
39+
}
40+
}

tests/neg/t1048.scala

Lines changed: 0 additions & 21 deletions
This file was deleted.
File renamed without changes.

tests/neg/t1843.scala renamed to tests/pending/pos/t1843.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
* ... Or Will It?
44
*
55
*/
6-
76
object Crash {
87
trait UpdateType[A]
98
case class StateUpdate[A](updateType : UpdateType[A], value : A)

0 commit comments

Comments
 (0)