From e60aa9eeaca4993f2c8dc8b89c76baca40112efe Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 7 Jan 2024 12:39:09 +0100 Subject: [PATCH 1/3] Turn given loop prevention on for -source future Drop the experimental language import. I believe everybody agrees that this is a desirable improvement, and type inference and implicit search have traditionally been in the realm of the compiler implementers. Experimental language imports have to live forever (albeit as deprecated once the feature is accepted as standard). So they are rather heavyweight and it is unergonomic to require them for smallish improvements to type inference. The new road map is as follows: - In 3.4: warning if behavior would change in the future. - In 3.5: error if behavior would change in the future - In 3.future (at the earliest 3.6): new behavior. --- .../src/dotty/tools/dotc/config/Feature.scala | 1 - .../dotty/tools/dotc/typer/Implicits.scala | 5 ++- .../runtime/stdLibPatches/language.scala | 9 ---- tests/neg/i15474.check | 44 ++++++++++--------- tests/neg/i6716.check | 21 ++++----- tests/neg/i7294-a.check | 3 +- tests/pos/i15474.scala | 2 +- tests/pos/looping-givens.scala | 2 +- tests/run/i6716.scala | 6 +-- 9 files changed, 43 insertions(+), 50 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Feature.scala b/compiler/src/dotty/tools/dotc/config/Feature.scala index cdd83b15f4fc..fa262a5880ff 100644 --- a/compiler/src/dotty/tools/dotc/config/Feature.scala +++ b/compiler/src/dotty/tools/dotc/config/Feature.scala @@ -33,7 +33,6 @@ object Feature: val pureFunctions = experimental("pureFunctions") val captureChecking = experimental("captureChecking") val into = experimental("into") - val givenLoopPrevention = experimental("givenLoopPrevention") val globalOnlyImports: Set[TermName] = Set(pureFunctions, captureChecking) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 1672c94fd969..37086cff0b4c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1595,7 +1595,7 @@ trait Implicits: def checkResolutionChange(result: SearchResult) = if (eligible ne preEligible) - && !Feature.enabled(Feature.givenLoopPrevention) + && !sourceVersion.isAtLeast(SourceVersion.future) then val prevResult = searchImplicit(preEligible, contextual) prevResult match @@ -1621,7 +1621,8 @@ trait Implicits: |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.givenLoopPrevention` language import. + |To opt into the new rules, compile with `-source future` or use + |the `scala.language.future` 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, diff --git a/library/src/scala/runtime/stdLibPatches/language.scala b/library/src/scala/runtime/stdLibPatches/language.scala index ae323befa1a4..c37555f26653 100644 --- a/library/src/scala/runtime/stdLibPatches/language.scala +++ b/library/src/scala/runtime/stdLibPatches/language.scala @@ -91,15 +91,6 @@ object language: @compileTimeOnly("`into` can only be used at compile time in import statements") object into - /** Experimental support for new given resolution rules that avoid looping - * 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/given-loop-prevention]] - */ - @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. * @see [[http://dotty.epfl.ch/docs/reference/contextual/extension-methods]] diff --git a/tests/neg/i15474.check b/tests/neg/i15474.check index 6a60aed304aa..3205f703cd50 100644 --- a/tests/neg/i15474.check +++ b/tests/neg/i15474.check @@ -1,29 +1,31 @@ -- Error: tests/neg/i15474.scala:6:39 ---------------------------------------------------------------------------------- 6 | given c: Conversion[ String, Int ] = _.toInt // error | ^ - | 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. + | 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, compile with `-source future` or use + | the `scala.language.future` 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 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. + | 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 | ^ - | 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. + | 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, compile with `-source future` or use + | the `scala.language.future` 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 Prices.Price.given_Ordering_Price comes earlier, - | - use an explicit argument. - | This will be an error in Scala 3.5 and later. + | 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 index e70ac4b15f9c..cdf655710452 100644 --- a/tests/neg/i6716.check +++ b/tests/neg/i6716.check @@ -1,14 +1,15 @@ -- Error: tests/neg/i6716.scala:12:39 ---------------------------------------------------------------------------------- 12 | given Monad[Bar] = summon[Monad[Foo]] // error | ^ - | 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. + | 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, compile with `-source future` or use + | the `scala.language.future` 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 Bar.given_Monad_Bar comes earlier, - | - use an explicit argument. - | This will be an error in Scala 3.5 and later. + | 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 index 6fac76a9faa5..2fe260fcf99a 100644 --- a/tests/neg/i7294-a.check +++ b/tests/neg/i7294-a.check @@ -14,7 +14,8 @@ | Current result foo.Test.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.givenLoopPrevention` language import. + | To opt into the new rules, compile with `-source future` or use + | the `scala.language.future` 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, diff --git a/tests/pos/i15474.scala b/tests/pos/i15474.scala index b006f8b61cf4..f2c85120e4b2 100644 --- 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.givenLoopPrevention +import scala.language.future 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 index 1b620b5c113e..0e615c8251df 100644 --- a/tests/pos/looping-givens.scala +++ b/tests/pos/looping-givens.scala @@ -1,4 +1,4 @@ -import language.experimental.givenLoopPrevention +import language.future class A class B diff --git a/tests/run/i6716.scala b/tests/run/i6716.scala index 4cca37f96a6f..3bef45ac7465 100644 --- a/tests/run/i6716.scala +++ b/tests/run/i6716.scala @@ -1,6 +1,4 @@ -//> using options -Xfatal-warnings - -import scala.language.experimental.givenLoopPrevention +//> using options -Xfatal-warnings -source future trait Monad[T]: def id: String @@ -11,7 +9,7 @@ object Foo { opaque type Bar = Foo object Bar { - given Monad[Bar] = summon[Monad[Foo]] // was error, fixed by givenLoopPrevention + given Monad[Bar] = summon[Monad[Foo]] // was error, fixed by given loop prevention } object Test extends App { From 5eceb3600f84b6f85d8e9f6cc44c7efa9d6ed8a9 Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 7 Jan 2024 18:46:43 +0100 Subject: [PATCH 2/3] Adapt docs --- .../changed-features/implicit-resolution.md | 32 +++++++++++++++++++ docs/sidebar.yml | 1 - 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/docs/_docs/reference/changed-features/implicit-resolution.md b/docs/_docs/reference/changed-features/implicit-resolution.md index 861a63bd4a05..1396ed04b6d3 100644 --- a/docs/_docs/reference/changed-features/implicit-resolution.md +++ b/docs/_docs/reference/changed-features/implicit-resolution.md @@ -164,3 +164,35 @@ 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.** The following change is currently enabled in `-source future`: + +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 currently enabled in `source.future` and will be enabled at the earliest in Scala 3.6. For earlier source versions, the behavior is as +follows: + + - Scala 3.3: no change + - Scala 3.4: A warning is issued where the behavior will change in 3.future. + - Scala 3.5: An error is issued where the behavior will change in 3.future. + +Old-style implicit definitions are unaffected by this change. + diff --git a/docs/sidebar.yml b/docs/sidebar.yml index d9f86d5141c3..65d7ac2f9ee4 100644 --- a/docs/sidebar.yml +++ b/docs/sidebar.yml @@ -153,7 +153,6 @@ 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 From eb3ca86b5954b289aba1e653059898d0b9755018 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 8 Jan 2024 09:38:11 +0100 Subject: [PATCH 3/3] Add test showing use of @nowarn --- tests/pos/given-loop-prevention.scala | 14 ++++++++++++++ tests/pos/i6716.scala | 14 ++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 tests/pos/given-loop-prevention.scala create mode 100644 tests/pos/i6716.scala diff --git a/tests/pos/given-loop-prevention.scala b/tests/pos/given-loop-prevention.scala new file mode 100644 index 000000000000..0bae0bb24fed --- /dev/null +++ b/tests/pos/given-loop-prevention.scala @@ -0,0 +1,14 @@ +//> using options -Xfatal-warnings + +class Foo + +object Bar { + given Foo with {} + given List[Foo] = List(summon[Foo]) // ok +} + +object Baz { + @annotation.nowarn + given List[Foo] = List(summon[Foo]) // gives a warning, which is suppressed + given Foo with {} +} diff --git a/tests/pos/i6716.scala b/tests/pos/i6716.scala new file mode 100644 index 000000000000..f02559af1e82 --- /dev/null +++ b/tests/pos/i6716.scala @@ -0,0 +1,14 @@ +//> using options -Xfatal-warnings -source 3.4 + +class Foo + +object Bar { + given Foo with {} + given List[Foo] = List(summon[Foo]) // ok +} + +object Baz { + @annotation.nowarn + given List[Foo] = List(summon[Foo]) // gives a warning, which is suppressed + given Foo with {} +}