Refine checking mode for for generators

Add a third mode, Ignore, for generators where the pattern
is refutable, but we know it has already been used as a filter
in an earlier step.

Author: odersky

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

@@ -1361,22 +1361,22 @@ object desugar {
         }
       }
 
-      def isIrrefutableGenFrom(gen: GenFrom): Boolean =
-        !gen.filtering ||
+      def needsFilter(gen: GenFrom): Boolean =
+        gen.checkMode != GenCheckMode.Filter ||
         IdPattern.unapply(gen.pat).isDefined ||
         isIrrefutable(gen.pat, gen.expr)
 
       /** rhs.name with a pattern filter on rhs unless `pat` is irrefutable when
        *  matched against `rhs`.
        */
       def rhsSelect(gen: GenFrom, name: TermName) = {
-        val rhs = if (isIrrefutableGenFrom(gen)) gen.expr else makePatFilter(gen.expr, gen.pat)
+        val rhs = if (needsFilter(gen)) gen.expr else makePatFilter(gen.expr, gen.pat)
         Select(rhs, name)
       }
 
       def checkMode(gen: GenFrom) =
-        if (gen.filtering) MatchCheck.None // refutable paterns were already eliminated in filter step
-        else MatchCheck.IrrefutableGenFrom
+        if (gen.checkMode == GenCheckMode.Check) MatchCheck.IrrefutableGenFrom
+        else MatchCheck.None // refutable paterns were already eliminated in filter step
 
       enums match {
         case (gen: GenFrom) :: Nil =>
@@ -1391,13 +1391,13 @@ object desugar {
           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, gen.expr, gen.filtering) :: Nil, Block(pdefs, makeTuple(id0 :: ids)))
+          val rhs1 = makeFor(nme.map, nme.flatMap, GenFrom(defpat0, gen.expr, gen.checkMode) :: Nil, Block(pdefs, makeTuple(id0 :: ids)))
           val allpats = gen.pat :: pats
-          val vfrom1 = new GenFrom(makeTuple(allpats), rhs1, filtering = false)
+          val vfrom1 = new GenFrom(makeTuple(allpats), rhs1, GenCheckMode.Ignore)
           makeFor(mapName, flatMapName, vfrom1 :: rest1, body)
         case (gen: GenFrom) :: test :: rest =>
           val filtered = Apply(rhsSelect(gen, nme.withFilter), makeLambda(gen.pat, test, MatchCheck.None))
-          val genFrom = GenFrom(gen.pat, filtered, filtering = false)
+          val genFrom = GenFrom(gen.pat, filtered, GenCheckMode.Ignore)
           makeFor(mapName, flatMapName, genFrom :: rest, body)
         case _ =>
           EmptyTree //may happen for erroneous input

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

@@ -99,7 +99,7 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
   case class DoWhile(body: Tree, cond: Tree)(implicit @constructorOnly src: SourceFile) extends TermTree
   case class ForYield(enums: List[Tree], expr: Tree)(implicit @constructorOnly src: SourceFile) extends TermTree
   case class ForDo(enums: List[Tree], body: Tree)(implicit @constructorOnly src: SourceFile) extends TermTree
-  case class GenFrom(pat: Tree, expr: Tree, filtering: Boolean)(implicit @constructorOnly src: SourceFile) extends Tree
+  case class GenFrom(pat: Tree, expr: Tree, checkMode: GenCheckMode)(implicit @constructorOnly src: SourceFile) extends Tree
   case class GenAlias(pat: Tree, expr: Tree)(implicit @constructorOnly src: SourceFile) extends Tree
   case class ContextBounds(bounds: TypeBoundsTree, cxBounds: List[Tree])(implicit @constructorOnly src: SourceFile) extends TypTree
   case class PatDef(mods: Modifiers, pats: List[Tree], tpt: Tree, rhs: Tree)(implicit @constructorOnly src: SourceFile) extends DefTree
@@ -116,6 +116,14 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
    *  `Positioned#checkPos` */
   class XMLBlock(stats: List[Tree], expr: Tree)(implicit @constructorOnly src: SourceFile) extends Block(stats, expr)
 
