scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

NoSuchMethodError when accessing an override val via super #12715

Closed Atry closed 1 year ago

Atry commented 1 year ago

Reproduction steps

Scala version: 2.13.10

trait A {
  def f: String
}

trait B extends A {
  def f = "B";
}

trait C extends A {
  override val f = "C"
}

trait D extends A with B {
  def d = super.f
}

object O extends B with C with D

O.f
O.d

See https://scastie.scala-lang.org/2L5tOgBESIu7v9D9yyK7gQ

Problem

java.lang.NoSuchMethodError: 'java.lang.String Playground$C.f$(Playground$C)'
    at Playground$O$.Playground$D$$super$f(main.scala:19)
    at Playground$D.d(main.scala:16)
    at Playground$D.d$(main.scala:15)
    at Playground$O$.d(main.scala:19)
    at Playground$.<clinit>(main.scala:22)
    at Main$.<clinit>(main.scala:27)
    at Main.main(main.scala)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at sbt.Run.invokeMain(Run.scala:143)
    at sbt.Run.execute$1(Run.scala:93)
    at sbt.Run.$anonfun$runWithLoader$5(Run.scala:120)
    at sbt.Run$.executeSuccess(Run.scala:186)
    at sbt.Run.runWithLoader(Run.scala:120)
    at sbt.Run.run(Run.scala:127)
    at com.olegych.scastie.sbtscastie.SbtScastiePlugin$$anon$1.$anonfun$run$1(SbtScastiePlugin.scala:38)
    at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
    at sbt.util.InterfaceUtil$$anon$1.get(InterfaceUtil.scala:17)
    at sbt.ScastieTrapExit$App.run(ScastieTrapExit.scala:258)
    at java.base/java.lang.Thread.run(Thread.java:831)

This bug is similar to #10308, but this one can be reproduced without abstract override.

SethTisue commented 1 year ago

This stuff requires spec lawyering to even figure out what the behavior should be (though obviously it shouldn't be NoSuchMethodError!).

As I understand it, the relevant spec sections are SLS 5.4 https://scala-lang.org/files/archive/spec/2.13/05-classes-and-objects.html#overriding which says:

Assume a trait D defines some aspect of an instance x of type C (i.e. D is a base class of C). Then the actual supertype of D in x is the compound type consisting of all the base classes in L[inearization](C) that succeed D. The actual supertype gives the context for resolving a super reference in a trait. Note that the actual supertype depends on the type to which the trait is added in a mixin composition; it is not statically known at the time the trait is defined

and SLS 6.5 https://scala-lang.org/files/archive/spec/2.13/06-expressions.html#this-and-super which says:

A reference super.m refers statically to a method or type m in the least proper supertype of the innermost template containing the reference. It evaluates to the member m' in the actual supertype of that template which is equal to m or which overrides m

som-snytt commented 1 year ago

I just reported a super crasher to dotty, based on a forum post. That was a restriction in scala 2.

I'm not sure how this wound up as a "corner" of the language. (Like the corners of tables that need those plastic bumpers so toddlers don't strike their heads; or the corner of a room into which one paints oneself.)

Would I remember if there were a FAQ entry on "When should I prefer a trait or a class?" that mentions these limits to mixability?

lrytz commented 1 year ago

Thanks for the spec pointers. So the resolution of super.f in a trait depends on how the trait is mixed in:

scala> trait A { def f: String }
     | trait B extends A { def f: String = "B" }
     | trait C extends A { override def f: String = "C" }
     | trait D extends B { def d = super.f }
     | (new B with C with D).d + " - " + (new B with D with C).d
     |

val res6: String = C - B

To implement super calls to "default" (concrete) methods in interface classfiles, every such method gets a static "super accessor" method next it, e.g., for trait T { def f = 1 }:

public static /*synthetic*/ int f$(T $this) { return $this.f(); } // invokespecial, prevents dynamic dispatch
default public int f() { return 1 }

Fields in traits are a different story, as interface classfiles cannot have fields. The interface gets an abstract getter method, the field is generated in the subclass classfile with a concrete getter.

Thinking about all this, I believe the example should be disallowed at compile time. Two observations:

trait A {
  def f: String
}

trait B extends A {
  def f = "B";
}

trait C extends A {
  // override val f = "C"
  override def f = "C"
}

trait D extends C {
  // override val f = "D"
  override def f = "D"
}

trait E extends A with B {
  def d = super.f
}

object O1 extends B with C with D with E // `O1.d` is "D"
object O2 extends B with C with E with D // `O2.d` is "C"
object O3 extends B with E with C with D // `O3.d` is "B"

If we change the two overrides to be val, we cannot implement the same semantics. All three objects have a single f field which holds the value "D" in all three instances. So there's no way O2.d could return the value "C".

odersky commented 1 year ago

I tend to agree that we should disallow it.