Merge pull request #13976 from dotty-staging/unsafe-nulls-fix-space

Fix case null on non-nullable type in unsafe nulls
scala · Feb 19, 2022 · 5061804 · 5061804
2 parents 62d54fe + d296dd4
commit 5061804
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 21 deletions.
diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -813,7 +813,7 @@ object SymDenotations {
     /** Is this symbol a class of which `null` is a value? */
     final def isNullableClass(using Context): Boolean =
       if ctx.mode.is(Mode.SafeNulls) && !ctx.phase.erasedTypes
-      then symbol == defn.NullClass || symbol == defn.AnyClass
+      then symbol == defn.NullClass || symbol == defn.AnyClass || symbol == defn.MatchableClass
       else isNullableClassAfterErasure
 
     /** Is this symbol a class of which `null` is a value after erasure?

diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
@@ -330,24 +330,15 @@ class SpaceEngine(using Context) extends SpaceLogic {
 
   private val constantNullType     = ConstantType(Constant(null))
 
-  /** Does the given tree stand for the literal `null`? */
-  def isNullLit(tree: Tree): Boolean = tree match {
-    case Literal(Constant(null)) => true
-    case _ => false
-  }
-
   override def intersectUnrelatedAtomicTypes(tp1: Type, tp2: Type): Space = trace(s"atomic intersection: ${AndType(tp1, tp2).show}", debug) {
     // Precondition: !isSubType(tp1, tp2) && !isSubType(tp2, tp1).
-    if (!ctx.explicitNulls && (tp1.isNullType || tp2.isNullType)) {
+    if !ctx.mode.is(Mode.SafeNulls) && (tp1.isNullType || tp2.isNullType) then
       // Since projections of types don't include null, intersection with null is empty.
       Empty
-    }
-    else {
+    else
       val res = TypeComparer.provablyDisjoint(tp1, tp2)
-
-      if (res) Empty
+      if res then Empty
       else Typ(AndType(tp1, tp2), decomposed = true)
-    }
   }
 
   /** Return the space that represents the pattern `pat` */
@@ -549,7 +540,8 @@ class SpaceEngine(using Context) extends SpaceLogic {
 
   /** Is `tp1` a subtype of `tp2`?  */
   def isSubType(tp1: Type, tp2: Type): Boolean = trace(i"$tp1 <:< $tp2", debug, show = true) {
-    if tp1 == constantNullType && !ctx.explicitNulls then tp2 == constantNullType
+    if tp1 == constantNullType && !ctx.mode.is(Mode.SafeNulls)
+    then tp2 == constantNullType
     else adaptType(tp1, tp2) <:< tp2
   }
 

diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala
@@ -247,6 +247,7 @@ class CompilationTests {
     aggregateTests(
       compileFilesInDir("tests/explicit-nulls/pos", explicitNullsOptions),
       compileFilesInDir("tests/explicit-nulls/pos-separate", explicitNullsOptions),
+      compileFilesInDir("tests/explicit-nulls/pos-patmat", explicitNullsOptions and "-Xfatal-warnings"),
       compileFilesInDir("tests/explicit-nulls/unsafe-common", explicitNullsOptions and "-language:unsafeNulls"),
     )
   }.checkCompile()

diff --git a/...s/explicit-nulls/neg-patmat/patmat1.scala → ...explicit-nulls/neg-patmat/match-pat.scala b/...s/explicit-nulls/neg-patmat/patmat1.scala → ...explicit-nulls/neg-patmat/match-pat.scala
@@ -1,12 +1,9 @@
 class Foo {
+
   val s: String = ???
-  s match {
-    case s: String => 100 // warning: type test will always succeed
-    case _ => 200 // error: unreachable
-  }
 
   s match {
-    case s: String => 100 // warning: type test will always succeed
+    case s: String => 100
     case _ => 200 // error: unreachable
   }
 
@@ -15,20 +12,23 @@ class Foo {
   case object Cat extends Animal
 
   val a: Animal = ???
+
   a match {
     case Dog(name) => 100
     case Cat => 200
     case _ => 300 // error: unreachable
   }
 
-  val a2: Animal|Null = ???
+  val a2: Animal | Null = ???
+
   a2 match {
     case Dog(_) => 100
     case Cat => 200
     case _ => 300
   }
 
-  val a3: Animal|Null = ???
+  val a3: Animal | Null = ???
+
   a3 match {
     case Dog(_) => 100
     case Cat => 200

diff --git a/tests/explicit-nulls/pos-patmat/unsafe-match-null-pat.scala b/tests/explicit-nulls/pos-patmat/unsafe-match-null-pat.scala
@@ -0,0 +1,14 @@
+import scala.language.unsafeNulls
+
+def test1 =
+  val s: String = ???
+  s match
+    case _: String =>
+    // under unsafeNulls, we should not get Match case Unreachable Warning
+    case null => // ok
+
+def test2 =
+  val s: String | Null = ???
+  s match
+    case _: String =>
+    case null =>
diff --git a/tests/explicit-nulls/unsafe-common/unsafe-match-null.scala b/tests/explicit-nulls/unsafe-common/unsafe-match-null.scala
@@ -0,0 +1,11 @@
+def test1 =
+  val s: String = ???
+  s match
+    case _: String =>
+    case null => // error: Values of types Null and String cannot be compared
+
+def test2 =
+  val s: String | Null = ???
+  s match
+    case _: String =>
+    case null =>