Fix #2099: Make param forwarders override the param they forward to

compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala

@@ -3,12 +3,13 @@ package transform
 
 import core._
 import dotty.tools.dotc.ast.tpd
-import dotty.tools.dotc.core.DenotTransformers.{SymTransformer, IdentityDenotTransformer}
+import dotty.tools.dotc.core.DenotTransformers.{SymTransformer, DenotTransformer}
 import Contexts.Context
 import Symbols._
 import Scopes._
 import Flags._
 import StdNames._
+import Denotations._
 import SymDenotations._
 import Types._
 import collection.mutable
@@ -25,15 +26,10 @@ import ValueClasses._
  *  Make private accessor in value class not-private. Ihis is necessary to unbox
  *  the value class when accessing it from separate compilation units
  *
- *  Also, make non-private any private parameter forwarders that forward to an inherited
- *  public or protected parameter accessor with the same name as the forwarder.
- *  This is necessary since private methods are not allowed to have the same name
- *  as inherited public ones.
- *
  *  See discussion in https://github.com/lampepfl/dotty/pull/784
  *  and https://github.com/lampepfl/dotty/issues/783
  */
-class ExpandPrivate extends MiniPhaseTransform with IdentityDenotTransformer { thisTransform =>
+class ExpandPrivate extends MiniPhaseTransform with DenotTransformer { thisTransform =>
   import ast.tpd._
 
   override def phaseName: String = "expandPrivate"
@@ -56,6 +52,15 @@ class ExpandPrivate extends MiniPhaseTransform with IdentityDenotTransformer { t
     }
   }
 
+  override def transform(ref: SingleDenotation)(implicit ctx: Context): SingleDenotation =
+    ref match {
+      case ref: SymDenotation if isVCPrivateParamAccessor(ref) =>
+        ref.ensureNotPrivate
+      case _ =>
+        ref
+    }
+
+
   private def isVCPrivateParamAccessor(d: SymDenotation)(implicit ctx: Context) =
     d.isTerm && d.is(PrivateParamAccessor) && isDerivedValueClass(d.owner)
 
@@ -65,9 +70,7 @@ class ExpandPrivate extends MiniPhaseTransform with IdentityDenotTransformer { t
    *  static members of the companion class, we should tighten the condition below.
    */
   private def ensurePrivateAccessible(d: SymDenotation)(implicit ctx: Context) =
-    if (isVCPrivateParamAccessor(d))
-      d.ensureNotPrivate.installAfter(thisTransform)
-    else if (d.is(PrivateTerm) && d.owner != ctx.owner.enclosingClass) {
+    if (d.is(PrivateTerm) && d.owner != ctx.owner.enclosingClass) {
       // Paths `p1` and `p2` are similar if they have a common suffix that follows
       // possibly different directory paths. That is, their common suffix extends
       // in both cases either to the start of the path or to a file separator character.
@@ -95,17 +98,4 @@ class ExpandPrivate extends MiniPhaseTransform with IdentityDenotTransformer { t
     ensurePrivateAccessible(tree.symbol)
     tree
   }
-
-  override def transformDefDef(tree: DefDef)(implicit ctx: Context, info: TransformerInfo) = {
-    val sym = tree.symbol
-    tree.rhs match {
-      case Apply(sel @ Select(_: Super, _), _)
-      if sym.is(PrivateParamAccessor) && sel.symbol.is(ParamAccessor) && sym.name == sel.symbol.name =>
-        sym.ensureNotPrivate.installAfter(thisTransform)
-      case _ =>
-        if (isVCPrivateParamAccessor(sym))
-          sym.ensureNotPrivate.installAfter(thisTransform)
-    }
-    tree
-  }
 }

compiler/src/dotty/tools/dotc/transform/ParamForwarding.scala

@@ -4,6 +4,7 @@ package transform
 import core._
 import ast.Trees._
 import Contexts._, Types._, Symbols._, Flags._, TypeUtils._, DenotTransformers._, StdNames._
+import Names._, NameOps._
 
 /** For all parameter accessors
  *
@@ -32,6 +33,8 @@ class ParamForwarding(thisTransformer: DenotTransformer) {
           }
         case _ => (Nil, Nil)
       }
+      def findAccessor(cls: Symbol, name: Name): Symbol =
+        cls.info.decl(name).suchThat(_ is (ParamAccessor, butNot = Mutable)).symbol
       def inheritedAccessor(sym: Symbol): Symbol = {
         /**
          * Dmitry: having it have the same name is needed to maintain correctness in presence of subclassing
@@ -44,9 +47,17 @@ class ParamForwarding(thisTransformer: DenotTransformer) {
          *   def s = 3
          *   assert(this.b == 2)
          * }
-         */
-        val candidate = sym.owner.asClass.superClass
-          .info.decl(sym.name).suchThat(_ is (ParamAccessor, butNot = Mutable)).symbol
+          */
+        val superCls = sym.owner.asClass.superClass
+        val candidate = {
+          val candidate0 = findAccessor(superCls, sym.name)
+          if (candidate0.exists)
+            candidate0
+          else
+            // If a forwarder was private, it will have been name-mangled by `ensureNotPrivate`
+            findAccessor(superCls, sym.name.expandedName(superCls))
+        }
+
         if (candidate.isAccessibleFrom(currentClass.thisType, superAccess = true)) candidate
         else if (candidate is Method) inheritedAccessor(candidate)
         else NoSymbol
@@ -63,7 +74,9 @@ class ParamForwarding(thisTransformer: DenotTransformer) {
                 val alias = inheritedAccessor(sym)
                 if (alias.exists) {
                   def forwarder(implicit ctx: Context) = {
-                    sym.copySymDenotation(initFlags = sym.flags | Method | Stable, info = sym.info.ensureMethodic)
+                    val flags = sym.flags | Method | Stable
+                    sym.copySymDenotation(initFlags = flags, info = sym.info.ensureMethodic)
+                      .ensureNotPrivate
                       .installAfter(thisTransformer)
                     val superAcc =
                       Super(This(currentClass), tpnme.EMPTY, inConstrCall = false).select(alias)
@@ -86,9 +99,10 @@ class ParamForwarding(thisTransformer: DenotTransformer) {
   def adaptRef[T <: RefTree](tree: T)(implicit ctx: Context): T = tree.tpe match {
     case tpe: TermRefWithSignature
     if tpe.sig == Signature.NotAMethod && tpe.symbol.is(Method) =>
-      // It's a param forwarder; adapt the signature
+      // It's a param forwarder; adapt the signature and the name
+      val name = tpe.symbol.asTerm.name
       tree.withType(
-        TermRef.withSig(tpe.prefix, tpe.name, tpe.prefix.memberInfo(tpe.symbol).signature))
+        TermRef.withSig(tpe.prefix, name, tpe.prefix.memberInfo(tpe.symbol).signature))
         .asInstanceOf[T]
     case _ =>
       tree

tests/pos/i2099/A_1.scala

@@ -0,0 +1,3 @@
+class A(val member: Int)
+
+class SubA(member: Int) extends A(member)

tests/pos/i2099/B_2.scala

@@ -0,0 +1 @@
+class B(member: Int) extends SubA(member)