Make SIP-62 - betterFors a standard feature

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

@@ -11,7 +11,7 @@ import NameKinds.{UniqueName, ContextBoundParamName, ContextFunctionParamName, D
 import typer.{Namer, Checking}
 import util.{Property, SourceFile, SourcePosition, SrcPos, Chars}
 import config.{Feature, Config}
-import config.Feature.{sourceVersion, migrateTo3, enabled, betterForsEnabled}
+import config.Feature.{sourceVersion, migrateTo3, enabled}
 import config.SourceVersion.*
 import collection.mutable
 import reporting.*
@@ -1953,9 +1953,9 @@ object desugar {
     /** Create tree for for-comprehension `<for (enums) do body>` or
      *   `<for (enums) yield body>` where mapName and flatMapName are chosen
      *  corresponding to whether this is a for-do or a for-yield.
-     *  If betterFors are enabled, the creation performs the following rewrite rules:
+     *  If sourceVersion >= 3.7 are enabled, the creation performs the following rewrite rules:
      *
-     *  1. if betterFors is enabled:
+     *  1. if sourceVersion >= 3.7:
      *
      *    for () do E  ==>  E
      *      or
@@ -1986,13 +1986,13 @@ object desugar {
      *      ==>
      *    for (P <- G.withFilter (P => E); ...) ...
      *
-     *  6. For any N, if betterFors is enabled:
+     *  6. For any N, if sourceVersion >= 3.7:
      *
      *    for (P <- G; P_1 = E_1; ... P_N = E_N; P1 <- G1; ...) ...
      *      ==>
      *    G.flatMap (P => for (P_1 = E_1; ... P_N = E_N; ...))
      *
-     *  7. For any N, if betterFors is enabled:
+     *  7. For any N, if sourceVersion >= 3.7:
      *
      *    for (P <- G; P_1 = E_1; ... P_N = E_N) ...
      *      ==>
@@ -2013,7 +2013,7 @@ object desugar {
      *    If any of the P_i are variable patterns, the corresponding `x_i @ P_i` is not generated
      *    and the variable constituting P_i is used instead of x_i
      *
-     *  9. For any N, if betterFors is enabled:
+     *  9. For any N, if sourceVersion >= 3.7:
      *
      *    for (P_1 = E_1; ... P_N = E_N; ...)
      *      ==>
@@ -2044,6 +2044,16 @@ 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)
+        case _ => false
+      }
+
+      /** Does this pattern define any given bindings */
+      def isNestedGivenPattern(pat: Tree): Boolean = pat match
+        case pat @ Bind(_, pat1) => hasGivenBind(pat1)
+        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
@@ -2147,15 +2157,15 @@ object desugar {
           case _ => false
 
       def markTrailingMap(aply: Apply, gen: GenFrom, selectName: TermName): Unit =
-        if betterForsEnabled
+        if sourceVersion.isAtLeast(`3.7`)
           && selectName == mapName
           && gen.checkMode != GenCheckMode.Filtered // results of withFilter have the wrong type
           && (deepEquals(gen.pat, body) || deepEquals(body, Tuple(Nil)))
         then
           aply.putAttachment(TrailingForMap, ())
 
       enums match {
-        case Nil if betterForsEnabled => body
+        case Nil if sourceVersion.isAtLeast(`3.7`) => body
         case (gen: GenFrom) :: Nil =>
           val aply = Apply(rhsSelect(gen, mapName), makeLambda(gen, body))
           markTrailingMap(aply, gen, mapName)
@@ -2164,8 +2174,9 @@ object desugar {
           val cont = makeFor(mapName, flatMapName, rest, body)
           Apply(rhsSelect(gen, flatMapName), makeLambda(gen, cont))
         case (gen: GenFrom) :: rest
-        if betterForsEnabled
-           && rest.dropWhile(_.isInstanceOf[GenAlias]).headOption.forall(e => e.isInstanceOf[GenFrom]) => // possible aliases followed by a generator or end of for
+        if sourceVersion.isAtLeast(`3.7`)
+          && 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
@@ -2191,9 +2202,9 @@ object desugar {
           makeFor(mapName, flatMapName, vfrom1 :: rest1, body)
         case (gen: GenFrom) :: test :: rest =>
           val filtered = Apply(rhsSelect(gen, nme.withFilter), makeLambda(gen, test))
-          val genFrom = GenFrom(gen.pat, filtered, if betterForsEnabled then GenCheckMode.Filtered else GenCheckMode.Ignore)
+          val genFrom = GenFrom(gen.pat, filtered, if sourceVersion.isAtLeast(`3.7`) then GenCheckMode.Filtered else GenCheckMode.Ignore)
           makeFor(mapName, flatMapName, genFrom :: rest, body)
-        case GenAlias(_, _) :: _ if betterForsEnabled =>
+        case GenAlias(_, _) :: _ if sourceVersion.isAtLeast(`3.7`) =>
           val (valeqs, rest) = enums.span(_.isInstanceOf[GenAlias])
           val pats = valeqs.map { case GenAlias(pat, _) => pat }
           val rhss = valeqs.map { case GenAlias(_, rhs) => rhs }

compiler/src/dotty/tools/dotc/config/Feature.scala

@@ -37,7 +37,6 @@ object Feature:
   val modularity = experimental("modularity")
   val betterMatchTypeExtractors = experimental("betterMatchTypeExtractors")
   val quotedPatternsWithPolymorphicFunctions = experimental("quotedPatternsWithPolymorphicFunctions")
-  val betterFors = experimental("betterFors")
   val packageObjectValues = experimental("packageObjectValues")
 
   def experimentalAutoEnableFeatures(using Context): List[TermName] =
@@ -66,8 +65,7 @@ object Feature:
     (into, "Allow into modifier on parameter types"),
     (namedTuples, "Allow named tuples"),
     (modularity, "Enable experimental modularity features"),
-    (betterMatchTypeExtractors, "Enable better match type extractors"),
-    (betterFors, "Enable improvements in `for` comprehensions")
+    (betterMatchTypeExtractors, "Enable better match type extractors")
   )
 
   // legacy language features from Scala 2 that are no longer supported.
@@ -122,8 +120,6 @@ object Feature:
 
   def namedTypeArgsEnabled(using Context) = enabled(namedTypeArguments)
 
-  def betterForsEnabled(using Context) = enabled(betterFors)
-
   def genericNumberLiteralsEnabled(using Context) = enabled(genericNumberLiterals)
 
   def scala2ExperimentalMacroEnabled(using Context) = enabled(scala2macros)

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

@@ -2949,7 +2949,7 @@ object Parsers {
     /** Enumerators ::= Generator {semi Enumerator | Guard}
      */
     def enumerators(): List[Tree] =
-      if in.featureEnabled(Feature.betterFors) then
+      if sourceVersion.isAtLeast(`3.7`) then
         aliasesUntilGenerator() ++ enumeratorsRest()
       else
         generator() :: enumeratorsRest()

docs/_docs/reference/experimental/better-fors.md → docs/_docs/reference/changed-features/better-fors.md

@@ -1,12 +1,10 @@
 ---
 layout: doc-page
 title: "Better fors"
-nightlyOf: https://docs.scala-lang.org/scala3/reference/experimental/better-fors.html
+nightlyOf: https://docs.scala-lang.org/scala3/reference/changed-features/better-fors.html
 ---
 
-The `betterFors` language extension improves the usability of `for`-comprehensions.
-
-The extension is enabled by the language import `import scala.language.experimental.betterFors` or by setting the command line option `-language:experimental.betterFors`.
+Starting in Scala `3.7`, the usability of `for`-comprehensions is improved.
 
 The biggest user facing change is the new ability to start `for`-comprehensions with aliases. This means that the following previously invalid code is now valid:
 
@@ -30,11 +28,11 @@ for
 yield a + b
 ```
 
-Additionally this extension changes the way `for`-comprehensions are desugared. The desugaring is now done in a more intuitive way and the desugared code can be more efficient, because it avoids some unnecessary method calls. There are two main changes in the desugaring:
+Additionally, this extension changes the way `for`-comprehensions are desugared. The desugaring is now done in a more intuitive way and the desugared code can be more efficient, because it avoids some unnecessary method calls. There are two main changes in the desugaring:
 
 1. **Simpler Desugaring for Pure Aliases**:
     When an alias is not followed by a guard, the desugaring is simplified. The last generator and the aliases don't have to be wrapped in a tuple, and instead the aliases are simply introduced as local variables in a block with the next generator.
-    **Current Desugaring**:
+    **Previous Desugaring**:
       ```scala
       for {
         a <- doSth(arg)

docs/sidebar.yml

@@ -115,6 +115,7 @@ subsection:
           - page: reference/changed-features/lazy-vals-init.md
           - page: reference/changed-features/main-functions.md
           - page: reference/changed-features/interpolation-escapes.md
+          - page: reference/changed-features/better-fors.md
       - title: Dropped Features
         index: reference/dropped-features/dropped-features.md
         subsection:
@@ -162,7 +163,6 @@ subsection:
           - page: reference/experimental/modularity.md
           - page: reference/experimental/typeclasses.md
           - page: reference/experimental/runtimeChecked.md
-          - page: reference/experimental/better-fors.md
           - page: reference/experimental/unrolled-defs.md
           - page: reference/experimental/package-object-values.md
       - page: reference/syntax.md

library/src/scala/runtime/stdLibPatches/language.scala

@@ -139,6 +139,7 @@ object language:
      * @see [[https://github.com/scala/improvement-proposals/pull/79]]
      */
     @compileTimeOnly("`betterFors` can only be used at compile time in import statements")
+    @deprecated("The `experimental.betterFors` language import is no longer needed since the feature is now standard", since = "3.7")
     object betterFors
 
     /** Experimental support for package object values

tests/pos/better-fors-given.scala

@@ -0,0 +1,68 @@
+@main def Test: Unit =
+  for
+    x <- Option(23 -> "abc")
+    (a @ given Int, b @ given String) = x
+    _ <- Option(1)
+  yield
+    assert(summon[Int] == 23)
+
+  for
+    x <- Option((1.3, 23 -> "abc"))
+    (_, (a @ given Int, b @ given String)) = x
+    _ <- Option(1)
+  yield
+    assert(summon[Int] == 23)
+
+  for
+    x <- Option(Some(23 -> "abc"))
+    Some(a @ given Int, b @ given String) = x
+    _ <- Option(1)
+  yield
+    assert(summon[Int] == 23)
+
+  for
+    x <- Option(Some(23))
+    Some(a @ given Int) = x
+    _ <- Option(1)
+  yield
+    assert(summon[Int] == 23)
+
+  for
+    x <- Option(23)
+    a @ given Int = x
+  yield
+    assert(summon[Int] == 23)
+
+  for
+    x <- Option(23)
+    _ @ given Int = x
+  yield
+    assert(summon[Int] == 23)
+
+  for
+    x <- Option(23)
+    given Int = x
+  yield
+    assert(summon[Int] == 23)
+
+  for
+    x <- Option(23)
+    given Int = x
+    _ <- Option(1)
+  yield
+    assert(summon[Int] == 23)
+
+  for
+    a @ given Int <- Option(23)
+  yield
+    assert(summon[Int] == 23)
+
+  for
+    _ @ given Int <- Option(23)
+  yield
+    assert(summon[Int] == 23)
+
+  for
+    given Int <- Option(23)
+  yield
+    assert(summon[Int] == 23)

tests/run/better-fors.scala

@@ -1,5 +1,3 @@
-import scala.language.experimental.betterFors
-
 def for1 =
   for {
     a = 1

tests/run/fors.scala

@@ -113,8 +113,6 @@ object Test extends App {
 
   /////////////////// elimination of map ///////////////////
 
-  import scala.language.experimental.betterFors
-
   @tailrec
   def pair[B](xs: List[Int], ys: List[B], n: Int): List[(Int, B)] =
     if n == 0 then xs.zip(ys)