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

Improve binary compatibility for inner classes that lose the outer pointer #627

Open
retronym opened this issue May 22, 2019 · 0 comments
Open

Comments

@retronym
Copy link
Member

$ cat sandbox/A_1.scala
class C {
  class Inner
}

object Test {
  def main(args: Array[String]): Unit = {
    val c = new C
    val i = new c.Inner
    (i: Any) match {
      case _: c.Inner =>
        println("matched `case _: c.Inner`")
    }
  }
}

$ cat sandbox/A_2.scala
class C {
  final class Inner
}

$ scalac sandbox/A_1.scala && scala Test && scalac sandbox/A_2.scala && scala Test
matched `case _: c.Inner`
java.lang.NoSuchMethodError: C$Inner.C$Inner$$$outer()LC;
	at Test$.main(A_1.scala:10)
	at Test.main(A_1.scala)

Inner classes marked final do not store the outer pointer unless it is captured. They still have the outer parameter in their constructor, so new c.Inner is binary compatible in the example above.

  // access flags 0x1
  public main([Ljava/lang/String;)V
    // parameter final  args
    NEW C
    DUP
    INVOKESPECIAL C.<init> ()V
    ASTORE 3
    NEW C$Inner
    DUP
    ALOAD 3
    INVOKESPECIAL C$Inner.<init> (LC;)V

But type tests in pattern matching and isInstanceOf that check prefixes have a binary fragility.

    ASTORE 4
    ALOAD 4
    ASTORE 5
    ALOAD 5
    INSTANCEOF C$Inner
    IFEQ L0
    ALOAD 5
    CHECKCAST C$Inner
    INVOKEVIRTUAL C$Inner.C$Inner$$$outer ()LC;
    ALOAD 3
    IF_ACMPNE L0
    GETSTATIC scala/Predef$.MODULE$ : Lscala/Predef$;
    LDC "matched `case _: c.Inner`"

The runtime prefix check is elided if it can the prefix of the scrutinee is statically the same as that of the tested type, hence the upcast to Any in this test case.

We could make this more binary compatible by still generating the $outer accessor in final inner classes (it would return null) and treating a null prefix in the scrutinee as a wildcard for the prefix check.

@retronym retronym added this to the Backlog milestone May 22, 2019
@adriaanm adriaanm modified the milestones: Backlog, 2.14 May 22, 2019
dwijnand added a commit to dwijnand/mima that referenced this issue May 23, 2019
dwijnand added a commit to dwijnand/mima that referenced this issue May 23, 2019
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

3 participants