Skip to content
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

Missing trait initialisers in certain cases, compared to Scala 2 #10839

Closed
dwijnand opened this issue Dec 17, 2020 · 6 comments
Closed

Missing trait initialisers in certain cases, compared to Scala 2 #10839

dwijnand opened this issue Dec 17, 2020 · 6 comments
Milestone

Comments

@dwijnand
Copy link
Member

Minimized code

class A
trait B extends A {
  def foo = 2
}

or

trait A {
  def foo: Int
}
trait B extends A {
  def foo = 2
}

Output

$ echo 'class A; trait B extends A { def foo = 2 }' > A.scala && scalac3 A.scala && javap B
Compiled from "A.scala"
public interface B {
  public static int foo$(B);
  public default int foo();
}
$
$ echo 'trait A { def foo: Int }; trait B extends A { def foo = 2 }' > A.scala && scalac3 A.scala && javap B
Compiled from "A.scala"
public interface B extends A {
  public static int foo$(B);
  public default int foo();
}

Expectation

Same as Scala 2:

$ echo 'class A; trait B extends A { def foo = 2 }' > A.scala && scalac A.scala && javap B
Compiled from "A.scala"
public interface B {
  public static int foo$(B);
  public default int foo();
  public static void $init$(B);
}
$
$ echo 'trait A { def foo: Int }; trait B extends A { def foo = 2 }' > A.scala && scalac A.scala && javap B
Compiled from "A.scala"
public interface B extends A {
  public static int foo$(B);
  public default int foo();
  public static void $init$(B);
}

This came from some test cases in MiMa where changes within such hierarchies cause binary incompatibilities. I'd love if they didn't, but I think Scalac 2 has good reason to be the way it is and I believe we don't want to deviate from that in Scala 3. So perhaps this one is a 3.0 blocker...

@dwijnand dwijnand added this to the 3.0.0-RC1 milestone Dec 17, 2020
@dwijnand
Copy link
Member Author

Tentatively milestoned.

@sjrd
Copy link
Member

sjrd commented Dec 17, 2020

See #10530, where I left this intentionally "unsolved". (look for "diachronic" in there)

@dwijnand
Copy link
Member Author

I think Scalac 2 has good reason to be the way it is and I believe we don't want to deviate from that in Scala 3. So perhaps this one is a 3.0 blocker...

As I said in the meeting: I can believe there are reasons to do things differently, but that should be for 4.0 (3.T) IMO, not 3.0. We'll discuss again next time, when hopefully Seb is present.

@dwijnand
Copy link
Member Author

dwijnand commented Jan 4, 2021

So Scala 3 doesn't add $init$ when a trait def is concrete (has an impl). @sjrd so does that makes these binary compatible changes?

// functional-tests/src/test/trait-pushing-up-concrete-methods-is-nok/v1/A.scala
trait A
trait B extends A {
  def foo: Int = 2
}
// functional-tests/src/test/trait-pushing-up-concrete-methods-is-nok/v2/A.scala
trait A {
  def foo: Int = 2
}
trait B extends A
// functional-tests/src/test/moving-method-upward-from-trait-to-class-nok/v1/A.scala
class A
trait B extends A {
  def foo = 2
}

// functional-tests/src/test/moving-method-upward-from-trait-to-class-nok/v2/A.scala
class A {
  def foo = 2
}
trait B extends A
// functional-tests/src/test/trait-deleting-concrete-methods-is-nok/v1/A.scala
trait A {
  def foo: Int
}
trait B extends A {
  override def foo: Int = 2
}
class C extends B
// functional-tests/src/test/trait-deleting-concrete-methods-is-nok/v2/A.scala
trait A {
  def foo: Int = 2
}
trait B extends A
class C extends B

If yes, then great, the new scheme makes such changes compatible. If no, then less good, because while it may make more sense these changes are still incompatible (and adds some diachronic complexity).

@sjrd
Copy link
Member

sjrd commented Jan 4, 2021

They're unfortunately still incompatible, but not due to $init$. They're incompatible because of the static forwarders used for super calls.

@dwijnand
Copy link
Member Author

dwijnand commented Jan 4, 2021

Thanks for confirming.

I'm happy to close this, taking the status quo as "better because it makes more sense" (default methods).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants