From 632922e323d592637ae4bc876e355db702ed73bd Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 4 Aug 2020 11:13:49 +0200 Subject: [PATCH 1/9] Optimize TermRefSet The overwhelming majority of TermRefSets in implicit search has a single prefix per symbol (in the case of typer.scala: 100%). Optimize for this case. --- .../dotty/tools/dotc/typer/Implicits.scala | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index b8ba3396261c..ce6ffdb4888a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1657,28 +1657,29 @@ final class SearchRoot extends SearchHistory { /** A set of term references where equality is =:= */ sealed class TermRefSet(using Context): - private val elems = new java.util.LinkedHashMap[TermSymbol, List[Type]] + private val elems = new java.util.LinkedHashMap[TermSymbol, Type | List[Type]] def isEmpty = elems.size == 0 - def += (ref: TermRef): Unit = { + def += (ref: TermRef): Unit = val pre = ref.prefix val sym = ref.symbol.asTerm - elems.get(sym) match { + elems.get(sym) match case null => - elems.put(sym, pre :: Nil) - case prefixes => - if (!prefixes.exists(_ =:= pre)) - elems.put(sym, pre :: prefixes) - } - } + elems.put(sym, pre) + case prefix: Type => + if !(prefix =:= pre) then elems.put(sym, pre :: prefix :: Nil) + case prefixes: List[Type] => + if !prefixes.exists(_ =:= pre) then elems.put(sym, pre :: prefixes) def ++= (that: TermRefSet): Unit = that.foreach(+=) def foreach[U](f: TermRef => U): Unit = - elems.forEach((sym: TermSymbol, prefixes: List[Type]) => - prefixes.foreach(pre => f(TermRef(pre, sym)))) + elems.forEach((sym: TermSymbol, prefixes: Type | List[Type]) => + prefixes match + case prefix: Type => f(TermRef(prefix, sym)) + case prefixes: List[Type] => prefixes.foreach(pre => f(TermRef(pre, sym)))) // used only for debugging def toList: List[TermRef] = { From 48415d2e0a913850321e2952d91b3cc6d0fdc084 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 4 Aug 2020 11:14:59 +0200 Subject: [PATCH 2/9] Optimize SearchHistory Avoid a separate list of candidate/type pairs. Instead integrate them as fields of the search history itself. --- .../dotty/tools/dotc/typer/Implicits.scala | 65 ++++++++++--------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index ce6ffdb4888a..adb03c79edc0 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1362,11 +1362,11 @@ trait Implicits { self: Typer => * recursive references and emit a complete implicit dictionary when the outermost search * is complete. */ -abstract class SearchHistory { outer => +abstract class SearchHistory: val root: SearchRoot - val open: List[(Candidate, Type)] /** Does this search history contain any by name implicit arguments. */ val byname: Boolean + def open: List[(Candidate, Type)] /** * Create the state for a nested implicit search. @@ -1374,12 +1374,7 @@ abstract class SearchHistory { outer => * @param pt The target type for the above candidate. * @result The nested history. */ - def nest(cand: Candidate, pt: Type)(using Context): SearchHistory = - new SearchHistory { - val root = outer.root - val open = (cand, pt) :: outer.open - val byname = outer.byname || isByname(pt) - } + def nest(cand: Candidate, pt: Type)(using Context): OpenSearch = OpenSearch(cand, pt, this) def isByname(tp: Type): Boolean = tp.isInstanceOf[ExprType] @@ -1410,22 +1405,25 @@ abstract class SearchHistory { outer => // as we ascend the chain of open implicits to the outermost search context. @tailrec - def loop(ois: List[(Candidate, Type)], belowByname: Boolean): Boolean = - ois match { - case Nil => false - case (hd@(cand1, tp)) :: tl => - if (cand1.ref == cand.ref) { + def loop(history: SearchHistory, belowByname: Boolean): Boolean = + history match + case _: SearchRoot => false + case OpenSearch(cand1, tp, outer) => + if cand1.ref == cand.ref then val wideTp = tp.widenExpr lazy val wildTp = wildApprox(wideTp) lazy val tpSize = wideTp.typeSize - if (belowByname && (wildTp <:< wildPt)) false - else if (tpSize > ptSize || wideTp.coveringSet != ptCoveringSet) loop(tl, isByname(tp) || belowByname) - else tpSize < ptSize || wildTp =:= wildPt || loop(tl, isByname(tp) || belowByname) - } - else loop(tl, isByname(tp) || belowByname) - } + if belowByname && (wildTp <:< wildPt) then + false + else if tpSize > ptSize || wideTp.coveringSet != ptCoveringSet then + loop(outer, isByname(tp) || belowByname) + else + tpSize < ptSize + || wildTp =:= wildPt + || loop(outer, isByname(tp) || belowByname) + else loop(outer, isByname(tp) || belowByname) - loop(open, isByname(pt)) + loop(this, isByname(pt)) } /** @@ -1457,17 +1455,16 @@ abstract class SearchHistory { outer => // argument as we ascend the chain of open implicits to the outermost search // context. @tailrec - def loop(ois: List[(Candidate, Type)], belowByname: Boolean): Type = - ois match { - case (hd@(cand, tp)) :: tl if (belowByname || isByname(tp)) && tp.widenExpr <:< widePt => tp - case (_, tp) :: tl => loop(tl, belowByname || isByname(tp)) + def loop(history: SearchHistory, belowByname: Boolean): Type = + history match + case OpenSearch(cand, tp, outer) => + if (belowByname || isByname(tp)) && tp.widenExpr <:< widePt then tp + else loop(outer, belowByname || isByname(tp)) case _ => NoType - } - loop(open, bynamePt) match { + loop(this, bynamePt) match case NoType => NoType case tp => ctx.searchHistory.linkBynameImplicit(tp.widenExpr) - } } } } @@ -1484,15 +1481,21 @@ abstract class SearchHistory { outer => def emitDictionary(span: Span, result: SearchResult)(using Context): SearchResult = result override def toString: String = s"SearchHistory(open = $open, byname = $byname)" -} +end SearchHistory + +case class OpenSearch(cand: Candidate, pt: Type, outer: SearchHistory) extends SearchHistory: + val root = outer.root + val byname = outer.byname || isByname(pt) + def open = (cand, pt) :: outer.open +end OpenSearch /** * The the state corresponding to the outermost context of an implicit searcch. */ -final class SearchRoot extends SearchHistory { +final class SearchRoot extends SearchHistory: val root = this - val open = Nil val byname = false + def open = Nil /** The dictionary of recursive implicit types and corresponding terms for this search. */ var implicitDictionary0: mutable.Map[Type, (TermRef, tpd.Tree)] = null @@ -1653,7 +1656,7 @@ final class SearchRoot extends SearchHistory { success.copy(tree = blk)(success.tstate, success.gstate) } } -} +end SearchRoot /** A set of term references where equality is =:= */ sealed class TermRefSet(using Context): From 2266d1ddced82e4eb7771348cd78865c4bcef215 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 4 Aug 2020 12:15:25 +0200 Subject: [PATCH 3/9] Refactor divergence checking Move core operations from SearchHistory to ImplicitSearch. This gives more opportunities for sharing and optimizations. --- .../tools/dotc/transform/TypeUtils.scala | 3 + .../dotty/tools/dotc/typer/Implicits.scala | 230 +++++++++--------- 2 files changed, 118 insertions(+), 115 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeUtils.scala b/compiler/src/dotty/tools/dotc/transform/TypeUtils.scala index c282880ca6b6..cd4b9695047a 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeUtils.scala @@ -21,6 +21,9 @@ object TypeUtils { def isPrimitiveValueType(using Context): Boolean = self.classSymbol.isPrimitiveValueClass + def isByName: Boolean = + self.isInstanceOf[ExprType] + def ensureMethodic(using Context): Type = self match { case self: MethodicType => self case _ => if (ctx.erasedTypes) MethodType(Nil, self) else ExprType(self) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index adb03c79edc0..9a1a6d15b22a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -40,7 +40,7 @@ import scala.annotation.internal.sharable import scala.annotation.threadUnsafe /** Implicit resolution */ -object Implicits { +object Implicits: import tpd._ /** An implicit definition `implicitRef` that is visible under a different name, `alias`. @@ -499,7 +499,7 @@ object Implicits { class FailedExtension(extApp: Tree, val expectedType: Type) extends SearchFailureType: def argument = EmptyTree def explanation(using Context) = em"$extApp does not $qualify" -} +end Implicits import Implicits._ @@ -723,7 +723,8 @@ trait ImplicitRunInfo: end ImplicitRunInfo /** The implicit resolution part of type checking */ -trait Implicits { self: Typer => +trait Implicits: + self: Typer => import tpd._ @@ -1082,24 +1083,22 @@ trait Implicits { self: Typer => } /** An implicit search; parameters as in `inferImplicit` */ - class ImplicitSearch(protected val pt: Type, protected val argument: Tree, span: Span)(using Context) { + class ImplicitSearch(protected val pt: Type, protected val argument: Tree, span: Span)(using Context): assert(argument.isEmpty || argument.tpe.isValueType || argument.tpe.isInstanceOf[ExprType], em"found: $argument: ${argument.tpe}, expected: $pt") private def nestedContext() = ctx.fresh.setMode(ctx.mode &~ Mode.ImplicitsEnabled) - private def implicitProto(resultType: Type, f: Type => Type) = - if (argument.isEmpty) f(resultType) else ViewProto(f(argument.tpe.widen), f(resultType)) - // Not clear whether we need to drop the `.widen` here. All tests pass with it in place, though. - private def isCoherent = pt.isRef(defn.EqlClass) - /** The expected type for the searched implicit */ - @threadUnsafe lazy val fullProto: Type = implicitProto(pt, identity) + val wideProto = pt.widenExpr /** The expected type where parameters and uninstantiated typevars are replaced by wildcard types */ - val wildProto: Type = implicitProto(pt, wildApprox(_)) + val wildProto: Type = + if argument.isEmpty then wildApprox(pt) + else ViewProto(wildApprox(argument.tpe.widen), wildApprox(pt)) + // Not clear whether we need to drop the `.widen` here. All tests pass with it in place, though. val isNot: Boolean = wildProto.classSymbol == defn.NotClass @@ -1107,15 +1106,15 @@ trait Implicits { self: Typer => * a diverging search */ def tryImplicit(cand: Candidate, contextual: Boolean): SearchResult = - if (ctx.searchHistory.checkDivergence(cand, pt)) - SearchFailure(new DivergingImplicit(cand.ref, pt.widenExpr, argument)) + if checkDivergence(cand) then + SearchFailure(new DivergingImplicit(cand.ref, wideProto, argument)) else { val history = ctx.searchHistory.nest(cand, pt) val result = typedImplicit(cand, pt, argument, span)(using nestedContext().setNewTyperState().setFreshGADTBounds.setSearchHistory(history)) result match { case res: SearchSuccess => - ctx.searchHistory.defineBynameImplicit(pt.widenExpr, res) + ctx.searchHistory.defineBynameImplicit(wideProto, res) case _ => result } @@ -1306,7 +1305,7 @@ trait Implicits { self: Typer => // effectively in a more inner context than any other definition provided by // explicit definitions. Consequently these terms have the highest priority and no // other candidates need to be considered. - ctx.searchHistory.recursiveRef(pt) match { + recursiveRef(pt) match { case ref: TermRef => SearchSuccess(tpd.ref(ref).withSpan(span.startPos), ref, 0)(ctx.typerState, ctx.gadt) case _ => @@ -1346,8 +1345,101 @@ trait Implicits { self: Typer => case success: SearchSuccess => success.ref } } - } -} + + /** Fields needed for divergence checking */ + @threadUnsafe lazy val ptCoveringSet = wideProto.coveringSet + @threadUnsafe lazy val ptSize = wideProto.typeSize + @threadUnsafe lazy val wildPt = wildApprox(wideProto) + + /** + * Check if the supplied candidate implicit and target type indicate a diverging + * implicit search. + * + * @param cand The candidate implicit to be explored. + * @param pt The target type for the above candidate. + * @result True if this candidate/pt are divergent, false otherwise. + */ + def checkDivergence(cand: Candidate)(using Context): Boolean = + // For full details of the algorithm see the SIP: + // https://docs.scala-lang.org/sips/byname-implicits.html + util.Stats.record("checkDivergence") + + // Unless we are able to tie a recursive knot, we report divergence if there is an + // open implicit using the same candidate implicit definition which has a type which + // is larger (see `typeSize`) and is constructed using the same set of types and type + // constructors (see `coveringSet`). + // + // We are able to tie a recursive knot if there is compatible term already under + // construction which is separated from this context by at least one by name argument + // as we ascend the chain of open implicits to the outermost search context. + + @tailrec + def loop(history: SearchHistory, belowByname: Boolean): Boolean = + history match + case OpenSearch(cand1, tp, outer) => + if cand1.ref eq cand.ref then + util.Stats.record("checkDivergence for sure") + val wideTp = tp.widenExpr + lazy val wildTp = wildApprox(wideTp) + lazy val tpSize = wideTp.typeSize + if belowByname && (wildTp <:< wildPt) then + false + else if tpSize > ptSize || wideTp.coveringSet != ptCoveringSet then + loop(outer, tp.isByName || belowByname) + else + tpSize < ptSize + || wildTp =:= wildPt + || loop(outer, tp.isByName || belowByname) + else loop(outer, tp.isByName || belowByname) + case _ => false + + loop(ctx.searchHistory, pt.isByName) + end checkDivergence + + /** + * Return the reference, if any, to a term under construction or already constructed in + * the current search history corresponding to the supplied target type. + * + * A term is eligible if its type is a subtype of the target type and either it has + * already been constructed and is present in the current implicit dictionary, or it is + * currently under construction and is separated from the current search context by at + * least one by name argument position. + * + * Note that because any suitable term found is defined as part of this search it will + * always be effectively in a more inner context than any other definition provided by + * explicit definitions. Consequently these terms have the highest priority and no other + * candidates need to be considered. + * + * @param pt The target type being searched for. + * @result The corresponding dictionary reference if any, NoType otherwise. + */ + def recursiveRef(pt: Type)(using Context): Type = + val widePt = pt.widenExpr + + ctx.searchHistory.refBynameImplicit(widePt).orElse { + val bynamePt = pt.isByName + if (!ctx.searchHistory.byname && !bynamePt) NoType // No recursion unless at least one open implicit is by name ... + else { + // We are able to tie a recursive knot if there is compatible term already under + // construction which is separated from this context by at least one by name + // argument as we ascend the chain of open implicits to the outermost search + // context. + @tailrec + def loop(history: SearchHistory, belowByname: Boolean): Type = + history match + case OpenSearch(cand, tp, outer) => + if (belowByname || tp.isByName) && tp.widenExpr <:< widePt then tp + else loop(outer, belowByname || tp.isByName) + case _ => NoType + + loop(ctx.searchHistory, bynamePt) match + case NoType => NoType + case tp => ctx.searchHistory.linkBynameImplicit(tp.widenExpr) + } + } + end recursiveRef + end ImplicitSearch +end Implicits /** * Records the history of currently open implicit searches. @@ -1378,97 +1470,6 @@ abstract class SearchHistory: def isByname(tp: Type): Boolean = tp.isInstanceOf[ExprType] - /** - * Check if the supplied candidate implicit and target type indicate a diverging - * implicit search. - * - * @param cand The candidate implicit to be explored. - * @param pt The target type for the above candidate. - * @result True if this candidate/pt are divergent, false otherwise. - */ - def checkDivergence(cand: Candidate, pt: Type)(using Context): Boolean = { - // For full details of the algorithm see the SIP: - // https://docs.scala-lang.org/sips/byname-implicits.html - - val widePt = pt.widenExpr - lazy val ptCoveringSet = widePt.coveringSet - lazy val ptSize = widePt.typeSize - lazy val wildPt = wildApprox(widePt) - - // Unless we are able to tie a recursive knot, we report divergence if there is an - // open implicit using the same candidate implicit definition which has a type which - // is larger (see `typeSize`) and is constructed using the same set of types and type - // constructors (see `coveringSet`). - // - // We are able to tie a recursive knot if there is compatible term already under - // construction which is separated from this context by at least one by name argument - // as we ascend the chain of open implicits to the outermost search context. - - @tailrec - def loop(history: SearchHistory, belowByname: Boolean): Boolean = - history match - case _: SearchRoot => false - case OpenSearch(cand1, tp, outer) => - if cand1.ref == cand.ref then - val wideTp = tp.widenExpr - lazy val wildTp = wildApprox(wideTp) - lazy val tpSize = wideTp.typeSize - if belowByname && (wildTp <:< wildPt) then - false - else if tpSize > ptSize || wideTp.coveringSet != ptCoveringSet then - loop(outer, isByname(tp) || belowByname) - else - tpSize < ptSize - || wildTp =:= wildPt - || loop(outer, isByname(tp) || belowByname) - else loop(outer, isByname(tp) || belowByname) - - loop(this, isByname(pt)) - } - - /** - * Return the reference, if any, to a term under construction or already constructed in - * the current search history corresponding to the supplied target type. - * - * A term is eligible if its type is a subtype of the target type and either it has - * already been constructed and is present in the current implicit dictionary, or it is - * currently under construction and is separated from the current search context by at - * least one by name argument position. - * - * Note that because any suitable term found is defined as part of this search it will - * always be effectively in a more inner context than any other definition provided by - * explicit definitions. Consequently these terms have the highest priority and no other - * candidates need to be considered. - * - * @param pt The target type being searched for. - * @result The corresponding dictionary reference if any, NoType otherwise. - */ - def recursiveRef(pt: Type)(using Context): Type = { - val widePt = pt.widenExpr - - refBynameImplicit(widePt).orElse { - val bynamePt = isByname(pt) - if (!byname && !bynamePt) NoType // No recursion unless at least one open implicit is by name ... - else { - // We are able to tie a recursive knot if there is compatible term already under - // construction which is separated from this context by at least one by name - // argument as we ascend the chain of open implicits to the outermost search - // context. - @tailrec - def loop(history: SearchHistory, belowByname: Boolean): Type = - history match - case OpenSearch(cand, tp, outer) => - if (belowByname || isByname(tp)) && tp.widenExpr <:< widePt then tp - else loop(outer, belowByname || isByname(tp)) - case _ => NoType - - loop(this, bynamePt) match - case NoType => NoType - case tp => ctx.searchHistory.linkBynameImplicit(tp.widenExpr) - } - } - } - // The following are delegated to the root of this search history. def linkBynameImplicit(tpe: Type)(using Context): TermRef = root.linkBynameImplicit(tpe) @@ -1498,12 +1499,11 @@ final class SearchRoot extends SearchHistory: def open = Nil /** The dictionary of recursive implicit types and corresponding terms for this search. */ - var implicitDictionary0: mutable.Map[Type, (TermRef, tpd.Tree)] = null - def implicitDictionary = { - if (implicitDictionary0 == null) - implicitDictionary0 = mutable.Map.empty[Type, (TermRef, tpd.Tree)] - implicitDictionary0 - } + var myImplicitDictionary: mutable.Map[Type, (TermRef, tpd.Tree)] = null + private def implicitDictionary = + if myImplicitDictionary == null then + myImplicitDictionary = mutable.Map.empty[Type, (TermRef, tpd.Tree)] + myImplicitDictionary /** * Link a reference to an under-construction implicit for the provided type to its @@ -1593,7 +1593,7 @@ final class SearchRoot extends SearchHistory: } val pruned = prune(List(tree), implicitDictionary.map(_._2).toList, Nil) - implicitDictionary0 = null + myImplicitDictionary = null if (pruned.isEmpty) result else if (pruned.exists(_._2 == EmptyTree)) NoMatchingImplicitsFailure else { From b8e20de54a66240c3e773d5f2b2240f50e5b8d8d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 4 Aug 2020 12:38:51 +0200 Subject: [PATCH 4/9] Refactor bestImplicit No need to query recursiveRef twice --- .../dotty/tools/dotc/typer/Implicits.scala | 72 +++++++++---------- 1 file changed, 34 insertions(+), 38 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 9a1a6d15b22a..4bf6b1d6adc6 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -975,13 +975,10 @@ trait Implicits: if (argument.isEmpty) i"missing implicit parameter of type $pt after typer" else i"type error: ${argument.tpe} does not conform to $pt${err.whyNoMatchStr(argument.tpe, pt)}") val result0 = - try - new ImplicitSearch(pt, argument, span).bestImplicit(contextual = true) - catch { - case ce: CyclicReference => - ce.inImplicitSearch = true - throw ce - } + try ImplicitSearch(pt, argument, span).bestImplicit + catch case ce: CyclicReference => + ce.inImplicitSearch = true + throw ce val result = result0 match { @@ -1121,7 +1118,7 @@ trait Implicits: } /** Search a list of eligible implicit references */ - def searchImplicits(eligible: List[Candidate], contextual: Boolean): SearchResult = { + private def searchImplicit(eligible: List[Candidate], contextual: Boolean): SearchResult = /** Compare previous success with reference and level to determine which one would be chosen, if * an implicit starting with the reference was found. @@ -1292,11 +1289,33 @@ trait Implicits: } rank(sort(eligible), NoMatchingImplicitsFailure, Nil) - } - // end searchImplicits + end searchImplicit + + private def searchImplicit(contextual: Boolean): SearchResult = + val eligible = + if contextual then ctx.implicits.eligible(wildProto) + else implicitScope(wildProto).eligible + searchImplicit(eligible, contextual) match + case result: SearchSuccess => + result + case failure: SearchFailure => + failure.reason match + case _: AmbiguousImplicits => failure + case reason => + if contextual then + searchImplicit(contextual = false).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 /** Find a unique best implicit reference */ - def bestImplicit(contextual: Boolean): SearchResult = + def bestImplicit: SearchResult = // Before searching for contextual or implicit scope candidates we first check if // there is an under construction or already constructed term with which we can tie // the knot. @@ -1305,35 +1324,12 @@ trait Implicits: // effectively in a more inner context than any other definition provided by // explicit definitions. Consequently these terms have the highest priority and no // other candidates need to be considered. - recursiveRef(pt) match { + recursiveRef match case ref: TermRef => SearchSuccess(tpd.ref(ref).withSpan(span.startPos), ref, 0)(ctx.typerState, ctx.gadt) case _ => - val eligible = - if (contextual) ctx.implicits.eligible(wildProto) - else implicitScope(wildProto).eligible - searchImplicits(eligible, contextual) match { - case result: SearchSuccess => - result - case failure: SearchFailure => - failure.reason match { - case _: AmbiguousImplicits => failure - case reason => - if (contextual) - bestImplicit(contextual = false).recoverWith { - failure2 => failure2.reason match { - case _: AmbiguousImplicits => failure2 - case _ => - reason match { - case (_: DivergingImplicit) => failure - case _ => List(failure, failure2).maxBy(_.tree.treeSize) - } - } - } - else failure - } - } - } + searchImplicit(contextual = true) + end bestImplicit def implicitScope(tp: Type): OfTypeImplicits = ctx.run.implicitScope(tp) @@ -1413,7 +1409,7 @@ trait Implicits: * @param pt The target type being searched for. * @result The corresponding dictionary reference if any, NoType otherwise. */ - def recursiveRef(pt: Type)(using Context): Type = + def recursiveRef(using Context): Type = val widePt = pt.widenExpr ctx.searchHistory.refBynameImplicit(widePt).orElse { From 24f7a9d9704420f5a7f285f4ccfdcd61e6e23072 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 4 Aug 2020 15:39:56 +0200 Subject: [PATCH 5/9] Optimize recursiveRef --- .../dotty/tools/dotc/typer/Implicits.scala | 53 +++++++++---------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 4bf6b1d6adc6..817153015670 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1355,7 +1355,7 @@ trait Implicits: * @param pt The target type for the above candidate. * @result True if this candidate/pt are divergent, false otherwise. */ - def checkDivergence(cand: Candidate)(using Context): Boolean = + def checkDivergence(cand: Candidate): Boolean = // For full details of the algorithm see the SIP: // https://docs.scala-lang.org/sips/byname-implicits.html util.Stats.record("checkDivergence") @@ -1374,7 +1374,6 @@ trait Implicits: history match case OpenSearch(cand1, tp, outer) => if cand1.ref eq cand.ref then - util.Stats.record("checkDivergence for sure") val wideTp = tp.widenExpr lazy val wildTp = wildApprox(wideTp) lazy val tpSize = wideTp.typeSize @@ -1409,30 +1408,28 @@ trait Implicits: * @param pt The target type being searched for. * @result The corresponding dictionary reference if any, NoType otherwise. */ - def recursiveRef(using Context): Type = - val widePt = pt.widenExpr - - ctx.searchHistory.refBynameImplicit(widePt).orElse { - val bynamePt = pt.isByName - if (!ctx.searchHistory.byname && !bynamePt) NoType // No recursion unless at least one open implicit is by name ... - else { - // We are able to tie a recursive knot if there is compatible term already under - // construction which is separated from this context by at least one by name - // argument as we ascend the chain of open implicits to the outermost search - // context. - @tailrec - def loop(history: SearchHistory, belowByname: Boolean): Type = - history match - case OpenSearch(cand, tp, outer) => - if (belowByname || tp.isByName) && tp.widenExpr <:< widePt then tp - else loop(outer, belowByname || tp.isByName) - case _ => NoType - - loop(ctx.searchHistory, bynamePt) match - case NoType => NoType - case tp => ctx.searchHistory.linkBynameImplicit(tp.widenExpr) - } - } + def recursiveRef: Type = + val found = ctx.searchHistory.refBynameImplicit(wideProto) + if found.exists then + found + else if !ctx.searchHistory.byname && !pt.isByName then + NoType // No recursion unless at least one open implicit is by name ... + else + // We are able to tie a recursive knot if there is compatible term already under + // construction which is separated from this context by at least one by name + // argument as we ascend the chain of open implicits to the outermost search + // context. + @tailrec + def loop(history: SearchHistory, belowByname: Boolean): Type = + history match + case OpenSearch(cand, tp, outer) => + if (belowByname || tp.isByName) && tp.widenExpr <:< wideProto then tp + else loop(outer, belowByname || tp.isByName) + case _ => NoType + + loop(ctx.searchHistory, pt.isByName) match + case NoType => NoType + case tp => ctx.searchHistory.linkBynameImplicit(tp.widenExpr) end recursiveRef end ImplicitSearch end Implicits @@ -1529,7 +1526,9 @@ final class SearchRoot extends SearchHistory: * @result The corresponding TermRef, or NoType if none. */ override def refBynameImplicit(tpe: Type)(using Context): Type = - implicitDictionary.get(tpe).map(_._1).getOrElse(NoType) + implicitDictionary.get(tpe) match + case Some((tp, _)) => tp + case None => NoType /** * Define a pending dictionary entry if any. From 7d3adb84613595a46e568dc8e38b02e4c4a6a008 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 4 Aug 2020 15:26:55 +0200 Subject: [PATCH 6/9] Further tweaks to search history checking --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 817153015670..c2db69a7ea14 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1461,8 +1461,6 @@ abstract class SearchHistory: */ def nest(cand: Candidate, pt: Type)(using Context): OpenSearch = OpenSearch(cand, pt, this) - def isByname(tp: Type): Boolean = tp.isInstanceOf[ExprType] - // The following are delegated to the root of this search history. def linkBynameImplicit(tpe: Type)(using Context): TermRef = root.linkBynameImplicit(tpe) @@ -1479,7 +1477,7 @@ end SearchHistory case class OpenSearch(cand: Candidate, pt: Type, outer: SearchHistory) extends SearchHistory: val root = outer.root - val byname = outer.byname || isByname(pt) + val byname = outer.byname || pt.isByName def open = (cand, pt) :: outer.open end OpenSearch @@ -1560,7 +1558,7 @@ final class SearchRoot extends SearchHistory: * substituted with references into the dictionary. */ override def emitDictionary(span: Span, result: SearchResult)(using Context): SearchResult = - if (implicitDictionary == null || implicitDictionary.isEmpty) result + if (myImplicitDictionary == null || implicitDictionary.isEmpty) result else result match { case failure: SearchFailure => failure From cfe45b3f34f96c9e17dacdb7ee64416e4fd8352e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 4 Aug 2020 14:37:11 +0200 Subject: [PATCH 7/9] More optimzations of implicit search --- .../dotty/tools/dotc/typer/Implicits.scala | 105 +++++++++--------- 1 file changed, 51 insertions(+), 54 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index c2db69a7ea14..93ee5165430a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -239,23 +239,20 @@ object Implicits: Nil else val nestedCtx = ctx.fresh.addMode(Mode.TypevarsMissContext) + val candidates = new mutable.ListBuffer[Candidate] + var extensionOnly = true - def matchingCandidate(ref: ImplicitRef, extensionOnly: Boolean): Option[Candidate] = + val tryCandidate = (ref: ImplicitRef) => var ckind = explore(candidateKind(ref.underlyingRef))(using nestedCtx) if extensionOnly then ckind &= Candidate.Extension - if ckind == Candidate.None then None - else Some(new Candidate(ref, ckind, level)) - - val extensionCandidates = - if considerExtension then - companionRefs.toList - .filterConserve(!_.symbol.isOneOf(GivenOrImplicit)) // implicit objects are already in `refs` - .flatMap(matchingCandidate(_, extensionOnly = true)) - else - Nil - val implicitCandidates = - refs.flatMap(matchingCandidate(_, extensionOnly = false)) - extensionCandidates ::: implicitCandidates + if ckind != Candidate.None then + candidates += Candidate(ref, ckind, level) + + if considerExtension then + companionRefs.foreach(tryCandidate) + extensionOnly = false + refs.foreach(tryCandidate) + candidates.toList } } @@ -265,7 +262,7 @@ object Implicits: */ class OfTypeImplicits(tp: Type, override val companionRefs: TermRefSet)(initctx: Context) extends ImplicitRefs(initctx) { assert(initctx.typer != null) - implicits.println(i"implicit scope of type $tp = ${companionRefs.toList}%, %") + implicits.println(i"implicit scope of type $tp = ${companionRefs.showAsList}%, %") @threadUnsafe lazy val refs: List[ImplicitRef] = { val buf = new mutable.ListBuffer[TermRef] for (companion <- companionRefs) buf ++= companion.implicitMembers @@ -274,7 +271,7 @@ object Implicits: /** The candidates that are eligible for expected type `tp` */ @threadUnsafe lazy val eligible: List[Candidate] = - trace(i"eligible($tp), companions = ${companionRefs.toList}%, %", implicitsDetailed, show = true) { + trace(i"eligible($tp), companions = ${companionRefs.showAsList}%, %", implicitsDetailed, show = true) { if (refs.nonEmpty && monitored) record(s"check eligible refs in tpe", refs.length) filterMatching(tp) } @@ -283,7 +280,7 @@ object Implicits: ref.symbol.exists && !ref.symbol.is(Private) override def toString: String = - i"OfTypeImplicits($tp), companions = ${companionRefs.toList}%, %; refs = $refs%, %." + i"OfTypeImplicits($tp), companions = ${companionRefs.showAsList}%, %; refs = $refs%, %." } /** The implicit references coming from the context. @@ -681,41 +678,41 @@ trait ImplicitRunInfo: * - If `T` is some other type, S includes the implicit scopes of all anchors of `T`. */ def implicitScope(tp: Type)(using Context): OfTypeImplicits = - object liftToAnchors extends TypeMap: - override def stopAtStatic = true - private val seen = TypeHashSet() + implicitScopeCache.get(tp) match + case Some(is) => is + case None => + record(i"implicitScope") + val liftToAnchors = new TypeMap: + override def stopAtStatic = true + private val seen = TypeHashSet() + + def applyToUnderlying(t: TypeProxy) = + if seen.contains(t) then + WildcardType + else + seen.addEntry(t) + t.underlying match + case TypeBounds(lo, hi) => + if defn.isBottomTypeAfterErasure(lo) then apply(hi) + else AndType.make(apply(lo), apply(hi)) + case u => apply(u) - def applyToUnderlying(t: TypeProxy) = - if seen.contains(t) then - WildcardType + def apply(t: Type) = t.dealias match + case t: TypeRef => + if t.symbol.isClass || isAnchor(t.symbol) then t else applyToUnderlying(t) + case t: TypeVar => apply(t.underlying) + case t: ParamRef => applyToUnderlying(t) + case t: ConstantType => apply(t.underlying) + case t => mapOver(t) + end liftToAnchors + val liftedTp = liftToAnchors(tp) + if liftedTp eq tp then + record(i"implicitScope unlifted") + computeIScope(tp) else - seen.addEntry(t) - t.underlying match - case TypeBounds(lo, hi) => - if defn.isBottomTypeAfterErasure(lo) then apply(hi) - else AndType.make(apply(lo), apply(hi)) - case u => apply(u) - - def apply(t: Type) = t.dealias match - case t: TypeRef => - if t.symbol.isClass || isAnchor(t.symbol) then t else applyToUnderlying(t) - case t: TypeVar => apply(t.underlying) - case t: ParamRef => applyToUnderlying(t) - case t: ConstantType => apply(t.underlying) - case t => mapOver(t) - end liftToAnchors - - record(i"implicitScope") - implicitScopeCache.getOrElse(tp, { - val liftedTp = liftToAnchors(tp) - if liftedTp eq tp then - record(i"implicitScope unlifted") - computeIScope(tp) - else - record(i"implicitScope lifted") - val liftedIScope = implicitScopeCache.getOrElse(liftedTp, computeIScope(liftedTp)) - OfTypeImplicits(tp, liftedIScope.companionRefs)(runContext) - }) + record(i"implicitScope lifted") + val liftedIScope = implicitScopeCache.getOrElse(liftedTp, computeIScope(liftedTp)) + OfTypeImplicits(tp, liftedIScope.companionRefs)(runContext) end implicitScope protected def reset(): Unit = @@ -1204,7 +1201,7 @@ trait Implicits: else disambiguate(found, best) match { case retained: SearchSuccess => val newPending = - if (retained eq found) remaining + if (retained eq found) || remaining.isEmpty then remaining else remaining.filter(cand => compareCandidate(retained, cand.ref, cand.level) <= 0) rank(newPending, retained, rfailures) @@ -1669,7 +1666,7 @@ sealed class TermRefSet(using Context): if !prefixes.exists(_ =:= pre) then elems.put(sym, pre :: prefixes) def ++= (that: TermRefSet): Unit = - that.foreach(+=) + if !that.isEmpty then that.foreach(+=) def foreach[U](f: TermRef => U): Unit = elems.forEach((sym: TermSymbol, prefixes: Type | List[Type]) => @@ -1678,13 +1675,13 @@ sealed class TermRefSet(using Context): case prefixes: List[Type] => prefixes.foreach(pre => f(TermRef(pre, sym)))) // used only for debugging - def toList: List[TermRef] = { + def showAsList: List[TermRef] = { val buffer = new mutable.ListBuffer[TermRef] foreach(tr => buffer += tr) buffer.toList } - override def toString = toList.toString + override def toString = showAsList.toString object TermRefSet: From 9d158e0bc291908ff853c168c77b9ead75a35c88 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 13 Aug 2020 19:21:41 +0200 Subject: [PATCH 8/9] Fix #9504: Snapshot type size and covering set in search histories The problem with #9504 is that types in the search history grow larger over time, since they contain type variables that get instantiated with new types containing new type variables and so on. That's why divergence checking is fooled into believing that the newly searched type is always smaller than the history. To fix this, we need to take a snapshot of type size and covering set at the time the search history is created. --- .../dotty/tools/dotc/typer/Implicits.scala | 31 ++++++++++++------- tests/neg/i9504.scala | 14 +++++++++ 2 files changed, 33 insertions(+), 12 deletions(-) create mode 100644 tests/neg/i9504.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 93ee5165430a..3d00c149d1cb 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1369,17 +1369,15 @@ trait Implicits: @tailrec def loop(history: SearchHistory, belowByname: Boolean): Boolean = history match - case OpenSearch(cand1, tp, outer) => + case prev @ OpenSearch(cand1, tp, outer) => if cand1.ref eq cand.ref then - val wideTp = tp.widenExpr - lazy val wildTp = wildApprox(wideTp) - lazy val tpSize = wideTp.typeSize + lazy val wildTp = wildApprox(tp.widenExpr) if belowByname && (wildTp <:< wildPt) then false - else if tpSize > ptSize || wideTp.coveringSet != ptCoveringSet then + else if prev.typeSize > ptSize || prev.coveringSet != ptCoveringSet then loop(outer, tp.isByName || belowByname) else - tpSize < ptSize + prev.typeSize < ptSize || wildTp =:= wildPt || loop(outer, tp.isByName || belowByname) else loop(outer, tp.isByName || belowByname) @@ -1434,7 +1432,7 @@ end Implicits /** * Records the history of currently open implicit searches. * - * A search history maintains a list of open implicit searches (`open`) a shortcut flag + * A search history maintains a list of open implicit searches (`openSearchPairs`) a shortcut flag * indicating whether any of these are by name (`byname`) and a reference to the root * search history (`root`) which in turn maintains a possibly empty dictionary of * recursive implicit terms constructed during this search. @@ -1448,7 +1446,7 @@ abstract class SearchHistory: val root: SearchRoot /** Does this search history contain any by name implicit arguments. */ val byname: Boolean - def open: List[(Candidate, Type)] + def openSearchPairs: List[(Candidate, Type)] /** * Create the state for a nested implicit search. @@ -1469,13 +1467,22 @@ abstract class SearchHistory: // This is NOOP unless at the root of this search history. def emitDictionary(span: Span, result: SearchResult)(using Context): SearchResult = result - override def toString: String = s"SearchHistory(open = $open, byname = $byname)" + override def toString: String = s"SearchHistory(open = $openSearchPairs, byname = $byname)" end SearchHistory -case class OpenSearch(cand: Candidate, pt: Type, outer: SearchHistory) extends SearchHistory: +case class OpenSearch(cand: Candidate, pt: Type, outer: SearchHistory)(using Context) extends SearchHistory: val root = outer.root val byname = outer.byname || pt.isByName - def open = (cand, pt) :: outer.open + def openSearchPairs = (cand, pt) :: outer.openSearchPairs + + // The typeSize and coveringSet of the current search. + // Note: It is important to cache size and covering sets since types + // in search histories can contain type variables that can be instantiated + // by nested implicit searches, thus leading to types in search histories + // that grow larger the deeper the search gets. This can mask divergence. + // An example is in neg/9504.scala + lazy val typeSize = pt.typeSize + lazy val coveringSet = pt.coveringSet end OpenSearch /** @@ -1484,7 +1491,7 @@ end OpenSearch final class SearchRoot extends SearchHistory: val root = this val byname = false - def open = Nil + def openSearchPairs = Nil /** The dictionary of recursive implicit types and corresponding terms for this search. */ var myImplicitDictionary: mutable.Map[Type, (TermRef, tpd.Tree)] = null diff --git a/tests/neg/i9504.scala b/tests/neg/i9504.scala new file mode 100644 index 000000000000..f6388e58d1a2 --- /dev/null +++ b/tests/neg/i9504.scala @@ -0,0 +1,14 @@ +trait Monad[F[_]] { + def foo[A](fa: F[A]): Unit = {} +} + +class Bla[F[_], A] + +object Test { + type Id[A] = A + + val bla: Bla[Id, Unit] = ??? + implicit def blaMonad[F[_]: Monad, S]: Monad[({type L[X] = Bla[F, X]})#L] = ??? + + blaMonad.foo(bla) // error: divergence +} \ No newline at end of file From 12fb39b9c1ba28e71d77811db12a5f7dc1670f6e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 14 Aug 2020 18:00:38 +0200 Subject: [PATCH 9/9] Clean up tryCandidate --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 3d00c149d1cb..85bbafdc3426 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -240,18 +240,18 @@ object Implicits: else val nestedCtx = ctx.fresh.addMode(Mode.TypevarsMissContext) val candidates = new mutable.ListBuffer[Candidate] - var extensionOnly = true - - val tryCandidate = (ref: ImplicitRef) => + def tryCandidate(extensionOnly: Boolean)(ref: ImplicitRef) = var ckind = explore(candidateKind(ref.underlyingRef))(using nestedCtx) if extensionOnly then ckind &= Candidate.Extension if ckind != Candidate.None then candidates += Candidate(ref, ckind, level) if considerExtension then - companionRefs.foreach(tryCandidate) - extensionOnly = false - refs.foreach(tryCandidate) + val tryExtension = tryCandidate(extensionOnly = true) + companionRefs.foreach(tryExtension) + if refs.nonEmpty then + val tryGiven = tryCandidate(extensionOnly = false) + refs.foreach(tryGiven) candidates.toList } }