Adding a `lazy val` to a `trait` emits backwards-compatible code with wrong semantics

[armanbilge]

Reproduction steps

tl;dr sbt app/runMain Bar in https://github.com/armanbilge/sandbox/commit/48881c4402029c1bb8055d17ad53a52b2abf8e52

1. Compile old Foo.

trait Foo {
  def main(args: Array[String]): Unit = ()
}

2. Compile Bar.

   object Bar extends Foo

3. Compile new Foo.

   trait Foo {
   lazy val sameSame: AnyRef = new AnyRef
   
   def main(args: Array[String]): Unit = println(sameSame eq sameSame)
   }

4. Run Bar with new Foo.

   sbt:sandbox> app/runMain Bar
   [info] running Bar 
   false

Problem

Lazy val semantics dictate that sameSame eq sameSame should always be true.

Analysis

If we look at the decompiled bytecode for new Foo, we see it has emitted a default public Object sameSame() method which it is using as an accessor. However, because Bar was compiled against an old Foo, it is not overriding that method. Therefore the lazy val effectively has the semantics of a def.

It seems like that should not be a default method, since the default implementation does not have the correct semantics.

import scala.Predef$;
import scala.runtime.BoxesRunTime;

public interface Foo {
    public static /* synthetic */ Object sameSame$(Foo $this) {
        return $this.sameSame();
    }

    default public Object sameSame() {
        return new Object();
    }

    public static /* synthetic */ void main$(Foo $this, String[] args) {
        $this.main(args);
    }

    default public void main(String[] args) {
        Predef$.MODULE$.println((Object)BoxesRunTime.boxToBoolean((this.sameSame() == this.sameSame() ? 1 : 0) != 0));
    }

    public static void $init$(Foo $this) {
    }
}

Credits

@s5bug for stumbling on this by way of https://github.com/bkirwi/decline/issues/461.

[som-snytt]

Isn't this a MiMa bug? "Binary compatible" doesn't just mean JVM linkage. MiMa should tell me if clients require recompilation.

[armanbilge]

"Binary compatible" doesn't just mean JVM linkage. MiMa should tell me if clients require recompilation.

Hm, this has not been my understanding of MiMa to date. See also https://github.com/lightbend/mima/issues/200 which is not a linking error, but still requires client recompilation.

---

As I pointed out on the dotty issue, why not just emit an abstract method here instead of a default one? What is the utility of a default method that must be overridden to achieve the correct semantics? This would have the benefit of triggering MiMa, since it would also be a linking error.

[sjrd]

The default method carries the initialization code for the rhs of the lazy val. The override generated in the subclass calls it with super.sameSame() to initialize the field the first time it is accessed. So we cannot make it abstract.

We could argue that it would have been better to put that initialization code in a different method. But that's not something we can change now while remaining binary-compatible with existing compiled code.

[armanbilge]

But that's not something we can change now while remaining binary-compatible with existing compiled code.

But note that any code that is relying on this binary-compatibility is actually bugged.

[sjrd]

No. Every class that correctly implements the trait, i.e., that is compiled with the trait already has the lazy val, relies on actually calling that method. If you remove it, you break all lazy vals defined in traits everywhere.

[armanbilge]

Sorry, hum, I see now. The problem is that the initializer and the accessor have been made the same method, and it's too late to change that.

[som-snytt]

I was going to propose adding a line to the FAQ on when to avoid traits, but it seems necessary to first propose adding the FAQ to the docs.