Skip to content

Drop incorrect super accessor in trait subclass #17062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
35 changes: 19 additions & 16 deletions compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 _ =>
Expand Down
7 changes: 5 additions & 2 deletions compiler/test/dotty/tools/vulpix/ParallelTesting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 6 additions & 0 deletions tests/neg/i17021.ext-java/A.java
Original file line number Diff line number Diff line change
@@ -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; }
}
14 changes: 14 additions & 0 deletions tests/neg/i17021.ext-java/Test.scala
Original file line number Diff line number Diff line change
@@ -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)
4 changes: 2 additions & 2 deletions tests/neg/i11170a.scala → tests/pos/i11170a.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
16 changes: 16 additions & 0 deletions tests/run/i17021.defs.scala
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions tests/run/i17021.ext-java/A.java
Original file line number Diff line number Diff line change
@@ -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; }
}
15 changes: 15 additions & 0 deletions tests/run/i17021.ext-java/Test.scala
Original file line number Diff line number Diff line change
@@ -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)
18 changes: 18 additions & 0 deletions tests/run/i17021.scala
Original file line number Diff line number Diff line change
@@ -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