+  /** An enum to control checking or filtering of patterns in GenFrom trees */
+  class GenCheckMode(val x: Int) extends AnyVal
+  object GenCheckMode {
+    val Ignore = new GenCheckMode(0)  // neither filter nor check since filtering was done before
+    val Check = new GenCheckMode(1)   // check that pattern is irrefutable
+    val Filter = new GenCheckMode(2)  // filter out non-matching elements
+  }
+
   // ----- Modifiers -----------------------------------------------------
   /** Mod is intended to record syntactic information about modifiers, it's
     * NOT a replacement of FlagSet.
@@ -525,9 +533,9 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
       case tree: ForDo if (enums eq tree.enums) && (body eq tree.body) => tree
       case _ => finalize(tree, untpd.ForDo(enums, body)(tree.source))
     }
-    def GenFrom(tree: Tree)(pat: Tree, expr: Tree, filtering: Boolean)(implicit ctx: Context): Tree = tree match {
-      case tree: GenFrom if (pat eq tree.pat) && (expr eq tree.expr) && (filtering == tree.filtering) => tree
-      case _ => finalize(tree, untpd.GenFrom(pat, expr, filtering)(tree.source))
+    def GenFrom(tree: Tree)(pat: Tree, expr: Tree, checkMode: GenCheckMode)(implicit ctx: Context): Tree = tree match {
+      case tree: GenFrom if (pat eq tree.pat) && (expr eq tree.expr) && (checkMode == tree.checkMode) => tree
+      case _ => finalize(tree, untpd.GenFrom(pat, expr, checkMode)(tree.source))
     }
     def GenAlias(tree: Tree)(pat: Tree, expr: Tree)(implicit ctx: Context): Tree = tree match {
       case tree: GenAlias if (pat eq tree.pat) && (expr eq tree.expr) => tree
@@ -589,8 +597,8 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
         cpy.ForYield(tree)(transform(enums), transform(expr))
       case ForDo(enums, body) =>
         cpy.ForDo(tree)(transform(enums), transform(body))
-      case GenFrom(pat, expr, filtering) =>
-        cpy.GenFrom(tree)(transform(pat), transform(expr), filtering)
+      case GenFrom(pat, expr, checkMode) =>
+        cpy.GenFrom(tree)(transform(pat), transform(expr), checkMode)
       case GenAlias(pat, expr) =>
         cpy.GenAlias(tree)(transform(pat), transform(expr))
       case ContextBounds(bounds, cxBounds) =>

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -1741,7 +1741,10 @@ object Parsers {
 
     def generatorRest(pat: Tree, casePat: Boolean): GenFrom =
       atSpan(startOffset(pat), accept(LARROW)) {
-        GenFrom(pat, expr(), filtering = casePat || !ctx.settings.strict.value)  // don't filter under -strict
+        val checkMode =
+          if (casePat || !ctx.settings.strict.value) GenCheckMode.Filter  // don't filter under -strict
+          else GenCheckMode.Check
+        GenFrom(pat, expr(), checkMode)
       }
 
     /** ForExpr  ::= `for' (`(' Enumerators `)' | `{' Enumerators `}')

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

@@ -570,8 +570,9 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
         forText(enums, expr, keywordStr(" yield "))
       case ForDo(enums, expr) =>
         forText(enums, expr, keywordStr(" do "))
-      case GenFrom(pat, expr, filtering) =>
-        (Str("case ") provided filtering) ~ toText(pat) ~ " <- " ~ toText(expr)
+      case GenFrom(pat, expr, checkMode) =>
+        (Str("case ") provided checkMode == untpd.GenCheckMode.Filter) ~
+        toText(pat) ~ " <- " ~ toText(expr)
       case GenAlias(pat, expr) =>
         toText(pat) ~ " = " ~ toText(expr)
       case ContextBounds(bounds, cxBounds) =>

tests/neg-strict/filtering-fors.scala

@@ -0,0 +1,27 @@
+object Test {
+
+  val xs: List[Any] = ???
+
+  for (x <- xs) do ()   // OK
+  for (x: Any <- xs) do () // OK
+
+  for (x: String <- xs) do ()      // error
+  for ((x: String) <- xs) do ()    // error
+  for (y@ (x: String) <- xs) do () // error
+  for ((x, y) <- xs) do ()         // error
+
+  for ((x: String) <- xs if x.isEmpty) do ()   // error
+  for ((x: String) <- xs; y = x) do ()          // error
+  for ((x: String) <- xs; (y, z) <- xs) do ()   // error // error
+  for (case (x: String) <- xs; (y, z) <- xs) do () // error
+  for ((x: String) <- xs; case (y, z) <- xs) do () // error
+
+  for (case x: String <- xs) do ()      // OK
+  for (case (x: String) <- xs) do ()    // OK
+  for (case y@ (x: String) <- xs) do () // OK
+  for (case (x, y) <- xs) do ()         // OK
+
+  for (case (x: String) <- xs if x.isEmpty) do ()   // OK
+  for (case (x: String) <- xs; y = x) do ()          // OK
+  for (case (x: String) <- xs; case (y, z) <- xs) do ()   // OK
+}