Skip to content

Commit 84e9a9c

Browse files
authored
Fix algorithm to prevent recursive givens (#19411)
Fixes #19404 Fixes #19407 Fixes #19417
2 parents a954bc7 + 4a97dcd commit 84e9a9c

File tree

5 files changed

+72
-36
lines changed

5 files changed

+72
-36
lines changed

compiler/src/dotty/tools/dotc/typer/Implicits.scala

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ object Implicits:
9393
if (initctx eq NoContext) initctx else initctx.retractMode(Mode.ImplicitsEnabled)
9494
protected given Context = irefCtx
9595

96-
/** The nesting level of this context. Non-zero only in ContextialImplicits */
96+
/** The nesting level of this context. Non-zero only in ContextualImplicits */
9797
def level: Int = 0
9898

9999
/** The implicit references */
@@ -408,6 +408,13 @@ object Implicits:
408408
}
409409
}
410410

411+
/** Search mode to use for possibly avoiding looping givens */
412+
enum SearchMode:
413+
case Old, // up to 3.3, old mode w/o protection
414+
CompareWarn, // from 3.4, old mode, warn if new mode would change result
415+
CompareErr, // from 3.5, old mode, error if new mode would change result
416+
New // from future, new mode where looping givens are avoided
417+
411418
/** The result of an implicit search */
412419
sealed abstract class SearchResult extends Showable {
413420
def tree: Tree
@@ -1553,18 +1560,18 @@ trait Implicits:
15531560
/** Search implicit in context `ctxImplicits` or else in implicit scope
15541561
* of expected type if `ctxImplicits == null`.
15551562
*/
1556-
private def searchImplicit(ctxImplicits: ContextualImplicits | Null): SearchResult =
1563+
private def searchImplicit(ctxImplicits: ContextualImplicits | Null, mode: SearchMode): SearchResult =
15571564
if isUnderspecified(wildProto) then
15581565
SearchFailure(TooUnspecific(pt), span)
15591566
else
15601567
val contextual = ctxImplicits != null
15611568
val preEligible = // the eligible candidates, ignoring positions
1562-
if contextual then
1569+
if ctxImplicits != null then
15631570
if ctx.gadt.isNarrowing then
15641571
withoutMode(Mode.ImplicitsEnabled) {
1565-
ctx.implicits.uncachedEligible(wildProto)
1572+
ctxImplicits.uncachedEligible(wildProto)
15661573
}
1567-
else ctx.implicits.eligible(wildProto)
1574+
else ctxImplicits.eligible(wildProto)
15681575
else implicitScope(wildProto).eligible
15691576

15701577
/** Does candidate `cand` come too late for it to be considered as an
@@ -1589,16 +1596,13 @@ trait Implicits:
15891596
end comesTooLate
15901597

15911598
val eligible = // the eligible candidates that come before the search point
1592-
if contextual && sourceVersion.isAtLeast(SourceVersion.`3.4`)
1599+
if contextual && mode != SearchMode.Old
15931600
then preEligible.filterNot(comesTooLate)
15941601
else preEligible
15951602

15961603
def checkResolutionChange(result: SearchResult) =
1597-
if (eligible ne preEligible)
1598-
&& !sourceVersion.isAtLeast(SourceVersion.future)
1599-
then
1600-
val prevResult = searchImplicit(preEligible, contextual)
1601-
prevResult match
1604+
if (eligible ne preEligible) && mode != SearchMode.New then
1605+
searchImplicit(preEligible, contextual) match
16021606
case prevResult: SearchSuccess =>
16031607
def remedy = pt match
16041608
case _: SelectionProto =>
@@ -1628,41 +1632,38 @@ trait Implicits:
16281632
| - use a `given ... with` clause as the enclosing given,
16291633
| - rearrange definitions so that ${showResult(prevResult)} comes earlier,
16301634
| - use an explicit $remedy."""
1631-
if sourceVersion.isAtLeast(SourceVersion.`3.5`)
1635+
if mode == SearchMode.CompareErr
16321636
then report.error(msg, srcPos)
16331637
else report.warning(msg.append("\nThis will be an error in Scala 3.5 and later."), srcPos)
1634-
case _ =>
1635-
prevResult
1638+
prevResult
1639+
case prevResult: SearchFailure => result
16361640
else result
16371641
end checkResolutionChange
16381642

1639-
val result = searchImplicit(eligible, contextual)
1640-
result match
1641-
case result: SearchSuccess =>
1642-
checkResolutionChange(result)
1643-
case failure: SearchFailure =>
1644-
failure.reason match
1645-
case _: AmbiguousImplicits => failure
1646-
case reason =>
1647-
if contextual then
1648-
// If we filtered out some candidates for being too late, we should
1649-
// do another contextual search further out, since the dropped candidates
1650-
// might have shadowed an eligible candidate in an outer level.
1651-
// Otherwise, proceed with a search of the implicit scope.
1652-
val newCtxImplicits =
1653-
if eligible eq preEligible then null
1654-
else ctxImplicits.nn.outerImplicits: ContextualImplicits | Null
1655-
// !!! Dotty problem: without the ContextualImplicits | Null type ascription
1656-
// we get a Ycheck failure after arrayConstructors due to "Types differ"
1657-
checkResolutionChange:
1658-
searchImplicit(newCtxImplicits).recoverWith:
1643+
checkResolutionChange:
1644+
searchImplicit(eligible, contextual).recoverWith:
1645+
case failure: SearchFailure =>
1646+
failure.reason match
1647+
case _: AmbiguousImplicits => failure
1648+
case reason =>
1649+
if contextual then
1650+
// If we filtered out some candidates for being too late, we should
1651+
// do another contextual search further out, since the dropped candidates
1652+
// might have shadowed an eligible candidate in an outer level.
1653+
// Otherwise, proceed with a search of the implicit scope.
1654+
val newCtxImplicits =
1655+
if eligible eq preEligible then null
1656+
else ctxImplicits.nn.outerImplicits: ContextualImplicits | Null
1657+
// !!! Dotty problem: without the ContextualImplicits | Null type ascription
1658+
// we get a Ycheck failure after arrayConstructors due to "Types differ"
1659+
searchImplicit(newCtxImplicits, SearchMode.New).recoverWith:
16591660
failure2 => failure2.reason match
16601661
case _: AmbiguousImplicits => failure2
16611662
case _ =>
16621663
reason match
16631664
case (_: DivergingImplicit) => failure
16641665
case _ => List(failure, failure2).maxBy(_.tree.treeSize)
1665-
else failure
1666+
else failure
16661667
end searchImplicit
16671668

16681669
/** Find a unique best implicit reference */
@@ -1679,7 +1680,11 @@ trait Implicits:
16791680
case ref: TermRef =>
16801681
SearchSuccess(tpd.ref(ref).withSpan(span.startPos), ref, 0)(ctx.typerState, ctx.gadt)
16811682
case _ =>
1682-
searchImplicit(ctx.implicits)
1683+
searchImplicit(ctx.implicits,
1684+
if sourceVersion.isAtLeast(SourceVersion.future) then SearchMode.New
1685+
else if sourceVersion.isAtLeast(SourceVersion.`3.5`) then SearchMode.CompareErr
1686+
else if sourceVersion.isAtLeast(SourceVersion.`3.4`) then SearchMode.CompareWarn
1687+
else SearchMode.Old)
16831688
end bestImplicit
16841689

16851690
def implicitScope(tp: Type): OfTypeImplicits = ctx.run.nn.implicitScope(tp)

tests/pos/i19404.scala

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
given ipEncoder[IP <: IpAddress]: Encoder[IP] = Encoder[String].contramap(_.toString)
2+
3+
class Encoder[A] {
4+
final def contramap[B](f: B => A): Encoder[B] = new Encoder[B]
5+
}
6+
7+
object Encoder {
8+
final def apply[A](implicit instance: Encoder[A]): Encoder[A] = instance
9+
implicit final val encodeString: Encoder[String] = new Encoder[String]
10+
}
11+
12+
trait Json
13+
trait IpAddress

tests/pos/i19407.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
trait GeneratedEnum
2+
trait Decoder[A]
3+
4+
object Decoder:
5+
given Decoder[Int] = ???
6+
7+
object GeneratedEnumDecoder:
8+
9+
given [A <: GeneratedEnum]: Decoder[A] =
10+
summon[Decoder[Int]]
11+
???

tests/pos/i19417/defs_1.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
trait QueryParamDecoder[E]:
2+
def emap[T](fn: E => Either[Throwable, T]): QueryParamDecoder[T]
3+
object QueryParamDecoder:
4+
def apply[T](implicit ev: QueryParamDecoder[T]): QueryParamDecoder[T] = ev
5+
implicit lazy val stringQueryParamDecoder: QueryParamDecoder[String] = ???

tests/pos/i19417/usage_2.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
given[E](using e: EnumOf[E]): QueryParamDecoder[E] = QueryParamDecoder[String].emap(_ => Right(???))
2+
trait EnumOf[E]

0 commit comments

Comments
 (0)