Skip to content

Commit 0f708eb

Browse files
committed
Keep order of operands in AndTypes invariant
We previously could get the following: Given: class A trait B extends A trait C extends A Then (A & B) & C simplified to C & B This is usually not a problem, *except* for typing super calls, where we pick the symbol of the right operand. The problem was masked because due to the wrong computation of base classes for AndTypes, we always ended up picking the leftmost symbol in a joint denotation instead of the rightmost one. So we got a double swap! The problem is reproducible in the test case. It originally manifested itself only in the bootstrap, and only under a particular order of compiling files, which was exercised by the CI but initially not by my local setup. The summary of the problem is as follows: ElimRepeated has the structure of ER in the test case. Because of the problem, a super call in `transform` of `ElimRepeated` went to `InfoTransformers` instead of `AnnotationTransformers`. Consequently, the elim repeated transform was not done on annotations, which led to backend crashes of the bootstrapped compiler for those annotations that had repeated arguments. It took me two full days to get to the bottom of this. Must rank right at the top of the list of hard-to-find bugs for me. Note: It would seem logical to apply a similar technique to OrTypes. But this currently fails some tests. Need to investigate later.
1 parent 953e8db commit 0f708eb

File tree

3 files changed

+59
-5
lines changed

3 files changed

+59
-5
lines changed

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

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,11 +1143,11 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
11431143
case OrType(tp11, tp12) =>
11441144
tp11 & tp2 | tp12 & tp2
11451145
case _ =>
1146-
val t1 = mergeIfSub(tp1, tp2)
1147-
if (t1.exists) t1
1146+
val tp1a = dropIfSuper(tp1, tp2)
1147+
if (tp1a ne tp1) glb(tp1a, tp2)
11481148
else {
1149-
val t2 = mergeIfSub(tp2, tp1)
1150-
if (t2.exists) t2
1149+
val tp2a = dropIfSuper(tp2, tp1)
1150+
if (tp2a ne tp2) glb(tp1, tp2a)
11511151
else tp1 match {
11521152
case tp1: ConstantType =>
11531153
tp2 match {
@@ -1161,7 +1161,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
11611161
// However, maybe we can still apply the replacement to
11621162
// types which are not explicitly written.
11631163
defn.NothingType
1164-
case _ => andType(tp1, tp2)
1164+
case _ => andType(tp1, tp2)
11651165
}
11661166
case _ => andType(tp1, tp2)
11671167
}
@@ -1204,6 +1204,22 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
12041204
final def lub(tps: List[Type]): Type =
12051205
((defn.NothingType: Type) /: tps)(lub(_,_, canConstrain = false))
12061206

1207+
private def recombineAndOr(tp: AndOrType, tp1: Type, tp2: Type) =
1208+
if (!tp1.exists) tp2
1209+
else if (!tp2.exists) tp1
1210+
else tp.derivedAndOrType(tp1, tp2)
1211+
1212+
/** If some (&-operand of) this type is a supertype of `sub` replace it with `NoType`.
1213+
*/
1214+
private def dropIfSuper(tp: Type, sub: Type): Type =
1215+
if (isSubTypeWhenFrozen(sub, tp)) NoType
1216+
else tp match {
1217+
case tp @ AndType(tp1, tp2) =>
1218+
recombineAndOr(tp, dropIfSuper(tp1, sub), dropIfSuper(tp2, sub))
1219+
case _ =>
1220+
tp
1221+
}
1222+
12071223
/** Merge `t1` into `tp2` if t1 is a subtype of some &-summand of tp2.
12081224
*/
12091225
private def mergeIfSub(tp1: Type, tp2: Type): Type =

tests/run/supercalls-traits.check

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
C1A1A2B1B2C2
2+
C1B3B4C3
3+
IT
4+
AT
5+
ER

tests/run/supercalls-traits.scala

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,42 @@ class Base[A](exp: => Option[A])
1414

1515
object Empty extends Base[Nothing](None)
1616

17+
18+
trait B1 extends C1 { override def f() = { super.f(); print("B1") }}
19+
trait B2 extends B1 { override def f() = { super.f(); print("B2") }}
20+
trait A1 extends C1 { override def f() = { super.f(); print("A1") }}
21+
trait A2 extends A1 { override def f() = { super.f(); print("A2") }}
22+
class C1 { def f() = print("C1") }
23+
class C2 extends A2 with B2 { override def f() = { super.f(); print("C2") }}
24+
25+
26+
trait B3 extends C1 { override def f() = { super.f(); print("B3") }}
27+
trait B4 extends C1 { this: B3 => override def f() = { super.f(); print("B4") }}
28+
class C3 extends C1 with B3 with B4 { override def f() = { super.f(); print("C3") }}
29+
30+
trait DT {
31+
def f(): Unit
32+
}
33+
trait IT extends DT {
34+
def f() = { println("IT") }
35+
}
36+
abstract class MPT {
37+
}
38+
trait AT extends MPT with DT {
39+
abstract override def f() = { super.f(); println("AT") }
40+
}
41+
class ER extends MPT with IT with AT {
42+
override def f() = { super.f(); println("ER") }
43+
}
44+
1745
object Test {
1846
def main(args: Array[String]): Unit = {
1947
assert(new C().foo == 3)
48+
new C2().f()
49+
println()
50+
new C3().f()
51+
println()
52+
new ER().f()
2053
Empty
2154
}
2255
}

0 commit comments

Comments
 (0)