Skip to content

Scala 2.12.3 regression: interfaces of a class are removed if they exist in a superclass #10473

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

Closed
sjrd opened this issue Aug 17, 2017 · 9 comments

Comments

@sjrd
Copy link
Member

sjrd commented Aug 17, 2017

Scala 2.12.2:

$ scala
Welcome to Scala 2.12.2 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_60).
Type in expressions for evaluation. Or try :help.

scala> final class RuntimeLong extends java.lang.Number with java.io.Serializable {
     |   def intValue(): Int = 0; def longValue(): Long = 0L; def floatValue(): Float = 0.0f; def doubleValue(): Double = 0.0
     | }
defined class RuntimeLong

scala> classOf[RuntimeLong].getInterfaces
res0: Array[Class[_]] = Array(interface java.io.Serializable)

scala> class A extends java.io.Serializable
defined class A

scala> class B extends A with java.io.Serializable
defined class B

scala> classOf[B].getInterfaces
res1: Array[Class[_]] = Array(interface java.io.Serializable)

Scala 2.12.3:

$ scala
Welcome to Scala 2.12.3 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_60).
Type in expressions for evaluation. Or try :help.

scala> final class RuntimeLong extends java.lang.Number with java.io.Serializable {
     |   def intValue(): Int = 0; def longValue(): Long = 0L; def floatValue(): Float = 0.0f; def doubleValue(): Double = 0.0
     | }
defined class RuntimeLong

scala> classOf[RuntimeLong].getInterfaces
res0: Array[Class[_]] = Array()

scala> class A extends java.io.Serializable
defined class A

scala> class B extends A with java.io.Serializable
defined class B

scala> classOf[B].getInterfaces
res1: Array[Class[_]] = Array()

This causes MiMa to report compatibility errors like the following, when upgrading a library from Scala 2.12.2 to 2.12.3:

[info] Scala.js library: found 1 potential binary incompatibilities while checking against org.scala-js:scalajs-library_2.12:0.6.19
[error]  * the type hierarchy of class scala.scalajs.runtime.RuntimeLong is different in current version. Missing types {java.io.Serializable}
[error]    filter with: ProblemFilters.exclude[MissingTypesProblem]("scala.scalajs.runtime.RuntimeLong")

I discovered this in scala-js/scala-js#3098. See the log of the failing build at https://scala-webapps.epfl.ch/jenkins/job/scalajs-task-worker/189205/console

@lrytz
Copy link
Member

lrytz commented Aug 17, 2017

Isn't that change binary compatible and it's a MiMa issue? It might be caused by scala/scala#5792

@sjrd
Copy link
Member Author

sjrd commented Aug 17, 2017

It's binary compatible if you control your parent classes. MiMa (correctly IMO) assumes that classes from separate jars are not under your control.

In any case, IMO dropping the interface is wrong. If I explicitly write it in my source code, I want it to be 1) in the doc and 2) in the class files.

@gzm0
Copy link

gzm0 commented Aug 19, 2017

Just to be absolutely sure, does this mean that the following code produces different results on 2.12.2 and 2.12.3?

trait A
trait B extends A
class C

println(classOf[C].getInterfaces)

@sjrd
Copy link
Member Author

sjrd commented Aug 19, 2017

@gzm0 I think you forgot some extends ... after class C.

@lrytz
Copy link
Member

lrytz commented Aug 19, 2017

No, the change is only with respect to Java-defined interfaces. We're minimizing parent interfaces for a long time. For example in

trait A
trait B extends A
class C extends A with B

only B ends up as an interface in C. That's in 2.10, 2.11, 2.12.

@sjrd
Copy link
Member Author

sjrd commented Aug 19, 2017

So ... does that mean that this change is intended and will stay forever? It's not consistent with Java, FWIW:

public class Test {
  public static void main(String[] args) {
    for (Class<?> intf: C.class.getInterfaces())
      System.out.println(intf);
  }
}

interface A {}

class B extends Object implements A {}

class C extends B implements A {}

prints

interface A

A is not deduplicated in C.

@lrytz
Copy link
Member

lrytz commented Aug 28, 2017

One observation we made while changing the trait method implementation to default methods in 2.12: default method resolution (during classfile parsing, in the JVM) gets a huge slowdown by the number of parent interfaces (scala/scala-dev#98 (comment)).

Minimizing parents was originally added in scala/scala@7a99c03da1, the commit message mentions android (probably referring to https://groups.google.com/forum/#!topic/scala-debate/El3QZEbIwHg) and #5278.

@lrytz
Copy link
Member

lrytz commented Sep 4, 2017

So I'd still call this a MiMa bug. If one of your parents changes to remove one of its interfaces, then that's where the binary incompatible change is, and not in your code.

I'm for closing this ticket.

@sjrd
Copy link
Member Author

sjrd commented Sep 4, 2017

I still think it's not normal that the compiler would just ignore things I have textually specified.

But I won't fight this. For our specific issue we've anyway worked around it already: scala-js/scala-js@2cdbbfd

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

No branches or pull requests

4 participants