Merge pull request #2124 from dotty-staging/fix-param-fwd

Fix parameter accessor forwarding

Author: smarter

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -1719,7 +1719,7 @@ object Types {
      *  under both private and public names, so it could still be found by looking up
      *  the public name.
      */
-    final def shadowed(implicit ctx: Context): NamedType =
+    def shadowed(implicit ctx: Context): NamedType =
       NamedType(prefix, name.shadowedName)
 
     override def equals(that: Any) = that match {
@@ -1782,15 +1782,19 @@ object Types {
       else d.atSignature(sig).checkUnique
     }
 
-    override def newLikeThis(prefix: Type)(implicit ctx: Context): TermRef = {
-      val candidate = TermRef.withSig(prefix, name, sig)
+    private def fixDenot(candidate: TermRef, prefix: Type)(implicit ctx: Context): TermRef =
       if (symbol.exists && !candidate.symbol.exists) { // recompute from previous symbol
         val ownSym = symbol
         val newd = asMemberOf(prefix, allowPrivate = ownSym.is(Private))
         candidate.withDenot(newd.suchThat(_.signature == ownSym.signature))
       }
       else candidate
-    }
+
+    override def newLikeThis(prefix: Type)(implicit ctx: Context): TermRef =
+      fixDenot(TermRef.withSig(prefix, name, sig), prefix)
+
+    override def shadowed(implicit ctx: Context): NamedType = 
+      fixDenot(TermRef.withSig(prefix, name.shadowedName, sig), prefix)
 
     override def equals(that: Any) = that match {
       case that: TermRefWithSignature =>

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

@@ -4,6 +4,8 @@ package transform
 import core._
 import ast.Trees._
 import Contexts._, Types._, Symbols._, Flags._, TypeUtils._, DenotTransformers._, StdNames._
+import Decorators._
+import config.Printers.typr
 
 /** For all parameter accessors
  *
@@ -48,7 +50,7 @@ class ParamForwarding(thisTransformer: DenotTransformer) {
         val candidate = sym.owner.asClass.superClass
           .info.decl(sym.name).suchThat(_ is (ParamAccessor, butNot = Mutable)).symbol
         if (candidate.isAccessibleFrom(currentClass.thisType, superAccess = true)) candidate
-        else if (candidate is Method) inheritedAccessor(candidate)
+        else if (candidate.exists) inheritedAccessor(candidate)
         else NoSymbol
       }
       def forwardParamAccessor(stat: Tree): Tree = {
@@ -65,8 +67,13 @@ class ParamForwarding(thisTransformer: DenotTransformer) {
                   def forwarder(implicit ctx: Context) = {
                     sym.copySymDenotation(initFlags = sym.flags | Method | Stable, info = sym.info.ensureMethodic)
                       .installAfter(thisTransformer)
-                    val superAcc =
+                    var superAcc =
                       Super(This(currentClass), tpnme.EMPTY, inConstrCall = false).select(alias)
+                    if (alias.owner != currentClass.superClass)
+                      // need to use shadowed in order not to accidentally address an
+                      // intervening private forwarder in the superclass
+                      superAcc = superAcc.withType(superAcc.tpe.asInstanceOf[TermRef].shadowed)
+                    typr.println(i"adding param forwarder $superAcc")
                     DefDef(sym, superAcc.ensureConforms(sym.info.widen))
                   }
                   return forwarder(ctx.withPhase(thisTransformer.next))

tests/run/paramForwarding.check

@@ -13,4 +13,4 @@ NonVal:
 X:
 private final int X.theValue$$local
 Y:
-private final int Y.theValue$$local
+

tests/run/paramForwarding.scala

@@ -19,7 +19,7 @@ class Bz extends A(42) {
   val theValueInBz = theValue
 }
 
-// C does not contains a field C.theValue$$local, it contains
+// C does not contain a field C.theValue$$local, it contains
 // a getter C.theValue() which only calls super.theValue()
 class C(override val theValue: Int) extends A(theValue)
 
@@ -36,8 +36,8 @@ class NonVal(theValue: Int) extends A(theValue) {
 // X.theValue() which overrides A.theValue()
 class X(override val theValue: Int) extends NonVal(0)
 
-// Y contains a field Y.theValue$$local accessible using the getter
-// Y.theValue() which overrides A.theValue()
+// Y does not contain a field Y.theValue$$local, it contains
+// a getter Y.theValue() which only calls super.theValue()
 class Y(override val theValue: Int) extends NonVal(theValue)
 
 

tests/run/paramForwarding_separate/A_1.scala

@@ -1,3 +1,7 @@
-class A(val member: Int)
+class A(val member: Int) {
+  def getAMember = member
+}
 
-class SubA(member: Int) extends A(member)
+class SubA(member: Int) extends A(member) {
+  def getSubAMember = member
+}

tests/run/paramForwarding_separate/B_2.scala

@@ -1,4 +1,6 @@
-class B(member: Int) extends SubA(member)
+class B(member: Int) extends SubA(member) {
+  def getMember = member
+}
 
 object Test {
   def printFields(cls: Class[_]) =

tests/run/paramForwarding_together.check

@@ -0,0 +1,6 @@
+# Fields in A:
+private final int A.member$$local
+# Fields in SubA:
+
+# Fields in B:
+

tests/run/paramForwarding_together.scala

@@ -0,0 +1,29 @@
+class A(val member: Int) {
+  def getAMember = member
+}
+
+class SubA(member: Int) extends A(member) {
+  def getSubAMember = member
+}
+
+class B(member: Int) extends SubA(member) {
+  def getBMember = member
+}
+
+object Test {
+  def printFields(cls: Class[_]) =
+    println(cls.getDeclaredFields.map(_.toString).sorted.deep.mkString("\n"))
+
+  def main(args: Array[String]): Unit = {
+    val a = new A(10)
+    val subA = new SubA(11)
+    val b = new B(12)
+
+    println("# Fields in A:")
+    printFields(classOf[A])
+    println("# Fields in SubA:")
+    printFields(classOf[SubA])
+    println("# Fields in B:")
+    printFields(classOf[B])
+  }
+}

tests/run/paramForwarding_together_b.check

@@ -0,0 +1,6 @@
+# Fields in A:
+private final int A.member$$local
+# Fields in SubA:
+
+# Fields in B:
+

tests/run/paramForwarding_together_b.scala

@@ -0,0 +1,30 @@
+class B(member: Int) extends SubA(member) {
+  def getBMember = member
+}
+
+class SubA(member: Int) extends A(member) {
+  def getSubAMember = member
+}
+
+class A(val member: Int) {
+  def getAMember = member
+}
+
+
+object Test {
+  def printFields(cls: Class[_]) =
+    println(cls.getDeclaredFields.map(_.toString).sorted.deep.mkString("\n"))
+
+  def main(args: Array[String]): Unit = {
+    val a = new A(10)
+    val subA = new SubA(11)
+    val b = new B(12)
+
+    println("# Fields in A:")
+    printFields(classOf[A])
+    println("# Fields in SubA:")
+    printFields(classOf[SubA])
+    println("# Fields in B:")
+    printFields(classOf[B])
+  }
+}