Merge pull request #13290 from dwijnand/unreachable-literals

Author: dwijnand

compiler/src/dotty/tools/dotc/transform/patmat/Space.scala

@@ -165,7 +165,7 @@ trait SpaceLogic {
   }
 
   /** Is `a` a subspace of `b`? Equivalent to `a - b == Empty`, but faster */
-  def isSubspace(a: Space, b: Space)(using Context): Boolean = trace(s"${show(a)} < ${show(b)}", debug) {
+  def isSubspace(a: Space, b: Space)(using Context): Boolean = trace(s"isSubspace(${show(a)}, ${show(b)})", debug) {
     def tryDecompose1(tp: Type) = canDecompose(tp) && isSubspace(Or(decompose(tp)), b)
     def tryDecompose2(tp: Type) = canDecompose(tp) && isSubspace(a, Or(decompose(tp)))
 
@@ -212,14 +212,14 @@ trait SpaceLogic {
         if (isSubType(tp2, tp1)) b
         else if (canDecompose(tp1)) tryDecompose1(tp1)
         else if (isSubType(tp1, tp2)) a // problematic corner case: inheriting a case class
-        else Empty
+        else intersectUnrelatedAtomicTypes(tp1, tp2)
       case (Prod(tp1, fun, ss), Typ(tp2, _)) =>
         if (isSubType(tp1, tp2)) a
         else if (canDecompose(tp2)) tryDecompose2(tp2)
         else if (isSubType(tp2, tp1)) a  // problematic corner case: inheriting a case class
-        else Empty
+        else intersectUnrelatedAtomicTypes(tp1, tp2)
       case (Prod(tp1, fun1, ss1), Prod(tp2, fun2, ss2)) =>
-        if (!isSameUnapply(fun1, fun2)) Empty
+        if (!isSameUnapply(fun1, fun2)) intersectUnrelatedAtomicTypes(tp1, tp2)
         else if (ss1.zip(ss2).exists(p => simplify(intersect(p._1, p._2)) == Empty)) Empty
         else Prod(tp1, fun1, ss1.zip(ss2).map((intersect _).tupled))
     }
@@ -500,14 +500,34 @@ class SpaceEngine(using Context) extends SpaceLogic {
     }
   }
 
+  /** Numeric literals, while being constant values of unrelated types (e.g. Char and Int),
+   *  when used in a case may end up matching at runtime, because their equals may returns true.
+   *  Because these are universally available, general purpose types, it would be good to avoid
+   *  returning false positive warnings, such as in `(c: Char) match { case 67 => ... }` emitting a
+   *  reachability warning on the case.  So the type `ConstantType(Constant(67, IntTag))` is
+   *  converted to `ConstantType(Constant(67, CharTag))`.  #12805 */
+  def convertConstantType(tp: Type, pt: Type): Type = tp match
+    case tp @ ConstantType(const) =>
+      val converted = const.convertTo(pt)
+      if converted == null then tp else ConstantType(converted)
+    case _ => tp
+
+  /** Adapt types by performing primitive value boxing.  #12805 */
+  def maybeBox(tp1: Type, tp2: Type): Type =
+    if tp1.classSymbol.isPrimitiveValueClass && !tp2.classSymbol.isPrimitiveValueClass then
+      defn.boxedType(tp1).narrow
+    else tp1
+
   /** Is `tp1` a subtype of `tp2`?  */
-  def isSubType(tp1: Type, tp2: Type): Boolean = {
-    debug.println(TypeComparer.explained(_.isSubType(tp1, tp2)))
+  def isSubType(_tp1: Type, tp2: Type): Boolean = {
+    val tp1 = maybeBox(convertConstantType(_tp1, tp2), tp2)
+    //debug.println(TypeComparer.explained(_.isSubType(tp1, tp2)))
     val res = if (ctx.explicitNulls) {
       tp1 <:< tp2
     } else {
       (tp1 != constantNullType || tp2 == constantNullType) && tp1 <:< tp2
     }
+    debug.println(i"$tp1 <:< $tp2 = $res")
     res
   }
 
@@ -647,7 +667,6 @@ class SpaceEngine(using Context) extends SpaceLogic {
         parts.map(Typ(_, true))
     }
 
-
   /** Abstract sealed types, or-types, Boolean and Java enums can be decomposed */
   def canDecompose(tp: Type): Boolean =
     val res = tp.dealias match
@@ -663,7 +682,7 @@ class SpaceEngine(using Context) extends SpaceLogic {
         || cls.isAllOf(JavaEnumTrait)
         || tp.isRef(defn.BooleanClass)
         || tp.isRef(defn.UnitClass)
-    debug.println(s"decomposable: ${tp.show} = $res")
+    //debug.println(s"decomposable: ${tp.show} = $res")
     res
 
   /** Show friendly type name with current scope in mind
@@ -747,6 +766,7 @@ class SpaceEngine(using Context) extends SpaceLogic {
   }
 
   def show(ss: Seq[Space]): String = ss.map(show).mkString(", ")
+
   /** Display spaces */
   def show(s: Space): String = {
     def params(tp: Type): List[Type] = tp.classSymbol.primaryConstructor.info.firstParamTypes
@@ -885,49 +905,36 @@ class SpaceEngine(using Context) extends SpaceLogic {
 
     if (!redundancyCheckable(sel)) return
 
-    val targetSpace =
-      if !selTyp.classSymbol.isNullableClass then
-        project(selTyp)
-      else
-        project(OrType(selTyp, constantNullType, soft = false))
-
-    // in redundancy check, take guard as false in order to soundly approximate
-    val spaces = cases.map { x =>
-      val res =
-        if (x.guard.isEmpty) project(x.pat)
-        else Empty
+    val isNullable = selTyp.classSymbol.isNullableClass
+    val targetSpace = if isNullable
+      then project(OrType(selTyp, constantNullType, soft = false))
+      else project(selTyp)
+    debug.println(s"targetSpace: ${show(targetSpace)}")
 
-      debug.println(s"${x.pat.show} ====> ${res}")
-      res
-    }
-
-    (1 until cases.length).foreach { i =>
-      val pat = cases(i).pat
+    cases.iterator.zipWithIndex.foldLeft(Nil: List[Space]) { case (prevs, (CaseDef(pat, guard, _), i)) =>
+      debug.println(i"case pattern: $pat")
 
-      if (pat != EmptyTree) { // rethrow case of catch uses EmptyTree
-        val prevs = Or(spaces.take(i))
-        val curr = project(pat)
+      val curr = project(pat)
+      debug.println(i"reachable? ${show(curr)}")
 
-        debug.println(s"---------------reachable? ${show(curr)}")
-        debug.println(s"prev: ${show(prevs)}")
+      val prev = simplify(Or(prevs))
+      debug.println(s"prev: ${show(prev)}")
 
-        var covered = simplify(intersect(curr, targetSpace))
-        debug.println(s"covered: $covered")
+      val covered = simplify(intersect(curr, targetSpace))
+      debug.println(s"covered: ${show(covered)}")
 
-        // `covered == Empty` may happen for primitive types with auto-conversion
-        // see tests/patmat/reader.scala  tests/patmat/byte.scala
-        if (covered == Empty && !isNullLit(pat)) covered = curr
-
-        if (isSubspace(covered, prevs)) {
-          if i == cases.length - 1
-             && isWildcardArg(pat)
-             && pat.tpe.classSymbol.isNullableClass
-          then
-            report.warning(MatchCaseOnlyNullWarning(), pat.srcPos)
-          else
-            report.warning(MatchCaseUnreachable(), pat.srcPos)
-        }
+      if pat != EmptyTree // rethrow case of catch uses EmptyTree
+        && prev != Empty // avoid isSubspace(Empty, Empty) - one of the previous cases much be reachable
+        && isSubspace(covered, prev)
+      then {
+        if isNullable && i == cases.length - 1 && isWildcardArg(pat) then
+          report.warning(MatchCaseOnlyNullWarning(), pat.srcPos)
+        else
+          report.warning(MatchCaseUnreachable(), pat.srcPos)
       }
+
+      // in redundancy check, take guard as false in order to soundly approximate
+      (if guard.isEmpty then covered else Empty) :: prevs
     }
   }
 }

compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala

@@ -45,7 +45,7 @@ class PatmatExhaustivityTest {
     val baseFilePath  = path.toString.stripSuffix(".scala")
     val checkFilePath = baseFilePath + ".check"
 
-    FileDiff.checkAndDump(path.toString, actualLines, checkFilePath)
+    FileDiff.checkAndDumpOrUpdate(path.toString, actualLines, checkFilePath)
   }
 
   /** A single test with multiple files grouped in a folder */
@@ -57,13 +57,17 @@ class PatmatExhaustivityTest {
     val actualLines   = compile(files)
     val checkFilePath = s"${path}${File.separator}expected.check"
 
-    FileDiff.checkAndDump(path.toString, actualLines, checkFilePath)
+    FileDiff.checkAndDumpOrUpdate(path.toString, actualLines, checkFilePath)
   }
 
   @Test
   def patmatExhaustivity: Unit = {
     val res = Directory(testsDir).list.toList
       .filter(f => f.extension == "scala" || f.isDirectory)
+      .filter { f =>
+        val path = if f.isDirectory then f.path + "/" else f.path
+        path.contains(Properties.testsFilter.getOrElse(""))
+      }
       .map(f => if f.isDirectory then compileDir(f.jpath) else compileFile(f.jpath))
 
     val failed = res.filter(!_)

compiler/test/dotty/tools/vulpix/FileDiff.scala

@@ -63,4 +63,21 @@ object FileDiff {
     }
   }
 
+  def checkAndDumpOrUpdate(sourceTitle: String, actualLines: Seq[String], checkFilePath: String): Boolean = {
+    val outFilePath = checkFilePath + ".out"
+    FileDiff.check(sourceTitle, actualLines, checkFilePath) match {
+      case Some(msg) if dotty.Properties.testsUpdateCheckfile =>
+        FileDiff.dump(checkFilePath, actualLines)
+        println("Updated checkfile: " + checkFilePath)
+        false
+      case Some(msg) =>
+        FileDiff.dump(outFilePath, actualLines)
+        println(msg)
+        println(FileDiff.diffMessage(checkFilePath, outFilePath))
+        false
+      case _ =>
+        Files.deleteIfExists(Paths.get(outFilePath))
+        true
+    }
+  }
 }

tests/patmat/boxing.scala

@@ -0,0 +1,18 @@
+class C {
+  def matchUnboxed(i: Integer) = i match {
+    case null => 0
+    case 1    => 1
+    case _    => 9
+  }
+
+  def matchBoxed(i: Int) = i match {
+    case C.ZERO => 0
+    case C.ONE  => 1
+    case _      => 9
+  }
+}
+
+object C {
+  final val ZERO: Integer = 0
+  final val ONE: Integer  = 1
+}

tests/patmat/i12805-fallout.scala

@@ -0,0 +1,30 @@
+import scala.annotation.unchecked.uncheckedVariance
+
+type Untyped = Null
+
+class Type
+
+abstract class Tree[-T >: Untyped] {
+  type ThisTree[T >: Untyped] <: Tree[T]
+
+  protected var myTpe: T @uncheckedVariance = _
+
+  def withType(tpe: Type): ThisTree[Type] = {
+    val tree = this.asInstanceOf[ThisTree[Type]]
+    tree.myTpe = tpe
+    tree
+  }
+}
+
+case class   Ident[-T >: Untyped]() extends Tree[T]
+case class  DefDef[-T >: Untyped]() extends Tree[T]
+case class Inlined[-T >: Untyped]() extends Tree[T]
+case class CaseDef[-T >: Untyped]() extends Tree[T]
+
+def test[T >: Untyped](tree: Tree[T], tp: Type) = tree.withType(tp) match {
+  case Ident()       => 1
+  case DefDef()      => 2
+  case _: Inlined[_] => 3
+  case CaseDef()     => 4
+  case _             => 5
+}

tests/patmat/i12805.check

@@ -0,0 +1,3 @@
+10: Match case Unreachable
+16: Match case Unreachable
+22: Match case Unreachable

tests/patmat/i12805.scala

@@ -0,0 +1,22 @@
+import scala.language.implicitConversions
+
+type Timeframe = "1m" | "2m" | "1H"
+type TimeframeN = 1 | 2 | 60
+
+def manualConvertToN(tf: Timeframe): TimeframeN = tf match
+  case "1m" => 1
+  case "2m" => 2
+  case "1H" => 60
+  case "4H" => ??? // was: no reachability warning
+
+given Conversion[Timeframe, TimeframeN] =
+  case "1m" => 1
+  case "2m" => 2
+  case "1H" => 60
+  case "4H" => ??? // was: no reachability warning
+
+given Conversion[TimeframeN, Timeframe] =
+  case 1 => "1m"
+  case 2 => "2m"
+  case 60 => "1H"
+  case 240 => ??? // was: no reachability warning

tests/patmat/i12805b.check

@@ -0,0 +1,3 @@
+4: Match case Unreachable
+9: Match case Unreachable
+14: Match case Unreachable

tests/patmat/i12805b.scala

@@ -0,0 +1,14 @@
+def test1(a: 1 | 2) = a match
+  case 1 => true
+  case 2 => false
+  case 4 => ??? // unreachable case, was: no warning
+
+def test2(a: 1 | 2) = a match
+  case 1 => true
+  case 2 => false
+  case _ => ??? // unreachable
+
+def test3(a: 1 | 2) = a match
+  case 1 => true
+  case 2 => false
+  case a if a < 0 => ??? // unreachable