Add tweaks and docs

odersky · odersky · commit 07c821a1ad5b · 2023-12-18T23:48:50.000+01:00
diff --git a/compiler/src/dotty/tools/dotc/config/Feature.scala b/compiler/src/dotty/tools/dotc/config/Feature.scala
@@ -33,7 +33,7 @@ object Feature:
   val pureFunctions = experimental("pureFunctions")
   val captureChecking = experimental("captureChecking")
   val into = experimental("into")
-  val avoidLoopingGivens = experimental("avoidLoopingGivens")
+  val givenLoopPrevention = experimental("givenLoopPrevention")
 
   val globalOnlyImports: Set[TermName] = Set(pureFunctions, captureChecking)
 
diff --git a/compiler/src/dotty/tools/dotc/config/SourceVersion.scala b/compiler/src/dotty/tools/dotc/config/SourceVersion.scala
@@ -10,6 +10,7 @@ enum SourceVersion:
   case `3.2-migration`, `3.2`
   case `3.3-migration`, `3.3`
   case `3.4-migration`, `3.4`
+  case `3.5-migration`, `3.5`
   // !!! Keep in sync with scala.runtime.stdlibPatches.language !!!
   case `future-migration`, `future`
 
diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala
@@ -1569,19 +1569,18 @@ trait Implicits:
 
         /** Does candidate `cand` come too late for it to be considered as an
          *  eligible candidate? This is the case if `cand` appears in the same
-         *  scope as a given definition enclosing the search point (with no
-         *  class methods between the given definition and the search point)
-         *  and `cand` comes later in the source or coincides with that given
-         *  definition.
+         *  scope as a given definition of the form `given ... = ...` that
+         *  encloses the search point and `cand` comes later in the source or
+         *  coincides with that given definition.
          */
         def comesTooLate(cand: Candidate): Boolean =
           val candSym = cand.ref.symbol
           def candSucceedsGiven(sym: Symbol): Boolean =
-            if sym.owner == candSym.owner then
-              if sym.is(ModuleClass) then candSucceedsGiven(sym.sourceModule)
-              else sym.is(Given) && sym.span.exists && sym.span.start <= candSym.span.start
-            else if sym.owner.isClass then false
-            else candSucceedsGiven(sym.owner)
+            val owner = sym.owner
+            if owner == candSym.owner then
+              sym.is(GivenVal) && sym.span.exists && sym.span.start <= candSym.span.start
+            else if owner.isClass then false
+            else candSucceedsGiven(owner)
 
           ctx.isTyper
           && !candSym.isOneOf(TermParamOrAccessor | Synthetic)
@@ -1596,7 +1595,7 @@ trait Implicits:
 
         def checkResolutionChange(result: SearchResult) =
           if (eligible ne preEligible)
-              && !Feature.enabled(Feature.avoidLoopingGivens)
+              && !Feature.enabled(Feature.givenLoopPrevention)
           then
             val prevResult = searchImplicit(preEligible, contextual)
             prevResult match
@@ -1617,17 +1616,20 @@ trait Implicits:
                   case result: SearchSuccess if prevResult.ref frozen_=:= result.ref =>
                     // OK
                   case _ =>
