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.

src/compiler/scala/reflect/internal/Symbols.scala

@@ -345,22 +345,26 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     }
 
     // Lock a symbol, using the handler if the recursion depth becomes too great.
-    def lock(handler: => Unit) = {
+    def lock(handler: => Unit): Boolean = {
       if ((rawflags & LOCKED) != 0L) {
         if (settings.Yrecursion.value != 0) {
           recursionTable get this match {
             case Some(n) =>
               if (n > settings.Yrecursion.value) {
                 handler
+                false
               } else {
                 recursionTable += (this -> (n + 1))
+                true
               }
             case None =>
               recursionTable += (this -> 1)
+              true
           }
-        } else { handler }
+        } else { handler; false }
       } else {
         rawflags |= LOCKED
+        true
 //        activeLocks += 1
 //        lockedSyms += this
       }
@@ -963,7 +967,6 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
           phase = phaseOf(infos.validFrom)
           tp.complete(this)
         } finally {
-          // if (id == 431) println("completer ran "+tp.getClass+" for "+fullName)
           unlock()
           phase = current
         }

src/compiler/scala/reflect/internal/TreePrinters.scala

@@ -397,7 +397,6 @@ trait TreePrinters extends api.TreePrinters { self: SymbolTable =>
 //          case SelectFromArray(qualifier, name, _) =>
 //          print(qualifier); print(".<arr>"); print(symName(tree, name))
 
-
         case tree =>
           xprintTree(this, tree)
       }

src/compiler/scala/reflect/internal/Trees.scala

@@ -129,6 +129,9 @@ trait Trees extends api.Trees { self: SymbolTable =>
     }
     def shallowDuplicate: Tree = new ShallowDuplicator(tree) transform tree
     def shortClass: String = tree.getClass.getName split "[.$]" last
+
+    def isErrorTyped = (tree.tpe ne null) && tree.tpe.isError
+
     /** When you want to know a little more than the class, but a lot
      *  less than the whole tree.
      */

src/compiler/scala/reflect/internal/Types.scala

@@ -1388,7 +1388,7 @@ trait Types extends api.Types { self: SymbolTable =>
         //Console.println("baseTypeSeq(" + typeSymbol + ") = " + baseTypeSeqCache.toList);//DEBUG
       }
       if (baseTypeSeqCache eq undetBaseTypeSeq)
-        throw new TypeError("illegal cyclic inheritance involving " + typeSymbol)
+        throw new RecoverableCyclicReference(typeSymbol)
       baseTypeSeqCache
     }
 
@@ -1430,7 +1430,7 @@ trait Types extends api.Types { self: SymbolTable =>
         }
       }
       if (baseClassesCache eq null)
-        throw new TypeError("illegal cyclic reference involving " + typeSymbol)
+        throw new RecoverableCyclicReference(typeSymbol)
       baseClassesCache
     }
 
@@ -1946,7 +1946,7 @@ trait Types extends api.Types { self: SymbolTable =>
           // If a subtyping cycle is not detected here, we'll likely enter an infinite
           // loop before a sensible error can be issued.  SI-5093 is one example.
           case x: SubType if x.supertype eq this =>
-            throw new TypeError("illegal cyclic reference involving " + sym)
+            throw new RecoverableCyclicReference(sym)
           case tp => tp
         }
       }
@@ -2064,7 +2064,7 @@ trait Types extends api.Types { self: SymbolTable =>
         }
       }
       if (baseTypeSeqCache == undetBaseTypeSeq)
-        throw new TypeError("illegal cyclic inheritance involving " + sym)
+        throw new RecoverableCyclicReference(sym)
       baseTypeSeqCache
     }
 
@@ -2074,11 +2074,11 @@ trait Types extends api.Types { self: SymbolTable =>
       else pre.prefixString
     )
     private def argsString = if (args.isEmpty) "" else args.mkString("[", ",", "]")
-    private def refinementString = (
+    def refinementString = (
       if (sym.isStructuralRefinement) (
         decls filter (sym => sym.isPossibleInRefinement && sym.isPublic)
           map (_.defString)
-          mkString(" {", "; ", "}")
+          mkString("{", "; ", "}")
       )
       else ""
     )
@@ -2498,7 +2498,7 @@ trait Types extends api.Types { self: SymbolTable =>
         if (args.isEmpty && params.isEmpty)   new TypeVar(origin, constr)
         else if (args.size == params.size)    new AppliedTypeVar(origin, constr, params zip args)
         else if (args.isEmpty)                new HKTypeVar(origin, constr, params)
-        else throw new TypeError("Invalid TypeVar construction: " + ((origin, constr, args, params)))
+        else throw new Error("Invalid TypeVar construction: " + ((origin, constr, args, params)))
       )
 
       trace("create", "In " + tv.originLocation)(tv)
@@ -2599,7 +2599,7 @@ trait Types extends api.Types { self: SymbolTable =>
         TypeVar.trace("applyArgs", "In " + originLocation + ", apply args " + newArgs.mkString(", ") + " to " + originName)(tv)
       }
       else
-        throw new TypeError("Invalid type application in TypeVar: " + params + ", " + newArgs)
+        throw new Error("Invalid type application in TypeVar: " + params + ", " + newArgs)
     )
     // newArgs.length may differ from args.length (could've been empty before)
     //
