Merge pull request #8722 from dotty-staging/fix-8681

Fix #8681: handle scrutinee of union types

Author: liufengyun

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -2062,12 +2062,12 @@ object messages {
     def explain = ""
   }
 
-  class TypeTestAlwaysSucceeds(foundCls: Symbol, testCls: Symbol)(implicit ctx: Context) extends SyntaxMsg(TypeTestAlwaysSucceedsID) {
+  class TypeTestAlwaysSucceeds(scrutTp: Type, testTp: Type)(implicit ctx: Context) extends SyntaxMsg(TypeTestAlwaysSucceedsID) {
     def msg = {
       val addendum =
-        if (foundCls != testCls) s" is a subtype of $testCls"
+        if (scrutTp != testTp) s" is a subtype of ${testTp.show}"
         else " is the same as the tested type"
-      s"The highlighted type test will always succeed since the scrutinee type ($foundCls)" + addendum
+      s"The highlighted type test will always succeed since the scrutinee type ($scrutTp.show)" + addendum
     }
     def explain = ""
   }

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

@@ -722,7 +722,7 @@ object Erasure {
       }
 
     override def typedTypeApply(tree: untpd.TypeApply, pt: Type)(using Context): Tree = {
-      val ntree = interceptTypeApply(tree.asInstanceOf[TypeApply])(ctx.withPhase(ctx.erasurePhase)).withSpan(tree.span)
+      val ntree = interceptTypeApply(tree.asInstanceOf[TypeApply])(using ctx.withPhase(ctx.erasurePhase)).withSpan(tree.span)
 
       ntree match {
         case TypeApply(fun, args) =>

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

@@ -53,7 +53,7 @@ object TypeTestsCasts {
    *  7. if `P` is a refinement type, FALSE
    *  8. otherwise, TRUE
    */
-  def checkable(X: Type, P: Type, span: Span)(implicit ctx: Context): Boolean = {
+  def checkable(X: Type, P: Type, span: Span)(using Context): Boolean = {
     def isAbstract(P: Type) = !P.dealias.typeSymbol.isClass
     def isPatternTypeSymbol(sym: Symbol) = !sym.isClass && sym.is(Case)
 
@@ -155,7 +155,7 @@ object TypeTestsCasts {
     res
   }
 
-  def interceptTypeApply(tree: TypeApply)(implicit ctx: Context): Tree = trace(s"transforming ${tree.show}", show = true) {
+  def interceptTypeApply(tree: TypeApply)(using Context): Tree = trace(s"transforming ${tree.show}", show = true) {
     /** Intercept `expr.xyz[XYZ]` */
     def interceptWith(expr: Tree): Tree =
       if (expr.isEmpty) tree
@@ -172,7 +172,9 @@ object TypeTestsCasts {
           else if tp.isRef(defn.AnyValClass) then defn.AnyClass
           else tp.classSymbol
 
-        def foundCls = effectiveClass(expr.tpe.widen)
+        def foundClasses(tp: Type, acc: List[Symbol]): List[Symbol] = tp match
+          case OrType(tp1, tp2) => foundClasses(tp2, foundClasses(tp1, acc))
+          case _ => effectiveClass(tp) :: acc
 
         def inMatch =
           tree.fun.symbol == defn.Any_typeTest ||  // new scheme
@@ -181,7 +183,7 @@ object TypeTestsCasts {
         def transformIsInstanceOf(expr: Tree, testType: Type, flagUnrelated: Boolean): Tree = {
           def testCls = effectiveClass(testType.widen)
 
-          def unreachable(why: => String): Boolean = {
+          def unreachable(why: => String)(using ctx: Context): Boolean = {
             if (flagUnrelated)
               if (inMatch) ctx.error(em"this case is unreachable since $why", expr.sourcePos)
               else ctx.warning(em"this will always yield false since $why", expr.sourcePos)
@@ -191,7 +193,7 @@ object TypeTestsCasts {
           /** Are `foundCls` and `testCls` classes that allow checks
            *  whether a test would be always false?
            */
-          def isCheckable =
+          def isCheckable(foundCls: Symbol) =
             foundCls.isClass && testCls.isClass &&
             !(testCls.isPrimitiveValueClass && !foundCls.isPrimitiveValueClass) &&
                // if `test` is primitive but `found` is not, we might have a case like
@@ -203,8 +205,8 @@ object TypeTestsCasts {
           /** Check whether a runtime test that a value of `foundCls` can be a `testCls`
            *  can be true in some cases. Issues a warning or an error otherwise.
            */
-          def checkSensical: Boolean =
-            if (!isCheckable) true
+          def checkSensical(foundCls: Symbol)(using Context): Boolean =
+            if (!isCheckable(foundCls)) true
             else if (foundCls.isPrimitiveValueClass && !testCls.isPrimitiveValueClass) {
               ctx.error("cannot test if value types are references", tree.sourcePos)
               false
@@ -214,38 +216,50 @@ object TypeTestsCasts {
                 testCls.is(Final) || !testCls.is(Trait) && !foundCls.is(Trait)
               )
               if (foundCls.is(Final))
-                unreachable(i"$foundCls is not a subclass of $testCls")
+                unreachable(i"type ${expr.tpe.widen} is not a subclass of $testCls")
               else if (unrelated)
-                unreachable(i"$foundCls and $testCls are unrelated")
+                unreachable(i"type ${expr.tpe.widen} and $testCls are unrelated")
               else true
             }
             else true
 
           if (expr.tpe <:< testType)
             if (expr.tpe.isNotNull) {
-              if (!inMatch) ctx.warning(TypeTestAlwaysSucceeds(foundCls, testCls), tree.sourcePos)
+              if (!inMatch) ctx.warning(TypeTestAlwaysSucceeds(expr.tpe, testType), tree.sourcePos)
               constant(expr, Literal(Constant(true)))
             }
             else expr.testNotNull
-          else if (!checkSensical)
-            constant(expr, Literal(Constant(false)))
-          else if (testCls.isPrimitiveValueClass)
-            if (foundCls.isPrimitiveValueClass)
-              constant(expr, Literal(Constant(foundCls == testCls)))
+          else {
+            val nestedCtx = ctx.fresh.setNewTyperState()
+            val foundClsSyms = foundClasses(expr.tpe.widen, Nil)
+            val sensical = foundClsSyms.exists(sym => checkSensical(sym)(using nestedCtx))
+            if (!sensical) {
+              nestedCtx.typerState.commit()
+              constant(expr, Literal(Constant(false)))
+            }
+            else if (testCls.isPrimitiveValueClass)
+              foundClsSyms match
+                case List(cls) if cls.isPrimitiveValueClass =>
+                  constant(expr, Literal(Constant(foundClsSyms.head == testCls)))
+                case _ =>
+                  transformIsInstanceOf(expr, defn.boxedType(testCls.typeRef), flagUnrelated)
             else
-              transformIsInstanceOf(expr, defn.boxedType(testCls.typeRef), flagUnrelated)
-          else
-            derivedTree(expr, defn.Any_isInstanceOf, testType)
+              derivedTree(expr, defn.Any_isInstanceOf, testType)
+          }
         }
 
         def transformAsInstanceOf(testType: Type): Tree = {
-          def testCls = testType.widen.classSymbol
+          def testCls = effectiveClass(testType.widen)
+          def foundClsSymPrimitive = {
+            val foundClsSyms = foundClasses(expr.tpe.widen, Nil)
+            foundClsSyms.size == 1 && foundClsSyms.head.isPrimitiveValueClass
+          }
           if (erasure(expr.tpe) <:< testType)
             Typed(expr, tree.args.head) // Replace cast by type ascription (which does not generate any bytecode)
           else if (testCls eq defn.BoxedUnitClass)
             // as a special case, casting to Unit always successfully returns Unit
             Block(expr :: Nil, Literal(Constant(()))).withSpan(expr.span)
-          else if (foundCls.isPrimitiveValueClass)
+          else if (foundClsSymPrimitive)
             if (testCls.isPrimitiveValueClass) primitiveConversion(expr, testCls)
             else derivedTree(box(expr), defn.Any_asInstanceOf, testType)
           else if (testCls.isPrimitiveValueClass)

tests/neg-custom-args/fatal-warnings/i8681.scala

@@ -0,0 +1,14 @@
+case class A(a: Int)
+case class B(b: Int)
+case class C(c: Int)
+
+val a = (A(1): A) match {
+  case A(_) => "OK"
+  case B(_) => "NOT OK" // error: this case is unreachable since class A and class B are unrelated
+}
+
+val b = (A(1): A | B) match {
+  case A(_) => "OK"
+  case B(_) => "OK"
+  case C(_) => "NOT OK" // error
+}

tests/neg-custom-args/fatal-warnings/i8711.check

@@ -0,0 +1,8 @@
+-- Error: tests/neg-custom-args/fatal-warnings/i8711.scala:7:9 ---------------------------------------------------------
+7 |    case x: B => x // error: this case is unreachable since class A is not a subclass of class B
+  |         ^
+  |         this case is unreachable since type A and class B are unrelated
+-- Error: tests/neg-custom-args/fatal-warnings/i8711.scala:12:9 --------------------------------------------------------
+12 |    case x: C => x // error
+   |         ^
+   |         this case is unreachable since type A | B and class C are unrelated

tests/neg-custom-args/fatal-warnings/i8711.scala

@@ -0,0 +1,15 @@
+class A
+class B
+class C
+
+object Test {
+  def foo(x: A) = x match {
+    case x: B => x // error: this case is unreachable since class A is not a subclass of class B
+    case _ =>
+  }
+
+  def bar(x: A | B) = x match {
+    case x: C => x // error
+    case _ =>
+  }
+}