Drop incorrect super accessor in trait subclass (#17062)

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/SuperAccessors.scala

@@ -174,27 +174,30 @@ 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) &&
       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.
-          */
-        superAccessorCall(sel)
+        /* 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.
+         *
+         * That's because the protected method is actually public,
+         * so we can call it.  For truly protected methods, like from
+         * Java, we error instead of emitting the wrong code (i17021.ext-java).
+         *
+         * Otherwise, we need to go through an accessor,
+         * which the implementing class will provide an implementation for.
+         */
+        if ctx.owner.enclosingClass.derivesFrom(sym.owner) then
+          if sym.is(JavaDefined) then
+            report.error(em"${ctx.owner} accesses protected $sym inside a concrete trait method: use super.${sel.name} instead", sel.srcPos)
+          sel
+        else
+          superAccessorCall(sel)
       case Super(_, mix) =>
         transformSuperSelect(sel)
       case _ =>

compiler/test/dotty/tools/vulpix/ParallelTesting.scala

@@ -751,8 +751,11 @@ trait ParallelTesting extends RunnerOrchestration { self =>
           case _ =>
         }
         case Failure(output) =>
-          echo(s"Test '${testSource.title}' failed with output:")
-          echo(output)
+          if output == "" then
+            echo(s"Test '${testSource.title}' failed with no output")
+          else
+            echo(s"Test '${testSource.title}' failed with output:")
+            echo(output)
           failTestSource(testSource)
         case Timeout =>
           echo("failed because test " + testSource.title + " timed out")

tests/neg/i17021.ext-java/A.java

@@ -0,0 +1,6 @@
+// Derives from run/i17021.defs, but with a Java protected member
+package p1;
+
+public class A {
+  protected int foo() { return 1; }
+}

tests/neg/i17021.ext-java/Test.scala

@@ -0,0 +1,14 @@
+// Derives from run/i17021.defs
+// but with a Java protected member
+// which leads to a compile error
+package p2:
+  trait B extends p1.A:
+    def bar: Int = foo // error: method bar accesses protected method foo inside a concrete trait method: use super.foo instead
+
+  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)

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.ext-java/A.java

@@ -0,0 +1,6 @@
+// Derives from run/i17021.defs, but with a Java protected member
+package p1;
+
+public class A {
+  protected int foo() { return 1; }
+}

tests/run/i17021.ext-java/Test.scala

@@ -0,0 +1,15 @@
+// scalajs: --skip
+// Derives from run/i17021.defs
+// but with a Java protected member
+// and fixed calling code, that uses super
+package p2:
+  trait B extends p1.A:
+    def bar: Int = super.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 == 1, n)

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