Merge pull request #9140 from dwijnand/exhaust

project/ScalaOptionParser.scala

@@ -83,7 +83,7 @@ object ScalaOptionParser {
 
   // TODO retrieve these data programmatically, ala https://github.com/scala/scala-tool-support/blob/master/bash-completion/src/main/scala/BashCompletion.scala
   private def booleanSettingNames = List("-X", "-Xasync", "-Xcheckinit", "-Xdev", "-Xdisable-assertions", "-Xexperimental", "-Xfatal-warnings", "-Xlog-free-terms", "-Xlog-free-types", "-Xlog-implicit-conversions", "-Xlog-implicits", "-Xlog-reflective-calls",
-    "-Xno-forwarders", "-Xno-patmat-analysis", "-Xprint-pos", "-Xprint-types", "-Xprompt", "-Xresident", "-Xshow-phases", "-Xverify", "-Y",
+    "-Xno-forwarders", "-Xno-patmat-analysis", "-Xno-unsealed-patmat-analysis", "-Xnon-strict-patmat-analysis", "-Xprint-pos", "-Xprint-types", "-Xprompt", "-Xresident", "-Xshow-phases", "-Xverify", "-Y",
     "-Ybreak-cycles", "-Ydebug", "-Ycompact-trees", "-YdisableFlatCpCaching", "-Ydoc-debug",
     "-Yide-debug",
     "-Yissue-debug", "-Ylog-classpath", "-Ymacro-debug-lite", "-Ymacro-debug-verbose", "-Ymacro-no-expand",

src/compiler/scala/tools/nsc/settings/ScalaSettings.scala

@@ -168,6 +168,9 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
     def isAtLeastJunit = isTruthy || XmixinForceForwarders.value == "junit"
   }
 
+  val nonStrictPatmatAnalysis  = BooleanSetting("-Xnon-strict-patmat-analysis", "Disable strict exhaustivity analysis, which assumes guards are false and refutable extractors don't match")
+  val noUnsealedPatmatAnalysis = BooleanSetting("-Xno-unsealed-patmat-analysis", "Pattern match on an unsealed class without a catch-all.")
+
   // XML parsing options
   object XxmlSettings extends MultiChoiceEnumeration {
     val coalescing   = Choice("coalescing", "Convert PCData to Text and coalesce sibling nodes")

src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala

@@ -68,13 +68,14 @@ trait TreeAndTypeAnalysis extends Debugging {
   }
 
   def equivalentTree(a: Tree, b: Tree): Boolean = (a, b) match {
-    case (Select(qual1, _), Select(qual2, _)) => equivalentTree(qual1, qual2) && a.symbol == b.symbol
-    case (Ident(_), Ident(_)) => a.symbol == b.symbol
-    case (Literal(c1), Literal(c2)) => c1 == c2
-    case (This(_), This(_)) => a.symbol == b.symbol
-    case (Apply(fun1, args1), Apply(fun2, args2)) => equivalentTree(fun1, fun2) && args1.corresponds(args2)(equivalentTree)
-    // Those are the only cases we need to handle in the pattern matcher
-    case _ => false
+    case (Select(qual1, _), Select(qual2, _))             => equivalentTree(qual1, qual2) && a.symbol == b.symbol
+    case (Ident(_), Ident(_))                             => a.symbol == b.symbol
+    case (Literal(c1), Literal(c2))                       => c1 == c2
+    case (This(_), This(_))                               => a.symbol == b.symbol
+    case (Apply(fun1, args1), Apply(fun2, args2))         => equivalentTree(fun1, fun2) && args1.corresponds(args2)(equivalentTree)
+    case (TypeApply(fun1, args1), TypeApply(fun2, args2)) => equivalentTree(fun1, fun2) && args1.corresponds(args2)(equivalentTree)
+    case (a @ TypeTree(), b @ TypeTree())                 => a.tpe =:= b.tpe
+    case _                                                => false // Those are the only cases we need to handle in the pattern matcher
   }
 
   trait CheckableTreeAndTypeAnalysis {
@@ -238,8 +239,9 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT
         uniqueTypeProps.getOrElseUpdate((testedPath, pt), Eq(Var(testedPath), TypeConst(checkableType(pt))))
 
       // a variable in this set should never be replaced by a tree that "does not consist of a selection on a variable in this set" (intuitively)
-      private val pointsToBound = mutable.HashSet(root)
-      private val trees         = mutable.HashSet.empty[Tree]
+      private val pointsToBound  = mutable.HashSet(root)
+      private val trees          = mutable.HashSet.empty[Tree]
+      private val extractBinders = mutable.HashMap.empty[Tree, Symbol]
 
       // the substitution that renames variables to variables in pointsToBound
       private var normalize: Substitution  = EmptySubstitution
@@ -282,7 +284,21 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT
       // binderToUniqueTree uses the type of the first symbol that was encountered as the type for all future binders
       abstract class TreeMakerToProp extends (TreeMaker => Prop) {
         // requires(if (!substitutionComputed))
-        def updateSubstitution(subst: Substitution): Unit = {
+        def updateSubstitution(tm: TreeMaker): Unit = {
+          val subst = tm.subPatternsAsSubstitution
+
+          tm match {
+            case x @ ExtractorTreeMaker(_, None, binder) =>
+              val extractor = accumSubst(normalize(x.extractor))
+              extractBinders.collectFirst {
+                case (t, reuseBinder) if equivalentTree(t, extractor) => reuseBinder
+              } match {
+                case Some(reuseBinder) => normalize >>= Substitution(binder, binderToUniqueTree(reuseBinder))
+                case None              => extractBinders(extractor) = binder
+              }
+            case _ =>
+          }
+
           // find part of substitution that replaces bound symbols by new symbols, and reverse that part
           // so that we don't introduce new aliases for existing symbols, thus keeping the set of bound symbols minimal
 
@@ -307,7 +323,7 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT
 
           val okSubst = Substitution(unboundFrom.toList, unboundTo.toList) // it's important substitution does not duplicate trees here -- it helps to keep hash consing simple, anyway
           foreach2(okSubst.from, okSubst.to){(f, t) =>
-            if (pointsToBound exists (sym => t.exists(_.symbol == sym)))
+            if (pointsToBound.exists(sym => t.exists(_.symbol == sym)) || tm.isInstanceOf[ExtractorTreeMaker])
               pointsToBound += f
           }
           // debug.patmat("pointsToBound: "+ pointsToBound)
@@ -328,7 +344,7 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT
          * TODO: don't ignore outer-checks
          */
         def apply(tm: TreeMaker): Prop = {
-          if (!substitutionComputed) updateSubstitution(tm.subPatternsAsSubstitution)
+          if (!substitutionComputed) updateSubstitution(tm)
 
           tm match {
             case ttm@TypeTestTreeMaker(prevBinder, testedBinder, pt, _)   =>
@@ -372,10 +388,11 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT
       }
 
       // special-case: interpret pattern `List()` as `Nil`
+      // as of 2.13, List.unapply returns an UnapplySeqWrapper (rather than a List)
       // TODO: make it more general List(1, 2) => 1 :: 2 :: Nil  -- not sure this is a good idea...
       private val rewriteListPattern: PartialFunction[TreeMaker, Prop] = {
         case p @ ExtractorTreeMaker(_, _, testedBinder)
-          if testedBinder.tpe.typeSymbol == ListClass && p.checkedLength == Some(0) =>
+          if testedBinder.tpe.typeSymbol == UnapplySeqWrapperClass && p.checkedLength == Some(0) =>
             uniqueEqualityProp(binderToUniqueTree(p.prevBinder), unique(Ident(NilModule) setType NilModule.tpe))
       }
       val fullRewrite      = (irrefutableExtractor orElse rewriteListPattern)
@@ -497,21 +514,25 @@ trait MatchAnalysis extends MatchApproximation {
 
     // exhaustivity
 
-    def exhaustive(prevBinder: Symbol, cases: List[List[TreeMaker]], pt: Type): List[String] = if (uncheckableType(prevBinder.info)) Nil else {
+    def exhaustive(prevBinder: Symbol, cases: List[List[TreeMaker]], pt: Type): List[String] = if (settings.noUnsealedPatmatAnalysis && uncheckableType(prevBinder.info)) Nil else {
       // customize TreeMakersToProps (which turns a tree of tree makers into a more abstract DAG of tests)
       // - approximate the pattern `List()` (unapplySeq on List with empty length) as `Nil`,
       //   otherwise the common (xs: List[Any]) match { case List() => case x :: xs => } is deemed unexhaustive
-      // - back off (to avoid crying exhaustive too often) when:
-      //    - there are guards -->
-      //    - there are extractor calls (that we can't secretly/soundly) rewrite
+      // - back off (to avoid crying exhaustive too often) in unhandled cases
       val start = if (StatisticsStatics.areSomeColdStatsEnabled) statistics.startTimer(statistics.patmatAnaExhaust) else null
       var backoff = false
+      val strict = !settings.nonStrictPatmatAnalysis.value
 
       val approx = new TreeMakersToProps(prevBinder)
       val symbolicCases = approx.approximateMatch(cases, approx.onUnknown { tm =>
         approx.fullRewrite.applyOrElse[TreeMaker, Prop](tm, {
           case BodyTreeMaker(_, _) => True // irrelevant -- will be discarded by symbolCase later
-          case _ => // debug.patmat("backing off due to "+ tm)
+          case ExtractorTreeMaker(_, _, _)
+             | ProductExtractorTreeMaker(_, _)
+             | GuardTreeMaker(_) if strict =>
+            False
+          case _ =>
+            debug.patmat("backing off due to "+ tm)
             backoff = true
             False
         })
@@ -556,7 +577,8 @@ trait MatchAnalysis extends MatchApproximation {
           // sorting before pruning is important here in order to
           // keep neg/t7020.scala stable
           // since e.g. List(_, _) would cover List(1, _)
-          val pruned = CounterExample.prune(counterExamples.sortBy(_.toString)).map(_.toString)
+          // and make sure the strings are distinct, see Shmeez & TestSequence06 in run/patmatnew.scala
+          val pruned = CounterExample.prune(counterExamples.sortBy(_.toString)).map(_.toString).distinct
 
           if (StatisticsStatics.areSomeColdStatsEnabled) statistics.stopTimer(statistics.patmatAnaExhaust, start)
           pruned
@@ -746,10 +768,13 @@ trait MatchAnalysis extends MatchApproximation {
     //  so, naively, you might try to construct a counter example like _ :: Nil(_ :: _, _ :: _),
     //  since we didn't realize the tail of the outer cons was a Nil)
     def modelToCounterExample(scrutVar: Var)(varAssignment: Map[Var, (Seq[Const], Seq[Const])]): Option[CounterExample] = {
+      val strict = !settings.nonStrictPatmatAnalysis.value
+
       // chop a path into a list of symbols
       def chop(path: Tree): List[Symbol] = path match {
         case Ident(_) => List(path.symbol)
         case Select(pre, name) => chop(pre) :+ path.symbol
+        case Apply(fun, args) => chop(fun) :+ path.symbol
         case _ =>
           // debug.patmat("don't know how to chop "+ path)
           Nil
@@ -873,7 +898,8 @@ trait MatchAnalysis extends MatchApproximation {
               // if uniqueEqualTo contains more than one symbol of the same domain
               // then we can safely ignore these counter examples since we will eventually encounter
               // both counter examples separately
-              case _ if inSameDomain => None
+              // ... in strict mode, consider variable assignment as a wild counter-example
+              case _ if inSameDomain => if (strict) Some(WildcardExample) else None
 
               // not a valid counter-example, possibly since we have a definite type but there was a field mismatch
               // TODO: improve reasoning -- in the mean time, a false negative is better than an annoying false positive

src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala

@@ -269,13 +269,16 @@ trait MatchTranslation {
           val casesNoSubstOnly = caseDefs map { caseDef => (propagateSubstitution(translateCase(scrutSym, pt)(caseDef), EmptySubstitution))}
 
           val exSym = freshSym(pos, pureType(ThrowableTpe), "ex")
+          val suppression =
+            if (settings.XnoPatmatAnalysis) Suppression.FullSuppression
+            else Suppression.NoSuppression.copy(suppressExhaustive = true) // try/catches needn't be exhaustive
 
           List(
               atPos(pos) {
                 CaseDef(
                   Bind(exSym, Ident(nme.WILDCARD)), // TODO: does this need fixing upping?
                   EmptyTree,
-                  combineCasesNoSubstOnly(REF(exSym), scrutSym, casesNoSubstOnly, pt, selectorPos, matchOwner, Some(scrut => Throw(REF(exSym))))
+                  combineCasesNoSubstOnly(REF(exSym), scrutSym, casesNoSubstOnly, pt, selectorPos, matchOwner, Some(scrut => Throw(REF(exSym))), suppression)
                 )
               })
         }

src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala

@@ -583,53 +583,60 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
       removeSubstOnly(treeMakers)
     }
 
+    def getSuppression(scrut: Tree): Suppression = scrut match {
+      case _ if settings.XnoPatmatAnalysis => Suppression.FullSuppression
+      case Typed(tree, tpt)                =>
+        val suppressExhaustive  = tpt.tpe.hasAnnotation(UncheckedClass)
+        val suppressUnreachable = tree match {
+          case Ident(name) => name.startsWith(nme.CHECK_IF_REFUTABLE_STRING) // scala/bug#7183 don't warn for withFilter's that turn out to be irrefutable.
+          case _           => false
+        }
+        Suppression(suppressExhaustive, suppressUnreachable)
+      case _                               => Suppression.NoSuppression
+    }
+
     // calls propagateSubstitution on the treemakers
     def combineCases(scrut: Tree, scrutSym: Symbol, casesRaw: List[List[TreeMaker]], pt: Type, selectorPos: Position, owner: Symbol, matchFailGenOverride: Option[Tree => Tree]): Tree = {
       // drops SubstOnlyTreeMakers, since their effect is now contained in the TreeMakers that follow them
       val casesNoSubstOnly = casesRaw map (propagateSubstitution(_, EmptySubstitution))
-      combineCasesNoSubstOnly(scrut, scrutSym, casesNoSubstOnly, pt, selectorPos, owner, matchFailGenOverride)
+      combineCasesNoSubstOnly(scrut, scrutSym, casesNoSubstOnly, pt, selectorPos, owner, matchFailGenOverride, getSuppression(scrut))
     }
 
     // pt is the fully defined type of the cases (either pt or the lub of the types of the cases)
     def combineCasesNoSubstOnly(scrut: Tree, scrutSym: Symbol, casesNoSubstOnly: List[List[TreeMaker]], pt: Type,
-                                selectorPos: Position, owner: Symbol, matchFailGenOverride: Option[Tree => Tree]): Tree =
+                                selectorPos: Position, owner: Symbol, matchFailGenOverride: Option[Tree => Tree],
+                                suppression: Suppression,
+    ): Tree =
       fixerUpper(owner, scrut.pos) {
         def matchFailGen = matchFailGenOverride orElse Some(Throw(MatchErrorClass.tpe, _: Tree))
 
         debug.patmat("combining cases: "+ (casesNoSubstOnly.map(_.mkString(" >> ")).mkString("{", "\n", "}")))
 
-        val (suppression, requireSwitch): (Suppression, Boolean) =
-          if (settings.XnoPatmatAnalysis) (Suppression.FullSuppression, false)
+        val requireSwitch: Boolean =
+          if (settings.XnoPatmatAnalysis) false
           else scrut match {
             case Typed(tree, tpt) =>
-              val suppressExhaustive = tpt.tpe hasAnnotation UncheckedClass
-              val suppressUnreachable = tree match {
-                case Ident(name) if name startsWith nme.CHECK_IF_REFUTABLE_STRING => true // scala/bug#7183 don't warn for withFilter's that turn out to be irrefutable.
-                case _ => false
-              }
-              val suppression = Suppression(suppressExhaustive, suppressUnreachable)
               val hasSwitchAnnotation = treeInfo.isSwitchAnnotation(tpt.tpe)
               // matches with two or fewer cases need not apply for switchiness (if-then-else will do)
               // `case 1 | 2` is considered as two cases.
               def exceedsTwoCasesOrAlts = {
                 // avoids traversing the entire list if there are more than 3 elements
-                def lengthMax3[T](l: List[T]): Int = l match {
+                def lengthMax3(l: List[List[TreeMaker]]): Int = l match {
                   case a :: b :: c :: _ => 3
-                  case cases =>
-                    cases.map({
+                  case cases            =>
+                    cases.map {
                       case AlternativesTreeMaker(_, alts, _) :: _ => lengthMax3(alts)
                       case c => 1
-                    }).sum
+                    }.sum
                 }
                 lengthMax3(casesNoSubstOnly) > 2
               }
-              val requireSwitch = hasSwitchAnnotation && exceedsTwoCasesOrAlts
-              (suppression, requireSwitch)
+              hasSwitchAnnotation && exceedsTwoCasesOrAlts
             case _ =>
-              (Suppression.NoSuppression, false)
+              false
           }
 
-        emitSwitch(scrut, scrutSym, casesNoSubstOnly, pt, matchFailGenOverride, unchecked = suppression.suppressExhaustive).getOrElse{
+        emitSwitch(scrut, scrutSym, casesNoSubstOnly, pt, matchFailGenOverride, unchecked = suppression.suppressExhaustive).getOrElse {
           if (requireSwitch) typer.context.warning(scrut.pos, "could not emit switch for @switch annotated match", WarningCategory.OtherMatchAnalysis)
 
           if (!casesNoSubstOnly.isEmpty) {

src/compiler/scala/tools/nsc/transform/patmat/PatternMatching.scala

@@ -191,11 +191,14 @@ trait Interface extends ast.TreeDSL {
 
     def reportUnreachable(pos: Position) = typer.context.warning(pos, "unreachable code", WarningCategory.OtherMatchAnalysis)
     def reportMissingCases(pos: Position, counterExamples: List[String]) = {
-      val ceString =
-        if (counterExamples.tail.isEmpty) "input: " + counterExamples.head
-        else "inputs: " + counterExamples.mkString(", ")
+      val ceString = counterExamples match {
+        case Nil        => "" // never occurs, but not carried in the type
+        case "_" :: Nil => ""
+        case ex :: Nil  => s"\nIt would fail on the following input: $ex"
+        case exs        => s"\nIt would fail on the following inputs: ${exs.mkString(", ")}"
+      }
 
-      typer.context.warning(pos, "match may not be exhaustive.\nIt would fail on the following "+ ceString, WarningCategory.OtherMatchAnalysis)
+      typer.context.warning(pos, s"match may not be exhaustive.$ceString", WarningCategory.OtherMatchAnalysis)
     }
   }
 

src/interactive/scala/tools/nsc/interactive/Global.scala

@@ -56,6 +56,7 @@ trait InteractiveAnalyzer extends Analyzer {
     override def missingSelectErrorTree(tree: Tree, qual: Tree, name: Name): Tree = tree match {
       case Select(_, _)             => treeCopy.Select(tree, qual, name)
       case SelectFromTypeTree(_, _) => treeCopy.SelectFromTypeTree(tree, qual, name)
+      case _                        => tree
     }
   }
 

src/interactive/scala/tools/nsc/interactive/Pickler.scala

@@ -99,7 +99,7 @@ object Pickler {
    *  where a value of the given type `T` could not be unpickled from input.
    *  @tparam  T the type of unpickled values in case of success.
    */
-  abstract class Unpickled[+T] {
+  sealed abstract class Unpickled[+T] {
     /** Transforms success values to success values using given function,
      *  leaves failures alone
      *  @param   f the function to apply.
@@ -145,7 +145,7 @@ object Pickler {
    *  @param rd       the lexer unpickled values were read from (can be used to get
    *                  error position, for instance).
    */
-  class UnpickleFailure(msg: => String, val rd: Lexer) extends Unpickled[Nothing] {
+  final class UnpickleFailure(msg: => String, val rd: Lexer) extends Unpickled[Nothing] {
     def errMsg = msg
     override def toString = "Failure at "+rd.tokenPos+":\n"+msg
   }

src/library/scala/unchecked.scala

@@ -15,7 +15,7 @@ package scala
 /** An annotation to designate that the annotated entity
  *  should not be considered for additional compiler checks.
  *  Specific applications include annotating the subject of
- *  a match expression to suppress exhaustiveness warnings, and
+ *  a match expression to suppress exhaustiveness and reachability warnings, and
  *  annotating a type argument in a match case to suppress
  *  unchecked warnings.
  *