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

SI-9408 Avoid capturing outer class in local classes. #4652

Merged
merged 3 commits into from
Jul 23, 2015

Conversation

retronym
Copy link
Member

Review by @lrytz

@scala-jenkins scala-jenkins added this to the 2.12.0-M3 milestone Jul 22, 2015
@retronym
Copy link
Member Author

Commit message error:

-class NoSerializable$C(outer$: C)
+class NoSerializable$C(outer$: NoSerializable)

retronym added 3 commits July 23, 2015 14:32
Using the same facility that we use to record subclasses of
sealed classes, record the subclasses of term-owned ("local")
classes.

I have changed existing callers of `children` to use `sealedChildren`
so we don't start using this new information in pattern matching
and type pattern checkability analysis.

The following commit will build on this to infer finality of local
classes in the context of outer pointer elision in the constructors
phase.
 - Check if the clazz that owns all the decls we're filtering is
effectively final once, rather than for each decl.
 - Query the full set of decls once.
Previously, only local classes declared final would be candidates
for outer pointer elision in the constructor phase.

This commit infers finality of local classes to expand the scope
of this optimization.

== Background ==

This was brought to our attention when shapeless enabled
indylambda and found that a hitherto serializable
data structure started to capture the enclosing class and hence
lost its serializability.

    class NotSerializable {
      def test = () => {
        class C; assertSerializable(new C)
      }
    }

Under `-Ydelambdafy:inline`, it used to capture the enclosing anon
function class as its outer, which, being final, didn't in turn
capture the enclosing class.

    class NotSerializable {
      def test = new anonFun$1
    }
    class anonFun$1 {
      def apply = assertSerializable(new C(this))
    }
    class ...$C(outer$: anonFun)

indylambda perturbs the enclosing structure of the function body.

    class NotSerializable {
      def anonFun$1 = {class C; assertSerializable(new C()))
      def test = lambdaMetaFactory(<<anonFun$1>>)
    }

Which leads to:

    class NotSerializable$C(outer$: NotSerializable)
@retronym
Copy link
Member Author

@lrytz Ready for re-review.

@retronym
Copy link
Member Author

Previous review comments:
retronym@a4dbd57
retronym@9c89d20

@lrytz
Copy link
Member

lrytz commented Jul 23, 2015

LGTM!

@retronym
Copy link
Member Author

I've spun out a new ticket for the suggested improvements: https://issues.scala-lang.org/browse/SI-9414

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

Successfully merging this pull request may close these issues.

4 participants