Skip to content

Commit 8c76595

Browse files
committed
Take nesting into account when ranking implicits
This will need a spec change. It's necessary in order not to confuse synthetic implicits with each other or with explicit ones in the environment.
1 parent 1b4d574 commit 8c76595

File tree

4 files changed

+59
-31
lines changed

4 files changed

+59
-31
lines changed

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -912,9 +912,21 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
912912
}
913913

914914
/** In a set of overloaded applicable alternatives, is `alt1` at least as good as
915-
* `alt2`? `alt1` and `alt2` are non-overloaded references.
915+
* `alt2`? Also used for implicits disambiguation.
916+
*
917+
* @param alt1, alt2 Non-overloaded references indicating the two choices
918+
* @param level1, level2 If alternatives come from a comparison of two contextual
919+
* implicit candidates, the nesting levels of the candidates.
920+
* In all other cases the nesting levels are both 0.
921+
*
922+
* An alternative A1 is "as good as" an alternative A2 if it wins or draws in a tournament
923+
* that awards one point for each of the following
924+
*
925+
* - A1 is nested more deeply than A2
926+
* - The nesting levels of A1 and A2 are the same, and A1's owner derives from A2's owner
927+
* - A1's type is more specific than A2's type.
916928
*/
917-
def isAsGood(alt1: TermRef, alt2: TermRef)(implicit ctx: Context): Boolean = track("isAsGood") { ctx.traceIndented(i"isAsGood($alt1, $alt2)", overload) {
929+
def isAsGood(alt1: TermRef, alt2: TermRef, nesting1: Int = 0, nesting2: Int = 0)(implicit ctx: Context): Boolean = track("isAsGood") { ctx.traceIndented(i"isAsGood($alt1, $alt2)", overload) {
918930

919931
assert(alt1 ne alt2)
920932

@@ -1029,9 +1041,9 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
10291041
val tp1 = stripImplicit(alt1.widen)
10301042
val tp2 = stripImplicit(alt2.widen)
10311043

1032-
def winsOwner1 = isDerived(owner1, owner2)
1044+
def winsOwner1 = nesting1 > nesting2 || isDerived(owner1, owner2)
10331045
def winsType1 = isAsSpecific(alt1, tp1, alt2, tp2)
1034-
def winsOwner2 = isDerived(owner2, owner1)
1046+
def winsOwner2 = nesting2 > nesting1 || isDerived(owner2, owner1)
10351047
def winsType2 = isAsSpecific(alt2, tp2, alt1, tp1)
10361048

10371049
overload.println(i"isAsGood($alt1, $alt2)? $tp1 $tp2 $winsOwner1 $winsType1 $winsOwner2 $winsType2")

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

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,24 @@ import collection.mutable
3535
/** Implicit resolution */
3636
object Implicits {
3737

38+
/** An eligible implicit candidate, consisting of an implicit reference and a nesting level */
39+
case class Candidate(ref: TermRef, level: Int)
40+
3841
/** A common base class of contextual implicits and of-type implicits which
3942
* represents a set of implicit references.
4043
*/
4144
abstract class ImplicitRefs(initctx: Context) {
4245
implicit val ctx: Context =
4346
if (initctx == NoContext) initctx else initctx retractMode Mode.ImplicitsEnabled
4447

48+
/** The nesting level of this context. Non-zero only in ContextialImplicits */
49+
def level: Int = 0
50+
4551
/** The implicit references */
4652
def refs: List[TermRef]
4753

4854
/** Return those references in `refs` that are compatible with type `pt`. */
49-
protected def filterMatching(pt: Type)(implicit ctx: Context): List[TermRef] = track("filterMatching") {
55+
protected def filterMatching(pt: Type)(implicit ctx: Context): List[Candidate] = track("filterMatching") {
5056

5157
def refMatches(ref: TermRef)(implicit ctx: Context) = /*ctx.traceIndented(i"refMatches $ref $pt")*/ {
5258

@@ -97,8 +103,9 @@ object Implicits {
97103
}
98104
}
99105

100-
if (refs.isEmpty) refs
101-
else refs filter (refMatches(_)(ctx.fresh.addMode(Mode.TypevarsMissContext).setExploreTyperState)) // create a defensive copy of ctx to avoid constraint pollution
106+
if (refs.isEmpty) Nil
107+
else refs.filter(refMatches(_)(ctx.fresh.addMode(Mode.TypevarsMissContext).setExploreTyperState)) // create a defensive copy of ctx to avoid constraint pollution
108+
.map(Candidate(_, level))
102109
}
103110
}
104111

@@ -114,8 +121,8 @@ object Implicits {
114121
buf.toList
115122
}
116123

117-
/** The implicit references that are eligible for expected type `tp` */
118-
lazy val eligible: List[TermRef] =
124+
/** The candidates that are eligible for expected type `tp` */
125+
lazy val eligible: List[Candidate] =
119126
/*>|>*/ track("eligible in tpe") /*<|<*/ {
120127
/*>|>*/ ctx.traceIndented(i"eligible($tp), companions = ${companionRefs.toList}%, %", implicitsDetailed, show = true) /*<|<*/ {
121128
if (refs.nonEmpty && monitored) record(s"check eligible refs in tpe", refs.length)
@@ -135,10 +142,15 @@ object Implicits {
135142
* @param outerCtx the next outer context that makes visible further implicits
136143
*/
137144
class ContextualImplicits(val refs: List[TermRef], val outerImplicits: ContextualImplicits)(initctx: Context) extends ImplicitRefs(initctx) {
138-
private val eligibleCache = new mutable.AnyRefMap[Type, List[TermRef]]
145+
private val eligibleCache = new mutable.AnyRefMap[Type, List[Candidate]]
146+
147+
override val level: Int =
148+
if (outerImplicits == null) 1
149+
else if (ctx.scope eq outerImplicits.ctx.scope) outerImplicits.level
150+
else outerImplicits.level + 1
139151

140152
/** The implicit references that are eligible for type `tp`. */
141-
def eligible(tp: Type): List[TermRef] = /*>|>*/ track(s"eligible in ctx") /*<|<*/ {
153+
def eligible(tp: Type): List[Candidate] = /*>|>*/ track(s"eligible in ctx") /*<|<*/ {
142154
if (tp.hash == NotCached) computeEligible(tp)
143155
else eligibleCache get tp match {
144156
case Some(eligibles) =>
@@ -162,13 +174,13 @@ object Implicits {
162174
}
163175
}
164176

165-
private def computeEligible(tp: Type): List[TermRef] = /*>|>*/ ctx.traceIndented(i"computeEligible $tp in $refs%, %", implicitsDetailed) /*<|<*/ {
177+
private def computeEligible(tp: Type): List[Candidate] = /*>|>*/ ctx.traceIndented(i"computeEligible $tp in $refs%, %", implicitsDetailed) /*<|<*/ {
166178
if (monitored) record(s"check eligible refs in ctx", refs.length)
167179
val ownEligible = filterMatching(tp)
168180
if (outerImplicits == NoContext.implicits) ownEligible
169181
else ownEligible ::: {
170-
val shadowed = (ownEligible map (_.name)).toSet
171-
outerImplicits.eligible(tp) filterNot (ref => shadowed contains ref.name)
182+
val shadowed = ownEligible.map(_.ref.name).toSet
183+
outerImplicits.eligible(tp).filterNot(cand => shadowed.contains(cand.ref.name))
172184
}
173185
}
174186

@@ -198,7 +210,7 @@ object Implicits {
198210
* @param tree The typed tree that needs to be inserted
199211
* @param ctx The context after the implicit search
200212
*/
201-
case class SearchSuccess(tree: tpd.Tree, ref: TermRef, tstate: TyperState) extends SearchResult {
213+
case class SearchSuccess(tree: tpd.Tree, ref: TermRef, level: Int, tstate: TyperState) extends SearchResult {
202214
override def toString = s"SearchSuccess($tree, $ref)"
203215
}
204216

@@ -478,7 +490,7 @@ trait Implicits { self: Typer =>
478490
*/
479491
def inferImplicitArg(formal: Type, error: (String => String) => Unit, pos: Position)(implicit ctx: Context): Tree =
480492
inferImplicit(formal, EmptyTree, pos) match {
481-
case SearchSuccess(arg, _, _) =>
493+
case SearchSuccess(arg, _, _, _) =>
482494
arg
483495
case ambi: AmbiguousImplicits =>
484496
error(where => s"ambiguous implicits: ${ambi.explanation} of $where")
@@ -621,12 +633,13 @@ trait Implicits { self: Typer =>
621633
protected def failedSearch: SearchFailure = NoImplicitMatches
622634

623635
/** Search a list of eligible implicit references */
624-
def searchImplicits(eligible: List[TermRef], contextual: Boolean): SearchResult = {
636+
def searchImplicits(eligible: List[Candidate], contextual: Boolean): SearchResult = {
625637
val constr = ctx.typerState.constraint
626638

627639
/** Try to typecheck an implicit reference */
628-
def typedImplicit(ref: TermRef)(implicit ctx: Context): SearchResult = track("typedImplicit") { ctx.traceIndented(i"typed implicit $ref, pt = $pt, implicitsEnabled == ${ctx.mode is ImplicitsEnabled}", implicits, show = true) {
640+
def typedImplicit(cand: Candidate)(implicit ctx: Context): SearchResult = track("typedImplicit") { ctx.traceIndented(i"typed implicit ${cand.ref}, pt = $pt, implicitsEnabled == ${ctx.mode is ImplicitsEnabled}", implicits, show = true) {
629641
assert(constr eq ctx.typerState.constraint)
642+
val ref = cand.ref
630643
var generated: Tree = tpd.ref(ref).withPos(pos)
631644
if (!argument.isEmpty)
632645
generated = typedUnadapted(
@@ -667,7 +680,7 @@ trait Implicits { self: Typer =>
667680
if fn.symbol == defn.Predef_eqAny && !validEqAnyArgs(arg1.tpe, arg2.tpe) =>
668681
nonMatchingImplicit(ref, Nil)
669682
case _ =>
670-
SearchSuccess(generated1, ref, ctx.typerState)
683+
SearchSuccess(generated1, ref, cand.level, ctx.typerState)
671684
}
672685
}}
673686

@@ -676,19 +689,20 @@ trait Implicits { self: Typer =>
676689
* @param pending The list of implicit references that remain to be investigated
677690
* @param acc An accumulator of successful matches found so far.
678691
*/
679-
def rankImplicits(pending: List[TermRef], acc: List[SearchSuccess]): List[SearchSuccess] = pending match {
680-
case ref :: pending1 =>
692+
def rankImplicits(pending: List[Candidate], acc: List[SearchSuccess]): List[SearchSuccess] = pending match {
693+
case cand :: pending1 =>
681694
val history = ctx.searchHistory nest wildProto
682695
val result =
683-
if (history eq ctx.searchHistory) divergingImplicit(ref)
684-
else typedImplicit(ref)(nestedContext.setNewTyperState.setSearchHistory(history))
696+
if (history eq ctx.searchHistory) divergingImplicit(cand.ref)
697+
else typedImplicit(cand)(nestedContext.setNewTyperState.setSearchHistory(history))
685698
result match {
686699
case fail: SearchFailure =>
687700
rankImplicits(pending1, acc)
688701
case best: SearchSuccess =>
689702
if (ctx.mode.is(Mode.ImplicitExploration)) best :: Nil
690703
else {
691-
val newPending = pending1 filter (isAsGood(_, best.ref)(nestedContext.setExploreTyperState))
704+
val newPending = pending1.filter(cand1 =>
705+
isAsGood(cand1.ref, best.ref, cand1.level, best.level)(nestedContext.setExploreTyperState))
692706
rankImplicits(newPending, best :: acc)
693707
}
694708
}
@@ -717,7 +731,7 @@ trait Implicits { self: Typer =>
717731
/** Convert a (possibly empty) list of search successes into a single search result */
718732
def condense(hits: List[SearchSuccess]): SearchResult = hits match {
719733
case best :: alts =>
720-
alts find (alt => isAsGood(alt.ref, best.ref)(ctx.fresh.setExploreTyperState)) match {
734+
alts find (alt => isAsGood(alt.ref, best.ref, alt.level, best.level)(ctx.fresh.setExploreTyperState)) match {
721735
case Some(alt) =>
722736
/* !!! DEBUG
723737
println(i"ambiguous refs: ${hits map (_.ref) map (_.show) mkString ", "}")
@@ -735,16 +749,18 @@ trait Implicits { self: Typer =>
735749
failedSearch
736750
}
737751

752+
def ranking(cand: Candidate) = -ctx.runInfo.useCount(cand.ref)
753+
738754
/** Sort list of implicit references according to their popularity
739755
* (# of times each was picked in current run).
740756
*/
741-
def sort(eligible: List[TermRef]) = eligible match {
757+
def sort(eligible: List[Candidate]) = eligible match {
742758
case Nil => eligible
743759
case e1 :: Nil => eligible
744760
case e1 :: e2 :: Nil =>
745-
if (ctx.runInfo.useCount(e1) < ctx.runInfo.useCount(e2)) e2 :: e1 :: Nil
761+
if (ranking(e2) < ranking(e1)) e2 :: e1 :: Nil
746762
else eligible
747-
case _ => eligible.sortBy(-ctx.runInfo.useCount(_))
763+
case _ => eligible.sortBy(ranking)
748764
}
749765

750766
condense(rankImplicits(sort(eligible), Nil))

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ class ImportInfo(symf: => Symbol, val selectors: List[untpd.Tree], val isRootImp
9898
*
9999
* TODO: Once we have fully bootstrapped, I would prefer if we expressed
100100
* unimport with an `override` modifier, and generalized it to all imports.
101-
* I believe this would be more transparent than the curren set of conditions. E.g.
101+
* I believe this would be more transparent than the current set of conditions. E.g.
102102
*
103103
* override import Predef.{any2stringAdd => _, StringAdd => _, _} // disables String +
104104
* override import java.lang.{} // disables all imports

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
519519
case tref: TypeRef if !tref.symbol.isClass && !ctx.isAfterTyper =>
520520
inferImplicit(defn.ClassTagType.appliedTo(tref),
521521
EmptyTree, tpt1.pos)(ctx.retractMode(Mode.Pattern)) match {
522-
case SearchSuccess(arg, _, _) =>
522+
case SearchSuccess(arg, _, _, _) =>
523523
return typed(untpd.Apply(untpd.TypedSplice(arg), tree.expr), pt)
524524
case _ =>
525525
}
@@ -1956,7 +1956,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
19561956
}
19571957
// try an implicit conversion
19581958
inferView(tree, pt) match {
1959-
case SearchSuccess(inferred, _, _) =>
1959+
case SearchSuccess(inferred, _, _, _) =>
19601960
adapt(inferred, pt)
19611961
case failure: SearchFailure =>
19621962
if (pt.isInstanceOf[ProtoType] && !failure.isInstanceOf[AmbiguousImplicits]) tree

0 commit comments

Comments
 (0)