Merge pull request #13485 from dwijnand/emit-deferred-reachability-warnings

Author: dwijnand

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

@@ -1731,6 +1731,20 @@ class Definitions {
     else sys.error(s"Not a primitive value type: $tp")
   }.typeRef
 
+  def unboxedType(tp: Type)(using Context): TypeRef = {
+    val cls = tp.classSymbol
+    if (cls eq BoxedByteClass)         ByteType
+    else if (cls eq BoxedShortClass)   ShortType
+    else if (cls eq BoxedCharClass)    CharType
+    else if (cls eq BoxedIntClass)     IntType
+    else if (cls eq BoxedLongClass)    LongType
+    else if (cls eq BoxedFloatClass)   FloatType
+    else if (cls eq BoxedDoubleClass)  DoubleType
+    else if (cls eq BoxedUnitClass)    UnitType
+    else if (cls eq BoxedBooleanClass) BooleanType
+    else sys.error(s"Not a boxed primitive value type: $tp")
+  }
+
   /** The JVM tag for `tp` if it's a primitive, `java.lang.Object` otherwise. */
   def typeTag(tp: Type)(using Context): Name = typeTags(scalaClassName(tp))
 

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

@@ -512,23 +512,28 @@ class SpaceEngine(using Context) extends SpaceLogic {
       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
+  def isPrimToBox(tp: Type, pt: Type) =
+    tp.classSymbol.isPrimitiveValueClass && (defn.boxedType(tp).classSymbol eq pt.classSymbol)
+
+  /** Adapt types by performing primitive value unboxing or boxing, or numeric constant conversion.  #12805
+   *
+   *  This makes these isSubType cases work like this:
+   *  {{{
+   *   1      <:< Integer  => (<skolem> : Integer) <:< Integer  = true
+   *  ONE     <:< Int      => (<skolem> : Int)     <:< Int      = true
+   *  Integer <:< (1: Int) => (<skolem> : Int)     <:< (1: Int) = false
+   *  }}}
+   */
+  def adaptType(tp1: Type, tp2: Type): Type = trace(i"adaptType($tp1, $tp2)", show = true) {
+    if      isPrimToBox(tp1, tp2) then defn.boxedType(tp1).narrow
+    else if isPrimToBox(tp2, tp1) then defn.unboxedType(tp1).narrow
+    else convertConstantType(tp1, tp2)
+  }
 
   /** Is `tp1` a subtype of `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
+  def isSubType(tp1: Type, tp2: Type): Boolean = trace(i"$tp1 <:< $tp2", debug, show = true) {
+    if tp1 == constantNullType && !ctx.explicitNulls then tp2 == constantNullType
+    else adaptType(tp1, tp2) <:< tp2
   }
 
   def isSameUnapply(tp1: TermRef, tp2: TermRef): Boolean =
@@ -911,7 +916,8 @@ class SpaceEngine(using Context) extends SpaceLogic {
     && !sel.tpe.widen.isRef(defn.QuotedTypeClass)
 
   def checkRedundancy(_match: Match): Unit = {
-    val Match(sel, cases) = _match
+    val Match(sel, _) = _match
+    val cases = _match.cases.toIndexedSeq
     debug.println(i"checking redundancy in $_match")
 
     if (!redundancyCheckable(sel)) return
@@ -925,7 +931,14 @@ class SpaceEngine(using Context) extends SpaceLogic {
       else project(selTyp)
     debug.println(s"targetSpace: ${show(targetSpace)}")
 
-    cases.iterator.zipWithIndex.foldLeft(Nil: List[Space]) { case (prevs, (CaseDef(pat, guard, _), i)) =>
+    var i        = 0
+    val len      = cases.length
+    var prevs    = List.empty[Space]
+    var deferred = List.empty[Tree]
+
+    while (i < len) {
+      val CaseDef(pat, guard, _) = cases(i)
+
       debug.println(i"case pattern: $pat")
 
       val curr = project(pat)
@@ -937,18 +950,24 @@ class SpaceEngine(using Context) extends SpaceLogic {
       val covered = simplify(intersect(curr, targetSpace))
       debug.println(s"covered: ${show(covered)}")
 
-      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
+      if prev == Empty && covered == Empty then // defer until a case is reachable
+        deferred ::= pat
+      else {
+        for (pat <- deferred.reverseIterator)
           report.warning(MatchCaseUnreachable(), pat.srcPos)
+        if pat != EmptyTree // rethrow case of catch uses EmptyTree
+            && isSubspace(covered, prev)
+        then {
+          val nullOnly = isNullable && i == len - 1 && isWildcardArg(pat)
+          val msg = if nullOnly then MatchCaseOnlyNullWarning() else MatchCaseUnreachable()
+          report.warning(msg, pat.srcPos)
+        }
+        deferred = Nil
       }
 
       // in redundancy check, take guard as false in order to soundly approximate
-      (if guard.isEmpty then covered else Empty) :: prevs
+      prevs ::= (if guard.isEmpty then covered else Empty)
+      i += 1
     }
   }
 }

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

@@ -0,0 +1,25 @@
+package dotty.tools
+package dotc
+package transform
+
+import org.junit.*, Assert.*
+
+import core.*, Constants.*, Contexts.*, Decorators.*, Symbols.*, Types.*
+
+class SpaceEngineTest extends DottyTest:
+  @Test def testAdaptTest(): Unit =
+    given Context = ctx
+    val defn = ctx.definitions
+    import defn._
+    val e = patmat.SpaceEngine()
+
+    val BoxedIntType = BoxedIntClass.typeRef
+    val ConstOneType = ConstantType(Constant(1))
+
+    assertTrue(e.isPrimToBox(IntType, BoxedIntType))
+    assertFalse(e.isPrimToBox(BoxedIntType, IntType))
+    assertTrue(e.isPrimToBox(ConstOneType, BoxedIntType))
+
+    assertEquals(BoxedIntType, e.adaptType(IntType, BoxedIntType).widenSingleton)
+    assertEquals(IntType,      e.adaptType(BoxedIntType, IntType).widenSingleton)
+    assertEquals(IntType,      e.adaptType(BoxedIntType, ConstOneType).widenSingleton)

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

@@ -67,9 +67,12 @@ object FileDiff {
     val outFilePath = checkFilePath + ".out"
     FileDiff.check(sourceTitle, actualLines, checkFilePath) match {
       case Some(msg) if dotty.Properties.testsUpdateCheckfile =>
-        FileDiff.dump(checkFilePath, actualLines)
+        Files.deleteIfExists(Paths.get(outFilePath))
+        if actualLines.isEmpty
+          then Files.deleteIfExists(Paths.get(checkFilePath))
+          else FileDiff.dump(checkFilePath, actualLines)
         println("Updated checkfile: " + checkFilePath)
-        false
+        true
       case Some(msg) =>
         FileDiff.dump(outFilePath, actualLines)
         println(msg)

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

@@ -1,8 +1,8 @@
--- Error: tests/neg-custom-args/fatal-warnings/i8711.scala:7:9 ---------------------------------------------------------
+-- [E030] Match case Unreachable 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 --------------------------------------------------------
+  |         ^^^^
+  |         Unreachable case
+-- [E030] Match case Unreachable 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
+   |         ^^^^
+   |         Unreachable case

tests/patmat/i13485.check

@@ -0,0 +1,2 @@
+11: Match case Unreachable
+16: Match case Unreachable

tests/patmat/i13485.scala

@@ -0,0 +1,16 @@
+// The intent of this test is test that changing the order of cases doesn't affect whether
+// warnings, originally reachability warnings but exhaustivity warnings too, are emitted.
+// To do so we need a case that typechecks but is statically assessed to be unreachable.
+// How about... a type pattern on a sealed trait that the scrutinee type doesn't extend?
+
+sealed trait Foo
+
+class Bar
+
+def test1(bar: Bar) = bar match
+  case _: Foo => 1
+  case _: Bar => 2
+
+def test2(bar: Bar) = bar match
+  case _: Bar => 2
+  case _: Foo => 1