@@ -3079,7 +3079,7 @@ trait Types extends api.Types { self: SymbolTable =>
     // don't expand cyclical type alias
     // we require that object is initialized, thus info.typeParams instead of typeParams.
     if (sym1.isAliasType && sameLength(sym1.info.typeParams, args) && !sym1.lockOK)
-      throw new TypeError("illegal cyclic reference involving " + sym1)
+      throw new RecoverableCyclicReference(sym1)
 
     val pre1 = pre match {
       case x: SuperType if sym1.isEffectivelyFinal || sym1.isDeferred =>
@@ -3101,7 +3101,7 @@ trait Types extends api.Types { self: SymbolTable =>
   def copyTypeRef(tp: Type, pre: Type, sym: Symbol, args: List[Type]): Type = tp match {
     case TypeRef(pre0, sym0, _) if pre == pre0 && sym0.name == sym.name =>
       if (sym.isAliasType && sameLength(sym.info.typeParams, args) && !sym.lockOK)
-        throw new TypeError("illegal cyclic reference involving " + sym)
+        throw new RecoverableCyclicReference(sym)
 
       TypeRef(pre, sym, args)
     case _ =>
@@ -3985,15 +3985,17 @@ trait Types extends api.Types { self: SymbolTable =>
                       else instParamRelaxed(ps.tail, as.tail)
 
                     //Console.println("instantiating " + sym + " from " + basesym + " with " + basesym.typeParams + " and " + baseargs+", pre = "+pre+", symclazz = "+symclazz);//DEBUG
-                    if (sameLength(basesym.typeParams, baseargs)) {
+                    if (sameLength(basesym.typeParams, baseargs))
                       instParam(basesym.typeParams, baseargs)
-                    } else {
-                      throw new TypeError(
-                        "something is wrong (wrong class file?): "+basesym+
-                        " with type parameters "+
-                        basesym.typeParams.map(_.name).mkString("[",",","]")+
-                        " gets applied to arguments "+baseargs.mkString("[",",","]")+", phase = "+phase)
-                    }
+                    else
+                      if (symclazz.tpe.parents.exists(_.isErroneous))
+                        ErrorType // don't be to overzealous with throwing exceptions, see #2641
+                      else
+                        throw new Error(
+                          "something is wrong (wrong class file?): "+basesym+
+                          " with type parameters "+
+                          basesym.typeParams.map(_.name).mkString("[",",","]")+
+                          " gets applied to arguments "+baseargs.mkString("[",",","]")+", phase = "+phase)
                   case ExistentialType(tparams, qtpe) =>
                     capturedSkolems = capturedSkolems union tparams
                     toInstance(qtpe, clazz)
@@ -6278,6 +6280,12 @@ trait Types extends api.Types { self: SymbolTable =>
     def this(msg: String) = this(NoPosition, msg)
   }
 
+  // TODO: RecoverableCyclicReference should be separated from TypeError,
+  // but that would be a big change. Left for further refactoring.
+  /** An exception for cyclic references from which we can recover */
+  case class RecoverableCyclicReference(sym: Symbol)
+    extends TypeError("illegal cyclic reference involving " + sym)
+
   class NoCommonType(tps: List[Type]) extends Throwable(
     "lub/glb of incompatible types: " + tps.mkString("", " and ", "")) with ControlThrowable
 
@@ -6286,9 +6294,6 @@ trait Types extends api.Types { self: SymbolTable =>
     def this(pre: Type, tp: String) = this("malformed type: " + pre + "#" + tp)
   }
 
-  /** An exception signalling a variance annotation/usage conflict */
-  class VarianceError(msg: String) extends TypeError(msg)
-
   /** The current indentation string for traces */
   private var indent: String = ""
 

src/compiler/scala/tools/nsc/ast/TreeBrowsers.scala

@@ -33,17 +33,16 @@ abstract class TreeBrowsers {
 
   val borderSize = 10
 
-
   def create(): SwingBrowser = new SwingBrowser();
 
   /** Pseudo tree class, so that all JTree nodes are treated uniformly */
   case class ProgramTree(units: List[UnitTree]) extends Tree {
-    override def toString(): String = "Program"
+    override def toString: String = "Program"
   }
 
   /** Pseudo tree class, so that all JTree nodes are treated uniformly */
   case class UnitTree(unit: CompilationUnit) extends Tree {
-    override def toString(): String = unit.toString()
+    override def toString: String = unit.toString
   }
 
   /**

src/compiler/scala/tools/nsc/ast/Trees.scala

@@ -17,7 +17,6 @@ import scala.reflect.internal.Flags.TRAIT
 trait Trees extends reflect.internal.Trees { self: Global =>
 
   // --- additional cases --------------------------------------------------------
-
   /** Only used during parsing */
   case class Parens(args: List[Tree]) extends Tree
 
@@ -31,7 +30,6 @@ trait Trees extends reflect.internal.Trees { self: Global =>
     override def isType = definition.isType
   }
 
-
   /** Either an assignment or a named argument. Only appears in argument lists,
    *  eliminated by typecheck (doTypedApply)
    */
@@ -40,7 +38,7 @@ trait Trees extends reflect.internal.Trees { self: Global =>
 
  /** Array selection <qualifier> . <name> only used during erasure */
   case class SelectFromArray(qualifier: Tree, name: Name, erasure: Type)
-       extends TermTree with RefTree { }
+       extends TermTree with RefTree
 
   /** emitted by typer, eliminated by refchecks */
   case class TypeTreeWithDeferredRefCheck()(val check: () => TypeTree) extends TypTree
@@ -163,7 +161,7 @@ trait Trees extends reflect.internal.Trees { self: Global =>
       traverser.traverse(qualifier)
     case ReferenceToBoxed(idt) =>
       traverser.traverse(idt)
-    case TypeTreeWithDeferredRefCheck() => // TODO: should we traverse the wrapped tree?
+    case TypeTreeWithDeferredRefCheck() =>
       // (and rewrap the result? how to update the deferred check? would need to store wrapped tree instead of returning it from check)
     case _ => super.xtraverse(traverser, tree)
   }

src/compiler/scala/tools/nsc/interactive/Global.scala

@@ -1060,6 +1060,8 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
 
   implicit def addOnTypeError[T](x: => T): OnTypeError[T] = new OnTypeError(x)
 
+  // OnTypeError should still catch TypeError because of cyclic references,
+  // but DivergentImplicit shouldn't leak anymore here
   class OnTypeError[T](op: => T) {
     def onTypeError(alt: => T) = try {
       op

src/compiler/scala/tools/nsc/plugins/Plugins.scala

@@ -28,7 +28,7 @@ trait Plugins {
     val dirs = (settings.pluginsDir.value split File.pathSeparator).toList map Path.apply
     val classes = Plugin.loadAllFrom(jars, dirs, settings.disable.value)
 
-    // Lach plugin must only be instantiated once. A common pattern
+    // Each plugin must only be instantiated once. A common pattern
     // is to register annotation checkers during object construction, so
     // creating multiple plugin instances will leave behind stale checkers.
     classes map (Plugin.instantiate(_, this))

src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala

@@ -59,7 +59,7 @@ abstract class Pickler extends SubComponent {
         }
       }
       // If there are any erroneous types in the tree, then we will crash
-      // when we pickle it: so let's report an erorr instead.  We know next
+      // when we pickle it: so let's report an error instead.  We know next
       // to nothing about what happened, but our supposition is a lot better
       // than "bad type: <error>" in terms of explanatory power.
       for (t <- unit.body ; if t.isErroneous) {

src/compiler/scala/tools/nsc/transform/UnCurry.scala

@@ -422,7 +422,7 @@ abstract class UnCurry extends InfoTransform
             if (tp.typeSymbol.isBottomClass) getManifest(AnyClass.tpe)
             else if (!manifestOpt.tree.isEmpty) manifestOpt.tree
             else if (tp.bounds.hi ne tp) getManifest(tp.bounds.hi)
-            else localTyper.getManifestTree(tree.pos, tp, false)
+            else localTyper.getManifestTree(tree, tp, false)
           }
           atPhase(phase.next) {
             localTyper.typedPos(pos) {

src/compiler/scala/tools/nsc/typechecker/Analyzer.scala

@@ -23,6 +23,7 @@ trait Analyzer extends AnyRef
             with Macros
             with NamesDefaults
             with TypeDiagnostics
+            with ContextErrors
 {
   val global : Global
   import global._