Address review comments

Flip some conditions.
Remove isSubtypeOfBottom.
Add new tests.

Author: abeln

compiler/src/dotty/tools/dotc/ast/tpd.scala

@@ -890,12 +890,12 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
 
     /** `tree ne null` (might need a cast to be type correct) */
     def testNotNull(implicit ctx: Context): Tree = {
-      val receiver = if (!defn.isSubtypeOfBottom(tree.tpe)) tree.ensureConforms(defn.ObjectType)
-      else {
+      val receiver = if (defn.isBottomType(tree.tpe)) {
         // If the receiver is of type `Nothing` or `Null`, add an ascription so that the selection
         // succeeds: e.g. `null.ne(null)` doesn't type, but `(null: AnyRef).ne(null)` does.
         Typed(tree, TypeTree(defn.AnyRefType))
       }
+      else tree.ensureConforms(defn.ObjectType)
       receiver.select(defn.Object_ne).appliedTo(Literal(Constant(null)))
     }
 

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

@@ -1004,11 +1004,6 @@ class Definitions {
   def isBottomType(tp: Type): Boolean =
     tp.derivesFrom(NothingClass) || tp.derivesFrom(NullClass)
 
-  /** Is `tp` a subtype of `Nothing` or `Null`? Unlike `isBottomType`, this uses subtyping instead of inheritance. */
-  def isSubtypeOfBottom(tp: Type): Boolean = {
-    tp.frozen_<:<(NothingType) || tp.frozen_<:<(NullType)
-  }
-
   /** Is a function class.
    *   - FunctionXXL
    *   - FunctionN for N >= 0

compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala

@@ -335,15 +335,15 @@ object PatternMatcher {
               patternPlan(casted, pat, onSuccess)
             })
         case UnApply(extractor, implicits, args) =>
-          val unappPlan = if (!defn.isSubtypeOfBottom(scrutinee.info)) {
+          val unappPlan = if (defn.isBottomType(scrutinee.info)) {
+            // Generate a throwaway but type-correct plan.
+            // This plan will never execute because it'll be guarded by a `NonNullTest`.
+            ResultPlan(tpd.Throw(tpd.Literal(Constant(null))))
+          } else {
             val mt @ MethodType(_) = extractor.tpe.widen
             var unapp = extractor.appliedTo(ref(scrutinee).ensureConforms(mt.paramInfos.head))
             if (implicits.nonEmpty) unapp = unapp.appliedToArgs(implicits)
             unapplyPlan(unapp, args)
-          } else {
-            // Generate a throwaway but type-correct plan.
-            // This plan will never execute because it'll be guarded by a `NonNullTest`.
-            ResultPlan(tpd.Throw(tpd.Literal(Constant(null))))
           }
           if (scrutinee.info.isNotNull || nonNull(scrutinee)) unappPlan
           else TestPlan(NonNullTest, scrutinee, tree.span, unappPlan)

tests/run/i5067.check

@@ -1,3 +1,6 @@
 matches null literal
 matches null literal
 not implemented
+match error
+not implemented
+match error

tests/run/i5067.scala

@@ -17,10 +17,31 @@ object Test {
     type Y = Nothing
     try {
       (??? : Y) match {
+        case Some(_) => println("matches Some")
         case _ => println("matches anything")        
       }
     } catch {
       case e: NotImplementedError => println("not implemented")
     }
+
+
+    try {
+      val Some(_) = null
+    } catch {
+      case e: MatchError => println("match error")
+    }
+
+    try {
+      val Some(_) = ???
+    } catch {
+      case e: NotImplementedError => println("not implemented")
+    }
+    
+    val x: X = null
+    try {
+      val Some(_) = x
+    } catch {
+      case e: MatchError => println("match error")
+    }
   }
 }