Use context for buffering errors that cannot/shouldn't be reported in the given moment (instead of throwing type errors). This avoids previous problems where we were creating fake error trees in some incorrect places like in type completers in Namers etc. Implicits relied heavily on type errors being thrown but performance should stay the same due to some explicit checks/returns.

Some of the problems involved how ambiguous error messages were collected/reported because it was very random (similarly for divergent implicits). This should be more explicit now. Reduced the number of unnecessary cyclic references being thrown (apart from those in Symbols/Types which don't have a context and need to stay for now as is).

Review by @paulp, @odersky.

1 files changed, 81 insertions, 48 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
index faff4ccab2..d828b019f9 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
@@ -7,7 +7,7 @@ package scala.tools.nsc
 package typechecker
 
 import symtab.Flags._
-import scala.collection.mutable.ListBuffer
+import scala.collection.mutable.{LinkedHashSet, Set}
 import annotation.tailrec
 
 /**
@@ -66,8 +66,7 @@ trait Contexts { self: Analyzer =>
       sc.depth += 1
     }
     val c = sc.make(unit, tree, sc.owner, sc.scope, sc.imports)
-    c.reportAmbiguousErrors = !erasedTypes
-    c.reportGeneralErrors = !erasedTypes
+    if (erasedTypes) c.setThrowErrors() else c.setReportErrors()
     c.implicitsEnabled = !erasedTypes
     c
   }
@@ -83,7 +82,17 @@ trait Contexts { self: Analyzer =>
     }
   }
 
+  private object Errors {
+    final val ReportErrors     = 1 << 0
+    final val BufferErrors     = 1 << 1
+    final val AmbiguousErrors  = 1 << 2
+    final val notThrowMask     = ReportErrors | BufferErrors
+    final val AllMask          = ReportErrors | BufferErrors | AmbiguousErrors
+  }
+
   class Context private[typechecker] {
+    import Errors._
+
     var unit: CompilationUnit = NoCompilationUnit
     var tree: Tree = _                      // Tree associated with this context
     var owner: Symbol = NoSymbol            // The current owner
@@ -109,8 +118,6 @@ trait Contexts { self: Analyzer =>
     // (the call to the super or self constructor in the first line of a constructor)
     // in this context the object's fields should not be in scope
 
-    var reportAmbiguousErrors = false
-    var reportGeneralErrors = false
     var diagnostic: List[String] = Nil      // these messages are printed when issuing an error
     var implicitsEnabled = false
     var checking = false
@@ -138,12 +145,41 @@ trait Contexts { self: Analyzer =>
       tparams
     }
 
-    def withoutReportingErrors[T](op: => T): T = {
-      val saved = reportGeneralErrors
-      reportGeneralErrors = false
-      try op
-      finally reportGeneralErrors = saved
+    private[this] var mode = 0
+    private[this] val buffer = LinkedHashSet[AbsTypeError]()
+
+    def errBuffer = buffer
+    def hasErrors = buffer.nonEmpty
+
+    def state: Int = mode
+    def restoreState(state0: Int) = mode = state0
+
+    def reportErrors    = (state & ReportErrors)     != 0
+    def bufferErrors    = (state & BufferErrors)     != 0
+    def ambiguousErrors = (state & AmbiguousErrors)  != 0
+    def throwErrors     = (state & notThrowMask)     == 0
+
+    def setReportErrors()    = mode = (ReportErrors | AmbiguousErrors)
+    def setBufferErrors()    = {
+      assert(bufferErrors || !hasErrors, "When entering the buffer state, context has to be clean. Current buffer: " + buffer)
+      mode = BufferErrors
+    }
+    def setThrowErrors()     = mode &= (~AllMask)
+    def setAmbiguousErrors(report: Boolean) = if (report) mode |= AmbiguousErrors else mode &= notThrowMask
+
+    def updateBuffer(errors: Set[AbsTypeError]) = buffer ++= errors
+    def condBufferFlush(removeP: AbsTypeError => Boolean) {
+      val elems = buffer.filter(removeP)
+      buffer --= elems
     }
+    def flushBuffer() { buffer.clear() }
+    def flushAndReturnBuffer(): Set[AbsTypeError] = {
+      val current = buffer.clone()
+      buffer.clear()
+      current
+    }
+    
+    def logError(err: AbsTypeError) = buffer += err
 
     def withImplicitsDisabled[T](op: => T): T = {
       val saved = implicitsEnabled
@@ -183,8 +219,7 @@ trait Contexts { self: Analyzer =>
       c.depth = if (scope == this.scope) this.depth else this.depth + 1
       c.imports = imports
       c.inSelfSuperCall = inSelfSuperCall
-      c.reportAmbiguousErrors = this.reportAmbiguousErrors
-      c.reportGeneralErrors = this.reportGeneralErrors
+      c.restoreState(this.state)
       c.diagnostic = this.diagnostic
       c.typingIndentLevel = typingIndentLevel
       c.implicitsEnabled = this.implicitsEnabled
@@ -196,10 +231,10 @@ trait Contexts { self: Analyzer =>
       c
     }
 
+    // TODO: remove? Doesn't seem to be used
     def make(unit: CompilationUnit): Context = {
       val c = make(unit, EmptyTree, owner, scope, imports)
-      c.reportAmbiguousErrors = true
-      c.reportGeneralErrors = true
+      c.setReportErrors()
       c.implicitsEnabled = true
       c
     }
@@ -229,8 +264,8 @@ trait Contexts { self: Analyzer =>
 
     def makeSilent(reportAmbiguousErrors: Boolean, newtree: Tree = tree): Context = {
       val c = make(newtree)
-      c.reportGeneralErrors = false
-      c.reportAmbiguousErrors = reportAmbiguousErrors
+      c.setBufferErrors()
+      c.setAmbiguousErrors(reportAmbiguousErrors)
       c
     }
 
@@ -242,13 +277,11 @@ trait Contexts { self: Analyzer =>
 
     def makeConstructorContext = {
       var baseContext = enclClass.outer
-      //todo: find out why we need next line
       while (baseContext.tree.isInstanceOf[Template])
         baseContext = baseContext.outer
       val argContext = baseContext.makeNewScope(tree, owner)
       argContext.inSelfSuperCall = true
-      argContext.reportGeneralErrors = this.reportGeneralErrors
-      argContext.reportAmbiguousErrors = this.reportAmbiguousErrors
+      argContext.restoreState(this.state)
       def enterElems(c: Context) {
         def enterLocalElems(e: ScopeEntry) {
           if (e != null && e.owner == c.scope) {
@@ -275,41 +308,41 @@ trait Contexts { self: Analyzer =>
     private def unitError(pos: Position, msg: String) =
       unit.error(pos, if (checking) "\n**** ERROR DURING INTERNAL CHECKING ****\n" + msg else msg)
 
+    def issue(err: AbsTypeError) {
+      if (reportErrors) unitError(err.errPos, addDiagString(err.errMsg))
+      else if (bufferErrors) { buffer += err }
+      else throw new TypeError(err.errPos, err.errMsg)
+    }
+
+    def issueAmbiguousError(pre: Type, sym1: Symbol, sym2: Symbol, err: AbsTypeError) {
+      if (ambiguousErrors) {
+        if (!pre.isErroneous && !sym1.isErroneous && !sym2.isErroneous)
+          unitError(err.errPos, err.errMsg)
+      } else if (bufferErrors) { buffer += err }
+      else throw new TypeError(err.errPos, err.errMsg)
+    }
+
+    def issueAmbiguousError(err: AbsTypeError) {
+      if (ambiguousErrors)
+        unitError(err.errPos, addDiagString(err.errMsg))
+      else if (bufferErrors) { buffer += err }
+      else throw new TypeError(err.errPos, err.errMsg)
+    }
+
+    // TODO remove
     def error(pos: Position, err: Throwable) =
-      if (reportGeneralErrors) unitError(pos, addDiagString(err.getMessage()))
+      if (reportErrors) unitError(pos, addDiagString(err.getMessage()))
       else throw err
 
     def error(pos: Position, msg: String) = {
       val msg1 = addDiagString(msg)
-      if (reportGeneralErrors) unitError(pos, msg1)
+      if (reportErrors) unitError(pos, msg1)
       else throw new TypeError(pos, msg1)
     }
 
-    def warning(pos:  Position, msg: String) = {
-      if (reportGeneralErrors) unit.warning(pos, msg)
-    }
-
-    def ambiguousError(pos: Position, pre: Type, sym1: Symbol, sym2: Symbol, rest: String) {
-      val (reportPos, msg) = (
-        if (sym1.hasDefaultFlag && sym2.hasDefaultFlag && sym1.enclClass == sym2.enclClass) {
-          val methodName = nme.defaultGetterToMethod(sym1.name)
-          (sym1.enclClass.pos,
-           "in "+ sym1.enclClass +", multiple overloaded alternatives of " + methodName +
-                     " define default arguments")
-        }
-        else {
-          (pos,
-            ("ambiguous reference to overloaded definition,\n" +
-             "both " + sym1 + sym1.locationString + " of type " + pre.memberType(sym1) +
-             "\nand  " + sym2 + sym2.locationString + " of type " + pre.memberType(sym2) +
-             "\nmatch " + rest)
-          )
-        }
-      )
-      if (reportAmbiguousErrors) {
-        if (!pre.isErroneous && !sym1.isErroneous && !sym2.isErroneous)
-          unit.error(reportPos, msg)
-      } else throw new TypeError(pos, msg)
+    def warning(pos: Position, msg: String): Unit = warning(pos, msg, false)
+    def warning(pos: Position, msg: String, force: Boolean) {
+      if (reportErrors || force) unit.warning(pos, msg)
     }
 
     def isLocal(): Boolean = tree match {
@@ -343,8 +376,8 @@ trait Contexts { self: Analyzer =>
 
     def enclosingContextChain: List[Context] = this :: outer.enclosingContextChain
 
-    override def toString = "Context(%s@%s unit=%s scope=%s)".format(
-      owner.fullName, tree.shortClass, unit, scope.##
+    override def toString = "Context(%s@%s unit=%s scope=%s errors=%b)".format(
+      owner.fullName, tree.shortClass, unit, scope.##, hasErrors
     )
     /** Is `sub` a subclass of `base` or a companion object of such a subclass?
      */