Merge pull request #4253 from dotty-staging/fix-4225

Fix #4225: always do reachability check

Author: liufengyun

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

@@ -429,7 +429,6 @@ object desugar {
         }
         val hasRepeatedParam = constrVparamss.exists(_.exists {
           case ValDef(_, tpt, _) => isRepeated(tpt)
-          case _ => false
         })
         if (mods.is(Abstract) || hasRepeatedParam) Nil  // cannot have default arguments for repeated parameters, hence copy method is not issued
         else {

compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala

@@ -473,26 +473,21 @@ class PlainPrinter(_ctx: Context) extends Printer {
     ("Scope{" ~ dclsText(sc.toList) ~ "}").close
 
   def toText[T >: Untyped](tree: Tree[T]): Text = {
-    tree match {
-      case node: Positioned =>
-        def toTextElem(elem: Any): Text = elem match {
-          case elem: Showable => elem.toText(this)
-          case elem: List[_] => "List(" ~ Text(elem map toTextElem, ",") ~ ")"
-          case elem => elem.toString
-        }
-        val nodeName = node.productPrefix
-        val elems =
-          Text(node.productIterator.map(toTextElem).toList, ", ")
-        val tpSuffix =
-          if (ctx.settings.XprintTypes.value && tree.hasType)
-            " | " ~ toText(tree.typeOpt)
-          else
-            Text()
-
-        nodeName ~ "(" ~ elems ~ tpSuffix ~ ")" ~ (Str(node.pos.toString) provided ctx.settings.YprintPos.value)
-      case _ =>
-        tree.fallbackToText(this)
+    def toTextElem(elem: Any): Text = elem match {
+      case elem: Showable => elem.toText(this)
+      case elem: List[_] => "List(" ~ Text(elem map toTextElem, ",") ~ ")"
+      case elem => elem.toString
     }
+    val nodeName = tree.productPrefix
+    val elems =
+      Text(tree.productIterator.map(toTextElem).toList, ", ")
+    val tpSuffix =
+      if (ctx.settings.XprintTypes.value && tree.hasType)
+        " | " ~ toText(tree.typeOpt)
+      else
+        Text()
+
+    nodeName ~ "(" ~ elems ~ tpSuffix ~ ")" ~ (Str(tree.pos.toString) provided ctx.settings.YprintPos.value)
   }.close // todo: override in refined printer
 
   def toText(result: SearchResult): Text = result match {

compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java

@@ -128,6 +128,7 @@ public enum ErrorMessageID {
     ParamsNoInlineID,
     JavaSymbolIsNotAValueID,
     DoubleDeclarationID,
+    MatchCaseOnlyNullWarningID,
     ;
 
     public int errorNumber() {

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

@@ -909,8 +909,15 @@ object messages {
 
   case class MatchCaseUnreachable()(implicit ctx: Context)
   extends Message(MatchCaseUnreachableID) {
-    val kind = s"""Match ${hl"case"} Unreachable"""
-    val msg = "unreachable code"
+    val kind = "Match case Unreachable"
+    val msg = "unreachable case"
+    val explanation = ""
+  }
+
+  case class MatchCaseOnlyNullWarning()(implicit ctx: Context)
+  extends Message(MatchCaseOnlyNullWarningID) {
+    val kind = "Only null matched"
+    val msg = s"Only ${hl"null"} is matched. Consider using `case null =>` instead."
     val explanation = ""
   }
 

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

@@ -33,11 +33,8 @@ class PatternMatcher extends MiniPhase {
 
     // check exhaustivity and unreachability
     val engine = new patmat.SpaceEngine
-
-    if (engine.checkable(tree)) {
-      engine.checkExhaustivity(tree)
-      engine.checkRedundancy(tree)
-    }
+    engine.checkExhaustivity(tree)
+    engine.checkRedundancy(tree)
 
     translated.ensureConforms(tree.tpe)
   }

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

@@ -86,7 +86,6 @@ object Splicer {
          """.stripMargin
         ctx.error(msg, pos)
         None
-      case ex: Throwable => throw ex
     }
   }
 

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

@@ -63,7 +63,7 @@ case object Empty extends Space
  * @param decomposed: does the space result from decomposition? Used for pretty print
  *
  */
-case class Typ(tp: Type, decomposed: Boolean) extends Space
+case class Typ(tp: Type, decomposed: Boolean = true) extends Space
 
 /** Space representing an extractor pattern */
 case class Prod(tp: Type, unappTp: Type, unappSym: Symbol, params: List[Space], full: Boolean) extends Space
@@ -288,6 +288,9 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
   private val scalaNilType         = ctx.requiredModuleRef("scala.collection.immutable.Nil")
   private val scalaConsType        = ctx.requiredClassRef("scala.collection.immutable.::")
 
+  private val nullType             = ConstantType(Constant(null))
+  private val nullSpace            = Typ(nullType)
+
   override def intersectUnrelatedAtomicTypes(tp1: Type, tp2: Type) = {
     val and = AndType(tp1, tp2)
     // Precondition: !(tp1 <:< tp2) && !(tp2 <:< tp1)
@@ -296,7 +299,10 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
 
     debug.println(s"atomic intersection: ${and.show} = ${res}")
 
-    if (res) Typ(and, true) else Empty
+    if (!res) Empty
+    else if (tp1.isSingleton) Typ(tp1, true)
+    else if (tp2.isSingleton) Typ(tp2, true)
+    else Typ(and, true)
   }
 
   /** Whether the extractor is irrefutable */
@@ -315,6 +321,8 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
       else
         Typ(ConstantType(c), false)
     case _: BackquotedIdent => Typ(pat.tpe, false)
+    case Ident(nme.WILDCARD) =>
+      Or(Typ(pat.tpe.stripAnnots, false) :: nullSpace :: Nil)
     case Ident(_) | Select(_, _) =>
       Typ(pat.tpe.stripAnnots, false)
     case Alternative(trees) => Or(trees.map(project(_)))
@@ -331,6 +339,10 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
     case Typed(pat @ UnApply(_, _, _), _) => project(pat)
     case Typed(expr, tpt) =>
       Typ(erase(expr.tpe.stripAnnots), true)
+    case This(_) =>
+      Typ(pat.tpe.stripAnnots, false)
+    case EmptyTree =>         // default rethrow clause of try/catch, check tests/patmat/try2.scala
+      Typ(WildcardType, false)
     case _ =>
       debug.println(s"unknown pattern: $pat")
       Empty
@@ -345,8 +357,8 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
       OrType(erase(tp1), erase(tp2))
     case AndType(tp1, tp2) =>
       AndType(erase(tp1), erase(tp2))
-    case tp: RefinedType =>
-      tp.derivedRefinedType(erase(tp.parent), tp.refinedName, WildcardType)
+    case tp @ RefinedType(parent, refinedName, _) if refinedName.isTermName =>   // see pos/dependent-extractors.scala
+      tp.derivedRefinedType(erase(parent), refinedName, WildcardType)
     case _ => tp
   }
 
@@ -370,7 +382,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
 
   /** Is `tp1` a subtype of `tp2`?  */
   def isSubType(tp1: Type, tp2: Type): Boolean = {
-    val res = tp1 <:< tp2
+    val res = (tp1 != nullType || tp2 == nullType) && tp1 <:< tp2
     debug.println(s"${tp1.show} <:< ${tp2.show} = $res")
     res
   }
@@ -543,13 +555,6 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
       case tp             => tp.isSingleton
     }
 
-    def superType(tp: Type): Type = tp match {
-      case tp: TypeProxy     => tp.superType
-      case OrType(tp1, tp2)  => OrType(superType(tp1), superType(tp2))
-      case AndType(tp1, tp2) => AndType(superType(tp1), superType(tp2))
-      case _                 => tp
-    }
-
     def recur(tp: Type): Boolean = tp.dealias match {
       case AndType(tp1, tp2) =>
         recur(tp1) && recur(tp2) && {
@@ -570,8 +575,8 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
           noClassConflict &&
             (!isSingleton(tp1) || tp1 <:< tp2) &&
             (!isSingleton(tp2) || tp2 <:< tp1) &&
-            (!bases1.exists(_ is Final) || tp1 <:< superType(tp2)) &&
-            (!bases2.exists(_ is Final) || tp2 <:< superType(tp1))
+            (!bases1.exists(_ is Final) || tp1 <:< tp2) &&
+            (!bases2.exists(_ is Final) || tp2 <:< tp1)
         }
       case OrType(tp1, tp2) =>
         recur(tp1) || recur(tp2)
@@ -842,7 +847,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
     flatten(s).map(doShow(_, false)).distinct.mkString(", ")
   }
 
-  def checkable(tree: Match): Boolean = {
+  private def exhaustivityCheckable(sel: Tree): Boolean = {
     // Possible to check everything, but be compatible with scalac by default
     def isCheckable(tp: Type): Boolean =
       !tp.hasAnnotation(defn.UncheckedAnnot) && {
@@ -860,26 +865,25 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
         (defn.isTupleType(tpw) && tpw.argInfos.exists(isCheckable(_)))
       }
 
-    val Match(sel, cases) = tree
     val res = isCheckable(sel.tpe)
-    debug.println(s"checkable: ${sel.show} = $res")
+    debug.println(s"exhaustivity checkable: ${sel.show} = $res")
     res
   }
 
   /** Whehter counter-examples should be further checked? True for GADTs. */
-  def shouldCheckExamples(tp: Type): Boolean = {
+  private def shouldCheckExamples(tp: Type): Boolean =
     new TypeAccumulator[Boolean] {
       override def apply(b: Boolean, tp: Type): Boolean = tp match {
         case tref: TypeRef if tref.symbol.is(TypeParam) && variance != 1 => true
         case tp => b || foldOver(b, tp)
       }
     }.apply(false, tp)
-  }
 
   def checkExhaustivity(_match: Match): Unit = {
     val Match(sel, cases) = _match
     val selTyp = sel.tpe.widen.dealias
 
+    if (!exhaustivityCheckable(sel)) return
 
     val patternSpace = cases.map({ x =>
       val space = project(x.pat)
@@ -890,36 +894,72 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
     val checkGADTSAT = shouldCheckExamples(selTyp)
 
     val uncovered =
-      flatten(simplify(minus(Typ(selTyp, true), patternSpace), aggressive = true))
-        .filter(s => s != Empty && (!checkGADTSAT || satisfiable(s)))
+      flatten(simplify(minus(Typ(selTyp, true), patternSpace), aggressive = true)).filter { s =>
+        s != Empty && (!checkGADTSAT || satisfiable(s))
+      }
 
     if (uncovered.nonEmpty)
       ctx.warning(PatternMatchExhaustivity(show(Or(uncovered))), sel.pos)
   }
 
+  private def redundancyCheckable(sel: Tree): Boolean =
+    !sel.tpe.hasAnnotation(defn.UncheckedAnnot)
+
   def checkRedundancy(_match: Match): Unit = {
     val Match(sel, cases) = _match
-    // ignore selector type for now
     val selTyp = sel.tpe.widen.dealias
 
-    (0 until cases.length).foreach { i =>
-      // in redundancy check, take guard as false in order to soundly approximate
-      val prevs =
-        if (i == 0)
-          Empty
-        else
-          cases.take(i).map { x =>
-            if (x.guard.isEmpty) project(x.pat)
-            else Empty
-          }.reduce((a, b) => Or(List(a, b)))
+    if (!redundancyCheckable(sel)) return
+
+    val targetSpace =
+      if (selTyp.classSymbol.isPrimitiveValueClass)
+        Typ(selTyp, true)
+      else
+        Or(Typ(selTyp, true) :: nullSpace :: Nil)
 
-      val curr = project(cases(i).pat)
+    // in redundancy check, take guard as false in order to soundly approximate
+    def projectPrevCases(cases: List[CaseDef]): Space =
+      cases.map { x =>
+        if (x.guard.isEmpty) project(x.pat)
+        else Empty
+      }.reduce((a, b) => Or(List(a, b)))
 
-      debug.println(s"---------------reachable? ${show(curr)}")
-      debug.println(s"prev: ${show(prevs)}")
+    def isNull(tree: Tree): Boolean = tree match {
+      case Literal(Constant(null)) => true
+      case _ => false
+    }
 
-      if (isSubspace(intersect(curr, Typ(selTyp, false)), prevs)) {
-        ctx.warning(MatchCaseUnreachable(), cases(i).body.pos)
+    (1 until cases.length).foreach { i =>
+      val prevs = projectPrevCases(cases.take(i))
+
+      val pat = cases(i).pat
+
+      if (pat != EmptyTree) { // rethrow case of catch uses EmptyTree
+        val curr = project(pat)
+
+        debug.println(s"---------------reachable? ${show(curr)}")
+        debug.println(s"prev: ${show(prevs)}")
+
+        var covered = simplify(intersect(curr, targetSpace))
+        debug.println(s"covered: $covered")
+
+        // `covered == Empty` may happen for primitive types with auto-conversion
+        // see tests/patmat/reader.scala  tests/patmat/byte.scala
+        if (covered == Empty) covered = curr
+
+        if (isSubspace(covered, prevs)) {
+          ctx.warning(MatchCaseUnreachable(), pat.pos)
+        }
+
+        // if last case is `_` and only matches `null`, produce a warning
+        if (i == cases.length - 1 && !isNull(pat) ) {
+          simplify(minus(covered, prevs)) match {
+            case Typ(`nullType`, _) =>
+              ctx.warning(MatchCaseOnlyNullWarning(), pat.pos)
+            case _ =>
+          }
+
+        }
       }
     }
   }

tests/patmat/andtype-opentype-interaction.check

@@ -2,6 +2,5 @@
 27: Pattern Match Exhaustivity: _: SealedAbstractClass & OpenTrait & OpenTrait2, _: SealedClass & OpenTrait & OpenTrait2, _: SealedTrait & OpenTrait & OpenTrait2, _: AbstractClass & OpenTrait & OpenTrait2, _: Clazz & OpenTrait & OpenTrait2, _: Trait & OpenTrait & OpenTrait2
 31: Pattern Match Exhaustivity: _: SealedTrait & OpenClass, _: Trait & OpenClass
 35: Pattern Match Exhaustivity: _: SealedTrait & OpenTrait & OpenClass, _: Trait & OpenTrait & OpenClass
-40: Match case Unreachable
 43: Pattern Match Exhaustivity: _: SealedTrait & OpenAbstractClass, _: Trait & OpenAbstractClass
 47: Pattern Match Exhaustivity: _: SealedTrait & OpenClass & OpenTrait & OpenClassSubclass, _: Trait & OpenClass & OpenTrait & OpenClassSubclass

tests/patmat/andtype-refinedtype-interaction.check

@@ -1,9 +1,6 @@
 32: Pattern Match Exhaustivity: _: Trait & C1{x: Int}
-37: Match case Unreachable
-43: Match case Unreachable
 48: Pattern Match Exhaustivity: _: Clazz & (C1 | C2 | T1){x: Int} & (C3 | C4 | T2){x: Int}, _: Trait & (C1 | C2 | T1){x: Int} & (C3 | C4 | T2){x: Int}
 54: Pattern Match Exhaustivity: _: Trait & (C1 | C2 | T1){x: Int} & C3{x: Int}
-60: Match case Unreachable
 65: Pattern Match Exhaustivity: _: Trait & (C1 | C2){x: Int} & (C3 | SubC1){x: Int}
 72: Pattern Match Exhaustivity: _: Trait & (T1 & (C1 | SubC2)){x: Int} & (T2 & (C2 | C3 | SubC1)){x: Int} &
   SubSubC1{x: Int}

tests/patmat/byte.check

@@ -0,0 +1 @@
+5: Match case Unreachable