Detect clash of mixedin val and existing member.

Before, we looked only at the result type, which was silly.
This was originally motivated by a hack to get to the error
about conflicting paramaccessors. The error detection for that
can now be formulated more directly.

Fixes scala/scala-dev#244

2 files changed, 20 insertions, 14 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Constructors.scala b/src/compiler/scala/tools/nsc/transform/Constructors.scala
index daf645fd20..92823bafb2 100644
--- a/src/compiler/scala/tools/nsc/transform/Constructors.scala
+++ b/src/compiler/scala/tools/nsc/transform/Constructors.scala
@@ -710,13 +710,19 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
 
       // Initialize all parameters fields that must be kept.
       val paramInits = paramAccessors filterNot omittableSym map { acc =>
-        // Check for conflicting symbol amongst parents: see bug #1960.
-        // It would be better to mangle the constructor parameter name since
-        // it can only be used internally, but I think we need more robust name
-        // mangling before we introduce more of it.
-        val conflict = clazz.info.nonPrivateMember(acc.name) filter (s => (s ne acc) && s.isGetter && !s.isOuterField && s.enclClass.isTrait)
-        if (conflict ne NoSymbol)
-          reporter.error(acc.pos, "parameter '%s' requires field but conflicts with %s".format(acc.name, conflict.fullLocationString))
+        // Check for conflicting field mixed in for a val/var defined in a parent trait (neg/t1960.scala).
+        // Since the fields phase has already mixed in fields, we can just look for
+        // an existing decl with the local variant of our paramaccessor's name.
+        //
+        // TODO: mangle the constructor parameter name (it can only be used internally), though we probably first need more robust name mangling
+
+        // sometimes acc is a field with a local name (when it's a val/var constructor param) --> exclude the `acc` itself when looking for conflicting decl
+        // sometimes it's not (just a constructor param) --> any conflicting decl is a problem
+        val conflict = clazz.info.decl(acc.name.localName).filter(sym => sym ne acc)
+        if (conflict ne NoSymbol) {
+          val orig = exitingTyper(clazz.info.nonPrivateMember(acc.name).filter(_ hasFlag ACCESSOR))
+          reporter.error(acc.pos, s"parameter '${acc.name}' requires field but conflicts with ${(orig orElse conflict).fullLocationString}")
+        }
 
         val accSetter =
           if (clazz.isTrait) acc.setterIn(clazz, hasExpandedName = true)
diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala
index f66e00ce1a..0fe7a82b15 100644
--- a/src/compiler/scala/tools/nsc/transform/Fields.scala
+++ b/src/compiler/scala/tools/nsc/transform/Fields.scala
@@ -359,12 +359,12 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
         val site = clazz.thisType
 
         // setter conflicts cannot arise independently from a getter conflict, since a setter without a getter does not a val definition make
-        def accessorConflictsExistingVal(accessor: Symbol): Boolean = {
-          val existingGetter = oldDecls.lookup(accessor.name.getterName)
-//          println(s"$existingGetter from $accessor to ${accessor.name.getterName}")
-          val tp = fieldTypeOfAccessorIn(accessor, site)
-          (existingGetter ne NoSymbol) && (tp matches (site memberInfo existingGetter).resultType) // !existingGetter.isDeferred && -- see (3)
-        }
+        def getterConflictsExistingVal(getter: Symbol): Boolean =
+          getter.isGetter && {
+            val existingGetter = oldDecls.lookup(getter.name)
+            (existingGetter ne NoSymbol) &&
+              ((site memberInfo existingGetter) matches (site memberInfo getter))
+          }
 
         def newModuleVarMember(module: Symbol): TermSymbol = {
           val moduleVar =
@@ -443,7 +443,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
           }
           // don't cause conflicts, skip overridden accessors contributed by supertraits (only act on the last overriding one)
           // see pos/trait_fields_dependent_conflict.scala and neg/t1960.scala
-          else if (accessorConflictsExistingVal(member) || isOverriddenAccessor(member, clazz)) Nil
+          else if (getterConflictsExistingVal(member) || isOverriddenAccessor(member, clazz)) Nil
           else if (member hasFlag MODULE) {
             val moduleVar = newModuleVarMember(member)
             List(moduleVar, newModuleAccessor(member, clazz, moduleVar))