From 909117b25331f0ee124e256cd1264a730bec11cb Mon Sep 17 00:00:00 2001 From: Abel Nieto Date: Thu, 14 Feb 2019 13:53:58 -0500 Subject: [PATCH] Fix #5823: better erasure of bottom types Change the definition of lub so that: - lub(C, {Null, Nothing}) = C if C is a reference type - lub(C, {Null, Nothing}) = Object if C is a value type (as before) This allows us to generate more efficient code, and it better matches the user's expectations. --- .../dotty/tools/dotc/core/TypeErasure.scala | 104 ++++++++++-------- tests/neg/i5823.scala | 38 +++++++ tests/run/i5823.check | 8 ++ tests/run/i5823.scala | 60 ++++++++++ 4 files changed, 162 insertions(+), 48 deletions(-) create mode 100644 tests/neg/i5823.scala create mode 100644 tests/run/i5823.check create mode 100644 tests/run/i5823.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala index fafc82cf956f..165df00f6994 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala @@ -248,54 +248,62 @@ object TypeErasure { * The reason to pick last is that we prefer classes over traits that way, * which leads to more predictable bytecode and (?) faster dynamic dispatch. */ - def erasedLub(tp1: Type, tp2: Type)(implicit ctx: Context): Type = tp1 match { - case JavaArrayType(elem1) => - import dotty.tools.dotc.transform.TypeUtils._ - tp2 match { - case JavaArrayType(elem2) => - if (elem1.isPrimitiveValueType || elem2.isPrimitiveValueType) { - if (elem1.classSymbol eq elem2.classSymbol) // same primitive - JavaArrayType(elem1) - else defn.ObjectType - } else JavaArrayType(erasedLub(elem1, elem2)) - case _ => defn.ObjectType - } - case _ => - tp2 match { - case JavaArrayType(_) => defn.ObjectType - case _ => - val cls2 = tp2.classSymbol - - /** takeWhile+1 */ - def takeUntil[T](l: List[T])(f: T => Boolean): List[T] = { - @tailrec def loop(tail: List[T], acc: List[T]): List[T] = - tail match { - case h :: t => loop(if (f(h)) t else Nil, h :: acc) - case Nil => acc.reverse - } - loop(l, Nil) - } - - // We are not interested in anything that is not a supertype of tp2 - val tp2superclasses = tp1.baseClasses.filter(cls2.derivesFrom) - - // From the spec, "Linearization also satisfies the property that a - // linearization of a class always contains the linearization of its - // direct superclass as a suffix"; it's enough to consider every - // candidate up to the first class. - val candidates = takeUntil(tp2superclasses)(!_.is(Trait)) - - // Candidates st "no other common superclass or trait derives from S" - val minimums = candidates.filter { cand => - candidates.forall(x => !x.derivesFrom(cand) || x.eq(cand)) - } - - // Pick the last minimum to prioritise classes over traits - minimums.lastOption match { - case Some(lub) => valueErasure(lub.typeRef) - case _ => defn.ObjectType - } - } + def erasedLub(tp1: Type, tp2: Type)(implicit ctx: Context): Type = { + // After erasure, C | {Null, Nothing} is just C, if C is a reference type. + // We need to short-circuit this case here because the regular lub logic below + // relies on the class hierarchy, which doesn't properly capture `Null`s subtyping + // behaviour. + if (defn.isBottomType(tp1) && tp2.derivesFrom(defn.ObjectClass)) return tp2 + if (defn.isBottomType(tp2) && tp1.derivesFrom(defn.ObjectClass)) return tp1 + tp1 match { + case JavaArrayType(elem1) => + import dotty.tools.dotc.transform.TypeUtils._ + tp2 match { + case JavaArrayType(elem2) => + if (elem1.isPrimitiveValueType || elem2.isPrimitiveValueType) { + if (elem1.classSymbol eq elem2.classSymbol) // same primitive + JavaArrayType(elem1) + else defn.ObjectType + } else JavaArrayType(erasedLub(elem1, elem2)) + case _ => defn.ObjectType + } + case _ => + tp2 match { + case JavaArrayType(_) => defn.ObjectType + case _ => + val cls2 = tp2.classSymbol + + /** takeWhile+1 */ + def takeUntil[T](l: List[T])(f: T => Boolean): List[T] = { + @tailrec def loop(tail: List[T], acc: List[T]): List[T] = + tail match { + case h :: t => loop(if (f(h)) t else Nil, h :: acc) + case Nil => acc.reverse + } + loop(l, Nil) + } + + // We are not interested in anything that is not a supertype of tp2 + val tp2superclasses = tp1.baseClasses.filter(cls2.derivesFrom) + + // From the spec, "Linearization also satisfies the property that a + // linearization of a class always contains the linearization of its + // direct superclass as a suffix"; it's enough to consider every + // candidate up to the first class. + val candidates = takeUntil(tp2superclasses)(!_.is(Trait)) + + // Candidates st "no other common superclass or trait derives from S" + val minimums = candidates.filter { cand => + candidates.forall(x => !x.derivesFrom(cand) || x.eq(cand)) + } + + // Pick the last minimum to prioritise classes over traits + minimums.lastOption match { + case Some(lub) => valueErasure(lub.typeRef) + case _ => defn.ObjectType + } + } + } } /** The erased greatest lower bound of two erased type picks one of the two argument types. diff --git a/tests/neg/i5823.scala b/tests/neg/i5823.scala new file mode 100644 index 000000000000..673cdfc65e86 --- /dev/null +++ b/tests/neg/i5823.scala @@ -0,0 +1,38 @@ +// Test that `C|Null` is erased to `C` if `C` is +// a reference type. +// If `C` is a value type, then `C|Null = Object`. +// Ditto for `C|Nothing`. + +class A +class B + +class Foo { + + // ok, because A and B are <: Object. + def foo(a: A|Null): Unit = () + def foo(b: B|Null): Unit = () + + def bar(a: Int|Null): Unit = () + def bar(b: Boolean|Null): Unit = () // error: signatures match + + // ok, T is erased to `String` and `Integer`, respectively + def gen[T <: String](s: T|Null): Unit = () + def gen[T <: Integer](i: T|Null): Unit = () + + def gen2[T <: Int](i: T|Null): Unit = () + def gen2[T <: Boolean](b: T|Null): Unit = () // error: signatures match + + // ok, because A and B are <: Object. + def foo2(a: A|Nothing): Unit = () + def foo2(b: B|Nothing): Unit = () + + def bar2(a: Int|Nothing): Unit = () + def bar2(b: Boolean|Nothing): Unit = () // error: signatures match + + // ok, T is erased to `String` and `Integer`, respectively + def gen3[T <: String](s: T|Nothing): Unit = () + def gen3[T <: Integer](i: T|Nothing): Unit = () + + def gen4[T <: Int](i: T|Nothing): Unit = () + def gen4[T <: Boolean](b: T|Nothing): Unit = () // error: signatures match +} diff --git a/tests/run/i5823.check b/tests/run/i5823.check new file mode 100644 index 000000000000..5fd3b8a14fc5 --- /dev/null +++ b/tests/run/i5823.check @@ -0,0 +1,8 @@ +foo(A) called +foo(B) called +foo(C) called +foo(D) called +bar(A) called +bar(B) called +fooz(A) called +fooz(B) called diff --git a/tests/run/i5823.scala b/tests/run/i5823.scala new file mode 100644 index 000000000000..445eed10a89e --- /dev/null +++ b/tests/run/i5823.scala @@ -0,0 +1,60 @@ +// Test that `C|Null` and `C|Nothing` are erased to `C`. + +class A +class B +class C +class D + +object Foo { + // This code would not have compiled before, when `C|Null` was erased + // to `Object`, because post-erasure we would end up with multiple methods + // with the same signature. + + def foo(a: A|Null): Unit = { + println("foo(A) called") + } + + def foo(b: B|Null): Unit = { + println("foo(B) called") + } + + def foo(c: Null|C): Unit = { + println("foo(C) called") + } + + def foo(d: Null|D): Unit = { + println("foo(D) called") + } + + def bar[T <: A](a: Null|T): Unit = { + println("bar(A) called") + } + + def bar[T <: B](b: Null|T): Unit = { + println("bar(B) called") + } + + def fooz(a: A|Nothing): Unit = { + println("fooz(A) called") + } + + def fooz(b: B|Nothing): Unit = { + println("fooz(B) called") + } +} + +object Test { + def main(args: Array[String]): Unit = { + import Foo._ + foo(new A) + foo(new B) + foo(new C) + foo(new D) + + bar(new A) + bar(new B) + + fooz(new A) + fooz(new B) + } +}