Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn given loop prevention on for -source future #19392

Merged
merged 3 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/config/Feature.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
32 changes: 32 additions & 0 deletions docs/_docs/reference/changed-features/implicit-resolution.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

1 change: 0 additions & 1 deletion docs/sidebar.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 0 additions & 9 deletions library/src/scala/runtime/stdLibPatches/language.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down
44 changes: 23 additions & 21 deletions tests/neg/i15474.check
Original file line number Diff line number Diff line change
@@ -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.
21 changes: 11 additions & 10 deletions tests/neg/i6716.check
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 2 additions & 1 deletion tests/neg/i7294-a.check
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 14 additions & 0 deletions tests/pos/given-loop-prevention.scala
Original file line number Diff line number Diff line change
@@ -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 {}
}
2 changes: 1 addition & 1 deletion tests/pos/i15474.scala
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
14 changes: 14 additions & 0 deletions tests/pos/i6716.scala
Original file line number Diff line number Diff line change
@@ -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 {}
}
2 changes: 1 addition & 1 deletion tests/pos/looping-givens.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import language.experimental.givenLoopPrevention
import language.future

class A
class B
Expand Down
6 changes: 2 additions & 4 deletions tests/run/i6716.scala
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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 {
Expand Down
Loading