Invent given pattern name in for comprehension

Author: som-snytt

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

@@ -13,13 +13,14 @@ import util.{Property, SourceFile, SourcePosition, SrcPos, Chars}
 import config.{Feature, Config}
 import config.Feature.{sourceVersion, migrateTo3, enabled}
 import config.SourceVersion.*
-import collection.mutable
 import reporting.*
 import printing.Formatting.hl
 import config.Printers
 import parsing.Parsers
+import dotty.tools.dotc.util.chaining.*
 
 import scala.annotation.{unchecked as _, *}, internal.sharable
+import scala.collection.mutable, mutable.ListBuffer
 
 object desugar {
   import untpd.*
@@ -272,12 +273,12 @@ object desugar {
    */
   private def desugarContextBounds(
       tdef: TypeDef,
-      evidenceBuf: mutable.ListBuffer[ValDef],
+      evidenceBuf: ListBuffer[ValDef],
       evidenceFlags: FlagSet,
       freshName: untpd.Tree => TermName,
       allParamss: List[ParamClause])(using Context): TypeDef =
 
-    val evidenceNames = mutable.ListBuffer[TermName]()
+    val evidenceNames = ListBuffer.empty[TermName]
 
     def desugarRHS(rhs: Tree): Tree = rhs match
       case ContextBounds(tbounds, ctxbounds) =>
@@ -322,7 +323,7 @@ object desugar {
   end desugarContextBounds
 
   def elimContextBounds(meth: Tree, isPrimaryConstructor: Boolean = false)(using Context): Tree =
-    val evidenceParamBuf = mutable.ListBuffer[ValDef]()
+    val evidenceParamBuf = ListBuffer.empty[ValDef]
     var seenContextBounds: Int = 0
     def freshName(unused: Tree) =
       seenContextBounds += 1 // Start at 1 like FreshNameCreator.
@@ -647,7 +648,7 @@ object desugar {
    *  ultimately map to deferred givens.
    */
   def typeDef(tdef: TypeDef)(using Context): Tree =
-    val evidenceBuf = new mutable.ListBuffer[ValDef]
+    val evidenceBuf = ListBuffer.empty[ValDef]
     val result = desugarContextBounds(
         tdef, evidenceBuf,
         (tdef.mods.flags.toTermFlags & AccessFlags) | Lazy | DeferredGivenFlags,
@@ -1343,7 +1344,7 @@ object desugar {
       )).withSpan(tree.span)
   end makePolyFunctionType
 
-  /** Invent a name for an anonympus given of type or template `impl`. */
+  /** Invent a name for an anonymous given of type or template `impl`. */
   def inventGivenName(impl: Tree)(using Context): SimpleName =
     val str = impl match
       case impl: Template =>
@@ -1422,7 +1423,7 @@ object desugar {
         ids.map(expand(_, false))
     else {
       val pats1 = if (tpt.isEmpty) pats else pats map (Typed(_, tpt))
-      pats1 map (makePatDef(pdef, mods, _, rhs))
+      pats1.map(makePatDef(original = pdef, mods, _, rhs))
     }
   }
 
@@ -1453,10 +1454,10 @@ object desugar {
    *    val/var/lazy val p = e
    *
    *  Otherwise, in case there is exactly one variable x_1 in pattern
-   *   val/var/lazy val p = e  ==>  val/var/lazy val x_1 = (e: @unchecked) match (case p => (x_1))
+   *   val/var/lazy val p = e ==> val/var/lazy val x_1 = (e: @unchecked) match (case p => (x_1))
    *
    *   in case there are zero or more than one variables in pattern
-   *   val/var/lazy p = e  ==>  private[this] synthetic [lazy] val t$ = (e: @unchecked) match (case p => (x_1, ..., x_N))
+   *   val/var/lazy p = e ==> private[this] synthetic [lazy] val t$ = (e: @unchecked) match (case p => (x_1, ..., x_N))
    *                   val/var/def x_1 = t$._1
    *                   ...
    *                   val/var/def x_N = t$._N
@@ -1470,6 +1471,7 @@ object desugar {
         then cpy.Ident(id)(WildcardParamName.fresh())
         else id
       derivedValDef(original, id1, tpt, rhs, mods)
+
     case _ =>
 
       def filterWildcardGivenBinding(givenPat: Bind): Boolean =
@@ -1538,7 +1540,7 @@ object desugar {
             if useSelectors then Select(Ident(tmpName), nme.selectorName(n))
             else Apply(Select(Ident(tmpName), nme.apply), Literal(Constant(n)) :: Nil)
           val restDefs =
-            for (((named, tpt), n) <- vars.zipWithIndex if named.name != nme.WILDCARD)
+            for ((named, tpt), n) <- vars.zipWithIndex if named.name != nme.WILDCARD
             yield
               if mods.is(Lazy) then
                 DefDef(named.name.asTermName, Nil, tpt, selector(n))
@@ -2044,33 +2046,36 @@ object desugar {
           makeCaseLambda(CaseDef(gen.pat, EmptyTree, body) :: Nil, matchCheckMode)
       }
 
-      def hasGivenBind(pat: Tree): Boolean = pat.existsSubTree {
-        case pat @ Bind(_, pat1) => pat.mods.is(Given)
+      def hasGivenBind(pat: Tree): Boolean = pat.existsSubTree:
+        case pat @ Bind(_, _) => pat.mods.is(Given)
         case _ => false
-      }
 
       /** Does this pattern define any given bindings */
       def isNestedGivenPattern(pat: Tree): Boolean = pat match
-        case pat @ Bind(_, pat1) => hasGivenBind(pat1)
+        case Bind(_, pat) => hasGivenBind(pat)
         case _ => hasGivenBind(pat)
 
       /** If `pat` is not an Identifier, a Typed(Ident, _), or a Bind, wrap
        *  it in a Bind with a fresh name. Return the transformed pattern, and the identifier
        *  that refers to the bound variable for the pattern. Wildcard Binds are
        *  also replaced by Binds with fresh names.
        */
-      def makeIdPat(pat: Tree): (Tree, Ident) = pat match {
-        case bind @ Bind(name, pat1) =>
-          if name == nme.WILDCARD then
-            val name = UniqueName.fresh()
-            (cpy.Bind(pat)(name, pat1).withMods(bind.mods), Ident(name))
-          else (pat, Ident(name))
+      def makeIdPat(pat: Tree): (Tree, Ident) = pat match
+        case pat @ Bind(nme.WILDCARD, body) =>
+          val name =
+            body match
+            case Typed(Ident(nme.WILDCARD), tpt) if pat.mods.is(Given) => inventGivenName(tpt)
+            case _ => UniqueName.fresh()
+          cpy.Bind(pat)(name, body)
+            .withMods(pat.mods)
+          ->
+          Ident(name)
+        case Bind(name, _) => (pat, Ident(name))
         case id: Ident if isVarPattern(id) && id.name != nme.WILDCARD => (id, id)
         case Typed(id: Ident, _) if isVarPattern(id) && id.name != nme.WILDCARD => (pat, id)
         case _ =>
           val name = UniqueName.fresh()
           (Bind(name, pat), Ident(name))
-      }
 
       /** Make a pattern filter:
        *    rhs.withFilter { case pat => true case _ => false }
@@ -2128,11 +2133,11 @@ object desugar {
         }
       }
 
-      /** Is `pat` of the form `x`, `x T`, or `given T`? when used as the lhs of a generator,
+      /** Is `pat` of the form `x`, `x: T`, or `given T`? when used as the lhs of a generator,
        *  these are all considered irrefutable.
        */
       def isVarBinding(pat: Tree): Boolean = pat match
-        case pat @ Bind(_, pat1) if pat.mods.is(Given) => isVarBinding(pat1)
+        case bind @ Bind(_, pat) if bind.mods.is(Given) => isVarBinding(pat)
         case IdPattern(_) => true
         case _ => false
 
@@ -2173,36 +2178,42 @@ object desugar {
         case (gen: GenFrom) :: (rest @ (GenFrom(_, _, _) :: _)) =>
           val cont = makeFor(mapName, flatMapName, rest, body)
           Apply(rhsSelect(gen, flatMapName), makeLambda(gen, cont))
-        case (gen: GenFrom) :: rest
-        if sourceVersion.enablesBetterFors
-          && rest.dropWhile(_.isInstanceOf[GenAlias]).headOption.forall(e => e.isInstanceOf[GenFrom]) // possible aliases followed by a generator or end of for
-          && !rest.takeWhile(_.isInstanceOf[GenAlias]).exists(a => isNestedGivenPattern(a.asInstanceOf[GenAlias].pat)) =>
-          val cont = makeFor(mapName, flatMapName, rest, body)
-          val selectName =
-            if rest.exists(_.isInstanceOf[GenFrom]) then flatMapName
-            else mapName
-          val aply = Apply(rhsSelect(gen, selectName), makeLambda(gen, cont))
-          markTrailingMap(aply, gen, selectName)
-          aply
-        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(gen.pat)
-          val (defpats, ids) = (pats map makeIdPat).unzip
-          val pdefs = valeqs.lazyZip(defpats).lazyZip(rhss).map { (valeq, defpat, rhs) =>
-            val mods = defpat match
-              case defTree: DefTree => defTree.mods
-              case _ => Modifiers()
-            makePatDef(valeq, mods, defpat, rhs)
-          }
-          val rhs1 = makeFor(nme.map, nme.flatMap, GenFrom(defpat0, gen.expr, gen.checkMode) :: Nil, Block(pdefs, makeTuple(id0 :: ids).withAttachment(ForArtifact, ())))
-          val allpats = gen.pat :: pats
-          val vfrom1 = GenFrom(makeTuple(allpats), rhs1, GenCheckMode.Ignore)
-          makeFor(mapName, flatMapName, vfrom1 :: rest1, body)
+        case (gen: GenFrom) :: (tail @ GenAlias(_, _) :: _) =>
+          val (valeqs, suffix) = tail.span(_.isInstanceOf[GenAlias])
+          // possible aliases followed by a generator or end of for, when betterFors.
+          // exclude value definitions with a given pattern (given T = x)
+          val better = sourceVersion.enablesBetterFors
+            && suffix.headOption.forall(_.isInstanceOf[GenFrom])
+            && !valeqs.exists(a => isNestedGivenPattern(a.asInstanceOf[GenAlias].pat))
+          if better then
+            val cont = makeFor(mapName, flatMapName, enums = tail, body)
+            val selectName =
+              if suffix.exists(_.isInstanceOf[GenFrom]) then flatMapName
+              else mapName
+            Apply(rhsSelect(gen, selectName), makeLambda(gen, cont))
+              .tap(markTrailingMap(_, gen, selectName))
+          else
+            val (pats, rhss) = valeqs.map { case GenAlias(pat, rhs) => (pat, rhs) }.unzip
+            val (defpat0, id0) = makeIdPat(gen.pat)
+            val (defpats, ids) = pats.map(makeIdPat).unzip
+            val pdefs = valeqs.lazyZip(defpats).lazyZip(rhss).map: (valeq, defpat, rhs) =>
+              val mods = defpat match
+                case defTree: DefTree => defTree.mods
+                case _ => Modifiers()
+              makePatDef(original = valeq, mods, defpat, rhs)
+            val rhs1 =
+              val enums = GenFrom(defpat0, gen.expr, gen.checkMode) :: Nil
+              val body = Block(pdefs, makeTuple(id0 :: ids).withAttachment(ForArtifact, ()))
+              makeFor(nme.map, nme.flatMap, enums, body)
+            val allpats = gen.pat :: pats
+            val vfrom1 = GenFrom(makeTuple(allpats), rhs1, GenCheckMode.Ignore)
+            makeFor(mapName, flatMapName, enums = vfrom1 :: suffix, body)
+          end if
         case (gen: GenFrom) :: test :: rest =>
-          val filtered = Apply(rhsSelect(gen, nme.withFilter), makeLambda(gen, test))
-          val genFrom = GenFrom(gen.pat, filtered, if sourceVersion.enablesBetterFors then GenCheckMode.Filtered else GenCheckMode.Ignore)
+          val genFrom =
+            val filtered = Apply(rhsSelect(gen, nme.withFilter), makeLambda(gen, test))
+            val mode = if sourceVersion.enablesBetterFors then GenCheckMode.Filtered else GenCheckMode.Ignore
+            GenFrom(gen.pat, filtered, mode)
           makeFor(mapName, flatMapName, genFrom :: rest, body)
         case GenAlias(_, _) :: _ if sourceVersion.enablesBetterFors =>
           val (valeqs, rest) = enums.span(_.isInstanceOf[GenAlias])
@@ -2213,7 +2224,7 @@ object desugar {
             val mods = defpat match
               case defTree: DefTree => defTree.mods
               case _ => Modifiers()
-            makePatDef(valeq, mods, defpat, rhs)
+            makePatDef(original = valeq, mods, defpat, rhs)
           }
           Block(pdefs, makeFor(mapName, flatMapName, rest, body))
         case _ =>
@@ -2279,7 +2290,7 @@ object desugar {
         makeFor(nme.map, nme.flatMap, enums, body) orElse tree
       case PatDef(mods, pats, tpt, rhs) =>
         val pats1 = if (tpt.isEmpty) pats else pats map (Typed(_, tpt))
-        flatTree(pats1 map (makePatDef(tree, mods, _, rhs)))
+        flatTree(pats1.map(makePatDef(original = tree, mods, _, rhs)))
       case ext: ExtMethods =>
         Block(List(ext), syntheticUnitLiteral.withSpan(ext.span))
       case f: FunctionWithMods if f.hasErasedParams => makeFunctionWithValDefs(f, pt)
@@ -2396,7 +2407,7 @@ object desugar {
    *  without duplicates
    */
   private def getVariables(tree: Tree, shouldAddGiven: Context ?=> Bind => Boolean)(using Context): List[VarInfo] = {
-    val buf = mutable.ListBuffer[VarInfo]()
+    val buf = ListBuffer.empty[VarInfo]
     def seenName(name: Name) = buf exists (_._1.name == name)
     def add(named: NameTree, t: Tree): Unit =
       if (!seenName(named.name) && named.name.isTermName) buf += ((named, t))

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

@@ -19,7 +19,7 @@ import dotty.tools.dotc.rewrites.Rewrites
 import dotty.tools.dotc.transform.MegaPhase.MiniPhase
 import dotty.tools.dotc.typer.{ImportInfo, Typer}
 import dotty.tools.dotc.typer.Deriving.OriginalTypeClass
-import dotty.tools.dotc.util.{Property, Spans, SrcPos}, Spans.Span
+import dotty.tools.dotc.util.{Property, Spans, SrcPos}, Spans.{NoSpan, Span}
 import dotty.tools.dotc.util.Chars.{isLineBreakChar, isWhitespace}
 import dotty.tools.dotc.util.chaining.*
 
@@ -426,12 +426,14 @@ object CheckUnused:
   class PostInlining extends CheckUnused(PhaseMode.Report, "PostInlining")
 
   class RefInfos:
-    val defs = mutable.Set.empty[(Symbol, SrcPos)]    // definitions
-    val pats = mutable.Set.empty[(Symbol, SrcPos)]    // pattern variables
-    val refs = mutable.Set.empty[Symbol]              // references
-    val asss = mutable.Set.empty[Symbol]              // targets of assignment
-    val skip = mutable.Set.empty[Symbol]              // methods to skip (don't warn about their params)
-    val nowarn = mutable.Set.empty[Symbol]            // marked @nowarn
+    val defs,  // definitions
+        pats   // pattern variables
+      = mutable.Set.empty[(Symbol, SrcPos)]
+    val refs,  // references
+        asss,  // targets of assignment
+        skip,  // methods to skip (don't warn about their params)
+        nowarn // marked @nowarn
+      = mutable.Set.empty[Symbol]
     val imps = new IdentityHashMap[Import, Unit]         // imports
     val sels = new IdentityHashMap[ImportSelector, Unit] // matched selectors
     def register(tree: Tree)(using Context): Unit = if inlined.isEmpty then
@@ -606,6 +608,7 @@ object CheckUnused:
         warnAt(pos)(UnusedSymbol.localDefs)
 
     def checkPatvars() =
+      //println(infos.pats.mkString("PATVARS\n", "\n", "\n----\n"))
       // convert the one non-synthetic span so all are comparable; filter NoSpan below
       def uniformPos(sym: Symbol, pos: SrcPos): SrcPos =
         if pos.span.isSynthetic then pos else pos.sourcePos.withSpan(pos.span.toSynthetic)

tests/pos/i23119.scala

@@ -0,0 +1,29 @@
+//> using options -Wunused:patvars -Werror
+
+def make: IndexedSeq[FalsePositive] =
+  for {
+    i <- 1 to 2
+    given Int = i
+    fp = FalsePositive()
+  } yield fp
+
+def broken =
+  for
+    i <- List(42)
+    (x, y) = "hello" -> "world"
+  yield
+    s"$x, $y" * i
+
+def alt: IndexedSeq[FalsePositive] =
+  given String = "hi"
+  for
+    given Int <- 1 to 2
+    j: Int = summon[Int] // simple assign because irrefutable
+    _ = j + 1
+    k :: Nil = j :: Nil : @unchecked // pattern in one var
+    fp = FalsePositive(using k)
+  yield fp
+
+class FalsePositive(using Int):
+  def usage(): Unit =
+    println(summon[Int])