From c12de60ea66c71dc5ba5894b7101f623210e53d2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 21 May 2017 19:18:08 +0200 Subject: [PATCH 1/2] Don't use tryWithFallback Why not? - It's a hack - It can add a factor of two for a failed implicit search, which can add up - It swallows up useful info for the first implicit search which caused the fallback. The only test failing after removal was t2429.scala. That test was originally for an issue where scalac crashed. It involves some fairly exotic code over List[Nothing]'s. I think we can ignore it for now, and just add the type annotation to make it compile. --- .../dotty/tools/dotc/core/TyperState.scala | 41 ------------------- .../tools/dotc/printing/PlainPrinter.scala | 9 ++-- .../dotty/tools/dotc/typer/Implicits.scala | 1 + .../src/dotty/tools/dotc/typer/Typer.scala | 6 +-- tests/pos/t2429.scala | 6 +-- 5 files changed, 10 insertions(+), 53 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TyperState.scala b/compiler/src/dotty/tools/dotc/core/TyperState.scala index b33b3aa29553..8735a1fe8130 100644 --- a/compiler/src/dotty/tools/dotc/core/TyperState.scala +++ b/compiler/src/dotty/tools/dotc/core/TyperState.scala @@ -76,8 +76,6 @@ class TyperState(r: Reporter) extends DotClass with Showable { /** Can this state be transitively committed until the top-level? */ def isGlobalCommittable: Boolean = false - def tryWithFallback[T](op: => T)(fallback: => T)(implicit ctx: Context): T = unsupported("tryWithFallBack") - override def toText(printer: Printer): Text = "ImmutableTyperState" /** A string showing the hashes of all nested mutable typerstates */ @@ -170,45 +168,6 @@ extends TyperState(r) { constraint = constraint.remove(poly) } - /** Try operation `op`; if it produces errors, execute `fallback` with constraint and - * reporter as they were before `op` was executed. This is similar to `typer/tryEither`, - * but with one important difference: Any type variable instantiations produced by `op` - * are persisted even if `op` fails. This is normally not what one wants and therefore - * it is recommended to use - * - * tryEither { implicit ctx => op } { (_, _) => fallBack } - * - * instead of - * - * ctx.tryWithFallback(op)(fallBack) - * - * `tryWithFallback` is only used when an implicit parameter search fails - * and the whole expression is subsequently retype-checked with a Wildcard - * expected type (so as to allow an implicit conversion on the result and - * avoid over-constraining the implicit parameter search). In this case, - * the only type variables that might be falsely instantiated by `op` but - * not by `fallBack` are type variables in the typed expression itself, and - * these will be thrown away and new ones will be created on re-typing. - * So `tryWithFallback` is safe. It is also necessary because without it - * we do not propagate enough instantiation information into the implicit search - * and this might lead to a missing parameter type error. This is exhibited - * at several places in the test suite (for instance in `pos_typers`). - * Overall, this is rather ugly, but despite trying for 2 days I have not - * found a better solution. - */ - override def tryWithFallback[T](op: => T)(fallback: => T)(implicit ctx: Context): T = { - val storeReporter = new StoreReporter(myReporter) - val savedReporter = myReporter - myReporter = storeReporter - val savedConstraint = myConstraint - val result = try op finally myReporter = savedReporter - if (!storeReporter.hasErrors) result - else { - myConstraint = savedConstraint - fallback - } - } - override def toText(printer: Printer): Text = constraint.toText(printer) override def hashesStr: String = hashCode.toString + " -> " + previous.hashesStr diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index 8aec867921eb..6cfefa65c700 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -193,7 +193,8 @@ class PlainPrinter(_ctx: Context) extends Printer { val bounds = if (constr.contains(tp)) constr.fullBounds(tp.origin)(ctx.addMode(Mode.Printing)) else TypeBounds.empty - if (ctx.settings.YshowVarBounds.value) "(" ~ toText(tp.origin) ~ "?" ~ toText(bounds) ~ ")" + if (bounds.isAlias) toText(bounds.lo) ~ "^" + else if (ctx.settings.YshowVarBounds.value) "(" ~ toText(tp.origin) ~ "?" ~ toText(bounds) ~ ")" else toText(tp.origin) } case tp: LazyRef => @@ -487,9 +488,9 @@ 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 result: NonMatchingImplicit => + case _: NonMatchingImplicit | NoImplicitMatches => "NoImplicitMatches" - case result: DivergingImplicit => + case _: DivergingImplicit | DivergingImplicit => "Diverging Implicit" case result: ShadowedImplicit => "Shadowed Implicit" @@ -498,7 +499,7 @@ class PlainPrinter(_ctx: Context) extends Printer { case result: AmbiguousImplicits => "Ambiguous Implicit: " ~ toText(result.alt1) ~ " and " ~ toText(result.alt2) case _ => - "?Unknown Implicit Result?" + "?Unknown Implicit Result?" + result.getClass } def toText(importInfo: ImportInfo): Text = { diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index fe63b3b7504c..f18d4dedeee5 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -680,6 +680,7 @@ trait Implicits { self: Typer => 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 => diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 7ba39a78cd35..644bcd7cbe0a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1911,11 +1911,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit } else adapt(tpd.Apply(tree, args), pt) } - if ((pt eq WildcardType) || original.isEmpty) addImplicitArgs(argCtx(tree)) - else - ctx.typerState.tryWithFallback(addImplicitArgs(argCtx(tree))) { - adapt(typed(original, WildcardType), pt, EmptyTree) - } + addImplicitArgs(argCtx(tree)) case wtp: MethodType if !pt.isInstanceOf[SingletonType] => // Follow proxies and approximate type paramrefs by their upper bound // in the current constraint in order to figure out robustly diff --git a/tests/pos/t2429.scala b/tests/pos/t2429.scala index 4cda3bde1c48..3336b3794d3d 100644 --- a/tests/pos/t2429.scala +++ b/tests/pos/t2429.scala @@ -16,10 +16,10 @@ object Msg { } } } - } /*: Seq[T] Adding this type annotation avoids the compile error.*/) + }: Seq[T] /* Adding this type annotation avoids the compile error.*/) } } object Oops { - implicit def someImplicit(s: Seq[_]): String = sys.error("stub") - def item: String = Nil map { case e: Any => e } +// implicit def someImplicit(s: Seq[_]): String = sys.error("stub") +// def item: String = Nil map { case e: Any => e } } From 119a6cf382f8624b131c0d28ed9234a1cbc26aa7 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 21 May 2017 22:31:14 +0200 Subject: [PATCH 2/2] Fix neg test --- tests/neg/i1802.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/neg/i1802.scala b/tests/neg/i1802.scala index 93e790f18dc5..455ee9573a43 100644 --- a/tests/neg/i1802.scala +++ b/tests/neg/i1802.scala @@ -16,6 +16,6 @@ object Exception { def mkThrowableCatcher[T](isDef: Throwable => Boolean, f: Throwable => T) = mkCatcher(isDef, f) // error: undetermined ClassTag - implicit def throwableSubtypeToCatcher[Ex <: Throwable: ClassTag, T](pf: PartialFunction[Ex, T]) = + implicit def throwableSubtypeToCatcher[Ex <: Throwable: ClassTag, T](pf: PartialFunction[Ex, T]) = // error: result type needs to be given mkCatcher(pf.isDefinedAt _, pf.apply _) }