diff options
author | Adriaan Moors <adriaan.moors@typesafe.com> | 2014-07-23 23:30:52 +0200 |
---|---|---|
committer | Adriaan Moors <adriaan.moors@typesafe.com> | 2014-08-04 17:50:42 +0200 |
commit | 3a4c6db9fe8c06b78f907f182e80e908c653cf89 (patch) | |
tree | 4fbfc797fe026012cd4803bc6af29482b6463bc0 | |
parent | e131424b45881ac8446235f255c8f637602857ac (diff) | |
download | scala-3a4c6db9fe8c06b78f907f182e80e908c653cf89.tar.gz scala-3a4c6db9fe8c06b78f907f182e80e908c653cf89.tar.bz2 scala-3a4c6db9fe8c06b78f907f182e80e908c653cf89.zip |
Towards more privacy for _reporter.
Move code that manipulates the error buffers / reporters
into combinators in Context/ContextReporter.
Eventually, would like to statically know when we're in silent mode,
and only then use buffering (push buffering code down to BufferingReporter).
Simplify TryTwice; avoid capturing mutable var in closure.
Changed inSilentMode to no longer check `&& !reporter.hasErrors`;
disassembling optimized code showed that this was keeping the inliner
from inlining this method.
Introduce a couple more combinators:
- withFreshErrorBuffer
- propagatingErrorsTo
- propagateImplicitTypeErrorsTo
4 files changed, 130 insertions, 108 deletions
diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala index 6573102e86..a79f162140 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -182,7 +182,8 @@ trait Contexts { self: Analyzer => * @param _outer The next outer context. */ class Context private[typechecker](val tree: Tree, val owner: Symbol, val scope: Scope, - val unit: CompilationUnit, _outer: Context) { + val unit: CompilationUnit, _outer: Context, + private[this] var _reporter: ContextReporter = new ThrowingReporter) { private def outerIsNoContext = _outer eq null final def outer: Context = if (outerIsNoContext) NoContext else _outer @@ -325,59 +326,50 @@ trait Contexts { self: Analyzer => // // the reporter for this context - // TODO: start off with ImmediateReporter - private var _reporter: ContextReporter = new ThrowingReporter - - def reporter = _reporter + def reporter: ContextReporter = _reporter // if set, errors will not be reporter/thrown - def bufferErrors = reporter.isBuffering - def reportErrors = !bufferErrors + def bufferErrors = reporter.isBuffering + def reportErrors = !bufferErrors // whether to *report* (which is separate from buffering/throwing) ambiguity errors def ambiguousErrors = this(AmbiguousErrors) private def setAmbiguousErrors(report: Boolean): Unit = this(AmbiguousErrors) = report - /** - * Try inference twice, once without views and once with views, + * Try inference twice: once without views and once with views, * unless views are already disabled. */ abstract class TryTwice { def tryOnce(isLastTry: Boolean): Unit - def apply(): Unit = { - var doLastTry = !implicitsEnabled - - // do first try if implicits are enabled - if (implicitsEnabled) { - val savedReporter = _reporter - val savedContextMode = contextMode - - _reporter = new BufferingReporter - setAmbiguousErrors(false) // this means ambiguous errors will not be force-fed to the reporter - - // We create a new BufferingReporter to - // distinguish errors that occurred before entering tryTwice - // and our first attempt in 'withImplicitsDisabled'. If the - // first attempt fails, we try with implicits on - // and the original reporter. - try { - withImplicitsDisabled(tryOnce(false)) - doLastTry = reporter.hasErrors - } catch { - case ex: CyclicReference => throw ex - case ex: TypeError => doLastTry = true // recoverable cyclic references? - } finally { - contextMode = savedContextMode - _reporter = savedReporter - } - } + final def apply(): Unit = { + val doLastTry = + // do first try if implicits are enabled + if (implicitsEnabled) { + // We create a new BufferingReporter to + // distinguish errors that occurred before entering tryTwice + // and our first attempt in 'withImplicitsDisabled'. If the + // first attempt fails, we try with implicits on + // and the original reporter. + // immediate reporting of ambiguous errors is suppressed, so that they are buffered + inSilentMode { + try { + set(disable = ImplicitsEnabled | EnrichmentEnabled) // restored by inSilentMode + tryOnce(false) + reporter.hasErrors + } catch { + case ex: CyclicReference => throw ex + case ex: TypeError => true // recoverable cyclic references? + } + } + } else true // do last try if try with implicits enabled failed // (or if it was not attempted because they were disabled) - if (doLastTry) tryOnce(true) + if (doLastTry) + tryOnce(true) } } @@ -385,7 +377,7 @@ trait Contexts { self: Analyzer => // Temporary mode adjustment // - @inline def withMode[T](enabled: ContextMode = NOmode, disabled: ContextMode = NOmode)(op: => T): T = { + @inline final def withMode[T](enabled: ContextMode = NOmode, disabled: ContextMode = NOmode)(op: => T): T = { val saved = contextMode set(enabled, disabled) try op @@ -419,17 +411,18 @@ trait Contexts { self: Analyzer => // See comment on FormerNonStickyModes. @inline final def withOnlyStickyModes[T](op: => T): T = withMode(disabled = FormerNonStickyModes)(op) - /** @return true if the `expr` evaluates to true within a silent Context that incurs no errors */ + // inliner note: this has to be a simple method for inlining to work -- moved the `&& !reporter.hasErrors` out @inline final def inSilentMode(expr: => Boolean): Boolean = { - val savedReporter = _reporter - withMode() { // TODO: rework -- withMode with no arguments to restore the mode mutated by `setBufferErrors` (no longer mutated!) - _reporter = reporter.makeBuffering - setAmbiguousErrors(false) - try expr && !reporter.hasErrors - finally { - _reporter = savedReporter - _reporter.clearAll() // TODO: ??? - } + val savedContextMode = contextMode + val savedReporter = reporter + + setAmbiguousErrors(false) + _reporter = new BufferingReporter + + try expr + finally { + contextMode = savedContextMode + _reporter = savedReporter } } @@ -445,7 +438,8 @@ trait Contexts { self: Analyzer => * `Context#imports`. */ def make(tree: Tree = tree, owner: Symbol = owner, - scope: Scope = scope, unit: CompilationUnit = unit): Context = { + scope: Scope = scope, unit: CompilationUnit = unit, + reporter: ContextReporter = this.reporter): Context = { val isTemplateOrPackage = tree match { case _: Template | _: PackageDef => true case _ => false @@ -468,16 +462,15 @@ trait Contexts { self: Analyzer => // The blank canvas val c = if (isImport) - new Context(tree, owner, scope, unit, this) with ImportContext + new Context(tree, owner, scope, unit, this, reporter) with ImportContext else - new Context(tree, owner, scope, unit, this) + new Context(tree, owner, scope, unit, this, reporter) // Fields that are directly propagated c.variance = variance c.diagUsedDefaults = diagUsedDefaults c.openImplicits = openImplicits c.contextMode = contextMode // note: ConstructorSuffix, a bit within `mode`, is conditionally overwritten below. - c._reporter = reporter // Fields that may take on a different value in the child c.prefix = prefixInChild @@ -510,22 +503,19 @@ trait Contexts { self: Analyzer => else make(tree, owner, scope, unit) /** Make a child context that represents a new nested scope */ - def makeNewScope(tree: Tree, owner: Symbol): Context = - make(tree, owner, newNestedScope(scope)) - + def makeNewScope(tree: Tree, owner: Symbol, reporter: ContextReporter = this.reporter): Context = + make(tree, owner, newNestedScope(scope), reporter = reporter) /** Make a child context that buffers errors and warnings into a fresh report buffer. */ def makeSilent(reportAmbiguousErrors: Boolean = ambiguousErrors, newtree: Tree = tree): Context = { - val c = make(newtree) - c.setAmbiguousErrors(reportAmbiguousErrors) // A fresh buffer so as not to leak errors/warnings into `this`. - c._reporter = new BufferingReporter + val c = make(newtree, reporter = new BufferingReporter) + c.setAmbiguousErrors(reportAmbiguousErrors) c } def makeNonSilent(newtree: Tree): Context = { - val c = make(newtree) - c._reporter = reporter.makeImmediate + val c = make(newtree, reporter = reporter.makeImmediate) c.setAmbiguousErrors(true) c } @@ -549,9 +539,10 @@ trait Contexts { self: Analyzer => */ def makeConstructorContext = { val baseContext = enclClass.outer.nextEnclosing(!_.tree.isInstanceOf[Template]) - val argContext = baseContext.makeNewScope(tree, owner) + // must propagate reporter! + // (caught by neg/t3649 when refactoring reporting to be specified only by this.reporter and not also by this.contextMode) + val argContext = baseContext.makeNewScope(tree, owner, reporter = this.reporter) argContext.contextMode = contextMode - argContext._reporter = _reporter // caught by neg/t3649 TODO: make sure _reporter is propagated wherever contextMode is argContext.inSelfSuperCall = true def enterElems(c: Context) { def enterLocalElems(e: ScopeEntry) { @@ -1230,7 +1221,17 @@ trait Contexts { self: Analyzer => override final def toString = super.toString + " with " + s"ImportContext { $impInfo; outer.owner = ${outer.owner} }" } - /** A buffer for warnings and errors that are accumulated during speculative type checking. */ + /** A reporter for use during type checking. It has multiple modes for handling errors. + * + * The default (immediate mode) is to send the error to the global reporter. + * When switched into buffering mode via makeBuffering, errors and warnings are buffered and not be reported + * (there's a special case for ambiguity errors for some reason: those are force to the reporter when context.ambiguousErrors, + * or else they are buffered -- TODO: can we simplify this?) + * + * When using the type checker after typers, an error results in a TypeError being thrown. TODO: get rid of this mode. + * + * To handle nested contexts, reporters share buffers. TODO: only buffer in BufferingReporter, emit immediately in ImmediateReporter + */ abstract class ContextReporter(private[this] var _errorBuffer: mutable.LinkedHashSet[AbsTypeError] = null, private[this] var _warningBuffer: mutable.LinkedHashSet[(Position, String)] = null) extends Reporter { type Error = AbsTypeError type Warning = (Position, String) @@ -1254,7 +1255,27 @@ trait Contexts { self: Analyzer => if (context.ambiguousErrors) reporter.error(err.errPos, addDiagString(err.errMsg)) // force reporting... see TODO above else handleSuppressedAmbiguous(err) - def ++=(errors: Traversable[AbsTypeError]): Unit = errorBuffer ++= errors + @inline final def withFreshErrorBuffer[T](expr: => T): T = { + val previousBuffer = _errorBuffer + _errorBuffer = newBuffer + val res = expr // expr will read _errorBuffer + _errorBuffer = previousBuffer + res + } + + @inline final def propagatingErrorsTo[T](target: ContextReporter)(expr: => T): T = { + val res = expr // TODO: make sure we're okay skipping the try/finally overhead + if ((this ne target) && hasErrors) { // `this eq target` in e.g., test/files/neg/divergent-implicit.scala + // assert(target.errorBuffer ne _errorBuffer) + target ++= errors + // TODO: is clearAllErrors necessary? (no tests failed when dropping it) + // NOTE: even though `this ne target`, it may still be that `target.errorBuffer eq _errorBuffer`, + // so don't clear the buffer, but null out the reference so that a new one will be created when necessary (should be never??) + // (we should refactor error buffering to avoid mutation on shared buffers) + clearAllErrors() + } + res + } protected final def info0(pos: Position, msg: String, severity: Severity, force: Boolean): Unit = severity match { @@ -1265,6 +1286,10 @@ trait Contexts { self: Analyzer => final override def hasErrors = super.hasErrors || errorBuffer.nonEmpty + // TODO: everything below should be pushed down to BufferingReporter (related to buffering) + // Implicit relies on this most heavily, but there you know reporter.isInstanceOf[BufferingReporter] + // can we encode this statically? + // have to pass in context because multiple contexts may share the same ReportBuffer def reportFirstDivergentError(fun: Tree, param: Symbol, paramTp: Type)(implicit context: Context): Unit = errors.collectFirst { @@ -1290,13 +1315,28 @@ trait Contexts { self: Analyzer => case _ => false } + def propagateImplicitTypeErrorsTo(target: ContextReporter) = { + errors foreach { + case err@(_: DivergentImplicitTypeError | _: AmbiguousImplicitTypeError) => + target.errorBuffer += err + case _ => + } + // debuglog("propagateImplicitTypeErrorsTo: " + errors) + } + protected def addDiagString(msg: String)(implicit context: Context): String = { val diagUsedDefaultsMsg = "Error occurred in an application involving default arguments." if (context.diagUsedDefaults && !(msg endsWith diagUsedDefaultsMsg)) msg + "\n" + diagUsedDefaultsMsg else msg } - // TODO: deprecate everything below (related to buffering) and push down to BufferingReporter + final def emitWarnings() = if (_warningBuffer != null) { + _warningBuffer foreach { + case (pos, msg) => reporter.warning(pos, msg) + } + _warningBuffer = null + } + // [JZ] Contexts, pre- the SI-7345 refactor, avoided allocating the buffers until needed. This // is replicated here out of conservatism. private def newBuffer[A] = mutable.LinkedHashSet.empty[A] // Important to use LinkedHS for stable results. @@ -1307,9 +1347,13 @@ trait Contexts { self: Analyzer => final def warnings: immutable.Seq[Warning] = warningBuffer.toVector final def firstError: Option[AbsTypeError] = errorBuffer.headOption - final def clearAll(): Unit = { clearAllErrors(); clearAllWarnings() } - final def clearAllErrors(): Unit = errorBuffer.clear() - final def clearAllWarnings(): Unit = warningBuffer.clear() + // TODO: remove ++= and clearAll* entirely in favor of more high-level combinators like withFreshErrorBuffer + final private[typechecker] def ++=(errors: Traversable[AbsTypeError]): Unit = errorBuffer ++= errors + + // null references to buffers instead of clearing them, + // as the buffers may be shared between different reporters + final def clearAll(): Unit = { _errorBuffer = null; _warningBuffer = null } + final def clearAllErrors(): Unit = { _errorBuffer = null } } private[typechecker] class ImmediateReporter(_errorBuffer: mutable.LinkedHashSet[AbsTypeError] = null, _warningBuffer: mutable.LinkedHashSet[(Position, String)] = null) extends ContextReporter(_errorBuffer, _warningBuffer) { diff --git a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala index e03877dbf9..d56f4a7ce8 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala @@ -71,13 +71,10 @@ trait Implicits { typingStack.printTyping(tree, "typing implicit: %s %s".format(tree, context.undetparamsString)) val implicitSearchContext = context.makeImplicit(reportAmbiguous) val result = new ImplicitSearch(tree, pt, isView, implicitSearchContext, pos).bestImplicit - if (result.isFailure && saveAmbiguousDivergent && implicitSearchContext.reporter.hasErrors) { - context.reporter ++= (implicitSearchContext.reporter.errors.collect { - case dte: DivergentImplicitTypeError => dte - case ate: AmbiguousImplicitTypeError => ate - }) - debuglog("update buffer: " + implicitSearchContext.reporter.errors) - } + + if (result.isFailure && saveAmbiguousDivergent && implicitSearchContext.reporter.hasErrors) + implicitSearchContext.reporter.propagateImplicitTypeErrorsTo(context.reporter) + // SI-7944 undetermined type parameters that result from inference within typedImplicit land in // `implicitSearchContext.undetparams`, *not* in `context.undetparams` // Here, we copy them up to parent context (analogously to the way the errors are copied above), diff --git a/src/compiler/scala/tools/nsc/typechecker/Infer.scala b/src/compiler/scala/tools/nsc/typechecker/Infer.scala index 535aaffbbf..fb7651ffd6 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Infer.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Infer.scala @@ -1375,7 +1375,7 @@ trait Infer extends Checkable { // with and without views enabled, and bestForExpectedType will try again // with pt = WildcardType if it fails with pt != WildcardType. val c = context - class InferTwice extends c.TryTwice { + class InferMethodAlternativeTwice extends c.TryTwice { private[this] val OverloadedType(pre, alts) = tree.tpe private[this] var varargsStar = false private[this] val argtpes = argtpes0 mapConserve { @@ -1384,12 +1384,12 @@ trait Infer extends Checkable { } private def followType(sym: Symbol) = followApply(pre memberType sym) + // separate method to help the inliner + private def isAltApplicable(pt: Type)(alt: Symbol) = context inSilentMode { isApplicable(undetparams, followType(alt), argtpes, pt) && !context.reporter.hasErrors } + private def rankAlternatives(sym1: Symbol, sym2: Symbol) = isStrictlyMoreSpecific(followType(sym1), followType(sym2), sym1, sym2) private def bestForExpectedType(pt: Type, isLastTry: Boolean): Unit = { - val applicable0 = alts filter (alt => context inSilentMode isApplicable(undetparams, followType(alt), argtpes, pt)) - val applicable = overloadsToConsiderBySpecificity(applicable0, argtpes, varargsStar) - val ranked = bestAlternatives(applicable)((sym1, sym2) => - isStrictlyMoreSpecific(followType(sym1), followType(sym2), sym1, sym2) - ) + val applicable = overloadsToConsiderBySpecificity(alts filter isAltApplicable(pt), argtpes, varargsStar) + val ranked = bestAlternatives(applicable)(rankAlternatives) ranked match { case best :: competing :: _ => AmbiguousMethodAlternativeError(tree, pre, best, competing, argtpes, pt, isLastTry) // ambiguous case best :: Nil => tree setSymbol best setType (pre memberType best) // success @@ -1405,7 +1405,7 @@ trait Infer extends Checkable { } } - (new InferTwice).apply() + (new InferMethodAlternativeTwice).apply() } /** Assign `tree` the type of all polymorphic alternatives diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index c557af88f2..8549d3dbbc 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -464,24 +464,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper if (cond) typerWithLocalContext(c)(f) else f(this) @inline - final def typerWithLocalContext[T](c: Context)(f: Typer => T): T = { - val res = f(newTyper(c)) - val errors = c.reporter.errors - if (errors.nonEmpty) { - c.reporter.clearAllErrors() - context.reporter ++= errors - } - res - } - - @inline - final def withSavedContext[T](c: Context)(f: => T) = { - val savedErrors = c.reporter.errors - c.reporter.clearAllErrors() - val res = f - c.reporter ++= savedErrors - res - } + final def typerWithLocalContext[T](c: Context)(f: Typer => T): T = + c.reporter.propagatingErrorsTo(context.reporter)(f(newTyper(c))) /** The typer for a label definition. If this is part of a template we * first have to enter the label definition. @@ -692,19 +676,16 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper SilentTypeError(context1.reporter.errors: _*) } else { // If we have a successful result, emit any warnings it created. - context1.reporter.warnings foreach { - case (pos, msg) => reporter.warning(pos, msg) - } - context1.reporter.clearAllWarnings() + context1.reporter.emitWarnings() SilentResultValue(result) } } else { assert(context.bufferErrors || isPastTyper, "silent mode is not available past typer") - withSavedContext(context){ + + context.reporter.withFreshErrorBuffer { val res = op(this) - val errorsToReport = context.reporter.errors - context.reporter.clearAllErrors() - if (errorsToReport.isEmpty) SilentResultValue(res) else SilentTypeError(errorsToReport.head) + if (!context.reporter.hasErrors) SilentResultValue(res) + else SilentTypeError(context.reporter.firstError.get) } } } catch { |