-                    report.error(
-                      em"""Warning: result of implicit search for $pt will change.
+                    val msg =
+                      em"""Result of implicit search for $pt will change.
                           |Current result ${showResult(prevResult)} will be no longer eligible
                           |  because it is not defined before the search position.
                           |Result with new rules: ${showResult(result)}.
-                          |To opt into the new rules, use the `experimental.avoidLoopingGivens` language import.
+                          |To opt into the new rules, use the `experimental.givenLoopPrevention` language import.
                           |
                           |To fix the problem without the language import, you could try one of the following:
+                          |  - use a `given ... with` clause as the enclosing given,
                           |  - rearrange definitions so that ${showResult(prevResult)} comes earlier,
-                          |  - use an explicit $remedy.""",
-                      srcPos)
+                          |  - use an explicit $remedy."""
+                    if sourceVersion.isAtLeast(SourceVersion.`3.5`)
+                    then report.error(msg, srcPos)
+                    else report.warning(msg.append("\nThis will be an error in Scala 3.5 and later."), srcPos)
               case _ =>
             prevResult
           else result
diff --git a/docs/_docs/reference/changed-features/implicit-resolution.md b/docs/_docs/reference/changed-features/implicit-resolution.md
@@ -164,26 +164,3 @@ The new rules are as follows: An implicit `a` defined in `A` is more specific th
 Condition (*) is new. It is necessary to ensure that the defined relation is transitive.
 
 [//]: # todo: expand with precise rules
-
-**9.** Implicit resolution now tries to avoid recursive givens that can lead to an infinite loop at runtime. Here is an example:
-
-```scala
-object Prices {
-  opaque type Price = BigDecimal
-
-  object Price{
-    given Ordering[Price] = summon[Ordering[BigDecimal]] // was error, now avoided
-  }
-}
-```
-
-Previously, implicit resolution would resolve the `summon` to the given in `Price`, leading to an infinite loop (a warning was issued in that case). We now use the underlying given in `BigDecimal` instead. We achieve that by adding the following rule for implicit search:
-
- - When doing an implicit search while checking the implementation of a `given` definition `G`, discard all search results that lead back to `G` or to a given
-with the same owner as `G` that comes later in the source than `G`.
-
-The new behavior is enabled under `-source future`. In earlier versions, a
-warning is issued where that behavior will change.
-
-Old-style implicit definitions are unaffected by this change.
-
diff --git a/docs/_docs/reference/experimental/given-loop-prevention.md b/docs/_docs/reference/experimental/given-loop-prevention.md
@@ -0,0 +1,31 @@
+---
+layout: doc-page
+title: Given Loop Prevention
+redirectFrom: /docs/reference/other-new-features/into-modifier.html
+nightlyOf: https://docs.scala-lang.org/scala3/reference/experimental/into-modifier.html
+---
+
+Implicit resolution now avoids generating recursive givens that can lead to an infinite loop at runtime. Here is an example:
+
+```scala
+object Prices {
+  opaque type Price = BigDecimal
+
+  object Price{
+    given Ordering[Price] = summon[Ordering[BigDecimal]] // was error, now avoided
+  }
+}
+```
+
+Previously, implicit resolution would resolve the `summon` to the given in `Price`, leading to an infinite loop (a warning was issued in that case). We now use the underlying given in `BigDecimal` instead. We achieve that by adding the following rule for implicit search:
+
+ - When doing an implicit search while checking the implementation of a `given` definition `G` of the form
+    ```
+    given ... = ....
+    ```
+    discard all search results that lead back to `G` or to a given with the same owner as `G` that comes later in the source than `G`.
+
+The new behavior is enabled with the `experimental.givenLoopPrevention` language import. If no such import or setting is given, a warning is issued where the behavior would change under that import (for source version 3.4 and later).
+
+Old-style implicit definitions are unaffected by this change.
+
diff --git a/docs/sidebar.yml b/docs/sidebar.yml
@@ -153,6 +153,7 @@ subsection:
           - page: reference/experimental/cc.md
           - page: reference/experimental/purefuns.md
           - page: reference/experimental/tupled-function.md
+          - page: reference/experimental/given-loop-prevention.md
       - page: reference/syntax.md
       - title: Language Versions
         index: reference/language-versions/language-versions.md
diff --git a/library/src/scala/runtime/stdLibPatches/language.scala b/library/src/scala/runtime/stdLibPatches/language.scala
@@ -95,10 +95,10 @@ object language:
      *  givens. By the new rules, a given may not implicitly use itself or givens
      *  defined after it.
      *
-     *  @see [[https://dotty.epfl.ch/docs/reference/experimental/avoid-looping-givens]]
+     *  @see [[https://dotty.epfl.ch/docs/reference/experimental/given-loop-prevention]]
      */
-    @compileTimeOnly("`avoidLoopingGivens` can only be used at compile time in import statements")
-    object avoidLoopingGivens
+    @compileTimeOnly("`givenLoopPrevention` can only be used at compile time in import statements")
+    object givenLoopPrevention
 
     /** Was needed to add support for relaxed imports of extension methods.
       * The language import is no longer needed as this is now a standard feature since SIP was accepted.
diff --git a/tests/neg/i15474.check b/tests/neg/i15474.check
@@ -1,25 +1,29 @@
 -- Error: tests/neg/i15474.scala:6:39 ----------------------------------------------------------------------------------
 6 |  given c: Conversion[ String, Int ] = _.toInt   // error
   |                                       ^
-  |                             Warning: result of implicit search for ?{ toInt: ? } will change.
-  |                             Current result Test2.c will be no longer eligible
-  |                               because it is not defined before the search position.
-  |                             Result with new rules: augmentString.
-  |                             To opt into the new rules, use the `experimental.avoidLoopingGivens` language import.
+  |                            Result of implicit search for ?{ toInt: ? } will change.
+  |                            Current result Test2.c will be no longer eligible
+  |                              because it is not defined before the search position.
+  |                            Result with new rules: augmentString.
+  |                            To opt into the new rules, use the `experimental.givenLoopPrevention` language import.
   |
-  |                             To fix the problem without the language import, you could try one of the following:
-  |                               - rearrange definitions so that Test2.c comes earlier,
-  |                               - use an explicit conversion,
-  |                               - use an import to get extension method into scope.
+  |                            To fix the problem without the language import, you could try one of the following:
+  |                              - use a `given ... with` clause as the enclosing given,
+  |                              - rearrange definitions so that Test2.c comes earlier,
+  |                              - use an explicit conversion,
+  |                              - use an import to get extension method into scope.
+  |                            This will be an error in Scala 3.5 and later.
 -- Error: tests/neg/i15474.scala:12:56 ---------------------------------------------------------------------------------
 12 |    given Ordering[Price] = summon[Ordering[BigDecimal]] // error
    |                                                        ^
-   |                           Warning: result of implicit search for Ordering[BigDecimal] will change.
-   |                           Current result Prices.Price.given_Ordering_Price will be no longer eligible
-   |                             because it is not defined before the search position.
-   |                           Result with new rules: scala.math.Ordering.BigDecimal.
-   |                           To opt into the new rules, use the `experimental.avoidLoopingGivens` language import.
+   |                          Result of implicit search for Ordering[BigDecimal] will change.
+   |                          Current result Prices.Price.given_Ordering_Price will be no longer eligible
+   |                            because it is not defined before the search position.
+   |                          Result with new rules: scala.math.Ordering.BigDecimal.
+   |                          To opt into the new rules, use the `experimental.givenLoopPrevention` language import.
    |
-   |                           To fix the problem without the language import, you could try one of the following:
-   |                             - rearrange definitions so that Prices.Price.given_Ordering_Price comes earlier,
-   |                             - use an explicit argument.
+   |                          To fix the problem without the language import, you could try one of the following:
+   |                            - use a `given ... with` clause as the enclosing given,
+   |                            - rearrange definitions so that Prices.Price.given_Ordering_Price comes earlier,
+   |                            - use an explicit argument.
+   |                          This will be an error in Scala 3.5 and later.
diff --git a/tests/neg/i6716.check b/tests/neg/i6716.check
@@ -1,12 +1,14 @@
 -- Error: tests/neg/i6716.scala:12:39 ----------------------------------------------------------------------------------
 12 |  given Monad[Bar] = summon[Monad[Foo]] // error
    |                                       ^
-   |                           Warning: result of implicit search for Monad[Foo] will change.
-   |                           Current result Bar.given_Monad_Bar will be no longer eligible
-   |                             because it is not defined before the search position.
-   |                           Result with new rules: Foo.given_Monad_Foo.
-   |                           To opt into the new rules, use the `experimental.avoidLoopingGivens` language import.
+   |                          Result of implicit search for Monad[Foo] will change.
+   |                          Current result Bar.given_Monad_Bar will be no longer eligible
+   |                            because it is not defined before the search position.
+   |                          Result with new rules: Foo.given_Monad_Foo.
+   |                          To opt into the new rules, use the `experimental.givenLoopPrevention` language import.
    |
-   |                           To fix the problem without the language import, you could try one of the following:
-   |                             - rearrange definitions so that Bar.given_Monad_Bar comes earlier,
-   |                             - use an explicit argument.
+   |                          To fix the problem without the language import, you could try one of the following:
+   |                            - use a `given ... with` clause as the enclosing given,
+   |                            - rearrange definitions so that Bar.given_Monad_Bar comes earlier,
+   |                            - use an explicit argument.
+   |                          This will be an error in Scala 3.5 and later.
diff --git a/tests/neg/i7294-a.check b/tests/neg/i7294-a.check
@@ -1,23 +1,25 @@
--- Error: tests/neg/i7294-a.scala:6:10 ---------------------------------------------------------------------------------
-6 |  case x: T => x.g(10) // error // error
+-- [E007] Type Mismatch Error: tests/neg/i7294-a.scala:8:18 ------------------------------------------------------------
+8 |  case x: T => x.g(10) // error // error
+  |               ^^^^^^^
+  |               Found:    Any
+  |               Required: T
+  |
+  |               where:    T is a type in given instance f with bounds <: foo.Foo
+  |
+  | longer explanation available when compiling with `-explain`
+-- Error: tests/neg/i7294-a.scala:8:10 ---------------------------------------------------------------------------------
+8 |  case x: T => x.g(10) // error // error
   |          ^
-  |          Warning: result of implicit search for scala.reflect.TypeTest[Nothing, T] will change.
+  |          Result of implicit search for scala.reflect.TypeTest[Nothing, T] will change.
   |          Current result foo.i7294-a$package.f will be no longer eligible
   |            because it is not defined before the search position.
   |          Result with new rules: No Matching Implicit.
-  |          To opt into the new rules, use the `experimental.avoidLoopingGivens` language import.
+  |          To opt into the new rules, use the `experimental.givenLoopPrevention` language import.
   |
   |          To fix the problem without the language import, you could try one of the following:
+  |            - use a `given ... with` clause as the enclosing given,
   |            - rearrange definitions so that foo.i7294-a$package.f comes earlier,
   |            - use an explicit argument.
+  |          This will be an error in Scala 3.5 and later.
   |
   |          where:    T is a type in given instance f with bounds <: foo.Foo
--- [E007] Type Mismatch Error: tests/neg/i7294-a.scala:6:18 ------------------------------------------------------------
-6 |  case x: T => x.g(10) // error // error
-  |               ^^^^^^^
-  |               Found:    Any
-  |               Required: T
-  |
-  |               where:    T is a type in given instance f with bounds <: foo.Foo
-  |
-  | longer explanation available when compiling with `-explain`
diff --git a/tests/neg/i7294-a.scala b/tests/neg/i7294-a.scala
@@ -1,3 +1,5 @@
+//> using options -Xfatal-warnings
+
 package foo
 
 trait Foo { def g(x: Int): Any }
diff --git a/tests/neg/i7294-b.scala b/tests/neg/i7294-b.scala
@@ -1,3 +1,5 @@
+//> using options -Xfatal-warnings
+
 package foo
 
 trait Foo { def g(x: Any): Any }
diff --git a/tests/neg/looping-givens.scala b/tests/neg/looping-givens.scala
@@ -1,3 +1,5 @@
+//> using options -Xfatal-warnings
+
 class A
 class B
 
diff --git a/tests/pos/i15474.scala b/tests/pos/i15474.scala
@@ -1,6 +1,6 @@
 //> using options -Xfatal-warnings
 import scala.language.implicitConversions
-import scala.language.experimental.avoidLoopingGivens
+import scala.language.experimental.givenLoopPrevention
 
 object Test2:
   given c: Conversion[ String, Int ] = _.toInt   // now avoided, was loop not detected, could be used as a fallback to avoid the warning.
diff --git a/tests/pos/looping-givens.scala b/tests/pos/looping-givens.scala
@@ -1,4 +1,4 @@
-import language.experimental.avoidLoopingGivens
+import language.experimental.givenLoopPrevention
 
 class A
 class B
diff --git a/tests/run/i6716.scala b/tests/run/i6716.scala
@@ -1,6 +1,6 @@
 //> using options -Xfatal-warnings
 
-import scala.language.experimental.avoidLoopingGivens
+import scala.language.experimental.givenLoopPrevention
 
 trait Monad[T]:
   def id: String
@@ -11,7 +11,7 @@ object Foo {
 
 opaque type Bar = Foo
 object Bar {
-  given Monad[Bar] = summon[Monad[Foo]] // was error fixed by avoidLoopingGivens
+  given Monad[Bar] = summon[Monad[Foo]] // was error, fixed by givenLoopPrevention
 }
 
 object Test extends App {
