From 9c3e2e331424c561eccc32bf7302eed443d1ab7a Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 28 Jun 2025 14:07:18 -0700 Subject: [PATCH 1/5] Minor refactor of better for desugar --- .../src/dotty/tools/dotc/ast/Desugar.scala | 75 ++++++++++--------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index ab77350be26c..0bee0cc34595 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -18,6 +18,7 @@ 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 @@ -2251,49 +2252,53 @@ 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) + val (valeqs, suffix) = rest.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 = rest, 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(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]) - val pats = valeqs.map { case GenAlias(pat, _) => pat } - val rhss = valeqs.map { case GenAlias(_, rhs) => rhs } + case enums @ GenAlias(_, _) :: _ if sourceVersion.enablesBetterFors => + val (valeqs, suffix) = enums.span(_.isInstanceOf[GenAlias]) + val (pats, rhss) = valeqs.map { case GenAlias(pat, rhs) => (pat, rhs) }.unzip val (defpats, ids) = pats.map(makeIdPat).unzip - val pdefs = valeqs.lazyZip(defpats).lazyZip(rhss).map { (valeq, defpat, rhs) => + 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) - } - Block(pdefs, makeFor(mapName, flatMapName, rest, body)) + Block(pdefs, makeFor(mapName, flatMapName, enums = suffix, body)) case _ => EmptyTree //may happen for erroneous input } From c7af9abee36e3de6946c1ce3e0012ec5845cf4f8 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 28 Jun 2025 14:31:22 -0700 Subject: [PATCH 2/5] Prefer Buffer.empty, which is always mutable --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 0bee0cc34595..4f3db0220153 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -21,6 +21,7 @@ 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, @@ -2472,7 +2473,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)) From e2c0c94299c172d0e216adf152f798ae19e3bcaa Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 6 Dec 2025 00:00:18 -0800 Subject: [PATCH 3/5] Mark trailing map receives body to inspect --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 9 ++++----- tests/run/i24673.scala | 13 +++++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) create mode 100644 tests/run/i24673.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 4f3db0220153..e127af0aaa52 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -2236,7 +2236,7 @@ object desugar { case (Tuple(ts1), Tuple(ts2)) => ts1.corresponds(ts2)(deepEquals) case _ => false - def markTrailingMap(aply: Apply, gen: GenFrom, selectName: TermName): Unit = + def markTrailingMap(aply: Apply, gen: GenFrom, selectName: TermName, body: Tree): Unit = if sourceVersion.enablesBetterFors && selectName == mapName && gen.checkMode != GenCheckMode.Filtered // results of withFilter have the wrong type @@ -2247,9 +2247,8 @@ object desugar { enums match { case Nil if sourceVersion.enablesBetterFors => body case (gen: GenFrom) :: Nil => - val aply = Apply(rhsSelect(gen, mapName), makeLambda(gen, body)) - markTrailingMap(aply, gen, mapName) - aply + Apply(rhsSelect(gen, mapName), makeLambda(gen, body)) + .tap(markTrailingMap(_, gen, mapName, body)) case (gen: GenFrom) :: (rest @ (GenFrom(_, _, _) :: _)) => val cont = makeFor(mapName, flatMapName, rest, body) Apply(rhsSelect(gen, flatMapName), makeLambda(gen, cont)) @@ -2266,7 +2265,7 @@ object desugar { if suffix.exists(_.isInstanceOf[GenFrom]) then flatMapName else mapName Apply(rhsSelect(gen, selectName), makeLambda(gen, cont)) - .tap(markTrailingMap(_, gen, selectName)) + .tap(markTrailingMap(_, gen, selectName, cont)) else val (pats, rhss) = valeqs.map { case GenAlias(pat, rhs) => (pat, rhs) }.unzip val (defpat0, id0) = makeIdPat(gen.pat) diff --git a/tests/run/i24673.scala b/tests/run/i24673.scala new file mode 100644 index 000000000000..8e3e691aa7a4 --- /dev/null +++ b/tests/run/i24673.scala @@ -0,0 +1,13 @@ +@main def Test = { + def result = for { + a <- Option(2) + _ = if (true) { + sys.error("err") + } + } yield a + + try + result + ??? + catch case e: RuntimeException => assert(e.getMessage == "err") +} From bb5c8b33d9ea978470215085227fa3fc3e570959 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 6 Dec 2025 00:18:45 -0800 Subject: [PATCH 4/5] Comprehension with alias is never trailing map --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 10 ++++------ tests/run/better-fors-map-elim.scala | 2 ++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index e127af0aaa52..c7b7c6d81df2 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -13,15 +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 collection.mutable, mutable.ListBuffer import reporting.* import printing.Formatting.hl import config.Printers import parsing.Parsers -import dotty.tools.dotc.util.chaining.* +import util.chaining.* import scala.annotation.{unchecked as _, *}, internal.sharable -import scala.collection.mutable, mutable.ListBuffer object desugar { import untpd.* @@ -2236,7 +2235,7 @@ object desugar { case (Tuple(ts1), Tuple(ts2)) => ts1.corresponds(ts2)(deepEquals) case _ => false - def markTrailingMap(aply: Apply, gen: GenFrom, selectName: TermName, body: Tree): Unit = + def markTrailingMap(aply: Apply, gen: GenFrom, selectName: TermName): Unit = if sourceVersion.enablesBetterFors && selectName == mapName && gen.checkMode != GenCheckMode.Filtered // results of withFilter have the wrong type @@ -2248,7 +2247,7 @@ object desugar { case Nil if sourceVersion.enablesBetterFors => body case (gen: GenFrom) :: Nil => Apply(rhsSelect(gen, mapName), makeLambda(gen, body)) - .tap(markTrailingMap(_, gen, mapName, body)) + .tap(markTrailingMap(_, gen, mapName)) case (gen: GenFrom) :: (rest @ (GenFrom(_, _, _) :: _)) => val cont = makeFor(mapName, flatMapName, rest, body) Apply(rhsSelect(gen, flatMapName), makeLambda(gen, cont)) @@ -2265,7 +2264,6 @@ object desugar { if suffix.exists(_.isInstanceOf[GenFrom]) then flatMapName else mapName Apply(rhsSelect(gen, selectName), makeLambda(gen, cont)) - .tap(markTrailingMap(_, gen, selectName, cont)) else val (pats, rhss) = valeqs.map { case GenAlias(pat, rhs) => (pat, rhs) }.unzip val (defpat0, id0) = makeIdPat(gen.pat) diff --git a/tests/run/better-fors-map-elim.scala b/tests/run/better-fors-map-elim.scala index bdeb087258bd..2779a42d9094 100644 --- a/tests/run/better-fors-map-elim.scala +++ b/tests/run/better-fors-map-elim.scala @@ -42,3 +42,5 @@ object MyOption: extension (i: Int) def map[A](f: Int => A): A = ??? val _ = for j <- 42 yield j + + val _ = for x = 42; x <- MyOption(x) yield x From 7f60d2ffaf7da3a49eb5c3084cc30ebebd21eb2e Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 12 Dec 2025 02:24:49 -0800 Subject: [PATCH 5/5] Preserve FilterAlways in withFilter --- .../src/dotty/tools/dotc/ast/Desugar.scala | 22 +++++++++---------- tests/neg/filtering-fors.scala | 2 ++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index c7b7c6d81df2..73af7e14ec6f 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -1439,18 +1439,13 @@ object desugar { * IrrefutableGenFrom: sel with attachment `CheckIrrefutable -> checkMode` */ def makeSelector(sel: Tree, checkMode: MatchCheck)(using Context): Tree = + import MatchCheck.* checkMode match - case MatchCheck.None => - Annotated(sel, New(ref(defn.UncheckedAnnot.typeRef))) - - case MatchCheck.Exhaustive => - sel - - case MatchCheck.IrrefutablePatDef | MatchCheck.IrrefutableGenFrom => + case None => Annotated(sel, New(ref(defn.UncheckedAnnot.typeRef))) + case Exhaustive => sel + case IrrefutablePatDef + | IrrefutableGenFrom => sel.withAttachment(CheckIrrefutable, checkMode) // TODO: use `pushAttachment` and investigate duplicate attachment - sel.withAttachment(CheckIrrefutable, checkMode) - sel - end match case class TuplePatternInfo(arity: Int, varNum: Int, wildcardNum: Int) object TuplePatternInfo: @@ -2284,7 +2279,12 @@ object desugar { case (gen: GenFrom) :: test :: rest => val genFrom = val filtered = Apply(rhsSelect(gen, nme.withFilter), makeLambda(gen, test)) - val mode = if sourceVersion.enablesBetterFors then GenCheckMode.Filtered else GenCheckMode.Ignore + val mode = + import GenCheckMode.* + if sourceVersion.enablesBetterFors then + if gen.checkMode eq FilterAlways then FilterAlways + else Filtered + else Ignore GenFrom(gen.pat, filtered, mode) makeFor(mapName, flatMapName, genFrom :: rest, body) case enums @ GenAlias(_, _) :: _ if sourceVersion.enablesBetterFors => diff --git a/tests/neg/filtering-fors.scala b/tests/neg/filtering-fors.scala index d62a8722f4ba..18ac1472e48a 100644 --- a/tests/neg/filtering-fors.scala +++ b/tests/neg/filtering-fors.scala @@ -31,4 +31,6 @@ object Test { for (case (x: String) <- xs; case (y, z) <- xs) do () // OK for (case (x, y) <- pairs) yield (y, x) // OK + + for case x: String <- xs if x.length < 5 yield x // OK }