Fix #5823: better erasure of bottom types

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.

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
+}

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

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)
+  }
+}