Fix #16899: Better handle X instanceOf P where X is T1 | T2 (#17382)

Author: dwijnand

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

@@ -16,6 +16,8 @@ import util.Spans._
 import reporting._
 import config.Printers.{ transforms => debug }
 
+import patmat.Typ
+
 /** This transform normalizes type tests and type casts,
  *  also replacing type tests with singleton argument type with reference equality check
  *  Any remaining type tests
@@ -51,7 +53,8 @@ object TypeTestsCasts {
    *  6. if `P = T1 | T2` or `P = T1 & T2`, checkable(X, T1) && checkable(X, T2).
    *  7. if `P` is a refinement type, "it's a refinement type"
    *  8. if `P` is a local class which is not statically reachable from the scope where `X` is defined, "it's a local class"
-   *  9. otherwise, ""
+   *  9. if `X` is `T1 | T2`, checkable(T1, P) && checkable(T2, P) or (isCheckDefinitelyFalse(T1, P) && checkable(T2, P)) or (checkable(T1, P) && isCheckDefinitelyFalse(T2, P)).
+   *  10. otherwise, ""
    */
   def whyUncheckable(X: Type, P: Type, span: Span)(using Context): String = atPhase(Phases.refchecksPhase.next) {
     extension (inline s1: String) inline def &&(inline s2: String): String = if s1 == "" then s2 else s1
@@ -129,6 +132,41 @@ object TypeTestsCasts {
 
     }
 
+    /** Whether the check X.isInstanceOf[P] is definitely false? */
+    def isCheckDefinitelyFalse(X: Type, P: Type)(using Context): Boolean = trace(s"isCheckDefinitelyFalse(${X.show}, ${P.show})") {
+      X.widenDealias match
+      case AndType(x1, x2) =>
+        isCheckDefinitelyFalse(x1, P) || isCheckDefinitelyFalse(x2, P)
+
+      case x =>
+        P.widenDealias match
+        case AndType(p1, p2) =>
+          isCheckDefinitelyFalse(x, p1) || isCheckDefinitelyFalse(x, p2)
+
+        case p =>
+          val pSpace = Typ(p)
+          val xSpace = Typ(x)
+          if pSpace.canDecompose then
+            val ps = pSpace.decompose.map(_.tp)
+            ps.forall(p => isCheckDefinitelyFalse(x, p))
+          else if xSpace.canDecompose then
+            val xs = xSpace.decompose.map(_.tp)
+            xs.forall(x => isCheckDefinitelyFalse(x, p))
+          else
+            if x.typeSymbol.isClass && p.typeSymbol.isClass then
+              val xClass = effectiveClass(x)
+              val pClass = effectiveClass(p)
+
+              val bothAreClasses = !xClass.is(Trait) && !pClass.is(Trait)
+              val notXsubP = !xClass.derivesFrom(pClass)
+              val notPsubX = !pClass.derivesFrom(xClass)
+              bothAreClasses && notXsubP && notPsubX
+              || xClass.is(Final) && notXsubP
+              || pClass.is(Final) && notPsubX
+            else
+              false
+    }
+
     def recur(X: Type, P: Type): String = (X <:< P) ||| (P.dealias match {
       case _: SingletonType     => ""
       case _: TypeProxy
@@ -146,7 +184,20 @@ object TypeTestsCasts {
             //   - T1 <:< T2 | T3
             //   - T1 & T2 <:< T3
             // See TypeComparer#either
-            recur(tp1, P) && recur(tp2, P)
+            val res1 = recur(tp1, P)
+            val res2 = recur(tp2, P)
+
+            if res1.isEmpty && res2.isEmpty then
+              res1
+            else if res2.isEmpty then
+              if isCheckDefinitelyFalse(tp1, P) then res2
+              else res1
+            else if res1.isEmpty then
+              if isCheckDefinitelyFalse(tp2, P) then res1
+              else res2
+            else
+              res1
+
           case _ =>
             // always false test warnings are emitted elsewhere
             X.classSymbol.exists && P.classSymbol.exists &&
@@ -302,8 +353,8 @@ object TypeTestsCasts {
 
         /** Transform isInstanceOf
          *
-         *    expr.isInstanceOf[A | B]  ~~>  expr.isInstanceOf[A] | expr.isInstanceOf[B]
-         *    expr.isInstanceOf[A & B]  ~~>  expr.isInstanceOf[A] & expr.isInstanceOf[B]
+         *    expr.isInstanceOf[A | B]          ~~>  expr.isInstanceOf[A] | expr.isInstanceOf[B]
+         *    expr.isInstanceOf[A & B]          ~~>  expr.isInstanceOf[A] & expr.isInstanceOf[B]
          *    expr.isInstanceOf[Tuple]          ~~>  scala.runtime.Tuples.isInstanceOfTuple(expr)
          *    expr.isInstanceOf[EmptyTuple]     ~~>  scala.runtime.Tuples.isInstanceOfEmptyTuple(expr)
          *    expr.isInstanceOf[NonEmptyTuple]  ~~>  scala.runtime.Tuples.isInstanceOfNonEmptyTuple(expr)

tests/neg-custom-args/isInstanceOf/i5826.scala

@@ -1,6 +1,6 @@
 class Foo {
-  def test[A]: List[Int] | A => Int = {
-    case ls: List[Int] => ls.head       // error
+  def test[A]: (List[Int] | A) => Int = {
+    case ls: List[Int] => ls.head       // error, A = List[String]
     case _ => 0
   }
 
@@ -17,4 +17,25 @@ class Foo {
     case ls: A[X] => 4                 // error
     case _ => 0
   }
+
+  def test4[A](x: List[Int] | (A => Int)) = x match {
+    case ls: List[Int] => ls.head       // error, List extends Int => T
+    case _ => 0
+  }
+
+  final class C[T] extends A[T]
+
+  def test5[T](x: A[T] | B[T] | Option[T]): Boolean = x.isInstanceOf[C[String]] // error
+
+  def test6[T](x: A[T] | B[T] | Option[T]): Boolean = x.isInstanceOf[C[T]]
+
+  def test7[A](x: Option[Int] | (A => Int)) = x match {
+    case ls: Option[Int] => ls.head       // OK, Option decomposes to Some and None
+    case _ => 0
+  }
+
+  def test8(x: List[Int] | A[String]) = x match {
+    case ls: List[Int] => ls.head        // OK, List decomposes to :: and Nil
+    case _ => 0
+  }
 }

tests/pos-special/isInstanceOf/i16899.scala

@@ -0,0 +1,5 @@
+sealed trait Unset
+
+def foo(v: Unset|Option[Int]): Unit = v match
+  case v: Unset => ()
+  case v: Option[Int] => ()