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