Simplify (ambiguous) error issuing.

The two functional differences are:
  - always add the diagnostics string
  - check erroneousness in `issueAmbiguousTypeErrorUnlessErroneous`,
    before even constructing the error message.

Consider this nugget:

```
-    def issueAmbiguousError(pre: Type, sym1: Symbol, sym2: Symbol, err: AbsTypeError) {
-      issueCommon(err) { case _ if ambiguousErrors =>
-        if (!pre.isErroneous && !sym1.isErroneous && !sym2.isErroneous)
```

I'd like to state for the record that the if-erroneous in the
case of the partial function looked super-dodgy: it meant that,
when `ambiguousErrors`, `issueCommon` would not get to the `else` branches
that buffer or throw, and if the erroneous condition was met,
nothing would be issued/buffered/thrown.

This refactoring checks this condition up front.

3 files changed, 22 insertions, 39 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
index 9715fdaf00..5c31685bcd 100644
--- a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
@@ -90,10 +90,6 @@ trait ContextErrors {
       issueTypeError(SymbolTypeError(sym, msg))
     }
 
-    def issueAmbiguousTypeError(pre: Type, sym1: Symbol, sym2: Symbol, err: AmbiguousTypeError)(implicit context: Context) {
-      context.issueAmbiguousError(pre, sym1, sym2, err)
-    }
-
     def issueTypeError(err: AbsTypeError)(implicit context: Context) { context.issue(err) }
 
     def typeErrorMsg(found: Type, req: Type) = "type mismatch" + foundReqMsg(found, req)
@@ -883,19 +879,21 @@ trait ContextErrors {
         val WrongNumber, NoParams, ArgsDoNotConform = Value
       }
 
-      private def ambiguousErrorMsgPos(pos: Position, pre: Type, sym1: Symbol, sym2: Symbol, rest: String) =
-        if (sym1.hasDefault && sym2.hasDefault && 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)
-          )
+      private def issueAmbiguousTypeErrorUnlessErroneous(pos: Position, pre: Type, sym1: Symbol, sym2: Symbol, rest: String): Unit =
+        if (!(pre.isErroneous || sym1.isErroneous || sym2.isErroneous)) {
+          if (sym1.hasDefault && sym2.hasDefault && sym1.enclClass == sym2.enclClass) {
+            val methodName = nme.defaultGetterToMethod(sym1.name)
+            context.issueAmbiguousError(AmbiguousTypeError(sym1.enclClass.pos,
+             "in "+ sym1.enclClass +", multiple overloaded alternatives of " + methodName +
+                       " define default arguments"))
+          } else {
+            context.issueAmbiguousError(AmbiguousTypeError(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)
+            ))
+          }
         }
 
       def AccessError(tree: Tree, sym: Symbol, ctx: Context, explanation: String): AbsTypeError =
@@ -952,8 +950,7 @@ trait ContextErrors {
           val msg0 =
             "argument types " + argtpes.mkString("(", ",", ")") +
            (if (pt == WildcardType) "" else " and expected result type " + pt)
-          val (pos, msg) = ambiguousErrorMsgPos(tree.pos, pre, best, firstCompeting, msg0)
-          issueAmbiguousTypeError(pre, best, firstCompeting, AmbiguousTypeError(pos, msg))
+          issueAmbiguousTypeErrorUnlessErroneous(tree.pos, pre, best, firstCompeting, msg0)
           setErrorOnLastTry(lastTry, tree)
         } else setError(tree) // do not even try further attempts because they should all fail
                               // even if this is not the last attempt (because of the SO's possibility on the horizon)
@@ -966,8 +963,7 @@ trait ContextErrors {
       }
 
       def AmbiguousExprAlternativeError(tree: Tree, pre: Type, best: Symbol, firstCompeting: Symbol, pt: Type, lastTry: Boolean) = {
-        val (pos, msg) = ambiguousErrorMsgPos(tree.pos, pre, best, firstCompeting, "expected type " + pt)
-        issueAmbiguousTypeError(pre, best, firstCompeting, AmbiguousTypeError(pos, msg))
+        issueAmbiguousTypeErrorUnlessErroneous(tree.pos, pre, best, firstCompeting, "expected type " + pt)
         setErrorOnLastTry(lastTry, tree)
       }
 
diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
index 72ca9b879a..aa2394897c 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
@@ -543,37 +543,23 @@ trait Contexts { self: Analyzer =>
     private def unitError(pos: Position, msg: String): Unit =
       if (checking) onTreeCheckerError(pos, msg) else reporter.error(pos, msg)
 
-    @inline private def issueCommon(err: AbsTypeError)(pf: PartialFunction[AbsTypeError, Unit]) {
+    @inline private def issueCommon(err: AbsTypeError, reportError: Boolean) {
       // TODO: are errors allowed to have pos == NoPosition??
       // if not, Jason suggests doing: val pos = err.errPos.orElse( { devWarning("Que?"); context.tree.pos })
       if (settings.Yissuedebug) {
         log("issue error: " + err.errMsg)
         (new Exception).printStackTrace()
       }
-      if (pf isDefinedAt err) pf(err)
+      if (reportError) unitError(err.errPos, addDiagString(err.errMsg))
       else if (bufferErrors) { reportBuffer += err }
       else throw new TypeError(err.errPos, err.errMsg)
     }
 
     /** Issue/buffer/throw the given type error according to the current mode for error reporting. */
-    def issue(err: AbsTypeError) {
-      issueCommon(err) { case _ if reportErrors =>
-        unitError(err.errPos, addDiagString(err.errMsg))
-      }
-    }
+    private[typechecker] def issue(err: AbsTypeError) = issueCommon(err, reportErrors)
 
     /** Issue/buffer/throw the given implicit ambiguity error according to the current mode for error reporting. */
-    def issueAmbiguousError(pre: Type, sym1: Symbol, sym2: Symbol, err: AbsTypeError) {
-      issueCommon(err) { case _ if ambiguousErrors =>
-        if (!pre.isErroneous && !sym1.isErroneous && !sym2.isErroneous)
-          unitError(err.errPos, err.errMsg)
-      }
-    }
-
-    /** Issue/buffer/throw the given implicit ambiguity error according to the current mode for error reporting. */
-    def issueAmbiguousError(err: AbsTypeError) {
-      issueCommon(err) { case _ if ambiguousErrors => unitError(err.errPos, addDiagString(err.errMsg)) }
-    }
+    private[typechecker] def issueAmbiguousError(err: AbsTypeError) = issueCommon(err, ambiguousErrors)
 
     /** Issue/throw the given `err` according to the current mode for error reporting. */
     def error(pos: Position, err: Throwable) =
diff --git a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
index 73c3e6f016..6b5a70bffb 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
@@ -1381,6 +1381,7 @@ trait Implicits {
         def maybeInvalidConversionError(msg: String) {
           // We have to check context.ambiguousErrors even though we are calling "issueAmbiguousError"
           // which ostensibly does exactly that before issuing the error. Why? I have no idea. Test is pos/t7690.
+          // AM: I would guess it's because ambiguous errors will be buffered in silent mode if they are not reported
           if (context.ambiguousErrors)
             context.issueAmbiguousError(AmbiguousImplicitTypeError(tree, msg))
         }