From bc36acbf5fae1aaced587ebbad5543a3e37078c6 Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 17 Dec 2023 13:45:43 +0100 Subject: [PATCH 1/6] Avoid generating given definitions that loop --- .../dotty/tools/dotc/typer/Implicits.scala | 73 ++++++++++++++++--- .../changed-features/implicit-resolution.md | 21 +++++- tests/neg/i15474.check | 6 ++ tests/neg/i15474.scala | 8 +- tests/neg/i6716.check | 6 ++ tests/neg/i6716.scala | 18 +++++ tests/pos/i15474.scala | 20 +++++ tests/pos/i6716.scala | 15 ---- tests/run/i17115.check | 2 + tests/{pos => run}/i17115.scala | 0 tests/run/i6716.check | 2 + tests/run/i6716.scala | 20 +++++ 12 files changed, 161 insertions(+), 30 deletions(-) create mode 100644 tests/neg/i15474.check create mode 100644 tests/neg/i6716.check create mode 100644 tests/neg/i6716.scala create mode 100644 tests/pos/i15474.scala delete mode 100644 tests/pos/i6716.scala create mode 100644 tests/run/i17115.check rename tests/{pos => run}/i17115.scala (100%) create mode 100644 tests/run/i6716.check create mode 100644 tests/run/i6716.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index ff23e8180f1c..bb35306f696c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -26,8 +26,8 @@ import Scopes.newScope import Typer.BindingPrec, BindingPrec.* import Hashable.* import util.{EqHashMap, Stats} -import config.{Config, Feature} -import Feature.migrateTo3 +import config.{Config, Feature, SourceVersion} +import Feature.{migrateTo3, sourceVersion} import config.Printers.{implicits, implicitsDetailed} import collection.mutable import reporting.* @@ -324,7 +324,7 @@ object Implicits: /** Is this the outermost implicits? This is the case if it either the implicits * of NoContext, or the last one before it. */ - private def isOuterMost = { + private def isOutermost = { val finalImplicits = NoContext.implicits (this eq finalImplicits) || (outerImplicits eqn finalImplicits) } @@ -356,7 +356,7 @@ object Implicits: Stats.record("uncached eligible") if monitored then record(s"check uncached eligible refs in irefCtx", refs.length) val ownEligible = filterMatching(tp) - if isOuterMost then ownEligible + if isOutermost then ownEligible else combineEligibles(ownEligible, outerImplicits.nn.uncachedEligible(tp)) /** The implicit references that are eligible for type `tp`. */ @@ -383,7 +383,7 @@ object Implicits: private def computeEligible(tp: Type): List[Candidate] = /*>|>*/ trace(i"computeEligible $tp in $refs%, %", implicitsDetailed) /*<|<*/ { if (monitored) record(s"check eligible refs in irefCtx", refs.length) val ownEligible = filterMatching(tp) - if isOuterMost then ownEligible + if isOutermost then ownEligible else combineEligibles(ownEligible, outerImplicits.nn.eligible(tp)) } @@ -392,7 +392,7 @@ object Implicits: override def toString: String = { val own = i"(implicits: $refs%, %)" - if (isOuterMost) own else own + "\n " + outerImplicits + if (isOutermost) own else own + "\n " + outerImplicits } /** This context, or a copy, ensuring root import from symbol `root` @@ -1550,11 +1550,15 @@ trait Implicits: case _ => tp.isAny || tp.isAnyRef - private def searchImplicit(contextual: Boolean): SearchResult = + /** Search implicit in context `ctxImplicits` or else in implicit scope + * of expected type if `ctxImplicits == null`. + */ + private def searchImplicit(ctxImplicits: ContextualImplicits | Null): SearchResult = if isUnderspecified(wildProto) then SearchFailure(TooUnspecific(pt), span) else - val eligible = + val contextual = ctxImplicits != null + val preEligible = // the eligible candidates, ignoring positions if contextual then if ctx.gadt.isNarrowing then withoutMode(Mode.ImplicitsEnabled) { @@ -1562,6 +1566,43 @@ trait Implicits: } else ctx.implicits.eligible(wildProto) else implicitScope(wildProto).eligible + + /** 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 and 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.is(Package) then false + else candSucceedsGiven(sym.owner) + + ctx.isTyper + && !candSym.isOneOf(TermParamOrAccessor | Synthetic) + && candSym.span.exists + && candSucceedsGiven(ctx.owner) + end comesTooLate + + val eligible = if contextual then preEligible.filterNot(comesTooLate) else preEligible + + def checkResolutionChange(result: SearchResult) = result match + case result: SearchSuccess + if (eligible ne preEligible) && !sourceVersion.isAtLeast(SourceVersion.`future`) => + searchImplicit(preEligible.diff(eligible), contextual) match + case prevResult: SearchSuccess => + report.error( + em"""Warning: result of implicit search for $pt will change. + |current result: ${prevResult.ref.symbol.showLocated} + |result with -source future: ${result.ref.symbol.showLocated}""", + srcPos + ) + case _ => + case _ => + searchImplicit(eligible, contextual) match case result: SearchSuccess => result @@ -1570,14 +1611,24 @@ trait Implicits: case _: AmbiguousImplicits => failure case reason => if contextual then - searchImplicit(contextual = false).recoverWith { + // If we filtered out some candidates for being too late, we should + // do another contextual search further out, since the dropped candidates + // might have shadowed an eligible candidate in an outer level. + // Otherwise, proceed with a search of the implicit scope. + val newCtxImplicits = + if eligible eq preEligible then null + else ctxImplicits.nn.outerImplicits: ContextualImplicits | Null + // !!! Dotty problem: without the ContextualImplicits | Null type ascription + // we get a Ycheck failure after arrayConstructors due to "Types differ" + val result = searchImplicit(newCtxImplicits).recoverWith: failure2 => failure2.reason match case _: AmbiguousImplicits => failure2 case _ => reason match case (_: DivergingImplicit) => failure case _ => List(failure, failure2).maxBy(_.tree.treeSize) - } + checkResolutionChange(result) + result else failure end searchImplicit @@ -1595,7 +1646,7 @@ trait Implicits: case ref: TermRef => SearchSuccess(tpd.ref(ref).withSpan(span.startPos), ref, 0)(ctx.typerState, ctx.gadt) case _ => - searchImplicit(contextual = true) + searchImplicit(ctx.implicits) end bestImplicit def implicitScope(tp: Type): OfTypeImplicits = ctx.run.nn.implicitScope(tp) diff --git a/docs/_docs/reference/changed-features/implicit-resolution.md b/docs/_docs/reference/changed-features/implicit-resolution.md index 6a898690b565..ab8293724a4e 100644 --- a/docs/_docs/reference/changed-features/implicit-resolution.md +++ b/docs/_docs/reference/changed-features/implicit-resolution.md @@ -163,8 +163,27 @@ 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. -[//]: # todo: expand with precise rules diff --git a/tests/neg/i15474.check b/tests/neg/i15474.check new file mode 100644 index 000000000000..267a02a80786 --- /dev/null +++ b/tests/neg/i15474.check @@ -0,0 +1,6 @@ +-- Error: tests/neg/i15474.scala:16:56 --------------------------------------------------------------------------------- +16 | given Ordering[Price] = summon[Ordering[BigDecimal]] // error + | ^ + | Warning: result of implicit search for Ordering[BigDecimal] will change. + | current result: given instance given_Ordering_Price in object Price + | result with -source future: object BigDecimal in object Ordering diff --git a/tests/neg/i15474.scala b/tests/neg/i15474.scala index 8edf97a1e55a..c5cf934bdd7a 100644 --- a/tests/neg/i15474.scala +++ b/tests/neg/i15474.scala @@ -4,10 +4,10 @@ import scala.language.implicitConversions object Test1: given c: Conversion[ String, Int ] with - def apply(from: String): Int = from.toInt // error + def apply(from: String): Int = from.toInt // was error, now avoided object Test2: - given c: Conversion[ String, Int ] = _.toInt // loop not detected, could be used as a fallback to avoid the warning. + given c: Conversion[ String, Int ] = _.toInt // now avoided, was loop not detected, could be used as a fallback to avoid the warning. object Prices { opaque type Price = BigDecimal @@ -15,4 +15,6 @@ object Prices { object Price{ given Ordering[Price] = summon[Ordering[BigDecimal]] // error } -} \ No newline at end of file +} + + diff --git a/tests/neg/i6716.check b/tests/neg/i6716.check new file mode 100644 index 000000000000..1e1359442bec --- /dev/null +++ b/tests/neg/i6716.check @@ -0,0 +1,6 @@ +-- 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: given instance given_Monad_Bar in object Bar + | result with -source future: object given_Monad_Foo in object Foo diff --git a/tests/neg/i6716.scala b/tests/neg/i6716.scala new file mode 100644 index 000000000000..bbbd9d6d6cd0 --- /dev/null +++ b/tests/neg/i6716.scala @@ -0,0 +1,18 @@ +//> using options -Xfatal-warnings + +trait Monad[T]: + def id: String +class Foo +object Foo { + given Monad[Foo] with { def id = "Foo" } +} + +opaque type Bar = Foo +object Bar { + given Monad[Bar] = summon[Monad[Foo]] // error +} + +object Test extends App { + println(summon[Monad[Foo]].id) + println(summon[Monad[Bar]].id) +} \ No newline at end of file diff --git a/tests/pos/i15474.scala b/tests/pos/i15474.scala new file mode 100644 index 000000000000..e40e11d84581 --- /dev/null +++ b/tests/pos/i15474.scala @@ -0,0 +1,20 @@ +//> using options -Xfatal-warnings +import scala.language.implicitConversions +import language.future + +object Test1: + given c: Conversion[ String, Int ] with + def apply(from: String): Int = from.toInt // was error, now avoided + +object Test2: + given c: Conversion[ String, Int ] = _.toInt // now avoided, was loop not detected, could be used as a fallback to avoid the warning. + +object Prices { + opaque type Price = BigDecimal + + object Price{ + given Ordering[Price] = summon[Ordering[BigDecimal]] // was error, now avoided + } +} + + diff --git a/tests/pos/i6716.scala b/tests/pos/i6716.scala deleted file mode 100644 index 446cd49c9214..000000000000 --- a/tests/pos/i6716.scala +++ /dev/null @@ -1,15 +0,0 @@ -trait Monad[T] -class Foo -object Foo { - given Monad[Foo] with {} -} - -opaque type Bar = Foo -object Bar { - given Monad[Bar] = summon[Monad[Foo]] -} - -object Test { - val mf = summon[Monad[Foo]] - val mb = summon[Monad[Bar]] -} \ No newline at end of file diff --git a/tests/run/i17115.check b/tests/run/i17115.check new file mode 100644 index 000000000000..61c83cba41ce --- /dev/null +++ b/tests/run/i17115.check @@ -0,0 +1,2 @@ +4 +5 diff --git a/tests/pos/i17115.scala b/tests/run/i17115.scala similarity index 100% rename from tests/pos/i17115.scala rename to tests/run/i17115.scala diff --git a/tests/run/i6716.check b/tests/run/i6716.check new file mode 100644 index 000000000000..bb85bd267288 --- /dev/null +++ b/tests/run/i6716.check @@ -0,0 +1,2 @@ +Foo +Foo diff --git a/tests/run/i6716.scala b/tests/run/i6716.scala new file mode 100644 index 000000000000..7c4e7fe394d8 --- /dev/null +++ b/tests/run/i6716.scala @@ -0,0 +1,20 @@ +//> using options -Xfatal-warnings + +import language.future + +trait Monad[T]: + def id: String +class Foo +object Foo { + given Monad[Foo] with { def id = "Foo" } +} + +opaque type Bar = Foo +object Bar { + given Monad[Bar] = summon[Monad[Foo]] // error +} + +object Test extends App { + println(summon[Monad[Foo]].id) + println(summon[Monad[Bar]].id) +} \ No newline at end of file From fab71479ae2fa39483d86e3227a408ef5cf52db9 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 18 Dec 2023 14:10:56 +0100 Subject: [PATCH 2/6] Use experimental language import to enable the new behavior --- .../src/dotty/tools/dotc/config/Feature.scala | 1 + .../dotty/tools/dotc/semanticdb/Scala3.scala | 3 ++ .../dotty/tools/dotc/typer/Implicits.scala | 47 ++++++++++++++----- .../quoted/runtime/impl/QuotesImpl.scala | 4 ++ .../runtime/stdLibPatches/language.scala | 9 ++++ tests/neg/i15474.check | 12 +++-- tests/neg/i6716.check | 10 +++- tests/neg/i7294-a.check | 23 +++++++++ tests/neg/i7294-a.scala | 2 +- tests/neg/i7294-b.scala | 2 +- tests/pos/i15474.scala | 2 +- tests/run/i6716.scala | 4 +- 12 files changed, 96 insertions(+), 23 deletions(-) create mode 100644 tests/neg/i7294-a.check diff --git a/compiler/src/dotty/tools/dotc/config/Feature.scala b/compiler/src/dotty/tools/dotc/config/Feature.scala index fa262a5880ff..e8ca30ecb243 100644 --- a/compiler/src/dotty/tools/dotc/config/Feature.scala +++ b/compiler/src/dotty/tools/dotc/config/Feature.scala @@ -33,6 +33,7 @@ object Feature: val pureFunctions = experimental("pureFunctions") val captureChecking = experimental("captureChecking") val into = experimental("into") + val avoidLoopingGivens = experimental("avoidLoopingGivens") val globalOnlyImports: Set[TermName] = Set(pureFunctions, captureChecking) diff --git a/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala b/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala index f49b00089712..fdb9251951e5 100644 --- a/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala +++ b/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala @@ -77,6 +77,9 @@ object Scala3: type SemanticSymbol = Symbol | FakeSymbol given SemanticSymbolOps : AnyRef with + import SymbolOps.* + import StringOps.* + extension (sym: SemanticSymbol) def name(using Context): Name = sym match case s: Symbol => s.name diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index bb35306f696c..7cdc3d4a2508 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1589,19 +1589,40 @@ trait Implicits: val eligible = if contextual then preEligible.filterNot(comesTooLate) else preEligible - def checkResolutionChange(result: SearchResult) = result match - case result: SearchSuccess - if (eligible ne preEligible) && !sourceVersion.isAtLeast(SourceVersion.`future`) => - searchImplicit(preEligible.diff(eligible), contextual) match - case prevResult: SearchSuccess => - report.error( - em"""Warning: result of implicit search for $pt will change. - |current result: ${prevResult.ref.symbol.showLocated} - |result with -source future: ${result.ref.symbol.showLocated}""", - srcPos - ) - case _ => - case _ => + def checkResolutionChange(result: SearchResult) = + if (eligible ne preEligible) + && !Feature.enabled(Feature.avoidLoopingGivens) + then searchImplicit(preEligible.diff(eligible), contextual) match + case prevResult: SearchSuccess => + def remedy = pt match + case _: SelectionProto => + "conversion,\n - use an import to get extension method into scope" + case _: ViewProto => + "conversion" + case _ => + "argument" + + def showResult(r: SearchResult) = r match + case r: SearchSuccess => ctx.printer.toTextRef(r.ref).show + case r => r.show + + result match + case result: SearchSuccess if prevResult.ref frozen_=:= result.ref => + // OK + case _ => + report.error( + em"""Warning: 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 `avoidLoopingGivens` language import, + | + |To fix the problem you could try one of the following: + | - rearrange definitions, + | - use an explicit $remedy.""", + srcPos) + case _ => + end checkResolutionChange searchImplicit(eligible, contextual) match case result: SearchSuccess => diff --git a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala index 51f133c972b4..1203e309c484 100644 --- a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala +++ b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala @@ -1781,6 +1781,8 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler end TypeRepr given TypeReprMethods: TypeReprMethods with + import SymbolMethods.* + extension (self: TypeRepr) def show(using printer: Printer[TypeRepr]): String = printer.show(self) @@ -2608,6 +2610,8 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler end Symbol given SymbolMethods: SymbolMethods with + import FlagsMethods.* + extension (self: Symbol) def owner: Symbol = self.denot.owner def maybeOwner: Symbol = self.denot.maybeOwner diff --git a/library/src/scala/runtime/stdLibPatches/language.scala b/library/src/scala/runtime/stdLibPatches/language.scala index c2a12cec2ecc..9fa8bff120af 100644 --- a/library/src/scala/runtime/stdLibPatches/language.scala +++ b/library/src/scala/runtime/stdLibPatches/language.scala @@ -91,6 +91,15 @@ 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/avoid-looping-givens]] + */ + @compileTimeOnly("`avoidLoopingGivens` can only be used at compile time in import statements") + object avoidLoopingGivens + /** 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 267a02a80786..4bf344dc5a71 100644 --- a/tests/neg/i15474.check +++ b/tests/neg/i15474.check @@ -1,6 +1,12 @@ -- Error: tests/neg/i15474.scala:16:56 --------------------------------------------------------------------------------- 16 | given Ordering[Price] = summon[Ordering[BigDecimal]] // error | ^ - | Warning: result of implicit search for Ordering[BigDecimal] will change. - | current result: given instance given_Ordering_Price in object Price - | result with -source future: object BigDecimal in object Ordering + | 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 `avoidLoopingGivens` language import, + | + | To fix the problem you could try one of the following: + | - rearrange definitions, + | - use an explicit argument. diff --git a/tests/neg/i6716.check b/tests/neg/i6716.check index 1e1359442bec..3746eaafad50 100644 --- a/tests/neg/i6716.check +++ b/tests/neg/i6716.check @@ -2,5 +2,11 @@ 12 | given Monad[Bar] = summon[Monad[Foo]] // error | ^ | Warning: result of implicit search for Monad[Foo] will change. - | current result: given instance given_Monad_Bar in object Bar - | result with -source future: object given_Monad_Foo in object Foo + | 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 `avoidLoopingGivens` language import, + | + | To fix the problem you could try one of the following: + | - rearrange definitions, + | - use an explicit argument. diff --git a/tests/neg/i7294-a.check b/tests/neg/i7294-a.check new file mode 100644 index 000000000000..9541f7979a7a --- /dev/null +++ b/tests/neg/i7294-a.check @@ -0,0 +1,23 @@ +-- Error: tests/neg/i7294-a.scala:6:10 --------------------------------------------------------------------------------- +6 | case x: T => x.g(10) // error // error + | ^ + | Warning: 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 `avoidLoopingGivens` language import, + | + | To fix the problem you could try one of the following: + | - rearrange definitions, + | - use an explicit argument. + | + | where: T is a type in given instance f with bounds <: foo.Foo +-- [E007] Type Mismatch Error: tests/neg/i7294-a.scala:6:15 ------------------------------------------------------------ +6 | case x: T => x.g(10) // error // error + | ^ + | Found: (x : Nothing) + | Required: ?{ g: ? } + | Note that implicit conversions were not tried because the result of an implicit conversion + | must be more specific than ?{ g: [applied to (10) returning T] } + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/i7294-a.scala b/tests/neg/i7294-a.scala index 13981fa4d375..538dc3159fb8 100644 --- a/tests/neg/i7294-a.scala +++ b/tests/neg/i7294-a.scala @@ -3,7 +3,7 @@ package foo trait Foo { def g(x: Int): Any } inline given f[T <: Foo]: T = ??? match { - case x: T => x.g(10) // error + case x: T => x.g(10) // error // error } @main def Test = f diff --git a/tests/neg/i7294-b.scala b/tests/neg/i7294-b.scala index 423d5037db96..b06d814444e8 100644 --- a/tests/neg/i7294-b.scala +++ b/tests/neg/i7294-b.scala @@ -3,7 +3,7 @@ package foo trait Foo { def g(x: Any): Any } inline given f[T <: Foo]: T = ??? match { - case x: T => x.g(10) // error + case x: T => x.g(10) // error // error } @main def Test = f diff --git a/tests/pos/i15474.scala b/tests/pos/i15474.scala index e40e11d84581..8adc5ad7233d 100644 --- a/tests/pos/i15474.scala +++ b/tests/pos/i15474.scala @@ -1,6 +1,6 @@ //> using options -Xfatal-warnings import scala.language.implicitConversions -import language.future +import scala.language.experimental.avoidLoopingGivens object Test1: given c: Conversion[ String, Int ] with diff --git a/tests/run/i6716.scala b/tests/run/i6716.scala index 7c4e7fe394d8..6208a52190fe 100644 --- a/tests/run/i6716.scala +++ b/tests/run/i6716.scala @@ -1,6 +1,6 @@ //> using options -Xfatal-warnings -import language.future +import scala.language.experimental.avoidLoopingGivens trait Monad[T]: def id: String @@ -11,7 +11,7 @@ object Foo { opaque type Bar = Foo object Bar { - given Monad[Bar] = summon[Monad[Foo]] // error + given Monad[Bar] = summon[Monad[Foo]] // was error fixed by avoidLoopingGivens } object Test extends App { From ed9f427c122471156efb680f97935a3d044c5218 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 18 Dec 2023 17:52:30 +0100 Subject: [PATCH 3/6] Make the new behavior dependent on source >= 3.4 --- .../dotty/tools/dotc/typer/Implicits.scala | 91 ++++++++++--------- tests/neg/i15474.check | 42 +++++++-- tests/neg/i15474.scala | 4 +- tests/neg/i6716.check | 16 ++-- tests/neg/i7294-a.check | 18 ++-- tests/neg/looping-givens.scala | 9 ++ tests/pos/looping-givens.scala | 11 +++ 7 files changed, 122 insertions(+), 69 deletions(-) create mode 100644 tests/neg/looping-givens.scala create mode 100644 tests/pos/looping-givens.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 7cdc3d4a2508..77328cdbb33d 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1587,46 +1587,54 @@ trait Implicits: && candSucceedsGiven(ctx.owner) end comesTooLate - val eligible = if contextual then preEligible.filterNot(comesTooLate) else preEligible + val eligible = // the eligible candidates that come before the search point + if contextual && sourceVersion.isAtLeast(SourceVersion.`3.4`) + then preEligible.filterNot(comesTooLate) + else preEligible def checkResolutionChange(result: SearchResult) = if (eligible ne preEligible) && !Feature.enabled(Feature.avoidLoopingGivens) - then searchImplicit(preEligible.diff(eligible), contextual) match - case prevResult: SearchSuccess => - def remedy = pt match - case _: SelectionProto => - "conversion,\n - use an import to get extension method into scope" - case _: ViewProto => - "conversion" - case _ => - "argument" - - def showResult(r: SearchResult) = r match - case r: SearchSuccess => ctx.printer.toTextRef(r.ref).show - case r => r.show - - result match - case result: SearchSuccess if prevResult.ref frozen_=:= result.ref => - // OK - case _ => - report.error( - em"""Warning: 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 `avoidLoopingGivens` language import, - | - |To fix the problem you could try one of the following: - | - rearrange definitions, - | - use an explicit $remedy.""", - srcPos) - case _ => + then + val prevResult = searchImplicit(preEligible, contextual) + prevResult match + case prevResult: SearchSuccess => + def remedy = pt match + case _: SelectionProto => + "conversion,\n - use an import to get extension method into scope" + case _: ViewProto => + "conversion" + case _ => + "argument" + + def showResult(r: SearchResult) = r match + case r: SearchSuccess => ctx.printer.toTextRef(r.ref).show + case r => r.show + + result match + case result: SearchSuccess if prevResult.ref frozen_=:= result.ref => + // OK + case _ => + report.error( + em"""Warning: 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 fix the problem without the language import, you could try one of the following: + | - rearrange definitions so that ${showResult(prevResult)} comes earlier, + | - use an explicit $remedy.""", + srcPos) + case _ => + prevResult + else result end checkResolutionChange - searchImplicit(eligible, contextual) match + val result = searchImplicit(eligible, contextual) + result match case result: SearchSuccess => - result + checkResolutionChange(result) case failure: SearchFailure => failure.reason match case _: AmbiguousImplicits => failure @@ -1641,15 +1649,14 @@ trait Implicits: else ctxImplicits.nn.outerImplicits: ContextualImplicits | Null // !!! Dotty problem: without the ContextualImplicits | Null type ascription // we get a Ycheck failure after arrayConstructors due to "Types differ" - val result = searchImplicit(newCtxImplicits).recoverWith: - failure2 => failure2.reason match - case _: AmbiguousImplicits => failure2 - case _ => - reason match - case (_: DivergingImplicit) => failure - case _ => List(failure, failure2).maxBy(_.tree.treeSize) - checkResolutionChange(result) - result + checkResolutionChange: + searchImplicit(newCtxImplicits).recoverWith: + failure2 => failure2.reason match + case _: AmbiguousImplicits => failure2 + case _ => + reason match + case (_: DivergingImplicit) => failure + case _ => List(failure, failure2).maxBy(_.tree.treeSize) else failure end searchImplicit diff --git a/tests/neg/i15474.check b/tests/neg/i15474.check index 4bf344dc5a71..0b23628d3051 100644 --- a/tests/neg/i15474.check +++ b/tests/neg/i15474.check @@ -1,12 +1,38 @@ +-- Error: tests/neg/i15474.scala:7:35 ---------------------------------------------------------------------------------- +7 | def apply(from: String): Int = from.toInt // error + | ^^^^ + | Warning: result of implicit search for ?{ toInt: ? } will change. + | Current result Test1.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. + | + | To fix the problem without the language import, you could try one of the following: + | - rearrange definitions so that Test1.c comes earlier, + | - use an explicit conversion, + | - use an import to get extension method into scope. +-- Error: tests/neg/i15474.scala:10:39 --------------------------------------------------------------------------------- +10 | 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. + | + | 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. -- Error: tests/neg/i15474.scala:16:56 --------------------------------------------------------------------------------- 16 | 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 `avoidLoopingGivens` language import, + | 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. | - | To fix the problem you could try one of the following: - | - rearrange definitions, - | - use an explicit argument. + | 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. diff --git a/tests/neg/i15474.scala b/tests/neg/i15474.scala index c5cf934bdd7a..5a66ea016630 100644 --- a/tests/neg/i15474.scala +++ b/tests/neg/i15474.scala @@ -4,10 +4,10 @@ import scala.language.implicitConversions object Test1: given c: Conversion[ String, Int ] with - def apply(from: String): Int = from.toInt // was error, now avoided + def apply(from: String): Int = from.toInt // error object Test2: - given c: Conversion[ String, Int ] = _.toInt // now avoided, was loop not detected, could be used as a fallback to avoid the warning. + given c: Conversion[ String, Int ] = _.toInt // error object Prices { opaque type Price = BigDecimal diff --git a/tests/neg/i6716.check b/tests/neg/i6716.check index 3746eaafad50..6771d736b6af 100644 --- a/tests/neg/i6716.check +++ b/tests/neg/i6716.check @@ -1,12 +1,12 @@ -- 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 `avoidLoopingGivens` language import, + | 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. | - | To fix the problem you could try one of the following: - | - rearrange definitions, - | - use an explicit argument. + | 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. diff --git a/tests/neg/i7294-a.check b/tests/neg/i7294-a.check index 9541f7979a7a..887635d89a35 100644 --- a/tests/neg/i7294-a.check +++ b/tests/neg/i7294-a.check @@ -5,19 +5,19 @@ | 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 `avoidLoopingGivens` language import, + | To opt into the new rules, use the `experimental.avoidLoopingGivens` language import. | - | To fix the problem you could try one of the following: - | - rearrange definitions, + | To fix the problem without the language import, you could try one of the following: + | - rearrange definitions so that foo.i7294-a$package.f comes earlier, | - use an explicit argument. | | where: T is a type in given instance f with bounds <: foo.Foo --- [E007] Type Mismatch Error: tests/neg/i7294-a.scala:6:15 ------------------------------------------------------------ +-- [E007] Type Mismatch Error: tests/neg/i7294-a.scala:6:18 ------------------------------------------------------------ 6 | case x: T => x.g(10) // error // error - | ^ - | Found: (x : Nothing) - | Required: ?{ g: ? } - | Note that implicit conversions were not tried because the result of an implicit conversion - | must be more specific than ?{ g: [applied to (10) returning T] } + | ^^^^^^^ + | 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/looping-givens.scala b/tests/neg/looping-givens.scala new file mode 100644 index 000000000000..572f1707861f --- /dev/null +++ b/tests/neg/looping-givens.scala @@ -0,0 +1,9 @@ +class A +class B + +given joint(using a: A, b: B): (A & B) = ??? + +def foo(using a: A, b: B) = + given aa: A = summon // error + given bb: B = summon // error + given ab: (A & B) = summon // error diff --git a/tests/pos/looping-givens.scala b/tests/pos/looping-givens.scala new file mode 100644 index 000000000000..ed11981c1bf6 --- /dev/null +++ b/tests/pos/looping-givens.scala @@ -0,0 +1,11 @@ +import language.experimental.avoidLoopingGivens + +class A +class B + +given joint(using a: A, b: B): (A & B) = ??? + +def foo(using a: A, b: B) = + given aa: A = summon // error + given bb: B = summon // error + given ab: (A & B) = summon // error From f56aa22e915ec43a076773f06b586ab483d917d1 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 18 Dec 2023 21:51:19 +0100 Subject: [PATCH 4/6] Restrict the new rules to givens that don't define a new class `given ... with` or `given ... = new { ... }` kinds of definitions now follow the old rules. This allows recursive `given...with` definitions as they are found in protoQuill. We still have the old check in a later phase against directly recursive methods. Of the three loops in the original i15474 we now detect #2 and #3 with new new restrictions. #1 slips through since it is a loop involving a `given...with` instance of `Conversion`, but is caught later with the recursive method check. Previously tests #1 and #3 were detected with the recursive methods check and #2 slipped through altogether. The new rules are enough for defining simple givens with `=` without fear of looping. --- .../dotty/tools/dotc/semanticdb/Scala3.scala | 3 --- .../dotty/tools/dotc/typer/Implicits.scala | 8 +++--- .../quoted/runtime/impl/QuotesImpl.scala | 4 --- tests/neg/i15474.check | 27 +++++-------------- tests/neg/i15474.scala | 4 --- tests/neg/i15474b.check | 5 ++++ tests/neg/i15474b.scala | 8 ++++++ tests/pos/i15474.scala | 4 --- 8 files changed, 25 insertions(+), 38 deletions(-) create mode 100644 tests/neg/i15474b.check create mode 100644 tests/neg/i15474b.scala diff --git a/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala b/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala index fdb9251951e5..f49b00089712 100644 --- a/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala +++ b/compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala @@ -77,9 +77,6 @@ object Scala3: type SemanticSymbol = Symbol | FakeSymbol given SemanticSymbolOps : AnyRef with - import SymbolOps.* - import StringOps.* - extension (sym: SemanticSymbol) def name(using Context): Name = sym match case s: Symbol => s.name diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 77328cdbb33d..3d3320eb7589 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1569,8 +1569,10 @@ 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 and comes - * later in the source or coincides with that given definition. + * 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. */ def comesTooLate(cand: Candidate): Boolean = val candSym = cand.ref.symbol @@ -1578,7 +1580,7 @@ trait Implicits: 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.is(Package) then false + else if sym.owner.isClass then false else candSucceedsGiven(sym.owner) ctx.isTyper diff --git a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala index 1203e309c484..51f133c972b4 100644 --- a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala +++ b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala @@ -1781,8 +1781,6 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler end TypeRepr given TypeReprMethods: TypeReprMethods with - import SymbolMethods.* - extension (self: TypeRepr) def show(using printer: Printer[TypeRepr]): String = printer.show(self) @@ -2610,8 +2608,6 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler end Symbol given SymbolMethods: SymbolMethods with - import FlagsMethods.* - extension (self: Symbol) def owner: Symbol = self.denot.owner def maybeOwner: Symbol = self.denot.maybeOwner diff --git a/tests/neg/i15474.check b/tests/neg/i15474.check index 0b23628d3051..f99c6778d1ae 100644 --- a/tests/neg/i15474.check +++ b/tests/neg/i15474.check @@ -1,31 +1,18 @@ --- Error: tests/neg/i15474.scala:7:35 ---------------------------------------------------------------------------------- -7 | def apply(from: String): Int = from.toInt // error - | ^^^^ +-- 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 Test1.c will be no longer eligible + | 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. | | To fix the problem without the language import, you could try one of the following: - | - rearrange definitions so that Test1.c comes earlier, + | - rearrange definitions so that Test2.c comes earlier, | - use an explicit conversion, | - use an import to get extension method into scope. --- Error: tests/neg/i15474.scala:10:39 --------------------------------------------------------------------------------- -10 | 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. - | - | 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. --- Error: tests/neg/i15474.scala:16:56 --------------------------------------------------------------------------------- -16 | given Ordering[Price] = summon[Ordering[BigDecimal]] // error +-- 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 diff --git a/tests/neg/i15474.scala b/tests/neg/i15474.scala index 5a66ea016630..b196d1b400ef 100644 --- a/tests/neg/i15474.scala +++ b/tests/neg/i15474.scala @@ -2,10 +2,6 @@ import scala.language.implicitConversions -object Test1: - given c: Conversion[ String, Int ] with - def apply(from: String): Int = from.toInt // error - object Test2: given c: Conversion[ String, Int ] = _.toInt // error diff --git a/tests/neg/i15474b.check b/tests/neg/i15474b.check new file mode 100644 index 000000000000..73ef720af7e3 --- /dev/null +++ b/tests/neg/i15474b.check @@ -0,0 +1,5 @@ +-- Error: tests/neg/i15474b.scala:7:40 --------------------------------------------------------------------------------- +7 | def apply(from: String): Int = from.toInt // error: infinite loop in function body + | ^^^^^^^^^^ + | Infinite loop in function body + | Test1.c.apply(from).toInt diff --git a/tests/neg/i15474b.scala b/tests/neg/i15474b.scala new file mode 100644 index 000000000000..9d496c37ef00 --- /dev/null +++ b/tests/neg/i15474b.scala @@ -0,0 +1,8 @@ +//> using options -Xfatal-warnings + +import scala.language.implicitConversions + +object Test1: + given c: Conversion[ String, Int ] with + def apply(from: String): Int = from.toInt // error: infinite loop in function body + diff --git a/tests/pos/i15474.scala b/tests/pos/i15474.scala index 8adc5ad7233d..6b9e55806ae3 100644 --- a/tests/pos/i15474.scala +++ b/tests/pos/i15474.scala @@ -2,10 +2,6 @@ import scala.language.implicitConversions import scala.language.experimental.avoidLoopingGivens -object Test1: - given c: Conversion[ String, Int ] with - def apply(from: String): Int = from.toInt // was error, now avoided - object Test2: given c: Conversion[ String, Int ] = _.toInt // now avoided, was loop not detected, could be used as a fallback to avoid the warning. From 07c821a1ad5b545fa1f8548ec1ed889fcefc56e9 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 18 Dec 2023 22:58:34 +0100 Subject: [PATCH 5/6] Add tweaks and docs --- .../src/dotty/tools/dotc/config/Feature.scala | 2 +- .../tools/dotc/config/SourceVersion.scala | 1 + .../dotty/tools/dotc/typer/Implicits.scala | 32 ++++++++-------- .../changed-features/implicit-resolution.md | 23 ----------- .../experimental/given-loop-prevention.md | 31 +++++++++++++++ docs/sidebar.yml | 1 + .../runtime/stdLibPatches/language.scala | 6 +-- tests/neg/i15474.check | 38 ++++++++++--------- tests/neg/i6716.check | 18 +++++---- tests/neg/i7294-a.check | 28 +++++++------- tests/neg/i7294-a.scala | 2 + tests/neg/i7294-b.scala | 2 + tests/neg/looping-givens.scala | 2 + tests/pos/i15474.scala | 2 +- tests/pos/looping-givens.scala | 2 +- tests/run/i6716.scala | 4 +- 16 files changed, 110 insertions(+), 84 deletions(-) create mode 100644 docs/_docs/reference/experimental/given-loop-prevention.md diff --git a/compiler/src/dotty/tools/dotc/config/Feature.scala b/compiler/src/dotty/tools/dotc/config/Feature.scala index e8ca30ecb243..cdd83b15f4fc 100644 --- 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 index f6db0bac0452..33b946ed173f 100644 --- 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 index 3d3320eb7589..1672c94fd969 100644 --- 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 index ab8293724a4e..861a63bd4a05 100644 --- 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 new file mode 100644 index 000000000000..e306ba977d45 --- /dev/null +++ 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 index 65d7ac2f9ee4..d9f86d5141c3 100644 --- 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 index 9fa8bff120af..9a2a034c6b7d 100644 --- 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 index f99c6778d1ae..6a60aed304aa 100644 --- 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 index 6771d736b6af..e70ac4b15f9c 100644 --- 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 index 887635d89a35..319ed8e1c9d0 100644 --- 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 index 538dc3159fb8..b0710413eefd 100644 --- 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 index b06d814444e8..8c6f9328cc20 100644 --- 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 index 572f1707861f..357a417f0ed9 100644 --- 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 index 6b9e55806ae3..b006f8b61cf4 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.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 index ed11981c1bf6..1b620b5c113e 100644 --- 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 index 6208a52190fe..4cca37f96a6f 100644 --- 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 { From 87e45fa95a08d0f0a70ab295e12cd066de2cc858 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 19 Dec 2023 13:38:33 +0100 Subject: [PATCH 6/6] Swap two givens in Specs2 to satisfy new restriuction --- community-build/community-projects/specs2 | 2 +- tests/neg/i7294-a.check | 50 +++++++++++------------ tests/neg/i7294-a.scala | 10 +++-- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/community-build/community-projects/specs2 b/community-build/community-projects/specs2 index e42f7987b4ce..ba01cca013d9 160000 --- a/community-build/community-projects/specs2 +++ b/community-build/community-projects/specs2 @@ -1 +1 @@ -Subproject commit e42f7987b4ce30d95fca3f30b9d508021f2fdac7 +Subproject commit ba01cca013d9d99e390d17619664bdedd716e0d7 diff --git a/tests/neg/i7294-a.check b/tests/neg/i7294-a.check index 319ed8e1c9d0..6fac76a9faa5 100644 --- a/tests/neg/i7294-a.check +++ b/tests/neg/i7294-a.check @@ -1,25 +1,25 @@ --- [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 - | ^ - | 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.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:10:20 ----------------------------------------------------------- +10 | 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:10:12 -------------------------------------------------------------------------------- +10 | case x: T => x.g(10) // error // error + | ^ + | Result of implicit search for scala.reflect.TypeTest[Nothing, T] will change. + | 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 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.Test.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 diff --git a/tests/neg/i7294-a.scala b/tests/neg/i7294-a.scala index b0710413eefd..3453e88cf741 100644 --- a/tests/neg/i7294-a.scala +++ b/tests/neg/i7294-a.scala @@ -4,8 +4,10 @@ package foo trait Foo { def g(x: Int): Any } -inline given f[T <: Foo]: T = ??? match { - case x: T => x.g(10) // error // error -} +object Test: -@main def Test = f + inline given f[T <: Foo]: T = ??? match { + case x: T => x.g(10) // error // error + } + + @main def Test = f