Some cleanup from investigating #4041, with a c...

Some cleanup from investigating #4041, with a comment instead of a fix
for the ticket. Review by rytz in case he sees a good way to fix it.

2 files changed, 32 insertions, 16 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala
index 1560f32e04..120a2efc4b 100644
--- a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala
@@ -472,14 +472,24 @@ trait NamesDefaults { self: Analyzer =>
           val typedAssign = try {
             typer.silent(_.typed(arg, subst(paramtpe)))
           } catch {
-            // `silent` only catches and returns TypeErrors which are not CyclicReferences
-            // fix for #3685
-            case cr @ CyclicReference(sym, info) if (sym.name == param.name) =>
+            // `silent` only catches and returns TypeErrors which are not
+            // CyclicReferences.  Fix for #3685
+            case cr @ CyclicReference(sym, info) if sym.name == param.name =>
+              // If this condition included sym.isMethod, #4041 issues a sensible
+              // error instead of crashing; but some programs which now compile would
+              // require a return type annotation.
               if (sym.isVariable || sym.isGetter && sym.accessed.isVariable) {
                 // named arg not allowed
-                typer.context.error(sym.pos, "variable definition needs type because the name is used as named argument the definition.")
+                typer.context.error(sym.pos,
+                  "%s definition needs %s because '%s' is used as a named argument in its body.".format(
+                    "variable",   // "method"
+                    "type",       // "result type"
+                    sym.name
+                  )
+                )
                 typer.infer.setError(arg)
-              } else cr                                                            // named arg OK
+              }
+              else cr
           }
           val res = typedAssign match {
             case _: TypeError =>
@@ -491,11 +501,17 @@ trait NamesDefaults { self: Analyzer =>
               rhs
             case t: Tree =>
               if (!t.isErroneous) {
-              // this throws an exception that's caught in `tryTypedApply` (as it uses `silent`)
-              // unfortunately, tryTypedApply recovers from the exception if you use errorTree(arg, ...) and conforms is allowed as a view (see tryImplicit in Implicits)
-              // because it tries to produce a new qualifier (if the old one was P, the new one will be conforms.apply(P)), and if that works, it pretends nothing happened
-              // so, to make sure tryTypedApply fails, would like to pass EmptyTree instead of arg, but can't do that because eventually setType(ErrorType) is called, and EmptyTree only accepts NoType as its tpe
-              // thus, we need to disable conforms as a view...
+                // This throws an exception which is caught in `tryTypedApply` (as it
+                // uses `silent`) - unfortunately, tryTypedApply recovers from the
+                // exception if you use errorTree(arg, ...) and conforms is allowed as
+                // a view (see tryImplicit in Implicits) because it tries to produce a
+                // new qualifier (if the old one was P, the new one will be
+                // conforms.apply(P)), and if that works, it pretends nothing happened.
+                //
+                // To make sure tryTypedApply fails, we would like to pass EmptyTree
+                // instead of arg, but can't do that because eventually setType(ErrorType)
+                // is called, and EmptyTree can only be typed NoType.  Thus we need to
+                // disable conforms as a view...
                 errorTree(arg, "reference to "+ name +" is ambiguous; it is both, a parameter\n"+
                              "name of the method and the name of a variable currently in scope.")
               } else t // error was reported above
diff --git a/test/files/neg/names-defaults-neg.check b/test/files/neg/names-defaults-neg.check
index 3b3dd53b2e..5d409f18a8 100644
--- a/test/files/neg/names-defaults-neg.check
+++ b/test/files/neg/names-defaults-neg.check
@@ -131,26 +131,26 @@ names-defaults-neg.scala:135: error: parameter specified twice: a
 names-defaults-neg.scala:136: error: wrong number of parameters; expected = 2
   val taf4: (Int, String) => Unit = testAnnFun(_, b = _)
                                               ^
-names-defaults-neg.scala:144: error: variable definition needs type because the name is used as named argument the definition.
+names-defaults-neg.scala:144: error: variable definition needs type because 'x' is used as a named argument in its body.
   def t3 { var x = t.f(x = 1) }
                ^
-names-defaults-neg.scala:147: error: variable definition needs type because the name is used as named argument the definition.
+names-defaults-neg.scala:147: error: variable definition needs type because 'x' is used as a named argument in its body.
   object t6 { var x = t.f(x = 1) }
                   ^
-names-defaults-neg.scala:150: error: variable definition needs type because the name is used as named argument the definition.
+names-defaults-neg.scala:150: error: variable definition needs type because 'x' is used as a named argument in its body.
   class t9 { var x = t.f(x = 1) }
                  ^
-names-defaults-neg.scala:164: error: variable definition needs type because the name is used as named argument the definition.
+names-defaults-neg.scala:164: error: variable definition needs type because 'x' is used as a named argument in its body.
   def u3 { var x = u.f(x = 1) }
                ^
-names-defaults-neg.scala:167: error: variable definition needs type because the name is used as named argument the definition.
+names-defaults-neg.scala:167: error: variable definition needs type because 'x' is used as a named argument in its body.
   def u6 { var x = u.f(x = "32") }
                ^
 names-defaults-neg.scala:170: error: reference to x is ambiguous; it is both, a parameter
 name of the method and the name of a variable currently in scope.
   def u9 { var x: Int = u.f(x = 1) }
                               ^
-names-defaults-neg.scala:177: error: variable definition needs type because the name is used as named argument the definition.
+names-defaults-neg.scala:177: error: variable definition needs type because 'x' is used as a named argument in its body.
   class u15 { var x = u.f(x = 1) }
                   ^
 names-defaults-neg.scala:180: error: reference to x is ambiguous; it is both, a parameter