Skip to content

Commit 93bee55

Browse files
committed
SI-9408 Avoid capturing outer class in local classes.
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)
1 parent 342afbd commit 93bee55

File tree

3 files changed

+68
-0
lines changed

3 files changed

+68
-0
lines changed

src/compiler/scala/tools/nsc/transform/Constructors.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,12 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
165165
return
166166
}
167167

168+
// Note: elision of outer reference is based on a class-wise analysis, if a class might have subclasses,
169+
// it doesn't work. For example, `LocalParent` retains the outer reference in:
170+
//
171+
// class Outer { def test = {class LocalParent; class LocalChild extends LocalParent } }
172+
//
173+
// See run/t9408.scala for related test cases.
168174
val isEffectivelyFinal = clazz.isEffectivelyFinal
169175
def isParamCandidateForElision(sym: Symbol) = (sym.isParamAccessor && sym.isPrivateLocal)
170176
def isOuterCandidateForElision(sym: Symbol) = (sym.isOuterAccessor && isEffectivelyFinal && !sym.isOverridingSymbol)

src/reflect/scala/reflect/internal/Symbols.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -992,6 +992,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
992992
isPrivate
993993
|| isLocalToBlock
994994
)
995+
|| isClass && originalOwner.isTerm && children.isEmpty // we track known subclasses of term-owned classes, use that infer finality
995996
)
996997
/** Is this symbol effectively final or a concrete term member of sealed class whose children do not override it */
997998
final def isEffectivelyFinalOrNotOverridden: Boolean = isEffectivelyFinal || (isTerm && !isDeferred && isNotOverridden)

test/files/run/t9408.scala

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
class Outer {
2+
def assertNoFields(c: Class[_]) {
3+
assert(c.getDeclaredFields.isEmpty)
4+
}
5+
def assertHasOuter(c: Class[_]) {
6+
assert(c.getDeclaredFields.exists(_.getName.contains("outer")))
7+
}
8+
class Member
9+
final class FinalMember
10+
11+
def test {
12+
assertHasOuter(classOf[Member])
13+
assertNoFields(classOf[FinalMember])
14+
final class C
15+
assertNoFields(classOf[C])
16+
class D
17+
assertNoFields(classOf[D])
18+
(() => {class E; assertNoFields(classOf[E])}).apply()
19+
20+
// The outer reference elision currently runs on a class-by-class basis. If it cannot rule out that a class has
21+
// subclasses, it will not remove the outer reference. A smarter analysis here could detect if no members of
22+
// a sealed (or effectively sealed) hierarchy use the outer reference, the optimization could be performed.
23+
class Parent
24+
class Child extends Parent
25+
assertHasOuter(classOf[Parent])
26+
27+
// Note: outer references (if they haven't been elided) are used in pattern matching as follows.
28+
// This isn't relevant to term-owned classes, as you can't refer to them with a prefix that includes
29+
// the outer class.
30+
val outer1 = new Outer
31+
val outer2 = new Outer
32+
(new outer1.Member: Any) match {
33+
case _: outer2.Member => sys.error("wrong match!")
34+
case _: outer1.Member => // okay
35+
}
36+
37+
// ... continuing on that theme, note that `Member` isn't considered as a local class, it is owned by a the class
38+
// `LocalOuter`, which itself happens to be term-owned. So we expect that it has an outer reference, and that this
39+
// is respected in type tests.
40+
class LocalOuter {
41+
class Member
42+
final class FinalMember
43+
}
44+
assertNoFields(classOf[LocalOuter])
45+
assertHasOuter(classOf[LocalOuter#Member])
46+
val localOuter1 = new LocalOuter
47+
val localOuter2 = new LocalOuter
48+
(new localOuter1.Member: Any) match {
49+
case _: localOuter2.Member => sys.error("wrong match!")
50+
case _: localOuter1.Member => // okay
51+
}
52+
// Final member classes still lose the outer reference.
53+
assertNoFields(classOf[LocalOuter#FinalMember])
54+
}
55+
}
56+
57+
object Test {
58+
def main(args: Array[String]): Unit = {
59+
new Outer().test
60+
}
61+
}

0 commit comments

Comments
 (0)