From fd7ba2b317a501194d152a6e12e80b63b73c0dc1 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 1 Nov 2017 10:56:22 +0100 Subject: [PATCH 01/30] Fix #3396: Abort implicit search if result does not match Don't proceed with implicit search if result type cannot match - the search will likely by under-constrained, which means that an unbounded number of alternatives is tried. See strawman-contrib MapDecoratorTest.scala for an example where this happens. --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index b75a8cc75949..d2b4392698f1 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2150,13 +2150,19 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit def adaptNoArgs(wtp: Type): Tree = { val ptNorm = underlyingApplied(pt) - val functionExpected = defn.isFunctionType(ptNorm) + lazy val functionExpected = defn.isFunctionType(ptNorm) + lazy val resultMatch = constrainResult(wtp, followAlias(pt)) wtp match { case wtp: ExprType => adaptInterpolated(tree.withType(wtp.resultType), pt) - case wtp: MethodType - if wtp.isImplicitMethod && (constrainResult(wtp, followAlias(pt)) || !functionExpected) => - adaptNoArgsImplicitMethod(wtp) + case wtp: MethodType if wtp.isImplicitMethod && (resultMatch || !functionExpected) => + if (resultMatch || ctx.mode.is(Mode.ImplicitsEnabled)) adaptNoArgsImplicitMethod(wtp) + else { + // Don't proceed with implicit search if result type cannot match - the search + // will likely by under-constrained, which means that an unbounded number of alternatives + // is tried. See strawman-contrib MapDecoratorTest.scala for an example where this happens. + err.typeMismatch(tree, pt) + } case wtp: MethodType if !pt.isInstanceOf[SingletonType] => val arity = if (functionExpected) From fd70c616537ec937dad598ff9b3363df9075fb93 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 1 Nov 2017 13:24:57 +0100 Subject: [PATCH 02/30] Fix typo in comment --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index d2b4392698f1..66fc3cf96783 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2159,7 +2159,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit if (resultMatch || ctx.mode.is(Mode.ImplicitsEnabled)) adaptNoArgsImplicitMethod(wtp) else { // Don't proceed with implicit search if result type cannot match - the search - // will likely by under-constrained, which means that an unbounded number of alternatives + // will likely be under-constrained, which means that an unbounded number of alternatives // is tried. See strawman-contrib MapDecoratorTest.scala for an example where this happens. err.typeMismatch(tree, pt) } From 2d4efa0b465300da4720f7f0e4fd77d6a46cd7f6 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 3 Nov 2017 17:39:56 +0100 Subject: [PATCH 03/30] Reflect nesting of lazy_implicit values in their levels This caused nested ambiguity errors with the next commit. --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 657f10564787..5be72776ddde 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -15,6 +15,7 @@ import TypeErasure.{erasure, hasStableErasure} import Mode.ImplicitsEnabled import Denotations._ import NameOps._ +import NameKinds.LazyImplicitName import SymDenotations._ import Symbols._ import Types._ @@ -185,7 +186,8 @@ object Implicits { if (outerImplicits == null) 1 else if (ctx.scala2Mode || (ctx.owner eq outerImplicits.ctx.owner) && - (ctx.scope eq outerImplicits.ctx.scope)) outerImplicits.level + (ctx.scope eq outerImplicits.ctx.scope) && + !refs.head.name.is(LazyImplicitName)) outerImplicits.level else outerImplicits.level + 1 /** Is this the outermost implicits? This is the case if it either the implicits From d7c86040ab7c3b607ece7b650e625011c3956e42 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 3 Nov 2017 18:47:13 +0100 Subject: [PATCH 04/30] Propagate implicit ambiguity and divergence failures Propagate implicit ambiguity and divergence failures from implicit arguments to their callers. In particular, and ambiguity in an implicit argument means that the whole implicit search is ambiguous. Previously, the local ambiguity translated to an "implicit not found" on the next outer level, which meant that another implicit on that level could qualify and the ambiguity would be forgotten. The code is still a bit rough and could profit from some refactorings. --- .../tools/dotc/printing/PlainPrinter.scala | 2 +- .../src/dotty/tools/dotc/typer/Dynamic.scala | 3 +- .../dotty/tools/dotc/typer/Implicits.scala | 153 +++++++++++------- .../src/dotty/tools/dotc/typer/Typer.scala | 66 +++++--- tests/run/iterator-from.scala | 6 +- 5 files changed, 147 insertions(+), 83 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index fc291a7b5620..49743176470f 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -503,7 +503,7 @@ class PlainPrinter(_ctx: Context) extends Printer { "SearchSuccess: " ~ toText(result.ref) ~ " via " ~ toText(result.tree) case _: NonMatchingImplicit | NoImplicitMatches => "NoImplicitMatches" - case _: DivergingImplicit | DivergingImplicit => + case _: DivergingImplicit => "Diverging Implicit" case result: ShadowedImplicit => "Shadowed Implicit" diff --git a/compiler/src/dotty/tools/dotc/typer/Dynamic.scala b/compiler/src/dotty/tools/dotc/typer/Dynamic.scala index 1d6643763365..ee5e9aab7195 100644 --- a/compiler/src/dotty/tools/dotc/typer/Dynamic.scala +++ b/compiler/src/dotty/tools/dotc/typer/Dynamic.scala @@ -150,9 +150,8 @@ trait Dynamic { self: Typer with Applications => fail(i"""takes too many parameters. |Structural types only support methods taking up to ${Definitions.MaxStructuralMethodArity} arguments""") else { - def issueError(msgFn: String => String): Unit = ctx.error(msgFn(""), tree.pos) val ctags = tpe.paramInfos.map(pt => - inferImplicitArg(defn.ClassTagType.appliedTo(pt :: Nil), issueError, tree.pos.endPos)) + implicitArgTree(defn.ClassTagType.appliedTo(pt :: Nil), tree.pos.endPos)) structuralCall(nme.selectDynamicMethod, ctags).asInstance(tpe.toFunctionType()) } case tpe: ValueType => diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 5be72776ddde..1664efa5272b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -278,7 +278,11 @@ object Implicits { /** A "no matching implicit found" failure */ case object NoImplicitMatches extends SearchFailure - case object DivergingImplicit extends SearchFailure + /** A tree representing a failed search for an implicit argument */ + case class FailedSearch(failure: SearchFailure, errorFn: String => String) extends WithoutTypeOrPos[Type] { + override def tag = -1 + override def isEmpty = true + } /** A search failure that can show information about the cause */ abstract class ExplainedSearchFailure extends SearchFailure { @@ -552,7 +556,7 @@ trait Implicits { self: Typer => * which is itself parameterized by another string, * indicating where the implicit parameter is needed */ - def inferImplicitArg(formal: Type, error: (String => String) => Unit, pos: Position)(implicit ctx: Context): Tree = { + def inferImplicitArg(formal: Type, pos: Position)(implicit ctx: Context): Tree = { /** If `formal` is of the form ClassTag[T], where `T` is a class type, * synthesize a class tag for `T`. @@ -562,16 +566,18 @@ trait Implicits { self: Typer => case arg :: Nil => fullyDefinedType(arg, "ClassTag argument", pos) match { case defn.ArrayOf(elemTp) => - val etag = inferImplicitArg(defn.ClassTagType.appliedTo(elemTp), error, pos) + val etag = inferImplicitArg(defn.ClassTagType.appliedTo(elemTp), pos) if (etag.isEmpty) etag else etag.select(nme.wrap) case tp if hasStableErasure(tp) => if (defn.isBottomClass(tp.typeSymbol)) - error(where => i"attempt to take ClassTag of undetermined type for $where") - ref(defn.ClassTagModule) - .select(nme.apply) - .appliedToType(tp) - .appliedTo(clsOf(erasure(tp))) - .withPos(pos) + FailedSearch(NoImplicitMatches, + where => i"attempt to take ClassTag of undetermined type for $where") + else + ref(defn.ClassTagModule) + .select(nme.apply) + .appliedToType(tp) + .appliedTo(clsOf(erasure(tp))) + .withPos(pos) case tp => EmptyTree } @@ -635,17 +641,16 @@ trait Implicits { self: Typer => else arg case ambi: AmbiguousImplicits => - error(where => s"ambiguous implicits: ${ambi.explanation} of $where") - EmptyTree + FailedSearch(ambi, where => s"ambiguous implicits: ${ambi.explanation} of $where") case failure: SearchFailure => - val arg = + val fallbackArg = if (formalValue.isRef(defn.ClassTagClass)) synthesizedClassTag(formalValue) else if (formalValue.isRef(defn.EqClass)) synthesizedEq(formalValue) else EmptyTree - if (!arg.isEmpty) arg + if (!fallbackArg.isEmpty) fallbackArg else { var msgFn = (where: String) => em"no implicit argument of type $formal found for $where" + failure.postscript @@ -659,12 +664,21 @@ trait Implicits { self: Typer => formalValue.typeSymbol.typeParams.map(_.name.unexpandedName.toString), formalValue.argInfos) } - error(msgFn) - EmptyTree + FailedSearch(failure, msgFn) } } } + /** Search an implicit argument and report error if not found */ + def implicitArgTree(formal: Type, pos: Position, where: String = "")(implicit ctx: Context): Tree = + inferImplicitArg(formal, pos) match { + case FailedSearch(fail, errorFn) => + ctx.error(errorFn(where), pos) + EmptyTree + case tree => + tree + } + private def assumedCanEqual(ltp: Type, rtp: Type)(implicit ctx: Context) = { def eqNullable: Boolean = { val other = @@ -694,8 +708,7 @@ trait Implicits { self: Typer => /** Check that equality tests between types `ltp` and `rtp` make sense */ def checkCanEqual(ltp: Type, rtp: Type, pos: Position)(implicit ctx: Context): Unit = if (!ctx.isAfterTyper && !assumedCanEqual(ltp, rtp)) { - val res = inferImplicitArg( - defn.EqType.appliedTo(ltp, rtp), msgFun => ctx.error(msgFun(""), pos), pos) + val res = implicitArgTree(defn.EqType.appliedTo(ltp, rtp), pos) implicits.println(i"Eq witness found for $ltp / $rtp: $res: ${res.tpe}") } @@ -771,7 +784,10 @@ trait Implicits { self: Typer => /** Search failures; overridden in ExplainedImplicitSearch */ protected def nonMatchingImplicit(ref: TermRef, trail: List[MessageContainer]): SearchFailure = NoImplicitMatches - protected def divergingImplicit(ref: TermRef): SearchFailure = NoImplicitMatches + protected def divergingImplicit(ref: TermRef): SearchFailure = { + implicits.println(i"diverging implicit: $ref for $pt, search history = ${ctx.searchHistory}") + new DivergingImplicit(ref, pt, argument) + } protected def shadowedImplicit(ref: TermRef, shadowing: Type): SearchFailure = NoImplicitMatches protected def failedSearch: SearchFailure = NoImplicitMatches @@ -802,11 +818,16 @@ trait Implicits { self: Typer => } } - if (ctx.reporter.hasErrors) - nonMatchingImplicit(ref, ctx.reporter.removeBufferedMessages) + if (ctx.reporter.hasErrors) { + val trail = ctx.reporter.removeBufferedMessages + generated1 match { + case failed: FailedSearch => failed.failure + case _ => nonMatchingImplicit(ref, trail) + } + } else if (contextual && !ctx.mode.is(Mode.ImplicitShadowing) && !shadowing.tpe.isError && !refSameAs(shadowing)) { - implicits.println(i"SHADOWING $ref in ${ref.termSymbol.owner} is shadowed by $shadowing in ${shadowing.symbol.owner}") + implicits.println(i"SHADOWING $ref in ${ref.termSymbol.maybeOwner} is shadowed by $shadowing in ${shadowing.symbol.maybeOwner}") shadowedImplicit(ref, methPart(shadowing).tpe) } else @@ -815,28 +836,38 @@ trait Implicits { self: Typer => /** Given a list of implicit references, produce a list of all implicit search successes, * where the first is supposed to be the best one. + * Except if one of the results is a diverging implicit, produce a list consisting + * of just that result. * @param pending The list of implicit references that remain to be investigated * @param acc An accumulator of successful matches found so far. */ - def rankImplicits(pending: List[Candidate], acc: List[SearchSuccess]): List[SearchSuccess] = pending match { - case cand :: pending1 => - val history = ctx.searchHistory nest wildProto - val result = - if (history eq ctx.searchHistory) divergingImplicit(cand.ref) - else typedImplicit(cand)(nestedContext.setNewTyperState().setSearchHistory(history)) - result match { - case fail: SearchFailure => - rankImplicits(pending1, acc) - case best: SearchSuccess => - if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) best :: Nil - else { - val newPending = pending1.filter(cand1 => - ctx.typerState.test(isAsGood(cand1.ref, best.ref, cand1.level, best.level)(nestedContext))) - rankImplicits(newPending, best :: acc) - } - } - case nil => acc - } + def rankImplicits(pending: List[Candidate], + successes: List[SearchSuccess], + failures: mutable.ListBuffer[SearchFailure]): (List[SearchSuccess], List[SearchFailure]) = + pending match { + case cand :: pending1 => + val history = ctx.searchHistory nest wildProto + val result = + if (history eq ctx.searchHistory) divergingImplicit(cand.ref) + else typedImplicit(cand)(nestedContext.setNewTyperState().setSearchHistory(history)) + result match { + case fail: AmbiguousImplicits => + (Nil, fail :: Nil) + case fail: SearchFailure => + rankImplicits(pending1, successes, + if (fail.isInstanceOf[DivergingImplicit]) fail +=: failures + else failures += fail) + case best: SearchSuccess => + if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) + (best :: Nil, failures.toList) + else { + val newPending = pending1.filter(cand1 => + ctx.typerState.test(isAsGood(cand1.ref, best.ref, cand1.level, best.level)(nestedContext))) + rankImplicits(newPending, best :: successes, failures) + } + } + case nil => (successes, failures.toList) + } /** If the (result types of) the expected type, and both alternatives * are all numeric value types, return the alternative which has @@ -858,26 +889,28 @@ trait Implicits { self: Typer => } /** Convert a (possibly empty) list of search successes into a single search result */ - def condense(hits: List[SearchSuccess]): SearchResult = hits match { - case best :: alts => + def condense(successes: List[SearchSuccess], failures: List[SearchFailure]): SearchResult = successes match { + case (best: SearchSuccess) :: (alts: List[SearchSuccess] @ unchecked) => alts.find(alt => ctx.typerState.test(isAsGood(alt.ref, best.ref, alt.level, best.level))) match { - case Some(alt) => - typr.println(i"ambiguous implicits for $pt: ${best.ref} @ ${best.level}, ${alt.ref} @ ${alt.level}") - /* !!! DEBUG - println(i"ambiguous refs: ${hits map (_.ref) map (_.show) mkString ", "}") - isAsGood(best.ref, alt.ref, explain = true)(ctx.fresh.withExploreTyperState) - */ - numericValueTieBreak(best, alt) match { - case eliminated: SearchSuccess => condense(hits.filter(_ ne eliminated)) - case _ => new AmbiguousImplicits(best.ref, alt.ref, pt, argument) - } - case None => - ctx.runInfo.useCount(best.ref) += 1 - best - } + case Some(alt) => + implicits.println(i"ambiguous implicits for $pt: ${best.ref} @ ${best.level}, ${alt.ref} @ ${alt.level}") + /* !!! DEBUG + println(i"ambiguous refs: ${hits map (_.ref) map (_.show) mkString ", "}") + isAsGood(best.ref, alt.ref, explain = true)(ctx.fresh.withExploreTyperState) + */ + numericValueTieBreak(best, alt) match { + case eliminated: SearchSuccess => + condense(successes.filter(_ ne eliminated), failures) + case _ => + new AmbiguousImplicits(best.ref, alt.ref, pt, argument) + } + case None => + ctx.runInfo.useCount(best.ref) += 1 + best + } case Nil => - failedSearch + failures.headOption.getOrElse(NoImplicitMatches) } def ranking(cand: Candidate) = -ctx.runInfo.useCount(cand.ref) @@ -894,7 +927,8 @@ trait Implicits { self: Typer => case _ => eligible.sortBy(ranking) } - condense(rankImplicits(sort(eligible), Nil)) + val (successes, failures) = rankImplicits(sort(eligible), Nil, new mutable.ListBuffer) + condense(successes, failures) } /** Find a unique best implicit reference */ @@ -905,6 +939,7 @@ trait Implicits { self: Typer => searchImplicits(eligible, contextual) match { case result: SearchSuccess => result case result: AmbiguousImplicits => result + case result: DivergingImplicit => result case result: SearchFailure => if (contextual) bestImplicit(contextual = false) else result } @@ -994,6 +1029,8 @@ class SearchHistory(val searchDepth: Int, val seen: Map[ClassSymbol, Int]) { else updateMap(proto.classSymbols, seen) } } + + override def toString = s"SearchHistory(depth = $searchDepth, seen = $seen)" } /** A set of term references where equality is =:= */ diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 66fc3cf96783..730b91be1c21 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2001,35 +2001,60 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit wtp.paramInfos.foreach(instantiateSelected(_, tvarsToInstantiate)) val constr = ctx.typerState.constraint def addImplicitArgs(implicit ctx: Context) = { - val errors = new mutable.ListBuffer[() => String] - def implicitArgError(msg: => String) = { - errors += (() => msg) - EmptyTree - } - def issueErrors() = { - for (err <- errors) ctx.error(err(), tree.pos.endPos) - tree.withType(wtp.resultType) - } - val args = (wtp.paramNames, wtp.paramInfos).zipped map { (pname, formal) => - def implicitArgError(msg: String => String) = - errors += (() => msg(em"parameter $pname of $methodStr")) - if (errors.nonEmpty) EmptyTree - else inferImplicitArg(formal, implicitArgError, tree.pos.endPos) + def implicitArgs(formals: List[Type]): List[Tree] = formals match { + case Nil => Nil + case formal :: formals1 => + inferImplicitArg(formal, tree.pos.endPos) match { + case arg: FailedSearch + if !arg.failure.isInstanceOf[AmbiguousImplicits] && !tree.symbol.hasDefaultParams => + // no need to search further, the adapt fails in any case + // the reason why we continue inferring in case of an AmbiguousImplicits + // is that we need to know whether there are further errors. + // If there are none, we have to propagate the ambiguity to the caller. + arg :: Nil + case arg => + arg :: implicitArgs(formals1) + } } - if (errors.nonEmpty) { + val args = implicitArgs(wtp.paramInfos) + + val failedArgs = new mutable.ListBuffer[Tree] + val ambiguousArgs = new mutable.ListBuffer[Tree] + for (arg @ FailedSearch(failure, _) <- args) + (if (failure.isInstanceOf[AmbiguousImplicits]) ambiguousArgs else failedArgs) += arg + + if (ambiguousArgs.isEmpty && failedArgs.isEmpty) + adapt(tpd.Apply(tree, args), pt) + else { // If there are several arguments, some arguments might already // have influenced the context, binding variables, but later ones // might fail. In that case the constraint needs to be reset. ctx.typerState.constraint = constr + def issueErrors() = { + def recur(paramNames: List[TermName], remainingArgs: List[Tree]): Tree = remainingArgs match { + case Nil => + tree.withType(wtp.resultType) + case (arg @ FailedSearch(failure, msgFn)) :: args1 => + val msg = msgFn(em"parameter ${paramNames.head} of $methodStr") + def closedArg = FailedSearch(failure, _ => msg) + ctx.error(msg, tree.pos.endPos) + failure match { + case _: AmbiguousImplicits if failedArgs.isEmpty => closedArg // propagate + case _: DivergingImplicit => closedArg // propagate + case _ => tree.withType(wtp.resultType) // show error at enclosing level instead + } + case arg :: args1 => + recur(paramNames.tail, args1) + } + recur(wtp.paramNames, args) + } + // If method has default params, fall back to regular application // where all inferred implicits are passed as named args. - if (tree.symbol.hasDefaultParams) { + if (failedArgs.nonEmpty && tree.symbol.hasDefaultParams) { val namedArgs = (wtp.paramNames, args).zipped.flatMap { (pname, arg) => - arg match { - case EmptyTree => Nil - case _ => untpd.NamedArg(pname, untpd.TypedSplice(arg)) :: Nil - } + if (arg.isEmpty) Nil else untpd.NamedArg(pname, untpd.TypedSplice(arg)) :: Nil } tryEither { implicit ctx => typed(untpd.Apply(untpd.TypedSplice(tree), namedArgs), pt) @@ -2038,7 +2063,6 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit } } else issueErrors() } - else adapt(tpd.Apply(tree, args), pt) } addImplicitArgs(argCtx(tree)) } diff --git a/tests/run/iterator-from.scala b/tests/run/iterator-from.scala index c7c0f9809cc5..7194a34c9e96 100644 --- a/tests/run/iterator-from.scala +++ b/tests/run/iterator-from.scala @@ -63,7 +63,11 @@ object Test extends dotty.runtime.LegacyApp { testSet(immutable.TreeSet(keys:_*), keys) testSet(mutable.TreeSet(keys:_*), keys) val days = keys map {n => Weekday(n % Weekday.values.size)} - testSet(Weekday.ValueSet(days:_*), days) + + // The following does not work with the implicit change that propagates nested ambiguous errors to the top. + // We get ambiguous implicits because Predef.$conforms and convertIfView both fit WeekDay.Value => Ordered[WeekDay.Value]. + // It does not work in scalac either; there we get a divergent implicit. + // testSet(Weekday.ValueSet(days:_*), days) val treeMap = immutable.TreeMap(keyValues:_*) testMap(treeMap, keyValues) From c311bf3eb10abb5e44b556cdc2edae4b2f15757d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 3 Nov 2017 19:20:47 +0100 Subject: [PATCH 05/30] Move iterator-from test to pending This is a temporary fix. iterator-from seems to cause non-deterministic ambiguity errors. Moving to pending until we have figured out what goes wrong. --- tests/{ => pending}/run/iterator-from.scala | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{ => pending}/run/iterator-from.scala (100%) diff --git a/tests/run/iterator-from.scala b/tests/pending/run/iterator-from.scala similarity index 100% rename from tests/run/iterator-from.scala rename to tests/pending/run/iterator-from.scala From beb90b44164deb1ce08c580d60c21005bd16636a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 3 Nov 2017 19:26:15 +0100 Subject: [PATCH 06/30] Add test --- tests/neg/i3430.scala | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 tests/neg/i3430.scala diff --git a/tests/neg/i3430.scala b/tests/neg/i3430.scala new file mode 100644 index 000000000000..5e88377dfdde --- /dev/null +++ b/tests/neg/i3430.scala @@ -0,0 +1,5 @@ +object Test extends App { + + println(Nil.min) // error: no implicit found + +} From c1c9198c610a1a993200ffb597afed7e222e3eb0 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 3 Nov 2017 22:30:55 +0100 Subject: [PATCH 07/30] Refine order in which implicit candidates are tried --- .../dotty/tools/dotc/typer/Implicits.scala | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 1664efa5272b..b53008d76f47 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -915,6 +915,27 @@ trait Implicits { self: Typer => def ranking(cand: Candidate) = -ctx.runInfo.useCount(cand.ref) + /** Prefer `cand1` over `cand2` if they are in the same compilation unit + * and `cand1` is defined before `cand2`, or they are in different units and + * `cand1` has been selected as an implicit more often than `cand2`. + */ + def prefer(cand1: Candidate, cand2: Candidate): Boolean = { + val sym1 = cand1.ref.symbol + val sym2 = cand2.ref.symbol + if (sym1.associatedFile == sym2.associatedFile) { + val coord1 = sym1.coord + val coord2 = sym2.coord + if (coord1.isPosition && coord2.isPosition) { + val pos1 = coord1.toPosition + val pos2 = coord2.toPosition + return pos1.exists && (!pos2.exists || pos1.start < pos2.start) + } + if (coord1.isIndex && coord2.isIndex) + return coord1.toIndex < coord2.toIndex + } + ranking(cand1) < ranking(cand2) + } + /** Sort list of implicit references according to their popularity * (# of times each was picked in current run). */ @@ -922,9 +943,9 @@ trait Implicits { self: Typer => case Nil => eligible case e1 :: Nil => eligible case e1 :: e2 :: Nil => - if (ranking(e2) < ranking(e1)) e2 :: e1 :: Nil + if (prefer(e2, e1)) e2 :: e1 :: Nil else eligible - case _ => eligible.sortBy(ranking) + case _ => eligible.sortWith(prefer) } val (successes, failures) = rankImplicits(sort(eligible), Nil, new mutable.ListBuffer) From a32b4b8b1f63e41327ef41b13807b9f83697fb86 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 5 Nov 2017 21:30:10 +0100 Subject: [PATCH 08/30] The great implicits re-org Refactoring of implicits with the aim of clearer code and better error messages. Here's an example: -- Error: implicitSearch.scala:15:12 ------------------------------------------- 15 | sort(xs) // error (with a partially constructed implicit argument shown) | ^ |no implicit argument of type Test.Ord[scala.collection.immutable.List[scala.collection.immutable.List[T]]] was found for parameter o of method sort in object Test. |I found: | | Test.listOrd[T](Test.listOrd[T](/* missing */implicitly[Test.Ord[T]])) | |But no implicit values were found that match type Test.Ord[T]. --- compiler/src/dotty/tools/dotc/ast/Trees.scala | 10 +- compiler/src/dotty/tools/dotc/ast/untpd.scala | 1 + .../tools/dotc/config/ScalaSettings.scala | 1 - .../src/dotty/tools/dotc/core/Types.scala | 21 +- .../tools/dotc/printing/PlainPrinter.scala | 28 +- .../tools/dotc/printing/RefinedPrinter.scala | 9 +- .../tools/dotc/typer/ErrorReporting.scala | 4 +- .../dotty/tools/dotc/typer/Implicits.scala | 355 ++++++++---------- .../src/dotty/tools/dotc/typer/ReTyper.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 90 +++-- tests/neg/implicitSearch.scala | 2 +- 11 files changed, 261 insertions(+), 262 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala index 0453f65dabdb..2b5d25f83504 100644 --- a/compiler/src/dotty/tools/dotc/ast/Trees.scala +++ b/compiler/src/dotty/tools/dotc/ast/Trees.scala @@ -145,9 +145,9 @@ object Trees { } /** A unique identifier for this tree. Used for debugging, and potentially - * tracking presentation compiler interactions + * tracking presentation compiler interactions. */ - private var myUniqueId: Int = nxId + @sharable private var myUniqueId: Int = nxId def uniqueId = myUniqueId @@ -449,6 +449,11 @@ object Trees { override def toString = s"BackquotedIdent($name)" } + class SearchFailureIdent[-T >: Untyped] private[ast] (name: Name) + extends Ident[T](name) { + override def toString = s"SearchFailureIdent($name)" + } + /** qualifier.name, or qualifier#name, if qualifier is a type */ case class Select[-T >: Untyped] private[ast] (qualifier: Tree[T], name: Name) extends RefTree[T] { @@ -948,6 +953,7 @@ object Trees { type Ident = Trees.Ident[T] type BackquotedIdent = Trees.BackquotedIdent[T] + type SearchFailureIdent = Trees.SearchFailureIdent[T] type Select = Trees.Select[T] type SelectWithSig = Trees.SelectWithSig[T] type This = Trees.This[T] diff --git a/compiler/src/dotty/tools/dotc/ast/untpd.scala b/compiler/src/dotty/tools/dotc/ast/untpd.scala index 28ef978a5012..1708cfc398df 100644 --- a/compiler/src/dotty/tools/dotc/ast/untpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/untpd.scala @@ -282,6 +282,7 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo { def Ident(name: Name): Ident = new Ident(name) def BackquotedIdent(name: Name): BackquotedIdent = new BackquotedIdent(name) + def SearchFailureIdent(name: Name): SearchFailureIdent = new SearchFailureIdent(name) def Select(qualifier: Tree, name: Name): Select = new Select(qualifier, name) def SelectWithSig(qualifier: Tree, name: Name, sig: Signature): Select = new SelectWithSig(qualifier, name, sig) def This(qual: Ident): This = new This(qual) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index f921ce5041a9..646fbfcb157a 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -26,7 +26,6 @@ class ScalaSettings extends Settings.SettingGroup { val migration = BooleanSetting("-migration", "Emit warning and location for migration issues from Scala 2.") val encoding = StringSetting("-encoding", "encoding", "Specify character encoding used by source files.", Properties.sourceEncoding) val explainTypes = BooleanSetting("-explain-types", "Explain type errors in more detail.") - val explainImplicits = BooleanSetting("-explain-implicits", "Explain implicit search errors in more detail.") val explain = BooleanSetting("-explain", "Explain errors in more detail.") val feature = BooleanSetting("-feature", "Emit warning and location for usages of features that should be imported explicitly.") val help = BooleanSetting("-help", "Print a synopsis of standard options") diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 5509415d827d..4a369805d255 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1981,7 +1981,7 @@ object Types { else candidate def withPrefix(prefix: Type)(implicit ctx: Context): NamedType = designator match { - case designator: TermSymbol => + case designator: TermSymbol @unchecked => TermRef(prefix, designator) case _ => // If symbol exists, the new signature is the symbol's signature as seen @@ -3641,22 +3641,25 @@ object Types { */ abstract class FlexType extends UncachedGroundType with ValueType - class ErrorType private[Types] () extends FlexType { - def msg(implicit ctx: Context): Message = - ctx.errorTypeMsg.get(this) match { - case Some(msgFun) => msgFun() - case None => "error message from previous run no longer available" - } + abstract class ErrorType extends FlexType { + def msg(implicit ctx: Context): Message } + object ErrorType { def apply(msg: => Message)(implicit ctx: Context): ErrorType = { - val et = new ErrorType + val et = new ErrorType { + def msg(implicit ctx: Context): Message = + ctx.errorTypeMsg.get(this) match { + case Some(msgFun) => msgFun() + case None => "error message from previous run no longer available" + } + } ctx.base.errorTypeMsg(et) = () => msg et } } - object UnspecifiedErrorType extends ErrorType() { + object UnspecifiedErrorType extends ErrorType { override def msg(implicit ctx: Context): Message = "unspecified error" } diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index 49743176470f..e1d0e99ac9ff 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -171,8 +171,8 @@ class PlainPrinter(_ctx: Context) extends Printer { changePrec(AndPrec) { toText(tp1) ~ " & " ~ toText(tp2) } case OrType(tp1, tp2) => changePrec(OrPrec) { toText(tp1) ~ " | " ~ toText(tp2) } - case _: ErrorType => - "" + case tp: ErrorType => + s"" case tp: WildcardType => if (tp.optBounds.exists) "(?" ~ toTextRHS(tp.bounds) ~ ")" else "?" case NoType => @@ -202,8 +202,6 @@ class PlainPrinter(_ctx: Context) extends Printer { ParamRefNameString(tp) ~ ".type" case AnnotatedType(tpe, annot) => toTextLocal(tpe) ~ " " ~ toText(annot) - case AppliedType(tycon, args) => - toTextLocal(tycon) ~ "[" ~ Text(args.map(argText), ", ") ~ "]" case tp: TypeVar => if (tp.isInstantiated) toTextLocal(tp.instanceOpt) ~ ("^" provided ctx.settings.YprintDebug.value) @@ -501,18 +499,16 @@ class PlainPrinter(_ctx: Context) extends Printer { def toText(result: SearchResult): Text = result match { case result: SearchSuccess => "SearchSuccess: " ~ toText(result.ref) ~ " via " ~ toText(result.tree) - case _: NonMatchingImplicit | NoImplicitMatches => - "NoImplicitMatches" - case _: DivergingImplicit => - "Diverging Implicit" - case result: ShadowedImplicit => - "Shadowed Implicit" - case result: FailedImplicit => - "Failed Implicit" - case result: AmbiguousImplicits => - "Ambiguous Implicit: " ~ toText(result.alt1) ~ " and " ~ toText(result.alt2) - case _ => - "?Unknown Implicit Result?" + result.getClass + case result: SearchFailure => + result.reason match { + case _: NoMatchingImplicits => "No Matching Implicit" + case _: DivergingImplicit => "Diverging Implicit" + case _: ShadowedImplicit => "Shadowed Implicit" + case result: AmbiguousImplicits => + "Ambiguous Implicit: " ~ toText(result.alt1) ~ " and " ~ toText(result.alt2) + case _ => + "?Unknown Implicit Result?" + result.getClass + } } def toText(importInfo: ImportInfo): Text = { diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 8e4e3fc160f1..c9dd0d0046c4 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -7,7 +7,7 @@ import TypeErasure.ErasedValueType import Contexts.Context, Scopes.Scope, Denotations._, SymDenotations._, Annotations.Annotation import StdNames.{nme, tpnme} import ast.{Trees, untpd, tpd} -import typer.{Namer, Inliner} +import typer.{Namer, Inliner, Implicits} import typer.ProtoTypes.{SelectionProto, ViewProto, FunProto, IgnoredProto, dummyTreeOfType} import Trees._ import TypeApplications._ @@ -332,6 +332,13 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { def toTextCore(tree: Tree): Text = tree match { case id: Trees.BackquotedIdent[_] if !homogenizedView => "`" ~ toText(id.name) ~ "`" + case id: Trees.SearchFailureIdent[_] => + tree.typeOpt match { + case reason: Implicits.SearchFailureType => + toText(id.name) ~ "implicitly[" ~ toText(reason.expectedType) ~ "]" + case _ => + toText(id.name) + } case Ident(name) => tree.typeOpt match { case tp: NamedType if name != nme.WILDCARD => diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala index 9714549b6bee..0427b4c76a66 100644 --- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -104,11 +104,11 @@ object ErrorReporting { def patternConstrStr(tree: Tree): String = ??? - def typeMismatch(tree: Tree, pt: Type, implicitFailure: SearchFailure = NoImplicitMatches): Tree = { + def typeMismatch(tree: Tree, pt: Type, implicitFailure: SearchFailureType = NoMatchingImplicits): Tree = { val normTp = normalize(tree.tpe, pt) val treeTp = if (normTp <:< pt) tree.tpe else normTp // use normalized type if that also shows an error, original type otherwise - errorTree(tree, typeMismatchMsg(treeTp, pt, implicitFailure.postscript)) + errorTree(tree, typeMismatchMsg(treeTp, pt, implicitFailure.whyNoConversion)) } /** A subtype log explaining why `found` does not conform to `expected` */ diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index b53008d76f47..29f5b8288389 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -26,7 +26,7 @@ import Constants._ import Applications._ import ProtoTypes._ import ErrorReporting._ -import reporting.diagnostic.MessageContainer +import reporting.diagnostic.{Message, MessageContainer} import Inferencing.fullyDefinedType import Trees._ import Hashable._ @@ -255,109 +255,96 @@ object Implicits { /** The result of an implicit search */ sealed abstract class SearchResult extends Showable { + def tree: tpd.Tree def toText(printer: Printer): Text = printer.toText(this) } /** A successful search - * @param ref The implicit reference that succeeded - * @param tree The typed tree that needs to be inserted - * @param ctx The context after the implicit search + * @param tree The typed tree that needs to be inserted + * @param ref The implicit reference that succeeded + * @param level The level where the reference was found + * @param tstate The typer state to be committed if this alternative is chosen */ - case class SearchSuccess(tree: tpd.Tree, ref: TermRef, level: Int, tstate: TyperState) extends SearchResult with Showable { - override def toString = s"SearchSuccess($tree, $ref, $level)" - } + case class SearchSuccess(tree: tpd.Tree, ref: TermRef, level: Int)(val tstate: TyperState) extends SearchResult with Showable /** A failed search */ - abstract class SearchFailure extends SearchResult { - /** A note describing the failure in more detail - this - * is either empty or starts with a '\n' - */ - def postscript(implicit ctx: Context): String = "" + case class SearchFailure(tree: tpd.Tree) extends SearchResult { + final def isAmbiguous = tree.tpe.isInstanceOf[AmbiguousImplicits] + final def reason = tree.tpe.asInstanceOf[SearchFailureType] } - /** A "no matching implicit found" failure */ - case object NoImplicitMatches extends SearchFailure - - /** A tree representing a failed search for an implicit argument */ - case class FailedSearch(failure: SearchFailure, errorFn: String => String) extends WithoutTypeOrPos[Type] { - override def tag = -1 - override def isEmpty = true + object SearchFailure { + def apply(tpe: SearchFailureType): SearchFailure = { + val id = + if (tpe.isInstanceOf[AmbiguousImplicits]) "/* ambiguous */" + else "/* missing */" + SearchFailure(untpd.SearchFailureIdent(id.toTermName).withTypeUnchecked(tpe)) + } } - /** A search failure that can show information about the cause */ - abstract class ExplainedSearchFailure extends SearchFailure { - protected def pt: Type + abstract class SearchFailureType extends ErrorType { + def expectedType: Type protected def argument: tpd.Tree - protected def qualify(implicit ctx: Context) = - if (argument.isEmpty) em"match type $pt" - else em"convert from ${argument.tpe} to $pt" + + final protected def qualify(implicit ctx: Context) = + if (expectedType.exists) + if (argument.isEmpty) em"match type $expectedType" + else em"convert from ${argument.tpe} to $expectedType" + else + if (argument.isEmpty) em"match expected type" + else em"convert from ${argument.tpe} to expected type" /** An explanation of the cause of the failure as a string */ def explanation(implicit ctx: Context): String + + def msg(implicit ctx: Context): Message = explanation + + /** If search was a for an implicit conversion, a note describing the failure + * in more detail - this is either empty or starts with a '\n' + */ + def whyNoConversion(implicit ctx: Context): String = "" + } + + class NoMatchingImplicits(val expectedType: Type, val argument: tpd.Tree) extends SearchFailureType { + def explanation(implicit ctx: Context): String = + em"no implicit values were found that $qualify" } + object NoMatchingImplicits extends NoMatchingImplicits(NoType, tpd.EmptyTree) + + val NoMatchingImplicitsFailure: SearchFailure = + SearchFailure(NoMatchingImplicits) + /** An ambiguous implicits failure */ - class AmbiguousImplicits(val alt1: TermRef, val alt2: TermRef, val pt: Type, val argument: tpd.Tree) extends ExplainedSearchFailure { + class AmbiguousImplicits(val alt1: TermRef, val alt2: TermRef, val expectedType: Type, val argument: tpd.Tree) extends SearchFailureType { def explanation(implicit ctx: Context): String = em"both ${err.refStr(alt1)} and ${err.refStr(alt2)} $qualify" - override def postscript(implicit ctx: Context) = + override def whyNoConversion(implicit ctx: Context) = "\nNote that implicit conversions cannot be applied because they are ambiguous;" + "\n " + explanation } - class NonMatchingImplicit(ref: TermRef, - val pt: Type, - val argument: tpd.Tree, - trail: List[MessageContainer]) extends ExplainedSearchFailure { - private val separator = "\n**** because ****\n" - - /** Replace repeated parts beginning with `separator` by ... */ - private def elideRepeated(str: String): String = { - val startIdx = str.indexOfSlice(separator) - val nextIdx = str.indexOfSlice(separator, startIdx + separator.length) - if (nextIdx < 0) str - else { - val prefix = str.take(startIdx) - val first = str.slice(startIdx, nextIdx) - var rest = str.drop(nextIdx) - if (rest.startsWith(first)) { - rest = rest.drop(first.length) - val dots = "\n\n ...\n" - if (!rest.startsWith(dots)) rest = dots ++ rest - } - prefix ++ first ++ rest - } - } - - def explanation(implicit ctx: Context): String = { - val headMsg = em"${err.refStr(ref)} does not $qualify" - val trailMsg = trail.map(mc => i"$separator ${mc.message}").mkString - elideRepeated(headMsg ++ trailMsg) - } + class MismatchedImplicit(ref: TermRef, + val expectedType: Type, + val argument: tpd.Tree) extends SearchFailureType { + def explanation(implicit ctx: Context): String = + em"${err.refStr(ref)} does not $qualify" } - class ShadowedImplicit(ref: TermRef, shadowing: Type, val pt: Type, val argument: tpd.Tree) extends ExplainedSearchFailure { + class ShadowedImplicit(ref: TermRef, + shadowing: Type, + val expectedType: Type, + val argument: tpd.Tree) extends SearchFailureType { def explanation(implicit ctx: Context): String = em"${err.refStr(ref)} does $qualify but is shadowed by ${err.refStr(shadowing)}" } - class DivergingImplicit(ref: TermRef, val pt: Type, val argument: tpd.Tree) extends ExplainedSearchFailure { + class DivergingImplicit(ref: TermRef, + val expectedType: Type, + val argument: tpd.Tree) extends SearchFailureType { def explanation(implicit ctx: Context): String = em"${err.refStr(ref)} produces a diverging implicit search when trying to $qualify" } - - class FailedImplicit(failures: List[ExplainedSearchFailure], val pt: Type, val argument: tpd.Tree) extends ExplainedSearchFailure { - def explanation(implicit ctx: Context): String = - if (failures.isEmpty) s" No implicit candidates were found that $qualify" - else failures.map(_.explanation).mkString("\n").replace("\n", "\n ") - override def postscript(implicit ctx: Context): String = { - val what = - if (argument.isEmpty) i"value of type $pt" - else i"conversion from ${argument.tpe.widen} to $pt" - i""" - |$explanation""" - } - } } import Implicits._ @@ -533,7 +520,7 @@ trait Implicits { self: Typer => || (from.tpe isRef defn.NothingClass) || (from.tpe isRef defn.NullClass) || !(ctx.mode is Mode.ImplicitsEnabled) - || (from.tpe eq NoPrefix)) NoImplicitMatches + || (from.tpe eq NoPrefix)) NoMatchingImplicitsFailure else { def adjust(to: Type) = to.stripTypeVar.widenExpr match { case SelectionProto(name, memberProto, compat, true) => @@ -567,17 +554,13 @@ trait Implicits { self: Typer => fullyDefinedType(arg, "ClassTag argument", pos) match { case defn.ArrayOf(elemTp) => val etag = inferImplicitArg(defn.ClassTagType.appliedTo(elemTp), pos) - if (etag.isEmpty) etag else etag.select(nme.wrap) - case tp if hasStableErasure(tp) => - if (defn.isBottomClass(tp.typeSymbol)) - FailedSearch(NoImplicitMatches, - where => i"attempt to take ClassTag of undetermined type for $where") - else - ref(defn.ClassTagModule) - .select(nme.apply) - .appliedToType(tp) - .appliedTo(clsOf(erasure(tp))) - .withPos(pos) + if (etag.tpe.isError) EmptyTree else etag.select(nme.wrap) + case tp if hasStableErasure(tp) && !defn.isBottomClass(tp.typeSymbol) => + ref(defn.ClassTagModule) + .select(nme.apply) + .appliedToType(tp) + .appliedTo(clsOf(erasure(tp))) + .withPos(pos) case tp => EmptyTree } @@ -631,7 +614,7 @@ trait Implicits { self: Typer => } inferImplicit(formalValue, EmptyTree, pos)(argCtx) match { - case SearchSuccess(arg, _, _, _) => + case SearchSuccess(arg, _, _) => def refersToLazyImplicit = arg.existsSubTree { case id: Ident => id.symbol == lazyImplicit case _ => false @@ -640,45 +623,57 @@ trait Implicits { self: Typer => Block(ValDef(lazyImplicit.asTerm, arg).withPos(pos) :: Nil, ref(lazyImplicit)) else arg + case fail @ SearchFailure(tree) => + if (fail.isAmbiguous) + tree + else if (formalValue.isRef(defn.ClassTagClass)) + synthesizedClassTag(formalValue).orElse(tree) + else if (formalValue.isRef(defn.EqClass)) + synthesizedEq(formalValue).orElse(tree) + else + tree + } + } + + /** Search an implicit argument and report error if not found */ + def implicitArgTree(formal: Type, pos: Position)(implicit ctx: Context): Tree = { + val arg = inferImplicitArg(formal, pos) + if (arg.tpe.isInstanceOf[SearchFailureType]) ctx.error(missingArgMsg(arg, formal, ""), pos) + arg + } + + def missingArgMsg(arg: tpd.Tree, pt: Type, where: String)(implicit ctx: Context): String = { + def msg(shortForm: String)(headline: String = shortForm) = arg match { + case arg: Trees.SearchFailureIdent[_] => + shortForm + case _ => + i"""$headline. + |I found: + | + | ${arg.show.replace("\n", "\n ")} + | + |But ${arg.tpe.asInstanceOf[SearchFailureType].explanation}.""" + } + arg.tpe match { case ambi: AmbiguousImplicits => - FailedSearch(ambi, where => s"ambiguous implicits: ${ambi.explanation} of $where") - case failure: SearchFailure => - val fallbackArg = - if (formalValue.isRef(defn.ClassTagClass)) - synthesizedClassTag(formalValue) - else if (formalValue.isRef(defn.EqClass)) - synthesizedEq(formalValue) - else - EmptyTree - if (!fallbackArg.isEmpty) fallbackArg - else { - var msgFn = (where: String) => - em"no implicit argument of type $formal found for $where" + failure.postscript + msg(s"ambiguous implicit arguments: ${ambi.explanation} of $where")( + s"ambiguous implicit arguments of type $pt found for $where") + case _ => + val userDefined = for { - notFound <- formalValue.typeSymbol.getAnnotation(defn.ImplicitNotFoundAnnot) + notFound <- pt.typeSymbol.getAnnotation(defn.ImplicitNotFoundAnnot) Trees.Literal(Constant(raw: String)) <- notFound.argument(0) - } { - msgFn = where => - err.implicitNotFoundString( - raw, - formalValue.typeSymbol.typeParams.map(_.name.unexpandedName.toString), - formalValue.argInfos) } - FailedSearch(failure, msgFn) - } + yield { + err.implicitNotFoundString( + raw, + pt.typeSymbol.typeParams.map(_.name.unexpandedName.toString), + pt.argInfos) + } + msg(userDefined.getOrElse(em"no implicit argument of type $pt was found for $where"))() } } - /** Search an implicit argument and report error if not found */ - def implicitArgTree(formal: Type, pos: Position, where: String = "")(implicit ctx: Context): Tree = - inferImplicitArg(formal, pos) match { - case FailedSearch(fail, errorFn) => - ctx.error(errorFn(where), pos) - EmptyTree - case tree => - tree - } - private def assumedCanEqual(ltp: Type, rtp: Type)(implicit ctx: Context) = { def eqNullable: Boolean = { val other = @@ -725,24 +720,21 @@ trait Implicits { self: Typer => else i"type error: ${argument.tpe} does not conform to $pt${err.whyNoMatchStr(argument.tpe, pt)}") trace(s"search implicit ${pt.show}, arg = ${argument.show}: ${argument.tpe.show}", implicits, show = true) { assert(!pt.isInstanceOf[ExprType]) - val isearch = - if (ctx.settings.explainImplicits.value) new ExplainedImplicitSearch(pt, argument, pos) - else new ImplicitSearch(pt, argument, pos) - val result = isearch.bestImplicit(contextual = true) + val result = new ImplicitSearch(pt, argument, pos).bestImplicit(contextual = true) result match { case result: SearchSuccess => result.tstate.commit() implicits.println(i"success: $result") implicits.println(i"committing ${result.tstate.constraint} yielding ${ctx.typerState.constraint} ${ctx.typerState.hashesStr}") result - case result: AmbiguousImplicits => + case result: SearchFailure if result.isAmbiguous => val deepPt = pt.deepenProto if (deepPt ne pt) inferImplicit(deepPt, argument, pos) else if (ctx.scala2Mode && !ctx.mode.is(Mode.OldOverloadingResolution)) { inferImplicit(pt, argument, pos)(ctx.addMode(Mode.OldOverloadingResolution)) match { case altResult: SearchSuccess => ctx.migrationWarning( - s"According to new implicit resolution rules, this will be ambiguous:\n ${result.explanation}", + s"According to new implicit resolution rules, this will be ambiguous:\n ${result.reason.explanation}", pos) altResult case _ => @@ -782,15 +774,6 @@ trait Implicits { self: Typer => /** The expected type where parameters and uninstantiated typevars are replaced by wildcard types */ val wildProto = implicitProto(pt, wildApprox(_, null, Set.empty)) - /** Search failures; overridden in ExplainedImplicitSearch */ - protected def nonMatchingImplicit(ref: TermRef, trail: List[MessageContainer]): SearchFailure = NoImplicitMatches - protected def divergingImplicit(ref: TermRef): SearchFailure = { - implicits.println(i"diverging implicit: $ref for $pt, search history = ${ctx.searchHistory}") - new DivergingImplicit(ref, pt, argument) - } - protected def shadowedImplicit(ref: TermRef, shadowing: Type): SearchFailure = NoImplicitMatches - protected def failedSearch: SearchFailure = NoImplicitMatches - /** Search a list of eligible implicit references */ def searchImplicits(eligible: List[Candidate], contextual: Boolean): SearchResult = { val constr = ctx.typerState.constraint @@ -819,54 +802,62 @@ trait Implicits { self: Typer => } if (ctx.reporter.hasErrors) { - val trail = ctx.reporter.removeBufferedMessages - generated1 match { - case failed: FailedSearch => failed.failure - case _ => nonMatchingImplicit(ref, trail) + ctx.reporter.removeBufferedMessages + SearchFailure { + generated1.tpe match { + case _: SearchFailureType => generated1 + case _ => generated1.withType(new MismatchedImplicit(ref, pt, argument)) + } } } else if (contextual && !ctx.mode.is(Mode.ImplicitShadowing) && !shadowing.tpe.isError && !refSameAs(shadowing)) { implicits.println(i"SHADOWING $ref in ${ref.termSymbol.maybeOwner} is shadowed by $shadowing in ${shadowing.symbol.maybeOwner}") - shadowedImplicit(ref, methPart(shadowing).tpe) + SearchFailure(generated1.withTypeUnchecked( + new ShadowedImplicit(ref, methPart(shadowing).tpe, pt, argument))) } else - SearchSuccess(generated1, ref, cand.level, ctx.typerState) + SearchSuccess(generated1, ref, cand.level)(ctx.typerState) }} - /** Given a list of implicit references, produce a list of all implicit search successes, - * where the first is supposed to be the best one. - * Except if one of the results is a diverging implicit, produce a list consisting - * of just that result. + /** Given a list of implicit references, produce a list of search results, + * which is either a list of successes or a list of failures. + * - if one of the references produces an ambiguity error, return it + * as only element + * - if some of the references produce successful searches, return those that do + * - otherwise return a list of all failures + * + * If mode is ImplicitExploration or we assume coherence, stop at first + * succesfull search. + * * @param pending The list of implicit references that remain to be investigated - * @param acc An accumulator of successful matches found so far. */ def rankImplicits(pending: List[Candidate], successes: List[SearchSuccess], - failures: mutable.ListBuffer[SearchFailure]): (List[SearchSuccess], List[SearchFailure]) = + rfailures: List[SearchFailure]): List[SearchResult] = pending match { case cand :: pending1 => val history = ctx.searchHistory nest wildProto val result = - if (history eq ctx.searchHistory) divergingImplicit(cand.ref) - else typedImplicit(cand)(nestedContext.setNewTyperState().setSearchHistory(history)) + if (history eq ctx.searchHistory) + SearchFailure(new DivergingImplicit(cand.ref, pt, argument)) + else + typedImplicit(cand)(nestedContext.setNewTyperState().setSearchHistory(history)) result match { - case fail: AmbiguousImplicits => - (Nil, fail :: Nil) case fail: SearchFailure => - rankImplicits(pending1, successes, - if (fail.isInstanceOf[DivergingImplicit]) fail +=: failures - else failures += fail) + if (fail.isAmbiguous) fail :: Nil + else rankImplicits(pending1, successes, fail :: rfailures) case best: SearchSuccess => if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) - (best :: Nil, failures.toList) + best :: Nil else { val newPending = pending1.filter(cand1 => ctx.typerState.test(isAsGood(cand1.ref, best.ref, cand1.level, best.level)(nestedContext))) - rankImplicits(newPending, best :: successes, failures) + rankImplicits(newPending, best :: successes, rfailures) } } - case nil => (successes, failures.toList) + case nil => + if (successes.nonEmpty) successes else rfailures.reverse } /** If the (result types of) the expected type, and both alternatives @@ -884,12 +875,19 @@ trait Implicits { self: Typer => if (isNumeric(rpt) && isNumeric(rt1) && isNumeric(rt2)) if (isProperSubType(rt1, rt2)) alt1 else if (isProperSubType(rt2, rt1)) alt2 - else NoImplicitMatches - else NoImplicitMatches + else NoMatchingImplicitsFailure + else NoMatchingImplicitsFailure } - /** Convert a (possibly empty) list of search successes into a single search result */ - def condense(successes: List[SearchSuccess], failures: List[SearchFailure]): SearchResult = successes match { + /** Convert a (possibly empty) list of search results into a single search result + * - if the list consists of one or more successes + * - if a following success is as good as the first one, issue an ambiguity error + * - otherwise return the first success + * - if the list consists of one or more failues, pick the failure with the largest + * associated tree. + * - if the list is empty, issue a "no matching implicits" error. + */ + def condense(results: List[SearchResult]): SearchResult = results match { case (best: SearchSuccess) :: (alts: List[SearchSuccess] @ unchecked) => alts.find(alt => ctx.typerState.test(isAsGood(alt.ref, best.ref, alt.level, best.level))) match { @@ -901,16 +899,18 @@ trait Implicits { self: Typer => */ numericValueTieBreak(best, alt) match { case eliminated: SearchSuccess => - condense(successes.filter(_ ne eliminated), failures) + condense(results.filter(_ ne eliminated)) case _ => - new AmbiguousImplicits(best.ref, alt.ref, pt, argument) + SearchFailure(new AmbiguousImplicits(best.ref, alt.ref, pt, argument)) } case None => ctx.runInfo.useCount(best.ref) += 1 best } + case (fail: SearchFailure) :: _ => + results.maxBy(_.tree.treeSize) case Nil => - failures.headOption.getOrElse(NoImplicitMatches) + SearchFailure(new NoMatchingImplicits(pt, argument)) } def ranking(cand: Candidate) = -ctx.runInfo.useCount(cand.ref) @@ -948,8 +948,7 @@ trait Implicits { self: Typer => case _ => eligible.sortWith(prefer) } - val (successes, failures) = rankImplicits(sort(eligible), Nil, new mutable.ListBuffer) - condense(successes, failures) + condense(rankImplicits(sort(eligible), Nil, Nil)) } /** Find a unique best implicit reference */ @@ -958,37 +957,17 @@ trait Implicits { self: Typer => if (contextual) ctx.implicits.eligible(wildProto) else implicitScope(wildProto).eligible searchImplicits(eligible, contextual) match { - case result: SearchSuccess => result - case result: AmbiguousImplicits => result - case result: DivergingImplicit => result - case result: SearchFailure => - if (contextual) bestImplicit(contextual = false) else result + case success: SearchSuccess => success + case failure: SearchFailure => + failure.reason match { + case (_: AmbiguousImplicits) | (_: DivergingImplicit) => failure + case _ => if (contextual) bestImplicit(contextual = false) else failure + } } } def implicitScope(tp: Type): OfTypeImplicits = ctx.runInfo.implicitScope(tp, ctx) } - - final class ExplainedImplicitSearch(pt: Type, argument: Tree, pos: Position)(implicit ctx: Context) - extends ImplicitSearch(pt, argument, pos) { - private[this] var myFailures = new mutable.ListBuffer[ExplainedSearchFailure] - private def record(fail: ExplainedSearchFailure) = { - myFailures += fail - fail - } - def failures = myFailures.toList - override def nonMatchingImplicit(ref: TermRef, trail: List[MessageContainer]) = - record(new NonMatchingImplicit(ref, pt, argument, trail)) - override def divergingImplicit(ref: TermRef) = - record(new DivergingImplicit(ref, pt, argument)) - override def shadowedImplicit(ref: TermRef, shadowing: Type): SearchFailure = - record(new ShadowedImplicit(ref, shadowing, pt, argument)) - override def failedSearch: SearchFailure = { - //println(s"wildProto = $wildProto") - //println(s"implicit scope = ${implicitScope(wildProto).companionRefs}") - new FailedImplicit(failures, pt, argument) - } - } } /** Records the history of currently open implicit searches diff --git a/compiler/src/dotty/tools/dotc/typer/ReTyper.scala b/compiler/src/dotty/tools/dotc/typer/ReTyper.scala index 3768d4f26598..8298889a70f9 100644 --- a/compiler/src/dotty/tools/dotc/typer/ReTyper.scala +++ b/compiler/src/dotty/tools/dotc/typer/ReTyper.scala @@ -101,7 +101,7 @@ class ReTyper extends Typer { override def checkVariance(tree: Tree)(implicit ctx: Context) = () override def inferView(from: Tree, to: Type)(implicit ctx: Context): Implicits.SearchResult = - Implicits.NoImplicitMatches + Implicits.NoMatchingImplicitsFailure override def checkCanEqual(ltp: Type, rtp: Type, pos: Position)(implicit ctx: Context): Unit = () override def inlineExpansion(mdef: DefDef)(implicit ctx: Context): List[Tree] = mdef :: Nil diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 730b91be1c21..ffd945d18671 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -548,7 +548,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit require(ctx.mode.is(Mode.Pattern)) inferImplicit(defn.ClassTagType.appliedTo(tref), EmptyTree, tree.tpt.pos)(ctx.retractMode(Mode.Pattern)) match { - case SearchSuccess(clsTag, _, _, _) => + case SearchSuccess(clsTag, _, _) => typed(untpd.Apply(untpd.TypedSplice(clsTag), untpd.TypedSplice(tree.expr)), pt) case _ => tree @@ -2000,61 +2000,68 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit val tvarsToInstantiate = tvarsInParams(tree) wtp.paramInfos.foreach(instantiateSelected(_, tvarsToInstantiate)) val constr = ctx.typerState.constraint + + def dummyArg(tp: Type) = untpd.Ident(nme.???).withTypeUnchecked(tp) + def addImplicitArgs(implicit ctx: Context) = { def implicitArgs(formals: List[Type]): List[Tree] = formals match { case Nil => Nil case formal :: formals1 => - inferImplicitArg(formal, tree.pos.endPos) match { - case arg: FailedSearch - if !arg.failure.isInstanceOf[AmbiguousImplicits] && !tree.symbol.hasDefaultParams => + val arg = inferImplicitArg(formal, tree.pos.endPos) + arg.tpe match { + case failed: SearchFailureType + if !failed.isInstanceOf[AmbiguousImplicits] && !tree.symbol.hasDefaultParams => // no need to search further, the adapt fails in any case - // the reason why we continue inferring in case of an AmbiguousImplicits + // the reason why we continue inferring arguments in case of an AmbiguousImplicits // is that we need to know whether there are further errors. // If there are none, we have to propagate the ambiguity to the caller. - arg :: Nil - case arg => + arg :: formals1.map(dummyArg) + case _ => arg :: implicitArgs(formals1) } } val args = implicitArgs(wtp.paramInfos) - val failedArgs = new mutable.ListBuffer[Tree] - val ambiguousArgs = new mutable.ListBuffer[Tree] - for (arg @ FailedSearch(failure, _) <- args) - (if (failure.isInstanceOf[AmbiguousImplicits]) ambiguousArgs else failedArgs) += arg + def propagatedFailure(args: List[Tree]): Type = args match { + case arg :: args1 => + arg.tpe match { + case ambi: AmbiguousImplicits => + propagatedFailure(args1) match { + case NoType | (_: AmbiguousImplicits) => ambi + case failed => failed + } + case failed: SearchFailureType => failed + case _ => propagatedFailure(args1) + } + case Nil => NoType + } - if (ambiguousArgs.isEmpty && failedArgs.isEmpty) - adapt(tpd.Apply(tree, args), pt) - else { + val propFail = propagatedFailure(args) + + def issueErrors(): Tree = { + (wtp.paramNames, wtp.paramInfos, args).zipped.foreach { (paramName, formal, arg) => + arg.tpe match { + case failure: SearchFailureType => + ctx.error( + missingArgMsg(arg, formal, em"parameter ${paramName} of $methodStr"), + tree.pos.endPos) + case _ => + } + } + untpd.Apply(tree, args).withType(propFail) + } + + if (propFail.exists) { // If there are several arguments, some arguments might already // have influenced the context, binding variables, but later ones // might fail. In that case the constraint needs to be reset. ctx.typerState.constraint = constr - def issueErrors() = { - def recur(paramNames: List[TermName], remainingArgs: List[Tree]): Tree = remainingArgs match { - case Nil => - tree.withType(wtp.resultType) - case (arg @ FailedSearch(failure, msgFn)) :: args1 => - val msg = msgFn(em"parameter ${paramNames.head} of $methodStr") - def closedArg = FailedSearch(failure, _ => msg) - ctx.error(msg, tree.pos.endPos) - failure match { - case _: AmbiguousImplicits if failedArgs.isEmpty => closedArg // propagate - case _: DivergingImplicit => closedArg // propagate - case _ => tree.withType(wtp.resultType) // show error at enclosing level instead - } - case arg :: args1 => - recur(paramNames.tail, args1) - } - recur(wtp.paramNames, args) - } - // If method has default params, fall back to regular application // where all inferred implicits are passed as named args. - if (failedArgs.nonEmpty && tree.symbol.hasDefaultParams) { + if (tree.symbol.hasDefaultParams && !propFail.isInstanceOf[AmbiguousImplicits]) { val namedArgs = (wtp.paramNames, args).zipped.flatMap { (pname, arg) => - if (arg.isEmpty) Nil else untpd.NamedArg(pname, untpd.TypedSplice(arg)) :: Nil + if (arg.tpe.isError) Nil else untpd.NamedArg(pname, untpd.TypedSplice(arg)) :: Nil } tryEither { implicit ctx => typed(untpd.Apply(untpd.TypedSplice(tree), namedArgs), pt) @@ -2063,6 +2070,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit } } else issueErrors() } + else adapt(tpd.Apply(tree, args), pt) } addImplicitArgs(argCtx(tree)) } @@ -2250,23 +2258,23 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit } // try an implicit conversion val prevConstraint = ctx.typerState.constraint - def recover(failure: SearchFailure) = + def recover(failure: SearchFailureType) = if (isFullyDefined(wtp, force = ForceDegree.all) && ctx.typerState.constraint.ne(prevConstraint)) adapt(tree, pt) else err.typeMismatch(tree, pt, failure) if (ctx.mode.is(Mode.ImplicitsEnabled)) inferView(tree, pt) match { - case SearchSuccess(inferred, _, _, _) => + case SearchSuccess(inferred, _, _) => adapt(inferred, pt)(ctx.retractMode(Mode.ImplicitsEnabled)) case failure: SearchFailure => - if (pt.isInstanceOf[ProtoType] && !failure.isInstanceOf[AmbiguousImplicits]) + if (pt.isInstanceOf[ProtoType] && !failure.reason.isInstanceOf[AmbiguousImplicits]) // don't report the failure but return the tree unchanged. This - // wil cause a failure at the next level out, which usually gives + // will cause a failure at the next level out, which usually gives // a better error message. tree - else recover(failure) + else recover(failure.reason) } - else recover(NoImplicitMatches) + else recover(NoMatchingImplicits) } def adaptType(tp: Type): Tree = { diff --git a/tests/neg/implicitSearch.scala b/tests/neg/implicitSearch.scala index bcb12c3e9a7c..a5be79474ffd 100644 --- a/tests/neg/implicitSearch.scala +++ b/tests/neg/implicitSearch.scala @@ -12,7 +12,7 @@ object Test { sort(xs) def f[T](xs: List[List[List[T]]]) = - sort(xs) // error TODO: improve the -explain-implicts diagnostic + sort(xs) // error (with a partially constructed implicit argument shown) listOrd(listOrd(implicitly[Ord[T]] /*not found*/)) // error } From e1cf2c98ac72e01227846673b15116882e49278f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 5 Nov 2017 22:27:00 +0100 Subject: [PATCH 09/30] Add @sharable to global data --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 29f5b8288389..f78a4b408b96 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -310,9 +310,9 @@ object Implicits { em"no implicit values were found that $qualify" } - object NoMatchingImplicits extends NoMatchingImplicits(NoType, tpd.EmptyTree) + @sharable object NoMatchingImplicits extends NoMatchingImplicits(NoType, tpd.EmptyTree) - val NoMatchingImplicitsFailure: SearchFailure = + @sharable val NoMatchingImplicitsFailure: SearchFailure = SearchFailure(NoMatchingImplicits) /** An ambiguous implicits failure */ From 7da033bdf3c36a78c178a6594b3c46167b14bb02 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 5 Nov 2017 23:11:55 +0100 Subject: [PATCH 10/30] Disable refined preference scheme for implicits Turns out it's not transitive when combined with the old scheme. So it should be one or the other. Conservatively, this commit reverts to the old scheme. --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index f78a4b408b96..89978a56fc54 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -920,7 +920,7 @@ trait Implicits { self: Typer => * `cand1` has been selected as an implicit more often than `cand2`. */ def prefer(cand1: Candidate, cand2: Candidate): Boolean = { - val sym1 = cand1.ref.symbol + /*val sym1 = cand1.ref.symbol val sym2 = cand2.ref.symbol if (sym1.associatedFile == sym2.associatedFile) { val coord1 = sym1.coord @@ -933,6 +933,7 @@ trait Implicits { self: Typer => if (coord1.isIndex && coord2.isIndex) return coord1.toIndex < coord2.toIndex } + */ ranking(cand1) < ranking(cand2) } From e6ffe35da8265e7415af773c8fecfe603336d11d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 6 Nov 2017 08:35:34 +0100 Subject: [PATCH 11/30] Bring back iterator-from from pending --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 2 +- tests/{pending => }/run/iterator-from.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename tests/{pending => }/run/iterator-from.scala (98%) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 89978a56fc54..4165bf7f08b5 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -657,7 +657,7 @@ trait Implicits { self: Typer => arg.tpe match { case ambi: AmbiguousImplicits => msg(s"ambiguous implicit arguments: ${ambi.explanation} of $where")( - s"ambiguous implicit arguments of type $pt found for $where") + s"ambiguous implicit arguments of type ${pt.show} found for $where") case _ => val userDefined = for { diff --git a/tests/pending/run/iterator-from.scala b/tests/run/iterator-from.scala similarity index 98% rename from tests/pending/run/iterator-from.scala rename to tests/run/iterator-from.scala index 7194a34c9e96..c5d016423b41 100644 --- a/tests/pending/run/iterator-from.scala +++ b/tests/run/iterator-from.scala @@ -67,7 +67,7 @@ object Test extends dotty.runtime.LegacyApp { // The following does not work with the implicit change that propagates nested ambiguous errors to the top. // We get ambiguous implicits because Predef.$conforms and convertIfView both fit WeekDay.Value => Ordered[WeekDay.Value]. // It does not work in scalac either; there we get a divergent implicit. - // testSet(Weekday.ValueSet(days:_*), days) + testSet(Weekday.ValueSet(days:_*), days) val treeMap = immutable.TreeMap(keyValues:_*) testMap(treeMap, keyValues) From d06d5aee48ccf0f9e6671c8bce6635c91c3dc4f3 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 6 Nov 2017 14:24:09 +0100 Subject: [PATCH 12/30] Replace isAsGood with ternary operator The aim is to have ultimately fewer comparisons. The downside is that some of the comparisons are now more expensive, because types have to be compared where they previously weren't. --- .../dotty/tools/dotc/typer/Applications.scala | 84 +++++++++++-------- 1 file changed, 48 insertions(+), 36 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 3ac531eed049..452ad3233621 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1046,17 +1046,25 @@ trait Applications extends Compatibility { self: Typer with Dynamic => * - The nesting levels of A1 and A2 are the same, and A1's owner derives from A2's owner * - A1's type is more specific than A2's type. */ - def isAsGood(alt1: TermRef, alt2: TermRef, nesting1: Int = 0, nesting2: Int = 0)(implicit ctx: Context): Boolean = track("isAsGood") { trace(i"isAsGood($alt1, $alt2)", overload) { + def compare(alt1: TermRef, alt2: TermRef, nesting1: Int = 0, nesting2: Int = 0)(implicit ctx: Context): Int = track("compare") { trace(i"compare($alt1, $alt2)", overload) { assert(alt1 ne alt2) - /** Is class or module class `sym1` derived from class or module class `sym2`? + /** Compare owner inheritance level. + * @param sym1 The first owner + * @param sym2 The second owner + * @return 1 if `sym1` properly derives from `sym2` + * -1 if `sym2` properly derives from `sym1` + * 0 otherwise * Module classes also inherit the relationship from their companions. */ - def isDerived(sym1: Symbol, sym2: Symbol): Boolean = - if (sym1 isSubClass sym2) true - else if (sym2 is Module) isDerived(sym1, sym2.companionClass) - else (sym1 is Module) && isDerived(sym1.companionClass, sym2) + def compareOwner(sym1: Symbol, sym2: Symbol): Int = + if (sym1 == sym2) 0 + else if (sym1 isSubClass sym2) 1 + else if (sym2 isSubClass sym1) -1 + else if (sym2 is Module) compareOwner(sym1, sym2.companionClass) + else if (sym1 is Module) compareOwner(sym1.companionClass, sym2) + else 0 /** Is alternative `alt1` with type `tp1` as specific as alternative * `alt2` with type `tp2` ? @@ -1165,55 +1173,59 @@ trait Applications extends Compatibility { self: Typer with Dynamic => val owner1 = if (alt1.symbol.exists) alt1.symbol.owner else NoSymbol val owner2 = if (alt2.symbol.exists) alt2.symbol.owner else NoSymbol + val ownerScore = + if (nesting1 > nesting2) 1 + else if (nesting1 < nesting2) -1 + else compareOwner(owner1, owner2) + val tp1 = stripImplicit(alt1.widen) val tp2 = stripImplicit(alt2.widen) - - def winsOwner1 = - nesting1 > nesting2 || nesting1 == nesting2 && isDerived(owner1, owner2) def winsType1 = isAsSpecific(alt1, tp1, alt2, tp2) - def winsOwner2 = - nesting2 > nesting1 || nesting1 == nesting2 && isDerived(owner2, owner1) def winsType2 = isAsSpecific(alt2, tp2, alt1, tp1) - overload.println(i"isAsGood($alt1, $alt2)? $tp1 $tp2 $winsOwner1 $winsType1 $winsOwner2 $winsType2") - - // Assume the following probabilities: - // - // P(winsOwnerX) = 2/3 - // P(winsTypeX) = 1/3 - // - // Then the call probabilities of the 4 basic operations are as follows: - // - // winsOwner1: 1/1 - // winsOwner2: 1/1 - // winsType1 : 7/9 - // winsType2 : 4/9 - - if (winsOwner1) /* 6/9 */ !winsOwner2 || /* 4/9 */ winsType1 || /* 8/27 */ !winsType2 - else if (winsOwner2) /* 2/9 */ winsType1 && /* 2/27 */ !winsType2 - else /* 1/9 */ winsType1 || /* 2/27 */ !winsType2 + overload.println(i"compare($alt1, $alt2)? $tp1 $tp2 $ownerScore $winsType1 $winsType2") + + if (ownerScore == 1) + if (winsType1 || !winsType2) 1 else 0 + else if (ownerScore == -1) + if (winsType2 || !winsType1) -1 else 0 + else if (winsType1) + if (winsType2) 0 else 1 + else + if (winsType2) -1 else 0 }} + def isAsGood(alt1: TermRef, alt2: TermRef, nesting1: Int = 0, nesting2: Int = 0)(implicit ctx: Context): Boolean = + compare(alt1, alt2, nesting1, nesting2) >= 0 + def narrowMostSpecific(alts: List[TermRef])(implicit ctx: Context): List[TermRef] = track("narrowMostSpecific") { alts match { case Nil => alts case _ :: Nil => alts + case alt1 :: alt2 :: Nil => + compare(alt1, alt2) match { + case 1 => alt1 :: Nil + case -1 => alt2 :: Nil + case 0 => alts + } case alt :: alts1 => - def winner(bestSoFar: TermRef, alts: List[TermRef]): TermRef = alts match { + def survivors(previous: List[TermRef], alts: List[TermRef]): List[TermRef] = alts match { case alt :: alts1 => - winner(if (isAsGood(alt, bestSoFar)) alt else bestSoFar, alts1) - case nil => - bestSoFar + compare(previous.head, alt) match { + case 1 => survivors(previous, alts1) + case -1 => survivors(alt :: previous.tail, alts1) + case 0 => survivors(alt :: previous, alts1) + } + case Nil => previous } - val best = winner(alt, alts1) + val best :: rest = survivors(alt :: Nil, alts1) def asGood(alts: List[TermRef]): List[TermRef] = alts match { case alt :: alts1 => - if ((alt eq best) || !isAsGood(alt, best)) asGood(alts1) - else alt :: asGood(alts1) + if (compare(alt, best) < 0) asGood(alts1) else alt :: asGood(alts1) case nil => Nil } - best :: asGood(alts) + best :: asGood(rest) } } From d8028de51fde223a262d75998d650251a9987744 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 6 Nov 2017 21:07:01 +0100 Subject: [PATCH 13/30] WIP: Try out some alternatives --- .../dotty/tools/dotc/core/TyperState.scala | 2 +- .../dotty/tools/dotc/typer/Applications.scala | 32 +-- .../dotty/tools/dotc/typer/Implicits.scala | 182 +++++++++++++++--- .../dotty/tools/dotc/util/PriorityGraph.scala | 96 +++++++++ 4 files changed, 269 insertions(+), 43 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/util/PriorityGraph.scala diff --git a/compiler/src/dotty/tools/dotc/core/TyperState.scala b/compiler/src/dotty/tools/dotc/core/TyperState.scala index f7e6926cfe92..532d7f90d408 100644 --- a/compiler/src/dotty/tools/dotc/core/TyperState.scala +++ b/compiler/src/dotty/tools/dotc/core/TyperState.scala @@ -89,7 +89,7 @@ class TyperState(previous: TyperState /* | Null */) extends DotClass with Showab private[this] var testReporter: StoreReporter = null /** Test using `op`, restoring typerState to previous state afterwards */ - def test(op: => Boolean): Boolean = { + def test[T](op: => T): T = { val savedReporter = myReporter val savedConstraint = myConstraint val savedCommittable = myIsCommittable diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 452ad3233621..addfea8e34b4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1031,6 +1031,22 @@ trait Applications extends Compatibility { self: Typer with Dynamic => tp.member(nme.apply).hasAltWith(d => p(TermRef(tp, nme.apply, d))) } + /** Compare owner inheritance level. + * @param sym1 The first owner + * @param sym2 The second owner + * @return 1 if `sym1` properly derives from `sym2` + * -1 if `sym2` properly derives from `sym1` + * 0 otherwise + * Module classes also inherit the relationship from their companions. + */ + def compareOwner(sym1: Symbol, sym2: Symbol)(implicit ctx: Context): Int = + if (sym1 == sym2) 0 + else if (sym1 isSubClass sym2) 1 + else if (sym2 isSubClass sym1) -1 + else if (sym2 is Module) compareOwner(sym1, sym2.companionClass) + else if (sym1 is Module) compareOwner(sym1.companionClass, sym2) + else 0 + /** In a set of overloaded applicable alternatives, is `alt1` at least as good as * `alt2`? Also used for implicits disambiguation. * @@ -1050,22 +1066,6 @@ trait Applications extends Compatibility { self: Typer with Dynamic => assert(alt1 ne alt2) - /** Compare owner inheritance level. - * @param sym1 The first owner - * @param sym2 The second owner - * @return 1 if `sym1` properly derives from `sym2` - * -1 if `sym2` properly derives from `sym1` - * 0 otherwise - * Module classes also inherit the relationship from their companions. - */ - def compareOwner(sym1: Symbol, sym2: Symbol): Int = - if (sym1 == sym2) 0 - else if (sym1 isSubClass sym2) 1 - else if (sym2 isSubClass sym1) -1 - else if (sym2 is Module) compareOwner(sym1, sym2.companionClass) - else if (sym1 is Module) compareOwner(sym1.companionClass, sym2) - else 0 - /** Is alternative `alt1` with type `tp1` as specific as alternative * `alt2` with type `tp2` ? * diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 4165bf7f08b5..cd32d4261a79 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -30,7 +30,7 @@ import reporting.diagnostic.{Message, MessageContainer} import Inferencing.fullyDefinedType import Trees._ import Hashable._ -import util.Property +import util.{Property, PriorityGraph} import config.Config import config.Printers.{implicits, implicitsDetailed, typr} import collection.mutable @@ -257,6 +257,10 @@ object Implicits { sealed abstract class SearchResult extends Showable { def tree: tpd.Tree def toText(printer: Printer): Text = printer.toText(this) + def orElse(other: => SearchResult) = this match { + case _: SearchSuccess => this + case _ => other + } } /** A successful search @@ -316,9 +320,9 @@ object Implicits { SearchFailure(NoMatchingImplicits) /** An ambiguous implicits failure */ - class AmbiguousImplicits(val alt1: TermRef, val alt2: TermRef, val expectedType: Type, val argument: tpd.Tree) extends SearchFailureType { + class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: tpd.Tree) extends SearchFailureType { def explanation(implicit ctx: Context): String = - em"both ${err.refStr(alt1)} and ${err.refStr(alt2)} $qualify" + em"both ${err.refStr(alt1.ref)} and ${err.refStr(alt2.ref)} $qualify" override def whyNoConversion(implicit ctx: Context) = "\nNote that implicit conversions cannot be applied because they are ambiguous;" + "\n " + explanation @@ -750,8 +754,10 @@ trait Implicits { self: Typer => /** An implicit search; parameters as in `inferImplicit` */ class ImplicitSearch(protected val pt: Type, protected val argument: Tree, pos: Position)(implicit ctx: 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 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)) @@ -759,8 +765,8 @@ trait Implicits { self: Typer => private def isCoherent = pt.isRef(defn.EqClass) - assert(argument.isEmpty || argument.tpe.isValueType || argument.tpe.isInstanceOf[ExprType], - em"found: $argument: ${argument.tpe}, expected: $pt") + private val cmpContext = nestedContext() + private val cmpCandidates = (c1: Candidate, c2: Candidate) => compare(c1.ref, c2.ref, c1.level, c2.level)(cmpContext) /** The expected type for the searched implicit */ lazy val fullProto = implicitProto(pt, identity) @@ -778,6 +784,8 @@ trait Implicits { self: Typer => def searchImplicits(eligible: List[Candidate], contextual: Boolean): SearchResult = { val constr = ctx.typerState.constraint + //println(i"search implicits $pt / ${eligible.map(_.ref)}") + /** Try to typecheck an implicit reference */ def typedImplicit(cand: Candidate)(implicit ctx: Context): SearchResult = track("typedImplicit") { trace(i"typed implicit ${cand.ref}, pt = $pt, implicitsEnabled == ${ctx.mode is ImplicitsEnabled}", implicits, show = true) { assert(constr eq ctx.typerState.constraint) @@ -790,7 +798,7 @@ trait Implicits { self: Typer => val generated1 = adapt(generated, pt) lazy val shadowing = typed(untpd.Ident(ref.name) withPos pos.toSynthetic, funProto)( - nestedContext.addMode(Mode.ImplicitShadowing).setExploreTyperState()) + nestedContext().addMode(Mode.ImplicitShadowing).setExploreTyperState()) def refSameAs(shadowing: Tree): Boolean = ref.symbol == closureBody(shadowing).symbol || { shadowing match { @@ -820,6 +828,126 @@ trait Implicits { self: Typer => SearchSuccess(generated1, ref, cand.level)(ctx.typerState) }} + def tryImplicit(cand: Candidate): SearchResult = { + val history = ctx.searchHistory nest wildProto + if (history eq ctx.searchHistory) + SearchFailure(new DivergingImplicit(cand.ref, pt, argument)) + else + typedImplicit(cand)(nestedContext().setNewTyperState().setSearchHistory(history)) + } + + def tryInOrder(cand1: Candidate, cand2: Candidate) = + tryImplicit(cand1) match { + case success: SearchSuccess => success + case _ => tryImplicit(cand2) + } + + def compareCandidate(prev: SearchSuccess, ref: TermRef, level: Int): Int = + if (prev.ref eq ref) 0 + else ctx.typerState.test(compare(prev.ref, ref, prev.level, level)(nestedContext())) + + def disambiguate(alt1: SearchResult, alt2: SearchSuccess) = alt1 match { + case alt1: SearchSuccess => + def isNumeric(tp: Type) = tp.typeSymbol.isNumericValueClass + def isProperSubType(tp1: Type, tp2: Type) = + tp1.isValueSubType(tp2) && !tp2.isValueSubType(tp1) + val rpt = pt.resultType + val rt1 = alt1.ref.widen.resultType + val rt2 = alt2.ref.widen.resultType + def ambi = SearchFailure(new AmbiguousImplicits(alt1, alt2, pt, argument)) + val diff = compareCandidate(alt1, alt2.ref, alt2.level) + assert(diff <= 0) + if (diff < 0) alt2 + else if (isNumeric(rpt) && isNumeric(rt1) && isNumeric(rt2)) + if (isProperSubType(rt1, rt2)) alt2 + else if (isProperSubType(rt2, rt1)) alt1 + else ambi + else ambi + case _: SearchFailure => alt2 + } + + def rank(pending: List[Candidate], found: SearchResult, rfailures: List[SearchFailure]): SearchResult = pending match { + case cand :: pending1 => + tryImplicit(cand) match { + case fail: SearchFailure => + if (fail.isAmbiguous) healAmbiguous(pending1, fail) + else rank(pending1, found, fail :: rfailures) + case best: SearchSuccess => + if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) + best + else disambiguate(found, best) match { + case retained: SearchSuccess => + val newPending = + if (retained eq found) pending1 + else pending1.filter(cand => + compareCandidate(retained, cand.ref, cand.level) <= 0) + rank(newPending, retained, rfailures) + case fail: SearchFailure => + healAmbiguous(pending1, fail) + } + } + case nil => + if (rfailures.isEmpty) found + else found.orElse(rfailures.reverse.maxBy(_.tree.treeSize)) + } + + def healAmbiguous(pending: List[Candidate], fail: SearchFailure) = { + val ambi = fail.reason.asInstanceOf[AmbiguousImplicits] + val newPending = pending.filter(cand => + compareCandidate(ambi.alt1, cand.ref, cand.level) < 0 && + compareCandidate(ambi.alt2, cand.ref, cand.level) < 0) + rank(newPending, fail, Nil).orElse(fail) + } + + def go: SearchResult = eligible match { + case Nil => + SearchFailure(new NoMatchingImplicits(pt, argument)) + case cand :: Nil => + tryImplicit(cand) + case cand1 :: cand2 :: Nil => + def compareTwo = cmpCandidates(cand1, cand2) match { + case 1 => tryInOrder(cand1, cand2) + case -1 => tryInOrder(cand2, cand1) + case 0 => + val alt1 = tryImplicit(cand1) + val alt2 = tryImplicit(cand2) + alt2 match { + case alt2: SearchSuccess => disambiguate(alt1, alt2) + case _ => + alt1 match { + case alt1: SearchSuccess => alt1 + case _ => if (alt1.tree.treeSize < alt2.tree.treeSize) alt2 else alt1 + } + } + } + compareTwo + case _ => + val cands = eligible.toArray + val pg = new PriorityGraph(cands, cmpCandidates) + + def loop(found: SearchResult, rfailures: List[SearchFailure]): SearchResult = + if (pg.hasNextSource()) + tryImplicit(cands(pg.nextSource())) match { + case fail: SearchFailure => + if (fail.isAmbiguous) fail + else { + pg.dropLastSource() + loop(found, fail :: rfailures) + } + case best: SearchSuccess => + if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) + best + else disambiguate(found, best) match { + case retained: SearchSuccess => loop(retained, rfailures) + case ambi => ambi + } + } + else if (found.isInstanceOf[SearchSuccess]) found + else rfailures.reverse.maxBy(_.tree.treeSize) + + loop(NoMatchingImplicitsFailure, Nil) + } + /** Given a list of implicit references, produce a list of search results, * which is either a list of successes or a list of failures. * - if one of the references produces an ambiguity error, return it @@ -842,7 +970,7 @@ trait Implicits { self: Typer => if (history eq ctx.searchHistory) SearchFailure(new DivergingImplicit(cand.ref, pt, argument)) else - typedImplicit(cand)(nestedContext.setNewTyperState().setSearchHistory(history)) + typedImplicit(cand)(nestedContext().setNewTyperState().setSearchHistory(history)) result match { case fail: SearchFailure => if (fail.isAmbiguous) fail :: Nil @@ -852,7 +980,7 @@ trait Implicits { self: Typer => best :: Nil else { val newPending = pending1.filter(cand1 => - ctx.typerState.test(isAsGood(cand1.ref, best.ref, cand1.level, best.level)(nestedContext))) + ctx.typerState.test(isAsGood(cand1.ref, best.ref, cand1.level, best.level)(nestedContext()))) rankImplicits(newPending, best :: successes, rfailures) } } @@ -901,7 +1029,7 @@ trait Implicits { self: Typer => case eliminated: SearchSuccess => condense(results.filter(_ ne eliminated)) case _ => - SearchFailure(new AmbiguousImplicits(best.ref, alt.ref, pt, argument)) + SearchFailure(new AmbiguousImplicits(best, alt, pt, argument)) } case None => ctx.runInfo.useCount(best.ref) += 1 @@ -920,21 +1048,20 @@ trait Implicits { self: Typer => * `cand1` has been selected as an implicit more often than `cand2`. */ def prefer(cand1: Candidate, cand2: Candidate): Boolean = { - /*val sym1 = cand1.ref.symbol + val level1 = cand1.level + val level2 = cand2.level + if (level1 > level2) return true + if (level1 < level2) return false + val sym1 = cand1.ref.symbol val sym2 = cand2.ref.symbol - if (sym1.associatedFile == sym2.associatedFile) { - val coord1 = sym1.coord - val coord2 = sym2.coord - if (coord1.isPosition && coord2.isPosition) { - val pos1 = coord1.toPosition - val pos2 = coord2.toPosition - return pos1.exists && (!pos2.exists || pos1.start < pos2.start) - } - if (coord1.isIndex && coord2.isIndex) - return coord1.toIndex < coord2.toIndex - } - */ - ranking(cand1) < ranking(cand2) + val ownerScore = compareOwner(sym1.maybeOwner, sym2.maybeOwner) + if (ownerScore > 0) return true + if (ownerScore < 0) return false + val arity1 = sym1.info.firstParamTypes.length + val arity2 = sym2.info.firstParamTypes.length + if (arity1 < arity2) return true + if (arity1 > arity2) return false + false } /** Sort list of implicit references according to their popularity @@ -946,10 +1073,13 @@ trait Implicits { self: Typer => case e1 :: e2 :: Nil => if (prefer(e2, e1)) e2 :: e1 :: Nil else eligible - case _ => eligible.sortWith(prefer) + case _ => + eligible.sortWith(prefer) } - condense(rankImplicits(sort(eligible), Nil, Nil)) + if (true) rank(sort(eligible), NoMatchingImplicitsFailure, Nil) + else if (true) go + else condense(rankImplicits(sort(eligible), Nil, Nil)) } /** Find a unique best implicit reference */ diff --git a/compiler/src/dotty/tools/dotc/util/PriorityGraph.scala b/compiler/src/dotty/tools/dotc/util/PriorityGraph.scala new file mode 100644 index 000000000000..33d06ca402ef --- /dev/null +++ b/compiler/src/dotty/tools/dotc/util/PriorityGraph.scala @@ -0,0 +1,96 @@ +package dotty.tools.dotc.util + +class PriorityGraph[T <: AnyRef](elems: Array[T], cmp: (T, T) => Int) { + private val N = elems.length + + private val edge = Array.ofDim[Boolean](N, N) + + private def compare(i: Int, j: Int): Int = { + var k = 0 + while (k < N) { + if (edge(i)(k) && edge(k)(j)) return 1 + k += 1 + } + k = 0 + while (k < N) { + if (edge(j)(k) && edge(k)(i)) return -1 + k += 1 + } + cmp(elems(i), elems(j)) + } + + /** Construct `edge` array such that + * + * edge(i)(j) == i != j && cmp(elems(i), elems(j)) + * + * Make use of the fact that `cmp` is transitive to avoid evaluations + * of `cmp` (which is supposed to be expensive). + */ + private def build() = + for (i <- 0 until N) + for (j <- 0 until N) + if (i != j) + compare(i, j) match { + case 1 => edge(i)(j) = true + case -1 => edge(j)(i) = true + case 0 => + } + + /** Reduce `edge` array to sparsest array that has + * the same transitive closure as the original `edge` array. + */ + def reduce() = + for (i <- 0 until N) + for (j <- 0 until N) { + var k = 0 + while (k < N && !(edge(i)(k) && edge(k)(j))) + k += 1 + if (k < N) edge(i)(j) = false + } + + println(s"building ${elems.length} ${elems.toList}") + build() + println(s"reducing ${elems.toList}") + reduce() + println(s"starting ${elems.toList}") + + private val indegree = { + val deg = new Array[Int](N) + for (i <- 0 until N) { + var d = 0 + for (j <- 0 until N) + if (edge(j)(i)) d += 1 + deg(i) = d + } + deg + } + + private val curSources = new Array[Int](N) + private var firstSource = 0 + private var lastSource = 0 + + private def considerAsSource(n: Int) = + if (indegree(n) == 0) { + curSources(lastSource) = n + lastSource += 1 + } + + for (i <- 0 until N) considerAsSource(i) + + def hasNextSource() = firstSource < lastSource + + def nextSource(): Int = { + val n = curSources(firstSource) + firstSource += 1 + n + } + + def dropLastSource() = { + val i = firstSource - 1 + for (j <- 0 until N) + if (edge(i)(j)) { + indegree(j) -= 1 + considerAsSource(j) + } + } +} \ No newline at end of file From 2270899240040e520d34b819c5d82640259bca5c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 6 Nov 2017 21:33:55 +0100 Subject: [PATCH 14/30] New scheme for handling implicits - Merge `rankImplicits` and `condense` into one phase - Fix handling of ambiguities - the previous unconditional abort was wrong, because another alternative might be better than both ambiguous choices. --- .../dotty/tools/dotc/typer/Implicits.scala | 211 +++--------------- .../dotty/tools/dotc/util/PriorityGraph.scala | 96 -------- 2 files changed, 27 insertions(+), 280 deletions(-) delete mode 100644 compiler/src/dotty/tools/dotc/util/PriorityGraph.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index cd32d4261a79..264fd06502fa 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -257,9 +257,9 @@ object Implicits { sealed abstract class SearchResult extends Showable { def tree: tpd.Tree def toText(printer: Printer): Text = printer.toText(this) - def orElse(other: => SearchResult) = this match { + def recoverWith(other: SearchFailure => SearchResult) = this match { case _: SearchSuccess => this - case _ => other + case fail: SearchFailure => other(fail) } } @@ -484,14 +484,8 @@ trait ImplicitRunInfo { self: RunInfo => iscope(rootTp) } - /** A map that counts the number of times an implicit ref was picked */ - val useCount = new mutable.HashMap[TermRef, Int] { - override def default(key: TermRef) = 0 - } - def clear() = { implicitScopeCache.clear() - useCount.clear() } } @@ -836,12 +830,6 @@ trait Implicits { self: Typer => typedImplicit(cand)(nestedContext().setNewTyperState().setSearchHistory(history)) } - def tryInOrder(cand1: Candidate, cand2: Candidate) = - tryImplicit(cand1) match { - case success: SearchSuccess => success - case _ => tryImplicit(cand2) - } - def compareCandidate(prev: SearchSuccess, ref: TermRef, level: Int): Int = if (prev.ref eq ref) 0 else ctx.typerState.test(compare(prev.ref, ref, prev.level, level)(nestedContext())) @@ -866,6 +854,14 @@ trait Implicits { self: Typer => case _: SearchFailure => alt2 } + def healAmbiguous(pending: List[Candidate], fail: SearchFailure) = { + val ambi = fail.reason.asInstanceOf[AmbiguousImplicits] + val newPending = pending.filter(cand => + compareCandidate(ambi.alt1, cand.ref, cand.level) < 0 && + compareCandidate(ambi.alt2, cand.ref, cand.level) < 0) + rank(newPending, fail, Nil).recoverWith(_ => fail) + } + def rank(pending: List[Candidate], found: SearchResult, rfailures: List[SearchFailure]): SearchResult = pending match { case cand :: pending1 => tryImplicit(cand) match { @@ -888,164 +884,17 @@ trait Implicits { self: Typer => } case nil => if (rfailures.isEmpty) found - else found.orElse(rfailures.reverse.maxBy(_.tree.treeSize)) - } - - def healAmbiguous(pending: List[Candidate], fail: SearchFailure) = { - val ambi = fail.reason.asInstanceOf[AmbiguousImplicits] - val newPending = pending.filter(cand => - compareCandidate(ambi.alt1, cand.ref, cand.level) < 0 && - compareCandidate(ambi.alt2, cand.ref, cand.level) < 0) - rank(newPending, fail, Nil).orElse(fail) - } - - def go: SearchResult = eligible match { - case Nil => - SearchFailure(new NoMatchingImplicits(pt, argument)) - case cand :: Nil => - tryImplicit(cand) - case cand1 :: cand2 :: Nil => - def compareTwo = cmpCandidates(cand1, cand2) match { - case 1 => tryInOrder(cand1, cand2) - case -1 => tryInOrder(cand2, cand1) - case 0 => - val alt1 = tryImplicit(cand1) - val alt2 = tryImplicit(cand2) - alt2 match { - case alt2: SearchSuccess => disambiguate(alt1, alt2) - case _ => - alt1 match { - case alt1: SearchSuccess => alt1 - case _ => if (alt1.tree.treeSize < alt2.tree.treeSize) alt2 else alt1 - } - } - } - compareTwo - case _ => - val cands = eligible.toArray - val pg = new PriorityGraph(cands, cmpCandidates) - - def loop(found: SearchResult, rfailures: List[SearchFailure]): SearchResult = - if (pg.hasNextSource()) - tryImplicit(cands(pg.nextSource())) match { - case fail: SearchFailure => - if (fail.isAmbiguous) fail - else { - pg.dropLastSource() - loop(found, fail :: rfailures) - } - case best: SearchSuccess => - if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) - best - else disambiguate(found, best) match { - case retained: SearchSuccess => loop(retained, rfailures) - case ambi => ambi - } - } - else if (found.isInstanceOf[SearchSuccess]) found - else rfailures.reverse.maxBy(_.tree.treeSize) - - loop(NoMatchingImplicitsFailure, Nil) - } - - /** Given a list of implicit references, produce a list of search results, - * which is either a list of successes or a list of failures. - * - if one of the references produces an ambiguity error, return it - * as only element - * - if some of the references produce successful searches, return those that do - * - otherwise return a list of all failures - * - * If mode is ImplicitExploration or we assume coherence, stop at first - * succesfull search. - * - * @param pending The list of implicit references that remain to be investigated - */ - def rankImplicits(pending: List[Candidate], - successes: List[SearchSuccess], - rfailures: List[SearchFailure]): List[SearchResult] = - pending match { - case cand :: pending1 => - val history = ctx.searchHistory nest wildProto - val result = - if (history eq ctx.searchHistory) - SearchFailure(new DivergingImplicit(cand.ref, pt, argument)) - else - typedImplicit(cand)(nestedContext().setNewTyperState().setSearchHistory(history)) - result match { - case fail: SearchFailure => - if (fail.isAmbiguous) fail :: Nil - else rankImplicits(pending1, successes, fail :: rfailures) - case best: SearchSuccess => - if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) - best :: Nil - else { - val newPending = pending1.filter(cand1 => - ctx.typerState.test(isAsGood(cand1.ref, best.ref, cand1.level, best.level)(nestedContext()))) - rankImplicits(newPending, best :: successes, rfailures) - } - } - case nil => - if (successes.nonEmpty) successes else rfailures.reverse - } - - /** If the (result types of) the expected type, and both alternatives - * are all numeric value types, return the alternative which has - * the smaller numeric subtype as result type, if it exists. - * (This alternative is then discarded). - */ - def numericValueTieBreak(alt1: SearchSuccess, alt2: SearchSuccess): SearchResult = { - def isNumeric(tp: Type) = tp.typeSymbol.isNumericValueClass - def isProperSubType(tp1: Type, tp2: Type) = - tp1.isValueSubType(tp2) && !tp2.isValueSubType(tp1) - val rpt = pt.resultType - val rt1 = alt1.ref.widen.resultType - val rt2 = alt2.ref.widen.resultType - if (isNumeric(rpt) && isNumeric(rt1) && isNumeric(rt2)) - if (isProperSubType(rt1, rt2)) alt1 - else if (isProperSubType(rt2, rt1)) alt2 - else NoMatchingImplicitsFailure - else NoMatchingImplicitsFailure + else found.recoverWith(_ => rfailures.reverse.maxBy(_.tree.treeSize)) } - /** Convert a (possibly empty) list of search results into a single search result - * - if the list consists of one or more successes - * - if a following success is as good as the first one, issue an ambiguity error - * - otherwise return the first success - * - if the list consists of one or more failues, pick the failure with the largest - * associated tree. - * - if the list is empty, issue a "no matching implicits" error. - */ - def condense(results: List[SearchResult]): SearchResult = results match { - case (best: SearchSuccess) :: (alts: List[SearchSuccess] @ unchecked) => - alts.find(alt => - ctx.typerState.test(isAsGood(alt.ref, best.ref, alt.level, best.level))) match { - case Some(alt) => - implicits.println(i"ambiguous implicits for $pt: ${best.ref} @ ${best.level}, ${alt.ref} @ ${alt.level}") - /* !!! DEBUG - println(i"ambiguous refs: ${hits map (_.ref) map (_.show) mkString ", "}") - isAsGood(best.ref, alt.ref, explain = true)(ctx.fresh.withExploreTyperState) - */ - numericValueTieBreak(best, alt) match { - case eliminated: SearchSuccess => - condense(results.filter(_ ne eliminated)) - case _ => - SearchFailure(new AmbiguousImplicits(best, alt, pt, argument)) - } - case None => - ctx.runInfo.useCount(best.ref) += 1 - best - } - case (fail: SearchFailure) :: _ => - results.maxBy(_.tree.treeSize) - case Nil => - SearchFailure(new NoMatchingImplicits(pt, argument)) - } - - def ranking(cand: Candidate) = -ctx.runInfo.useCount(cand.ref) - - /** Prefer `cand1` over `cand2` if they are in the same compilation unit - * and `cand1` is defined before `cand2`, or they are in different units and - * `cand1` has been selected as an implicit more often than `cand2`. + /** A relation that imfluences the order in which implicits are tried. + * We prefer (in order of importance) + * 1. more deeply nested definitions + * 2. definitions in subclasses + * 3. definitions with fewer implicit parameters + * The reason for (3) is that we want to fail fast if the search type + * is underconstrained. So we look for "small" goals first, because that + * will give an ambiguity quickly. */ def prefer(cand1: Candidate, cand2: Candidate): Boolean = { val level1 = cand1.level @@ -1064,9 +913,7 @@ trait Implicits { self: Typer => false } - /** Sort list of implicit references according to their popularity - * (# of times each was picked in current run). - */ + /** Sort list of implicit references according to `prefer` */ def sort(eligible: List[Candidate]) = eligible match { case Nil => eligible case e1 :: Nil => eligible @@ -1077,23 +924,19 @@ trait Implicits { self: Typer => eligible.sortWith(prefer) } - if (true) rank(sort(eligible), NoMatchingImplicitsFailure, Nil) - else if (true) go - else condense(rankImplicits(sort(eligible), Nil, Nil)) - } + rank(sort(eligible), NoMatchingImplicitsFailure, Nil) + } // end searchImplicits /** Find a unique best implicit reference */ def bestImplicit(contextual: Boolean): SearchResult = { val eligible = if (contextual) ctx.implicits.eligible(wildProto) else implicitScope(wildProto).eligible - searchImplicits(eligible, contextual) match { - case success: SearchSuccess => success - case failure: SearchFailure => - failure.reason match { - case (_: AmbiguousImplicits) | (_: DivergingImplicit) => failure - case _ => if (contextual) bestImplicit(contextual = false) else failure - } + searchImplicits(eligible, contextual).recoverWith { + failure => failure.reason match { + case (_: AmbiguousImplicits) | (_: DivergingImplicit) => failure + case _ => if (contextual) bestImplicit(contextual = false) else failure + } } } diff --git a/compiler/src/dotty/tools/dotc/util/PriorityGraph.scala b/compiler/src/dotty/tools/dotc/util/PriorityGraph.scala deleted file mode 100644 index 33d06ca402ef..000000000000 --- a/compiler/src/dotty/tools/dotc/util/PriorityGraph.scala +++ /dev/null @@ -1,96 +0,0 @@ -package dotty.tools.dotc.util - -class PriorityGraph[T <: AnyRef](elems: Array[T], cmp: (T, T) => Int) { - private val N = elems.length - - private val edge = Array.ofDim[Boolean](N, N) - - private def compare(i: Int, j: Int): Int = { - var k = 0 - while (k < N) { - if (edge(i)(k) && edge(k)(j)) return 1 - k += 1 - } - k = 0 - while (k < N) { - if (edge(j)(k) && edge(k)(i)) return -1 - k += 1 - } - cmp(elems(i), elems(j)) - } - - /** Construct `edge` array such that - * - * edge(i)(j) == i != j && cmp(elems(i), elems(j)) - * - * Make use of the fact that `cmp` is transitive to avoid evaluations - * of `cmp` (which is supposed to be expensive). - */ - private def build() = - for (i <- 0 until N) - for (j <- 0 until N) - if (i != j) - compare(i, j) match { - case 1 => edge(i)(j) = true - case -1 => edge(j)(i) = true - case 0 => - } - - /** Reduce `edge` array to sparsest array that has - * the same transitive closure as the original `edge` array. - */ - def reduce() = - for (i <- 0 until N) - for (j <- 0 until N) { - var k = 0 - while (k < N && !(edge(i)(k) && edge(k)(j))) - k += 1 - if (k < N) edge(i)(j) = false - } - - println(s"building ${elems.length} ${elems.toList}") - build() - println(s"reducing ${elems.toList}") - reduce() - println(s"starting ${elems.toList}") - - private val indegree = { - val deg = new Array[Int](N) - for (i <- 0 until N) { - var d = 0 - for (j <- 0 until N) - if (edge(j)(i)) d += 1 - deg(i) = d - } - deg - } - - private val curSources = new Array[Int](N) - private var firstSource = 0 - private var lastSource = 0 - - private def considerAsSource(n: Int) = - if (indegree(n) == 0) { - curSources(lastSource) = n - lastSource += 1 - } - - for (i <- 0 until N) considerAsSource(i) - - def hasNextSource() = firstSource < lastSource - - def nextSource(): Int = { - val n = curSources(firstSource) - firstSource += 1 - n - } - - def dropLastSource() = { - val i = firstSource - 1 - for (j <- 0 until N) - if (edge(i)(j)) { - indegree(j) -= 1 - considerAsSource(j) - } - } -} \ No newline at end of file From 01d8dfbfc42e085137b4f46b7283b3270735eaf2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 6 Nov 2017 21:43:51 +0100 Subject: [PATCH 15/30] Cleanups --- compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Applications.scala | 3 --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index e1d0e99ac9ff..1689ac392031 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -505,7 +505,7 @@ class PlainPrinter(_ctx: Context) extends Printer { case _: DivergingImplicit => "Diverging Implicit" case _: ShadowedImplicit => "Shadowed Implicit" case result: AmbiguousImplicits => - "Ambiguous Implicit: " ~ toText(result.alt1) ~ " and " ~ toText(result.alt2) + "Ambiguous Implicit: " ~ toText(result.alt1.ref) ~ " and " ~ toText(result.alt2.ref) case _ => "?Unknown Implicit Result?" + result.getClass } diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index addfea8e34b4..6bbf528c6fcb 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1195,9 +1195,6 @@ trait Applications extends Compatibility { self: Typer with Dynamic => if (winsType2) -1 else 0 }} - def isAsGood(alt1: TermRef, alt2: TermRef, nesting1: Int = 0, nesting2: Int = 0)(implicit ctx: Context): Boolean = - compare(alt1, alt2, nesting1, nesting2) >= 0 - def narrowMostSpecific(alts: List[TermRef])(implicit ctx: Context): List[TermRef] = track("narrowMostSpecific") { alts match { case Nil => alts diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 264fd06502fa..ce49a2aa1bd6 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -30,7 +30,7 @@ import reporting.diagnostic.{Message, MessageContainer} import Inferencing.fullyDefinedType import Trees._ import Hashable._ -import util.{Property, PriorityGraph} +import util.Property import config.Config import config.Printers.{implicits, implicitsDetailed, typr} import collection.mutable diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index ffd945d18671..1c46f2c574cc 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2267,7 +2267,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit case SearchSuccess(inferred, _, _) => adapt(inferred, pt)(ctx.retractMode(Mode.ImplicitsEnabled)) case failure: SearchFailure => - if (pt.isInstanceOf[ProtoType] && !failure.reason.isInstanceOf[AmbiguousImplicits]) + if (pt.isInstanceOf[ProtoType] && !failure.isAmbiguous) // don't report the failure but return the tree unchanged. This // will cause a failure at the next level out, which usually gives // a better error message. From 83f86ab0d8d81843d1233c16ad93a43c5359eec6 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 7 Nov 2017 00:36:26 +0100 Subject: [PATCH 16/30] Fix doc comment --- compiler/src/dotty/tools/dotc/typer/Applications.scala | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 6bbf528c6fcb..3e843e7b8693 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1047,16 +1047,18 @@ trait Applications extends Compatibility { self: Typer with Dynamic => else if (sym1 is Module) compareOwner(sym1.companionClass, sym2) else 0 - /** In a set of overloaded applicable alternatives, is `alt1` at least as good as - * `alt2`? Also used for implicits disambiguation. + /** Compare to alternatives of an overloaded call or an implicit search. forIn a set of overloaded applicable alternatives, is `alt1` at least as good as * * @param alt1, alt2 Non-overloaded references indicating the two choices * @param level1, level2 If alternatives come from a comparison of two contextual * implicit candidates, the nesting levels of the candidates. * In all other cases the nesting levels are both 0. + * @return 1 if 1st alternative is preferred over 2nd + * -1 if 2nd alternative is preferred over 1st + * 0 if neither alternative is preferred over the other * - * An alternative A1 is "as good as" an alternative A2 if it wins or draws in a tournament - * that awards one point for each of the following + * An alternative A1 is preferred over an alternative A2 if it wins in a tournament + * that awards one point for each of the following: * * - A1 is nested more deeply than A2 * - The nesting levels of A1 and A2 are the same, and A1's owner derives from A2's owner From 2df80313e8029227ec0d0443638ccc34f06c99c5 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 7 Nov 2017 00:38:35 +0100 Subject: [PATCH 17/30] Fix test comment --- tests/run/iterator-from.scala | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/run/iterator-from.scala b/tests/run/iterator-from.scala index c5d016423b41..1f73a7ffe8fb 100644 --- a/tests/run/iterator-from.scala +++ b/tests/run/iterator-from.scala @@ -64,10 +64,7 @@ object Test extends dotty.runtime.LegacyApp { testSet(mutable.TreeSet(keys:_*), keys) val days = keys map {n => Weekday(n % Weekday.values.size)} - // The following does not work with the implicit change that propagates nested ambiguous errors to the top. - // We get ambiguous implicits because Predef.$conforms and convertIfView both fit WeekDay.Value => Ordered[WeekDay.Value]. - // It does not work in scalac either; there we get a divergent implicit. - testSet(Weekday.ValueSet(days:_*), days) + testSet(Weekday.ValueSet(days:_*), days) // Note: produces divergent search in scalac val treeMap = immutable.TreeMap(keyValues:_*) testMap(treeMap, keyValues) From 50bf4629cc5a2f40cbb14c62f3e11210b84ee2cd Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 7 Nov 2017 00:40:07 +0100 Subject: [PATCH 18/30] Remove stray text in comment --- compiler/src/dotty/tools/dotc/typer/Applications.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 3e843e7b8693..e1a3cd830221 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1047,7 +1047,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic => else if (sym1 is Module) compareOwner(sym1.companionClass, sym2) else 0 - /** Compare to alternatives of an overloaded call or an implicit search. forIn a set of overloaded applicable alternatives, is `alt1` at least as good as + /** Compare to alternatives of an overloaded call or an implicit search. * * @param alt1, alt2 Non-overloaded references indicating the two choices * @param level1, level2 If alternatives come from a comparison of two contextual From ebb4ba2205f8f3f3093f4e5b278be4e84a81a220 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 7 Nov 2017 15:53:18 +0100 Subject: [PATCH 19/30] Avoid repeated `tpd.` prefixes in Implicits.scala --- .../dotty/tools/dotc/typer/Implicits.scala | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index ce49a2aa1bd6..7f8c9ca5e04b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -38,6 +38,7 @@ import reporting.trace /** Implicit resolution */ object Implicits { + import tpd._ /** A reference to an implicit value to be made visible on the next nested call to * inferImplicitArg with a by-name expected type. @@ -255,7 +256,7 @@ object Implicits { /** The result of an implicit search */ sealed abstract class SearchResult extends Showable { - def tree: tpd.Tree + def tree: Tree def toText(printer: Printer): Text = printer.toText(this) def recoverWith(other: SearchFailure => SearchResult) = this match { case _: SearchSuccess => this @@ -269,10 +270,10 @@ object Implicits { * @param level The level where the reference was found * @param tstate The typer state to be committed if this alternative is chosen */ - case class SearchSuccess(tree: tpd.Tree, ref: TermRef, level: Int)(val tstate: TyperState) extends SearchResult with Showable + case class SearchSuccess(tree: Tree, ref: TermRef, level: Int)(val tstate: TyperState) extends SearchResult with Showable /** A failed search */ - case class SearchFailure(tree: tpd.Tree) extends SearchResult { + case class SearchFailure(tree: Tree) extends SearchResult { final def isAmbiguous = tree.tpe.isInstanceOf[AmbiguousImplicits] final def reason = tree.tpe.asInstanceOf[SearchFailureType] } @@ -288,7 +289,7 @@ object Implicits { abstract class SearchFailureType extends ErrorType { def expectedType: Type - protected def argument: tpd.Tree + protected def argument: Tree final protected def qualify(implicit ctx: Context) = if (expectedType.exists) @@ -309,18 +310,18 @@ object Implicits { def whyNoConversion(implicit ctx: Context): String = "" } - class NoMatchingImplicits(val expectedType: Type, val argument: tpd.Tree) extends SearchFailureType { + class NoMatchingImplicits(val expectedType: Type, val argument: Tree) extends SearchFailureType { def explanation(implicit ctx: Context): String = em"no implicit values were found that $qualify" } - @sharable object NoMatchingImplicits extends NoMatchingImplicits(NoType, tpd.EmptyTree) + @sharable object NoMatchingImplicits extends NoMatchingImplicits(NoType, EmptyTree) @sharable val NoMatchingImplicitsFailure: SearchFailure = SearchFailure(NoMatchingImplicits) /** An ambiguous implicits failure */ - class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: tpd.Tree) extends SearchFailureType { + class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree) extends SearchFailureType { def explanation(implicit ctx: Context): String = em"both ${err.refStr(alt1.ref)} and ${err.refStr(alt2.ref)} $qualify" override def whyNoConversion(implicit ctx: Context) = @@ -330,7 +331,7 @@ object Implicits { class MismatchedImplicit(ref: TermRef, val expectedType: Type, - val argument: tpd.Tree) extends SearchFailureType { + val argument: Tree) extends SearchFailureType { def explanation(implicit ctx: Context): String = em"${err.refStr(ref)} does not $qualify" } @@ -338,14 +339,14 @@ object Implicits { class ShadowedImplicit(ref: TermRef, shadowing: Type, val expectedType: Type, - val argument: tpd.Tree) extends SearchFailureType { + val argument: Tree) extends SearchFailureType { def explanation(implicit ctx: Context): String = em"${err.refStr(ref)} does $qualify but is shadowed by ${err.refStr(shadowing)}" } class DivergingImplicit(ref: TermRef, val expectedType: Type, - val argument: tpd.Tree) extends SearchFailureType { + val argument: Tree) extends SearchFailureType { def explanation(implicit ctx: Context): String = em"${err.refStr(ref)} produces a diverging implicit search when trying to $qualify" } @@ -640,7 +641,7 @@ trait Implicits { self: Typer => arg } - def missingArgMsg(arg: tpd.Tree, pt: Type, where: String)(implicit ctx: Context): String = { + def missingArgMsg(arg: Tree, pt: Type, where: String)(implicit ctx: Context): String = { def msg(shortForm: String)(headline: String = shortForm) = arg match { case arg: Trees.SearchFailureIdent[_] => shortForm @@ -913,7 +914,10 @@ trait Implicits { self: Typer => false } - /** Sort list of implicit references according to `prefer` */ + /** Sort list of implicit references according to `prefer`. + * This is just an optimization that aims at reducing the average + * number of candidates to be tested. + */ def sort(eligible: List[Candidate]) = eligible match { case Nil => eligible case e1 :: Nil => eligible From 15744fbecd85b2c947ba6b0679802d6e44fc740e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 7 Nov 2017 17:33:24 +0100 Subject: [PATCH 20/30] Add some doc comments --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 7f8c9ca5e04b..e083c87129f4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -823,6 +823,9 @@ trait Implicits { self: Typer => SearchSuccess(generated1, ref, cand.level)(ctx.typerState) }} + /** Try to type-check implicit reference, after checking that this is not + * a diverging search + */ def tryImplicit(cand: Candidate): SearchResult = { val history = ctx.searchHistory nest wildProto if (history eq ctx.searchHistory) @@ -831,10 +834,18 @@ trait Implicits { self: Typer => typedImplicit(cand)(nestedContext().setNewTyperState().setSearchHistory(history)) } + /** Compare previous success with reference and level to determine which one would be chosen, if + * an implicit starting with the reference was found. + */ def compareCandidate(prev: SearchSuccess, ref: TermRef, level: Int): Int = if (prev.ref eq ref) 0 else ctx.typerState.test(compare(prev.ref, ref, prev.level, level)(nestedContext())) + /** If `alt1` is also a search success, try to disambiguate as follows: + * - If alt2 is preferred over alt1, pick alt2. + * - Otherwise, if both alternatives return numeric value types, pick the one + * with the larger type. + */ def disambiguate(alt1: SearchResult, alt2: SearchSuccess) = alt1 match { case alt1: SearchSuccess => def isNumeric(tp: Type) = tp.typeSymbol.isNumericValueClass From 0e8a74d22d9b312cb5b7dfd416e9adc266413d89 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 7 Nov 2017 18:22:54 +0100 Subject: [PATCH 21/30] Disable numeric value tie break All tests pass, and it is unclear what this is trying to achieve. --- .../dotty/tools/dotc/typer/Implicits.scala | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index e083c87129f4..686af3d1de97 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -841,28 +841,34 @@ trait Implicits { self: Typer => if (prev.ref eq ref) 0 else ctx.typerState.test(compare(prev.ref, ref, prev.level, level)(nestedContext())) + /* Seems we don't need this anymore. + def numericValueTieBreak(alt1: SearchSuccess, alt2: SearchSuccess) = { + def isNumeric(tp: Type) = tp.typeSymbol.isNumericValueClass + def isProperSubType(tp1: Type, tp2: Type) = + tp1.isValueSubType(tp2) && !tp2.isValueSubType(tp1) + val rpt = pt.resultType + val rt1 = alt1.ref.widen.resultType + val rt2 = alt2.ref.widen.resultType + if (isNumeric(rpt) && isNumeric(rt1) && isNumeric(rt2)) + if (isProperSubType(rt1, rt2)) alt2 + else if (isProperSubType(rt2, rt1)) alt1 + else NoMatchingImplicitsFailure + else NoMatchingImplicitsFailure + } + */ + /** If `alt1` is also a search success, try to disambiguate as follows: - * - If alt2 is preferred over alt1, pick alt2. - * - Otherwise, if both alternatives return numeric value types, pick the one - * with the larger type. + * - If alt2 is preferred over alt1, pick alt2, otherwise return an + * ambiguous implicits error. */ def disambiguate(alt1: SearchResult, alt2: SearchSuccess) = alt1 match { case alt1: SearchSuccess => - def isNumeric(tp: Type) = tp.typeSymbol.isNumericValueClass - def isProperSubType(tp1: Type, tp2: Type) = - tp1.isValueSubType(tp2) && !tp2.isValueSubType(tp1) - val rpt = pt.resultType - val rt1 = alt1.ref.widen.resultType - val rt2 = alt2.ref.widen.resultType - def ambi = SearchFailure(new AmbiguousImplicits(alt1, alt2, pt, argument)) val diff = compareCandidate(alt1, alt2.ref, alt2.level) assert(diff <= 0) if (diff < 0) alt2 - else if (isNumeric(rpt) && isNumeric(rt1) && isNumeric(rt2)) - if (isProperSubType(rt1, rt2)) alt2 - else if (isProperSubType(rt2, rt1)) alt1 - else ambi - else ambi + else + // numericValueTypeBreak(alt1, alt2) recoverWith + SearchFailure(new AmbiguousImplicits(alt1, alt2, pt, argument)) case _: SearchFailure => alt2 } From 9c1d942f2ace22f93260c1a78cceeb502b6e8965 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 7 Nov 2017 18:43:48 +0100 Subject: [PATCH 22/30] New test exercising implicit function types --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 12 ++++++++++++ compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 686af3d1de97..6a58c02cf042 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -673,6 +673,18 @@ trait Implicits { self: Typer => } } + /** A string indicating the formal parameter corresponding to a missing argument */ + def implicitParamString(paramName: TermName, methodStr: String, tree: Tree)(implicit ctx: Context): String = + tree match { + case Select(qual, nme.apply) if defn.isFunctionType(qual.tpe.widen) => + val qt = qual.tpe.widen + val qt1 = qt.dealias + def addendum = if (qt1 eq qt) "" else (i"\nwhich is an alias of: $qt1") + em"parameter of ${qual.tpe.widen}$addendum" + case _ => + em"parameter ${paramName} of $methodStr" + } + private def assumedCanEqual(ltp: Type, rtp: Type)(implicit ctx: Context) = { def eqNullable: Boolean = { val other = diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 1c46f2c574cc..489c72379160 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2043,7 +2043,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit arg.tpe match { case failure: SearchFailureType => ctx.error( - missingArgMsg(arg, formal, em"parameter ${paramName} of $methodStr"), + missingArgMsg(arg, formal, implicitParamString(paramName, methodStr, tree)), tree.pos.endPos) case _ => } From 0bae8a30e65271e2247ebc3b3a206f984e14c8ed Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 7 Nov 2017 18:47:32 +0100 Subject: [PATCH 23/30] New test exercising implicit function types --- tests/run/config.check | 2 + tests/run/config.scala | 123 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 tests/run/config.check create mode 100644 tests/run/config.scala diff --git a/tests/run/config.check b/tests/run/config.check new file mode 100644 index 000000000000..b20cb76502a7 --- /dev/null +++ b/tests/run/config.check @@ -0,0 +1,2 @@ +Some(Person(Name(John,Doe),Age(20))) +None diff --git a/tests/run/config.scala b/tests/run/config.scala new file mode 100644 index 000000000000..8caa7c2f771e --- /dev/null +++ b/tests/run/config.scala @@ -0,0 +1,123 @@ +case class Name(first: String, last: String) +case class Age(age: Int) +case class Person(name: Name, age: Age) +case class Config(name: String, age: Int) + + + +object Imperative { + import Configs._ + import Exceptions._ + + // Algebraic Effects + + def readName: Possibly[Configured[Name]] = { + val parts = config.name.split(" ") + require(parts.length >= 2) + Name(parts(0), parts.tail.mkString(" ")) + } + + def readAge: Configured[Possibly[Age]] = { + val age = config.age + require(1 <= age && age <= 150) + Age(age) + } + + def readPerson: Configured[Option[Person]] = + attempt( + Some(Person(readName, readAge)) + ).onError(None) + + def main(args: Array[String]) = { + println(readPerson(Config("John Doe", 20))) + println(readPerson(Config("Incognito", 99))) + } +} + +object Configs { + type Configured[T] = implicit Config => T + def config: Configured[Config] = implicitly[Config] +} + +object Exceptions { + + private class E extends Exception + + class CanThrow private[Exceptions] () { + private[Exceptions] def throwE() = throw new E + } + + type Possibly[T] = implicit CanThrow => T + + def require(p: Boolean)(implicit ct: CanThrow): Unit = + if (!p) ct.throwE() + + def attempt[T](op: Possibly[T]) = new OnError(op) + + class OnError[T](op: Possibly[T]) { + def onError(fallback: => T): T = + try op(new CanThrow) + catch { case ex: E => fallback } + } +} + +object Test extends App { + import Configs._ + import Exceptions._ + + type PC[T] = Possibly[Configured[T]] + + val names: PC[List[Name]] = readName :: Nil + val firstNames: PC[List[String]] = names.map(_.first) + val longest: PC[String] = firstNames.maxBy(_.length) + + def readName: Configured[Possibly[Name]] = { + val parts = config.name.split(" ") + require(parts.length >= 2) + Name(parts(0), parts.tail.mkString(" ")) + } + + def readAge: Possibly[Configured[Age]] = { + val age = config.age + require(1 <= age && age <= 150) + Age(age) + } + + def readPerson: Configured[Option[Person]] = + attempt( + Some(Person(readName, readAge)) + ).onError(None) + + val config1 = Config("John Doe", 20) + val config2 = Config("Incognito", 99) + + println(readPerson(config1)) + println(readPerson(config2)) +} + +object OptionTest extends App { + + def readName(config: Config): Option[Name] = { + val parts = config.name.split(" ") + if (parts.length >= 2) Some(Name(parts(0), parts.tail.mkString(" "))) + else None + } + + def readAge(config: Config): Option[Age] = { + val age = config.age + if (1 <= age && age <= 150) Some(Age(age)) else None + } + + def readPerson(config: Config): Option[Person] = + for { + name <- readName(config) + age <- readAge(config) + } + yield Person(name, age) + + val config1 = Config("John Doe", 20) + val config2 = Config("Incognito", 99) + + println(readPerson(config1)) + println(readPerson(config2)) +} From 2144367c2f99e777a08d12ef00b17e980a761625 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 13 Nov 2017 19:15:40 +0100 Subject: [PATCH 24/30] Implement negation of implicit searches Previously, ambiguous implicits were used to make a local implicit search fail so that another alternative could be considered globally. This is no longer possible because implicit search now propagates. This commit makes similar functionality available by implementing a special Not class. It also re-establishes the old behavior under -language:Scala2, issuing a migration warning. --- .../dotty/tools/dotc/core/Definitions.scala | 6 ++++ .../dotty/tools/dotc/typer/Implicits.scala | 36 +++++++++++++++---- library/src/scala/implicits/Not.scala | 9 +++++ tests/pos-scala2/i3396.scala | 34 ++++++++++++++++++ tests/run/i3396.scala | 25 +++++++++++++ 5 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 library/src/scala/implicits/Not.scala create mode 100644 tests/pos-scala2/i3396.scala create mode 100644 tests/run/i3396.scala diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 5e03220467be..bfc44b70bbcb 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -584,6 +584,12 @@ class Definitions { def Eq_eqAny(implicit ctx: Context) = EqModule.requiredMethod(nme.eqAny) + lazy val NotType = ctx.requiredClassRef("scala.implicits.Not") + def NotClass(implicit ctx: Context) = NotType.symbol.asClass + def NotModule(implicit ctx: Context) = NotClass.companionModule + + def Not_value(implicit ctx: Context) = NotModule.requiredMethod(nme.value) + lazy val XMLTopScopeModuleRef = ctx.requiredModuleRef("scala.xml.TopScope") // Annotation base classes diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 6a58c02cf042..149ccd59e005 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -262,6 +262,7 @@ object Implicits { case _: SearchSuccess => this case fail: SearchFailure => other(fail) } + def isSuccess = isInstanceOf[SearchSuccess] } /** A successful search @@ -503,8 +504,7 @@ trait Implicits { self: Typer => && from.isValueType && ( from.isValueSubType(to) || inferView(dummyTreeOfType(from), to) - (ctx.fresh.addMode(Mode.ImplicitExploration).setExploreTyperState()) - .isInstanceOf[SearchSuccess] + (ctx.fresh.addMode(Mode.ImplicitExploration).setExploreTyperState()).isSuccess // TODO: investigate why we can't TyperState#test here ) ) @@ -583,7 +583,7 @@ trait Implicits { self: Typer => } def hasEq(tp: Type): Boolean = - inferImplicit(defn.EqType.appliedTo(tp, tp), EmptyTree, pos).isInstanceOf[SearchSuccess] + inferImplicit(defn.EqType.appliedTo(tp, tp), EmptyTree, pos).isSuccess def validEqAnyArgs(tp1: Type, tp2: Type)(implicit ctx: Context) = { List(tp1, tp2).foreach(fullyDefinedType(_, "eqAny argument", pos)) @@ -787,6 +787,8 @@ trait Implicits { self: Typer => /** The expected type where parameters and uninstantiated typevars are replaced by wildcard types */ val wildProto = implicitProto(pt, wildApprox(_, null, Set.empty)) + val isNot = wildProto.classSymbol == defn.NotClass + /** Search a list of eligible implicit references */ def searchImplicits(eligible: List[Candidate], contextual: Boolean): SearchResult = { val constr = ctx.typerState.constraint @@ -896,10 +898,23 @@ trait Implicits { self: Typer => case cand :: pending1 => tryImplicit(cand) match { case fail: SearchFailure => - if (fail.isAmbiguous) healAmbiguous(pending1, fail) - else rank(pending1, found, fail :: rfailures) + if (isNot) + SearchSuccess(ref(defn.Not_value), defn.Not_value.termRef, 0)(ctx.typerState) + else if (fail.isAmbiguous) + if (ctx.scala2Mode) { + val result = rank(pending1, found, NoMatchingImplicitsFailure :: rfailures) + if (result.isSuccess) + warnAmbiguousNegation(fail.reason.asInstanceOf[AmbiguousImplicits]) + result + } + else + healAmbiguous(pending1, fail) + else + rank(pending1, found, fail :: rfailures) case best: SearchSuccess => - if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) + if (isNot) + NoMatchingImplicitsFailure + else if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) best else disambiguate(found, best) match { case retained: SearchSuccess => @@ -917,6 +932,15 @@ trait Implicits { self: Typer => else found.recoverWith(_ => rfailures.reverse.maxBy(_.tree.treeSize)) } + def warnAmbiguousNegation(ambi: AmbiguousImplicits) = + ctx.migrationWarning( + i"""Ambiguous implicits ${ambi.alt1.ref.symbol.showLocated} and ${ambi.alt2.ref.symbol.showLocated} + |seem to be used to implement a local failure in order to negate an implicit search. + |According to the new implicit resolution rules this is no longer possible; + |the search will fail with a global ambiguity error instead. + | + |Consider using the scala.implicits.Not class to implement similar functionality.""", pos) + /** A relation that imfluences the order in which implicits are tried. * We prefer (in order of importance) * 1. more deeply nested definitions diff --git a/library/src/scala/implicits/Not.scala b/library/src/scala/implicits/Not.scala new file mode 100644 index 000000000000..5722f2c4c6b0 --- /dev/null +++ b/library/src/scala/implicits/Not.scala @@ -0,0 +1,9 @@ +package scala.implicits + +final class Not[+T] private () + +object Not { + def value: Not[Nothing] = new Not[Nothing]() + implicit def amb1[T](implicit ev: T): Not[T] = ??? + implicit def amb2[T](implicit ev: T): Not[T] = ??? +} diff --git a/tests/pos-scala2/i3396.scala b/tests/pos-scala2/i3396.scala new file mode 100644 index 000000000000..5cf3ccba8980 --- /dev/null +++ b/tests/pos-scala2/i3396.scala @@ -0,0 +1,34 @@ +object Test { + + trait Tagged[A] + + // Negation Tagged: NotTagged[A] is available only if there are no Tagged[A] in scope. + trait NotTagged[A] + trait NotTaggedLowPrio { + implicit def notTaggedInstance[A]: NotTagged[A] = null + } + object NotTagged extends NotTaggedLowPrio { + implicit def notTaggedAmbiguity1[A](implicit ev: Tagged[A]): NotTagged[A] = null + implicit def notTaggedAmbiguity2[A](implicit ev: Tagged[A]): NotTagged[A] = null + } + + + case class Foo[A](value: Boolean) + trait FooLowPrio { + implicit def fooDefault[A]: Foo[A] = Foo(true) + } + object Foo extends FooLowPrio { + implicit def fooNotTagged[A](implicit ev: NotTagged[A]): Foo[A] = Foo(false) + } + + + def main(args: Array[String]): Unit = { + implicit val taggedInt: Tagged[Int] = null + + assert(implicitly[Foo[Int]].value) // fooDefault + + assert(!implicitly[Foo[String]].value) // fooNotTagged + + println(1) + } +} diff --git a/tests/run/i3396.scala b/tests/run/i3396.scala new file mode 100644 index 000000000000..fec007591d7a --- /dev/null +++ b/tests/run/i3396.scala @@ -0,0 +1,25 @@ +import implicits.Not + +object Test { + + trait Tagged[A] + + case class Foo[A](value: Boolean) + trait FooLowPrio { + implicit def fooDefault[A]: Foo[A] = Foo(true) + } + object Foo extends FooLowPrio { + implicit def fooNotTagged[A](implicit ev: Not[Tagged[A]]): Foo[A] = Foo(false) + } + + + def main(args: Array[String]): Unit = { + implicit val taggedInt: Tagged[Int] = null + + assert(implicitly[Foo[Int]].value) // fooDefault + + assert(!implicitly[Foo[String]].value) // fooNotTagged + + println(1) + } +} From 03779a12730e3060afdc80da4cd21e8b2f322d56 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 14 Nov 2017 13:54:38 +0100 Subject: [PATCH 25/30] Fixes and documentation for Not class --- library/src/scala/implicits/Not.scala | 38 ++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/library/src/scala/implicits/Not.scala b/library/src/scala/implicits/Not.scala index 5722f2c4c6b0..6c06444145fc 100644 --- a/library/src/scala/implicits/Not.scala +++ b/library/src/scala/implicits/Not.scala @@ -1,9 +1,45 @@ package scala.implicits +/** A special class used to implement negation in implicit search. + * + * Consider the problem of using implicit `i1` for a query type `D` if an implicit + * for some other class `C` is available, and using an implicit `i2` if no implicit + * value of type `C` is available. If we do not want to prioritize `i1` and `i2` by + * putting them in different traits we can instead define the following: + * + * implicit def i1: D(implicit ev: C) = ... + * implicit def i2: D(implicit ev: Not[C]) = ... + * + * `Not` is treated specially in implicit search, similar to the way logical negation + * is treated in Prolog: The implicit search for `Not[C]` succeeds if and only if the implicit + * search for `C` fails. + * + * In Scala 2 this form of negation can be simulated by setting up a conditional + * ambiguous implicit and an unconditional fallback, the way it is done with the + * `default`, `amb1` and `amb2` methods below. Due to the way these two methods are + * defined, `Not` is also usable from Scala 2. + * + * In Dotty, ambiguity is a global error, and therefore cannot be used to implement negation. + * Instead, `Not` is treated natively in implicit search. + */ final class Not[+T] private () -object Not { +trait LowPriorityNot { + + /** A fallback method used to emulate negation in Scala 2 */ + implicit def default[T]: Not[T] = Not.value +} +object Not extends LowPriorityNot { + + /** A value of type `Not` to signal a successful search for `Not[C]` (i.e. a failing + * search for `C`). A reference to this value will be explicitly constructed by Dotty's + * implicit search algorithm + */ def value: Not[Nothing] = new Not[Nothing]() + + /** One of two ambiguous methods used to emulate negation in Scala 2 */ implicit def amb1[T](implicit ev: T): Not[T] = ??? + + /** One of two ambiguous methods used to emulate negation in Scala 2 */ implicit def amb2[T](implicit ev: T): Not[T] = ??? } From 98c4b7db3c804e0634d4940e927fbbd57f84b453 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 14 Nov 2017 17:39:03 +0100 Subject: [PATCH 26/30] Fix handling divergent implicits If we hit a divergent implicit during a contextual search, we should continue with a type-based search. Only ambiguities are globally fatal. The removed test that prevented this logic was introduced when I experimented with fatal divergence. It was left in by oversight. --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 2 +- tests/pos/implicit-divergent.scala | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 tests/pos/implicit-divergent.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 149ccd59e005..d0f09e952f96 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -991,7 +991,7 @@ trait Implicits { self: Typer => else implicitScope(wildProto).eligible searchImplicits(eligible, contextual).recoverWith { failure => failure.reason match { - case (_: AmbiguousImplicits) | (_: DivergingImplicit) => failure + case _: AmbiguousImplicits => failure case _ => if (contextual) bestImplicit(contextual = false) else failure } } diff --git a/tests/pos/implicit-divergent.scala b/tests/pos/implicit-divergent.scala new file mode 100644 index 000000000000..91b561be8bc6 --- /dev/null +++ b/tests/pos/implicit-divergent.scala @@ -0,0 +1,12 @@ +class Foo[T] +object Foo { + implicit def intFoo: Foo[Int] = ??? +} + +object Test { + implicit def genFoo[T](implicit f: Foo[T => T]): Foo[T] = ??? + + def test: Unit = { + implicitly[Foo[Int]] + } +} From 39b4ec2386152dd5f25e7eb459c59bbb5da0882d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 15 Nov 2017 09:39:50 +0100 Subject: [PATCH 27/30] Address reviewers comments --- .../dotty/tools/dotc/typer/Implicits.scala | 102 +++++++++++------- 1 file changed, 65 insertions(+), 37 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index d0f09e952f96..27d09e446aae 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -305,7 +305,7 @@ object Implicits { def msg(implicit ctx: Context): Message = explanation - /** If search was a for an implicit conversion, a note describing the failure + /** If search was for an implicit conversion, a note describing the failure * in more detail - this is either empty or starts with a '\n' */ def whyNoConversion(implicit ctx: Context): String = "" @@ -878,7 +878,7 @@ trait Implicits { self: Typer => def disambiguate(alt1: SearchResult, alt2: SearchSuccess) = alt1 match { case alt1: SearchSuccess => val diff = compareCandidate(alt1, alt2.ref, alt2.level) - assert(diff <= 0) + assert(diff <= 0) // diff > 0 candidates should already have been eliminated in `rank` if (diff < 0) alt2 else // numericValueTypeBreak(alt1, alt2) recoverWith @@ -886,6 +886,10 @@ trait Implicits { self: Typer => case _: SearchFailure => alt2 } + /** Faced with an ambiguous implicits failure `fail`, try to find another + * alternative among `pending` that is strictly better than both ambiguous + * alternatives. If that fails, return `fail` + */ def healAmbiguous(pending: List[Candidate], fail: SearchFailure) = { val ambi = fail.reason.asInstanceOf[AmbiguousImplicits] val newPending = pending.filter(cand => @@ -894,42 +898,66 @@ trait Implicits { self: Typer => rank(newPending, fail, Nil).recoverWith(_ => fail) } - def rank(pending: List[Candidate], found: SearchResult, rfailures: List[SearchFailure]): SearchResult = pending match { - case cand :: pending1 => - tryImplicit(cand) match { - case fail: SearchFailure => - if (isNot) - SearchSuccess(ref(defn.Not_value), defn.Not_value.termRef, 0)(ctx.typerState) - else if (fail.isAmbiguous) - if (ctx.scala2Mode) { - val result = rank(pending1, found, NoMatchingImplicitsFailure :: rfailures) - if (result.isSuccess) - warnAmbiguousNegation(fail.reason.asInstanceOf[AmbiguousImplicits]) - result - } - else - healAmbiguous(pending1, fail) - else - rank(pending1, found, fail :: rfailures) - case best: SearchSuccess => - if (isNot) - NoMatchingImplicitsFailure - else if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) - best - else disambiguate(found, best) match { - case retained: SearchSuccess => - val newPending = - if (retained eq found) pending1 - else pending1.filter(cand => - compareCandidate(retained, cand.ref, cand.level) <= 0) - rank(newPending, retained, rfailures) - case fail: SearchFailure => - healAmbiguous(pending1, fail) + /** Try to find a best matching implicit term among all the candidates in `pending`. + * @param pending The list of candidates that remain to be tested + * @param found The result obtained from previously tried candidates + * @param rfailures A list of all failures from previously tried candidates in reverse order + * + * The scheme is to try candidates one-by-one. If a trial is successful: + * - if the query term is a `Not[T]` treat it a failure, + * - otherwise, if a previous search was also successful, handle the ambiguity + * in `disambiguate`, + * - otherwise, continue the search with all candidates that are not strictly + * worse than the succesful candidate. + * If a trial failed: + * - if the query term is a `Not[T]` treat it as a success, + * - otherwise, if the failure is an ambiguity, try to heal it (see @healAmbiguous) + * and return an ambiguous error otherwise. However, under Scala2 mode this is + * treated as a simple failure, with a warning that semantics will change. + * - otherwise add the failure to `rfailures` and continue testing the other candidates. + */ + def rank(pending: List[Candidate], found: SearchResult, rfailures: List[SearchFailure]): SearchResult = { + def recur(result: SearchResult, remaining: List[Candidate]): SearchResult = result match { + case fail: SearchFailure => + if (isNot) + recur( + SearchSuccess(ref(defn.Not_value), defn.Not_value.termRef, 0)( + ctx.typerState.fresh().setCommittable(true)) + remaining) + else if (fail.isAmbiguous) + if (ctx.scala2Mode) { + val result = rank(remaining, found, NoMatchingImplicitsFailure :: rfailures) + if (result.isSuccess) + warnAmbiguousNegation(fail.reason.asInstanceOf[AmbiguousImplicits]) + result } - } - case nil => - if (rfailures.isEmpty) found - else found.recoverWith(_ => rfailures.reverse.maxBy(_.tree.treeSize)) + else + healAmbiguous(remaining, fail) + else + rank(remaining, found, fail :: rfailures) + case best: SearchSuccess => + if (isNot) + recur(NoMatchingImplicitsFailure, remaining) + else if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) + best + else disambiguate(found, best) match { + case retained: SearchSuccess => + val newPending = + if (retained eq found) remaining + else remaining.filter(cand => + compareCandidate(retained, cand.ref, cand.level) <= 0) + rank(newPending, retained, rfailures) + case fail: SearchFailure => + healAmbiguous(remaining, fail) + } + } + pending match { + case cand :: pending1 => + recur(tryImplicit(cand), pending1) + case nil => + if (rfailures.isEmpty) found + else found.recoverWith(_ => rfailures.reverse.maxBy(_.tree.treeSize)) + } } def warnAmbiguousNegation(ambi: AmbiguousImplicits) = From 66b0d9a012a4ec45d5b1457dd7fee2f5376e3f4e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 15 Nov 2017 09:45:23 +0100 Subject: [PATCH 28/30] Fix typo --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 27d09e446aae..0d96e1e9a078 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -922,7 +922,7 @@ trait Implicits { self: Typer => if (isNot) recur( SearchSuccess(ref(defn.Not_value), defn.Not_value.termRef, 0)( - ctx.typerState.fresh().setCommittable(true)) + ctx.typerState.fresh().setCommittable(true)), remaining) else if (fail.isAmbiguous) if (ctx.scala2Mode) { From 0da21e192237d603e69b330e7962c1e43d543d98 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 15 Nov 2017 11:02:05 +0100 Subject: [PATCH 29/30] Avoid infinite loop --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 0d96e1e9a078..b1b3c6b67aed 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -917,13 +917,13 @@ trait Implicits { self: Typer => * - otherwise add the failure to `rfailures` and continue testing the other candidates. */ def rank(pending: List[Candidate], found: SearchResult, rfailures: List[SearchFailure]): SearchResult = { - def recur(result: SearchResult, remaining: List[Candidate]): SearchResult = result match { + def recur(result: SearchResult, remaining: List[Candidate], isNot: Boolean): SearchResult = result match { case fail: SearchFailure => if (isNot) recur( SearchSuccess(ref(defn.Not_value), defn.Not_value.termRef, 0)( ctx.typerState.fresh().setCommittable(true)), - remaining) + remaining, false) else if (fail.isAmbiguous) if (ctx.scala2Mode) { val result = rank(remaining, found, NoMatchingImplicitsFailure :: rfailures) @@ -937,7 +937,7 @@ trait Implicits { self: Typer => rank(remaining, found, fail :: rfailures) case best: SearchSuccess => if (isNot) - recur(NoMatchingImplicitsFailure, remaining) + recur(NoMatchingImplicitsFailure, remaining, false) else if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) best else disambiguate(found, best) match { @@ -953,7 +953,7 @@ trait Implicits { self: Typer => } pending match { case cand :: pending1 => - recur(tryImplicit(cand), pending1) + recur(tryImplicit(cand), pending1, this.isNot) case nil => if (rfailures.isEmpty) found else found.recoverWith(_ => rfailures.reverse.maxBy(_.tree.treeSize)) From db1b09ad1408271a4b185a3ec763b43128140409 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 15 Nov 2017 11:14:50 +0100 Subject: [PATCH 30/30] Simplify control flow in rank method --- .../dotty/tools/dotc/typer/Implicits.scala | 74 +++++++++---------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index b1b3c6b67aed..e922a1a55c14 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -916,49 +916,49 @@ trait Implicits { self: Typer => * treated as a simple failure, with a warning that semantics will change. * - otherwise add the failure to `rfailures` and continue testing the other candidates. */ - def rank(pending: List[Candidate], found: SearchResult, rfailures: List[SearchFailure]): SearchResult = { - def recur(result: SearchResult, remaining: List[Candidate], isNot: Boolean): SearchResult = result match { - case fail: SearchFailure => - if (isNot) - recur( - SearchSuccess(ref(defn.Not_value), defn.Not_value.termRef, 0)( - ctx.typerState.fresh().setCommittable(true)), - remaining, false) - else if (fail.isAmbiguous) - if (ctx.scala2Mode) { - val result = rank(remaining, found, NoMatchingImplicitsFailure :: rfailures) - if (result.isSuccess) - warnAmbiguousNegation(fail.reason.asInstanceOf[AmbiguousImplicits]) - result - } - else - healAmbiguous(remaining, fail) - else - rank(remaining, found, fail :: rfailures) - case best: SearchSuccess => - if (isNot) - recur(NoMatchingImplicitsFailure, remaining, false) - else if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) - best - else disambiguate(found, best) match { - case retained: SearchSuccess => - val newPending = - if (retained eq found) remaining - else remaining.filter(cand => - compareCandidate(retained, cand.ref, cand.level) <= 0) - rank(newPending, retained, rfailures) + def rank(pending: List[Candidate], found: SearchResult, rfailures: List[SearchFailure]): SearchResult = + pending match { + case cand :: remaining => + negateIfNot(tryImplicit(cand)) match { case fail: SearchFailure => - healAmbiguous(remaining, fail) + if (fail.isAmbiguous) + if (ctx.scala2Mode) { + val result = rank(remaining, found, NoMatchingImplicitsFailure :: rfailures) + if (result.isSuccess) + warnAmbiguousNegation(fail.reason.asInstanceOf[AmbiguousImplicits]) + result + } + else healAmbiguous(remaining, fail) + else rank(remaining, found, fail :: rfailures) + case best: SearchSuccess => + if (ctx.mode.is(Mode.ImplicitExploration) || isCoherent) + best + else disambiguate(found, best) match { + case retained: SearchSuccess => + val newPending = + if (retained eq found) remaining + else remaining.filter(cand => + compareCandidate(retained, cand.ref, cand.level) <= 0) + rank(newPending, retained, rfailures) + case fail: SearchFailure => + healAmbiguous(remaining, fail) + } } - } - pending match { - case cand :: pending1 => - recur(tryImplicit(cand), pending1, this.isNot) case nil => if (rfailures.isEmpty) found else found.recoverWith(_ => rfailures.reverse.maxBy(_.tree.treeSize)) } - } + + def negateIfNot(result: SearchResult) = + if (isNot) + result match { + case _: SearchFailure => + SearchSuccess(ref(defn.Not_value), defn.Not_value.termRef, 0)( + ctx.typerState.fresh().setCommittable(true)) + case _: SearchSuccess => + NoMatchingImplicitsFailure + } + else result def warnAmbiguousNegation(ambi: AmbiguousImplicits) = ctx.migrationWarning(