Filter only for generators starting with case.

But wait with this for now, since we can't cross-compile easily otherwise.
So currently this is enabled only under -strict.

Author: odersky

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

@@ -32,8 +32,19 @@ object desugar {
    */
   val DerivingCompanion: Property.Key[SourcePosition] = new Property.Key
 
-  /** An attachment for match expressions generated from a PatDef */
-  val PatDefMatch: Property.Key[Unit] = new Property.Key
+  /** An attachment for match expressions generated from a PatDef or GenFrom.
+   *  Value of key == one of IrrefutablePatDef, IrrefutableGenFrom
+   */
+  val CheckIrrefutable: Property.Key[MatchCheck] = new Property.StickyKey
+
+  /** What static check should be applied to a Match (none, irrefutable, exhaustive) */
+  class MatchCheck(val n: Int) extends AnyVal
+  object MatchCheck {
+    val None = new MatchCheck(0)
+    val Exhaustive = new MatchCheck(1)
+    val IrrefutablePatDef = new MatchCheck(2)
+    val IrrefutableGenFrom = new MatchCheck(3)
+  }
 
   /** Info of a variable in a pattern: The named tree and its type */
   private type VarInfo = (NameTree, Tree)
@@ -925,6 +936,22 @@ object desugar {
     }
   }
 
+  /** The selector of a match, which depends of the given `checkMode`.
+   *  @param  sel  the original selector
+   *  @return if `checkMode` is
+   *           - None              :  sel @unchecked
+   *           - Exhaustive        :  sel
+   *           - IrrefutablePatDef,
+   *             IrrefutableGenFrom:  sel @unchecked with attachment `CheckIrrefutable -> checkMode`
+   */
+  def makeSelector(sel: Tree, checkMode: MatchCheck)(implicit ctx: Context): Tree =
+    if (checkMode == MatchCheck.Exhaustive) sel
+    else {
+      val sel1 = Annotated(sel, New(ref(defn.UncheckedAnnotType)))
+      if (checkMode != MatchCheck.None) sel1.pushAttachment(CheckIrrefutable, checkMode)
+      sel1
+    }
+
   /** If `pat` is a variable pattern,
    *
    *    val/var/lazy val p = e
@@ -959,11 +986,6 @@ object desugar {
       // - `pat` is a tuple of N variables or wildcard patterns like `(x1, x2, ..., xN)`
       val tupleOptimizable = forallResults(rhs, isMatchingTuple)
 
-      def rhsUnchecked = {
-        val rhs1 = makeAnnotated("scala.unchecked", rhs)
-        rhs1.pushAttachment(PatDefMatch, ())
-        rhs1
-      }
       val vars =
         if (tupleOptimizable) // include `_`
           pat match {
@@ -976,7 +998,7 @@ object desugar {
       val caseDef = CaseDef(pat, EmptyTree, makeTuple(ids))
       val matchExpr =
         if (tupleOptimizable) rhs
-        else Match(rhsUnchecked, caseDef :: Nil)
+        else Match(makeSelector(rhs, MatchCheck.IrrefutablePatDef), caseDef :: Nil)
       vars match {
         case Nil =>
           matchExpr
@@ -1125,14 +1147,10 @@ object desugar {
    *
    *       (x$1, ..., x$n) => (x$0, ..., x${n-1} @unchecked?) match { cases }
    */
-  def makeCaseLambda(cases: List[CaseDef], nparams: Int = 1, unchecked: Boolean = true)(implicit ctx: Context): Function = {
+  def makeCaseLambda(cases: List[CaseDef], checkMode: MatchCheck, nparams: Int = 1)(implicit ctx: Context): Function = {
     val params = (1 to nparams).toList.map(makeSyntheticParameter(_))
     val selector = makeTuple(params.map(p => Ident(p.name)))
-
-    if (unchecked)
-      Function(params, Match(Annotated(selector, New(ref(defn.UncheckedAnnotType))), cases))
-    else
-      Function(params, Match(selector, cases))
+    Function(params, Match(makeSelector(selector, checkMode), cases))
   }
 
   /** Map n-ary function `(p1, ..., pn) => body` where n != 1 to unary function as follows:
@@ -1262,13 +1280,14 @@ object desugar {
 
       /** Make a function value pat => body.
        *  If pat is a var pattern id: T then this gives (id: T) => body
-       *  Otherwise this gives { case pat => body }
+       *  Otherwise this gives { case pat => body }, where `pat` is allowed to be
+       *  refutable only if `checkMode` is MatchCheck.None.
        */
-      def makeLambda(pat: Tree, body: Tree): Tree = pat match {
+      def makeLambda(pat: Tree, body: Tree, checkMode: MatchCheck): Tree = pat match {
         case IdPattern(named, tpt) =>
           Function(derivedValDef(pat, named, tpt, EmptyTree, Modifiers(Param)) :: Nil, body)
         case _ =>
-          makeCaseLambda(CaseDef(pat, EmptyTree, body) :: Nil)
+          makeCaseLambda(CaseDef(pat, EmptyTree, body) :: Nil, checkMode)
       }
 
       /** If `pat` is not an Identifier, a Typed(Ident, _), or a Bind, wrap
@@ -1314,7 +1333,7 @@ object desugar {
         val cases = List(
           CaseDef(pat, EmptyTree, Literal(Constant(true))),
           CaseDef(Ident(nme.WILDCARD), EmptyTree, Literal(Constant(false))))
-        Apply(Select(rhs, nme.withFilter), makeCaseLambda(cases))
+        Apply(Select(rhs, nme.withFilter), makeCaseLambda(cases, MatchCheck.None))
       }
 
       /** Is pattern `pat` irrefutable when matched against `rhs`?
@@ -1353,26 +1372,30 @@ object desugar {
         Select(rhs, name)
       }
 
+      def checkMode(gen: GenFrom) =
+        if (gen.filtering) MatchCheck.None // refutable paterns were already eliminated in filter step
+        else MatchCheck.IrrefutableGenFrom
+
       enums match {
         case (gen: GenFrom) :: Nil =>
-          Apply(rhsSelect(gen, mapName), makeLambda(gen.pat, body))
+          Apply(rhsSelect(gen, mapName), makeLambda(gen.pat, body, checkMode(gen)))
         case (gen: GenFrom) :: (rest @ (GenFrom(_, _, _) :: _)) =>
           val cont = makeFor(mapName, flatMapName, rest, body)
-          Apply(rhsSelect(gen, flatMapName), makeLambda(gen.pat, cont))
-        case (gen @ GenFrom(pat, rhs, _)) :: (rest @ GenAlias(_, _) :: _) =>
+          Apply(rhsSelect(gen, flatMapName), makeLambda(gen.pat, cont, checkMode(gen)))
+        case (gen: GenFrom) :: (rest @ GenAlias(_, _) :: _) =>
           val (valeqs, rest1) = rest.span(_.isInstanceOf[GenAlias])
           val pats = valeqs map { case GenAlias(pat, _) => pat }
           val rhss = valeqs map { case GenAlias(_, rhs) => rhs }
-          val (defpat0, id0) = makeIdPat(pat)
+          val (defpat0, id0) = makeIdPat(gen.pat)
           val (defpats, ids) = (pats map makeIdPat).unzip
           val pdefs = (valeqs, defpats, rhss).zipped.map(makePatDef(_, Modifiers(), _, _))
-          val rhs1 = makeFor(nme.map, nme.flatMap, GenFrom(defpat0, rhs, gen.filtering) :: Nil, Block(pdefs, makeTuple(id0 :: ids)))
-          val allpats = pat :: pats
+          val rhs1 = makeFor(nme.map, nme.flatMap, GenFrom(defpat0, gen.expr, gen.filtering) :: Nil, Block(pdefs, makeTuple(id0 :: ids)))
+          val allpats = gen.pat :: pats
           val vfrom1 = new GenFrom(makeTuple(allpats), rhs1, filtering = false)
           makeFor(mapName, flatMapName, vfrom1 :: rest1, body)
         case (gen: GenFrom) :: test :: rest =>
-          val filtered = Apply(rhsSelect(gen, nme.withFilter), makeLambda(gen.pat, test))
-          val genFrom = new GenFrom(gen.pat, filtered, filtering = false)
+          val filtered = Apply(rhsSelect(gen, nme.withFilter), makeLambda(gen.pat, test, MatchCheck.None))
+          val genFrom = GenFrom(gen.pat, filtered, filtering = false)
           makeFor(mapName, flatMapName, genFrom :: rest, body)
         case _ =>
           EmptyTree //may happen for erroneous input

compiler/src/dotty/tools/dotc/typer/Checking.scala

@@ -601,42 +601,48 @@ trait Checking {
    *  This means `pat` is either marked @unchecked or `pt` conforms to the
    *  pattern's type. If pattern is an UnApply, do the check recursively.
    */
-  def checkIrrefutable(pat: Tree, pt: Type)(implicit ctx: Context): Boolean = {
-    patmatch.println(i"check irrefutable $pat: ${pat.tpe} against $pt")
+  def checkIrrefutable(pat: Tree, pt: Type, isPatDef: Boolean)(implicit ctx: Context): Boolean = {
 
     def check(pat: Tree, pt: Type): Boolean = {
       if (pt <:< pat.tpe)
         true
       else {
+        var reportedPt = pt.dropAnnot(defn.UncheckedAnnot)
+        if (!pat.tpe.isSingleton) reportedPt = reportedPt.widen
+        val fix = if (isPatDef) "`: @unchecked` after" else "`case ` before"
         ctx.errorOrMigrationWarning(
-          ex"""pattern's type ${pat.tpe} is more specialized than the right hand side expression's type ${pt.dropAnnot(defn.UncheckedAnnot)}
+          ex"""pattern's type ${pat.tpe} is more specialized than the right hand side expression's type $reportedPt
               |
-              |If the narrowing is intentional, this can be communicated by writing `: @unchecked` after the full pattern.${err.rewriteNotice}""",
+              |If the narrowing is intentional, this can be communicated by writing $fix the full pattern.${err.rewriteNotice}""",
           pat.sourcePos)
         false
       }
     }
 
-    !ctx.settings.strict.value || // only in -strict mode for now since mitigations work only after this PR
-    pat.tpe.widen.hasAnnotation(defn.UncheckedAnnot) || {
-      pat match {
-        case Bind(_, pat1) =>
-          checkIrrefutable(pat1, pt)
-        case UnApply(fn, _, pats) =>
-          check(pat, pt) && {
-            val argPts = unapplyArgs(fn.tpe.finalResultType, fn, pats, pat.sourcePos)
-            pats.corresponds(argPts)(checkIrrefutable)
-          }
-        case Alternative(pats) =>
-          pats.forall(checkIrrefutable(_, pt))
-        case Typed(arg, tpt) =>
-          check(pat, pt) && checkIrrefutable(arg, pt)
-        case Ident(nme.WILDCARD) =>
-          true
-        case _ =>
-          check(pat, pt)
+    def recur(pat: Tree, pt: Type): Boolean =
+      !ctx.settings.strict.value || // only in -strict mode for now since mitigations work only after this PR
+      pat.tpe.widen.hasAnnotation(defn.UncheckedAnnot) || {
+        patmatch.println(i"check irrefutable $pat: ${pat.tpe} against $pt")
+        pat match {
+          case Bind(_, pat1) =>
+            recur(pat1, pt)
+          case UnApply(fn, _, pats) =>
+            check(pat, pt) && {
+              val argPts = unapplyArgs(fn.tpe.finalResultType, fn, pats, pat.sourcePos)
+              pats.corresponds(argPts)(recur)
+            }
+          case Alternative(pats) =>
+            pats.forall(recur(_, pt))
+          case Typed(arg, tpt) =>
+            check(pat, pt) && recur(arg, pt)
+          case Ident(nme.WILDCARD) =>
+            true
+          case _ =>
+            check(pat, pt)
+        }
       }
-    }
+
+    recur(pat, pt)
   }
 
   /** Check that `path` is a legal prefix for an import or export clause */

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -1029,19 +1029,26 @@ class Typer extends Namer
         }
         else {
           val (protoFormals, _) = decomposeProtoFunction(pt, 1)
-          val unchecked = pt.isRef(defn.PartialFunctionClass)
-          typed(desugar.makeCaseLambda(tree.cases, protoFormals.length, unchecked).withSpan(tree.span), pt)
+          val checkMode =
+            if (pt.isRef(defn.PartialFunctionClass)) desugar.MatchCheck.None
+            else desugar.MatchCheck.Exhaustive
+          typed(desugar.makeCaseLambda(tree.cases, checkMode, protoFormals.length).withSpan(tree.span), pt)
         }
       case _ =>
         if (tree.isInline) checkInInlineContext("inline match", tree.posd)
         val sel1 = typedExpr(tree.selector)
         val selType = fullyDefinedType(sel1.tpe, "pattern selector", tree.span).widen
         val result = typedMatchFinish(tree, sel1, selType, tree.cases, pt)
         result match {
-          case Match(sel, CaseDef(pat, _, _) :: _)
-          if (tree.selector.removeAttachment(desugar.PatDefMatch).isDefined) =>
-            if (!checkIrrefutable(pat, sel.tpe) && ctx.scala2Mode)
-              patch(Span(pat.span.end), ": @unchecked")
+          case Match(sel, CaseDef(pat, _, _) :: _) =>
+            tree.selector.removeAttachment(desugar.CheckIrrefutable) match {
+              case Some(checkMode) =>
+                val isPatDef = checkMode == desugar.MatchCheck.IrrefutablePatDef
+                if (!checkIrrefutable(pat, sel.tpe, isPatDef) && ctx.settings.migration.value)
+                  if (isPatDef) patch(Span(pat.span.end), ": @unchecked")
+                  else patch(Span(pat.span.start), "case ")
+              case _ =>
+            }
           case _ =>
         }
         result

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -194,6 +194,7 @@ class CompilationTests extends ParallelTesting {
       compileFilesInDir("tests/run-custom-args/Yretain-trees", defaultOptions and "-Yretain-trees"),
       compileFile("tests/run-custom-args/tuple-cons.scala", allowDeepSubtypes),
       compileFile("tests/run-custom-args/i5256.scala", allowDeepSubtypes),
+      compileFile("tests/run-custom-args/fors.scala", defaultOptions and "-strict"),
       compileFile("tests/run-custom-args/no-useless-forwarders.scala", defaultOptions and "-Xmixin-force-forwarders:false"),
       compileFilesInDir("tests/run", defaultOptions)
     ).checkRuns()

tests/neg/zipped.scala

@@ -0,0 +1,38 @@
+// This test shows some un-intuitive behavior of the `zipped` method.
+object Test {
+  val xs: List[Int] = ???
+
+  // 1. This works, since withFilter is not defined on Tuple3zipped. Instead,
+  // an implicit conversion from Tuple3zipped to Traversable[(Int, Int, Int)] is inserted.
+  // The subsequent map operation has the right type for this Traversable.
+  (xs, xs, xs).zipped
+    .withFilter( (x: (Int, Int, Int)) => x match { case (x, y, z) => true } )  // OK
+    .map( (x: (Int, Int, Int)) => x match { case (x, y, z) => x + y + z })     // OK
+
+
+  // 2. This works as well, because of auto untupling i.e. `case` is inserted.
+  // But it does not work in Scala2.
+  (xs, xs, xs).zipped
+  .withFilter( (x: (Int, Int, Int)) => x match { case (x, y, z) => true } )  // OK
+  .map( (x: Int, y: Int, z: Int) => x + y + z )  // OK
+      // works, because of auto untupling i.e. `case` is inserted
+      // does not work in Scala2
+
+  // 3. Now, without withFilter, it's the opposite, we need the 3 parameter map.
+  (xs, xs, xs).zipped
+    .map( (x: Int, y: Int, z: Int) => x + y + z )     // OK
+
+  // 4. The single parameter map does not work.
+  (xs, xs, xs).zipped
+    .map( (x: (Int, Int, Int)) => x match { case (x, y, z) => x + y + z })     // error
+
+  // 5. If we leave out the parameter type, we get a "Wrong number of parameters" error instead
+  (xs, xs, xs).zipped
+    .map( x => x match { case (x, y, z) => x + y + z })     // error
+
+  // This means that the following works in Dotty in normal mode, since a `withFilter`
+  // is inserted. But it does no work under -strict. And it will not work in Scala 3.1.
+  // The reason is that without -strict, the code below is mapped to (1), but with -strict
+  // it is mapped to (5).
+  for ((x, y, z) <- (xs, xs, xs).zipped) yield x + y + z
+}