Drop incorrect super accessor in trait subclass

When a trait extends a class and accesses a protected member (while in a
different package), we would emitted a super accessor and call that
instead.  That breaks the semantics because if the protected member is
overridden, that implementation won't be called, as the call is to the
super method instead.

In addition to that, the call to the super accessor was causing a false
positive error as we don't allow super calls to bind to vals.

Author: dwijnand

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

@@ -71,7 +71,7 @@ abstract class AccessProxies {
     def needsAccessor(sym: Symbol)(using Context): Boolean
 
     def ifNoHost(reference: RefTree)(using Context): Tree = {
-      assert(false, "no host found for $reference with ${reference.symbol.showLocated} from ${ctx.owner}")
+      assert(false, i"no host found for $reference with ${reference.symbol.showLocated} from ${ctx.owner}")
       reference
     }
 

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

@@ -33,17 +33,11 @@ object ProtectedAccessors {
       ctx.owner.isContainedIn(boundary) || ctx.owner.isContainedIn(boundary.linkedClass)
     }
 
-  /** Do we need a protected accessor if the current context's owner
-   *  is not in a subclass or subtrait of `sym`?
-   */
-  def needsAccessorIfNotInSubclass(sym: Symbol)(using Context): Boolean =
-    sym.isTerm && sym.is(Protected) &&
-    !sym.owner.is(Trait) && // trait methods need to be handled specially, are currently always public
-    !insideBoundaryOf(sym)
-
   /** Do we need a protected accessor for accessing sym from the current context's owner? */
   def needsAccessor(sym: Symbol)(using Context): Boolean =
-    needsAccessorIfNotInSubclass(sym) &&
+    sym.isTerm && sym.is(Protected) &&
+    !sym.owner.is(Trait) && // trait methods need to be handled specially, are currently always public
+    !insideBoundaryOf(sym) &&
     !ctx.owner.enclosingClass.derivesFrom(sym.owner)
 }
 

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

@@ -174,26 +174,20 @@ class SuperAccessors(thisPhase: DenotTransformer) {
     val sel @ Select(qual, name) = tree: @unchecked
     val sym = sel.symbol
 
-    /** If an accesses to protected member of a class comes from a trait,
-      *  or would need a protected accessor placed in a trait, we cannot
-      *  perform the access to the protected member directly since jvm access
-      *  restrictions require the call site to be in an actual subclass and
-      *  traits don't count as subclasses in this respect. In this case
-      *  we generate a super accessor instead. See SI-2296.
-      */
     def needsSuperAccessor =
-      ProtectedAccessors.needsAccessorIfNotInSubclass(sym) &&
+      ProtectedAccessors.needsAccessor(sym) &&
       AccessProxies.hostForAccessorOf(sym).is(Trait)
     qual match {
       case _: This if needsSuperAccessor =>
-        /*
-          * A trait which extends a class and accesses a protected member
-          *  of that class cannot implement the necessary accessor method
-          *  because jvm access restrictions require the call site to be in
-          *  an actual subclass and traits don't count as subclasses in this
-          *  respect. We generate a super accessor itself, which will be fixed
-          *  by the implementing class.  See SI-2296.
-          */
+        /* Given a protected member m defined in class C,
+         * and a trait T that calls m.
+         *
+         * If T extends C, then we can access it by casting
+         * the qualifier of the select to C.
+         *
+         * Otherwise, we need to go through an accessor,
+         * which the implementing class will provide an implementation for.
+         */
         superAccessorCall(sel)
       case Super(_, mix) =>
         transformSuperSelect(sel)

tests/neg/i11170a.scala renamed to tests/pos/i11170a.scala

@@ -23,6 +23,6 @@ package cpackage {
   import apackage._
   import bpackage._
 
-  case class C(override protected val x: Int) extends A with B  // error
+  case class C(override protected val x: Int) extends A with B
   case class C2(override val x: Int) extends A2 with B2
-}
+}

tests/run/i17021.defs.scala

@@ -0,0 +1,16 @@
+// Derives from run/i17021, but with defs instead of vals
+package p1:
+  class A:
+    protected def foo: Int = 1
+
+package p2:
+  trait B extends p1.A:
+    def bar: Int = foo
+
+  class C extends B:
+    override def foo: Int = 2
+
+object Test:
+  def main(args: Array[String]): Unit =
+    val n = new p2.C().bar
+    assert(n == 2, n) // was: assertion failed: 1

tests/run/i17021.scala

@@ -0,0 +1,18 @@
+package p1:
+  class A:
+    protected val foo: Int = 1
+
+package p2:
+  trait B extends p1.A:
+    def bar: Int = foo
+
+  class C extends B: // was: error: parent trait B has a super call which binds to the value p1.A.foo. Super calls can only target methods.
+    override val foo: Int = 2
+
+// Also, assert that the access continues to delegate:
+// i.e. B#bar delegates to this.foo and so C#bar returns 2,
+// not B#bar delegates to super.foo and so C#bar returns 1.
+object Test:
+  def main(args: Array[String]): Unit =
+    val n = new p2.C().bar
+    assert(n == 2, n) // was: assertion failed: 1