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

[backport] SI-9375 add synthetic readResolve only for static modules #4757

Merged
merged 1 commit into from
Sep 22, 2015

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Sep 21, 2015

Backport of #4671 to 2.11.x

For inner modules, the synthetic readResolve method would cause the
module constructor to be invoked on de-serialization in certain
situations. See the discussion in the ticket.

Adds a comprehensive test around serializing and de-serializing
modules.

@retronym
Copy link
Member

LGTM

@lrytz
Copy link
Member Author

lrytz commented Sep 21, 2015

MiMa failed. The reason is that the synthetic readResolve method can make the accessor method of a private nested object "notPrivate":

➜  sandbox git:(opt/heuristics) ✗ cat Test.scala
class O {
  private object N extends Serializable
}

➜  sandbox git:(opt/heuristics) ✗ scalac Test.scala
➜  sandbox git:(opt/heuristics) ✗ asm *.class
➜  sandbox git:(opt/heuristics) ✗ mv *.asm a

➜  sandbox git:(opt/heuristics) ✗ ~/scala/scala2/build/quick/bin/scalac Test.scala
➜  sandbox git:(opt/heuristics) ✗ asm *.class
➜  sandbox git:(opt/heuristics) ✗ mv *.asm b

➜  sandbox git:(opt/heuristics) ✗ diff -r a b
diff -r a/O$N$.asm b/O$N$.asm
12,23d11
<   // access flags 0x1012
<   private final synthetic LO; $outer
<
<   // access flags 0x2
<   private readResolve()Ljava/lang/Object;
<     ALOAD 0
<     GETFIELD O$N$.$outer : LO;
<     INVOKEVIRTUAL O.O$$N ()LO$N$;
<     ARETURN
<     MAXSTACK = 1
<     MAXLOCALS = 1
<
26,34d13
<     ALOAD 1
<     IFNONNULL L0
<     ACONST_NULL
<     ATHROW
<    L0
<    FRAME SAME
<     ALOAD 0
<     ALOAD 1
<     PUTFIELD O$N$.$outer : LO;
38c17
<     MAXSTACK = 2
---
>     MAXSTACK = 1
diff -r a/O.asm b/O.asm
18c18
<   private O$$N$lzycompute()LO$N$;
---
>   private N$lzycompute()LO$N$;
54,55c54,55
<   // access flags 0x1
<   public O$$N()LO$N$;
---
>   // access flags 0x2
>   private N()LO$N$;
60c60
<     INVOKESPECIAL O.O$$N$lzycompute ()LO$N$;
---
>     INVOKESPECIAL O.N$lzycompute ()LO$N$;

@lrytz
Copy link
Member Author

lrytz commented Sep 21, 2015

@retronym wdyt?

@retronym
Copy link
Member

I'd suggest to manually call clazz.sourceModule.makeNotPrivate(clazz.sourceModule.owner) keep the signatures identical in 2.11.x.

@lrytz
Copy link
Member Author

lrytz commented Sep 21, 2015

Calling makeNotPrivate during SyntheticMethods (i.e. during Typers) is too early, it needs to happen after everything is resolved:

class C {
  private object N extends Serializable
  def foo = N.toString
}

gives

Test.scala:3: error: not found: value N
  def foo = N.toString
            ^
one error found

@lrytz
Copy link
Member Author

lrytz commented Sep 21, 2015

Moved it to RefChecks. Thanks Jason for the suggestion!

For inner modules, the synthetic readResolve method would cause the
module constructor to be invoked on de-serialization in certain
situations. See the discussion in the ticket.

Adds a comprehensive test around serializing and de-serializing
modules.
@retronym
Copy link
Member

LGTM

lrytz added a commit that referenced this pull request Sep 22, 2015
[backport] SI-9375 add synthetic readResolve only for static modules
@lrytz lrytz merged commit a6c1687 into scala:2.11.x Sep 22, 2015
@lrytz lrytz deleted the t9375-2.11 branch September 23, 2015 10:02
@SethTisue SethTisue added this to the 2.11.8 milestone Sep 24, 2015
@retronym retronym added the release-notes worth highlighting in next release notes label Feb 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants