Skip to content

Don't use tryWithFallback #2493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 0 additions & 41 deletions compiler/src/dotty/tools/dotc/core/TyperState.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down Expand Up @@ -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"
Expand All @@ -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 = {
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down
6 changes: 1 addition & 5 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/neg/i1802.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 _)
}
6 changes: 3 additions & 3 deletions tests/pos/t2429.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}