Default methods, mixin and invokeSpecial on JVM

[DarkDimius]

class A { def m = 1 }
class B extends A { override def m = 2 }
trait T extends A
class C extends B with T {
  override def m = super[T].m // should invoke A.m currently invokes B.m
}

scala/scala-dev#143 (comment)
I have a fix in mind, but I will not have time to try it out before going for vacation.
Making a note for myself to come back to it in September.

[lrytz]

could you quickly summarize your idea here..? :)

[DarkDimius]

Two options:

1. Just before the phase ExpandPrivate move the original body to a method with a name mangled making it globally unique. Similar to how it is done by ExpandPrivate now.
   The very same phase will rewrite super calls to call those uniquely named methods.

   The major disadvantage would be increase in size of stack traces. AFAIK, unlike .NET, HotSpot does not expose a way to hide those entries from stack. I would also expect a minor increase in size of generated bytecode.

2. When compiling class containing super-call(C in your example) create a @static val that delegates to reflection in standard library to find the specific target. Compile the super-call itself to an invokeDynamic that evaluates to a ConstantCallSite that points to method that was found using reflection. This one can be benchmarked before implementing it in compiler.

   If this one works right and is fast, I would prefer it, as 90% of it can be implemented at library side and I'd prefer to start moving more such magic to library. This logic is complex, it will have bugs. If it is entirely implemented on library side, it can be fixed by a library bump without need for recompilation.

[DarkDimius]

btw: the 2 can be shared between Dotty and scalac.

[odersky]

I also think we should try (2). My logic is that we are working around a VM restriction, namely that one cannot call specific methods in superclasses. Scala.JS and Scala.native do not have the problem; for them it is trivial. So rather than generate avalanches of additional bytecode I would prefer if we could fix the problem by giving the target VM more capabilities through reflection-base libraries. The big unknown is how well this would optimize. To find out we have to get an implementation first.

[odersky]

I retract my remark. @DarkDimius and I just had session where we played with it. Here's the code:

class B { def m: Int = 0 }
class C extends B { override def m = 4 }
trait T1 extends B { override def m = 1 }
trait T2 extends T1 { override def m = 2 }
trait T3 extends T1 { override def m = 3 }

trait T4 extends T1
class D extends B {
  def f() = println(super[B].m)
}

object Test extends C with T2 with T3 with T4 with T0 {
  def main(args: Array[String]): Unit = {
    println(m)
    println(super[T2].m)
    println(super[T3].m)
    println(super[T4].m)
    println(super[T0].m) // should print 0, prints 4
    new D().f()
  }
}

@DarkDimius will summarize our conclusions.

[retronym]

I take it by "that points to method that was found using reflection" you mean to use unreflectSpecial which "will bypass checks for overriding methods on the receiver, as if called from an invokespecial instruction from within the explicitly specified specialCaller".

Sounds like a good plan.

I'd considered something similar once to implement access to private members without publicing them in bytecode.

[odersky]

@retronym Good find! If it is easy to do then let's do it. But the problem is less pervasive than I thought initially. Essentially it only applies when an explicit super call targets a class method, not a trait method. Mixin forwarders should not be affected.

[DarkDimius]

ping @lrytz @adriaanm.
Just had a long meeting with @odersky, considering the options here. Note that this bug also affects 2.11.8. Giving a proper fix for this bug would change the behavior of already existing code. If existing projects rely on current behavior, bug created because of this change could be very hard to track and may require the code to be rewritten.

We've decided, that at least for dotty we would prefer to warn for cases that may misbehave. We have played with HotSpot and reread the spec several times, and decided to:

• warn on super[X].foo, where:
  1. X is an interface that inherits a class C;
  2. X does not provide implementation for method foo;
  3. X inherits implementation of foo from a class.
• in traits, warn on super[C] syntax where C is a superclass of this trait. Calling other super[mixin] is allowed unless it's contradicts previous rule. Calling super[<empty>] is also allowed but means that runtime virtual dispatch would be used.

Depending on frequency of those warning we may decide not to implement the scheme 2 that I've proposed above, and instead simply reject the code that would require it.

[smarter]

Handled in scalac by scala/scala@a980fde which got refined by scala/scala#5377