Skip to content

Commit 3bb7358

Browse files
retronymretronym
committed
Merge pull request scala#5099 from retronym/ticket/9390
SI-9390 Emit local defs that don't capture this as static
2 parents 361f3f1 + f07019f commit 3bb7358

File tree

17 files changed

+192
-33
lines changed

17 files changed

+192
-33
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,9 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
715715
primaryConstrBody.expr)
716716
})
717717

718+
if (omittableAccessor.exists(_.isOuterField) && !constructorStats.exists(_.exists { case i: Ident if i.symbol.isOuterParam => true; case _ => false}))
719+
primaryConstructor.symbol.updateAttachment(OuterArgCanBeElided)
720+
718721
val constructors = primaryConstructor :: auxConstructors
719722

720723
// Unlink all fields that can be dropped from class scope

‎src/compiler/scala/tools/nsc/transform/Delambdafy.scala

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
225225
}
226226
}
227227

228+
228229
private def transformFunction(originalFunction: Function): Tree = {
229230
val target = targetMethod(originalFunction)
230231
assert(target.hasFlag(Flags.STATIC))
@@ -272,10 +273,21 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
272273
val Template(parents, self, body) = super.transform(deriveTemplate(tree)(_.mapConserve(pretransform)))
273274
Template(parents, self, body ++ boxingBridgeMethods)
274275
} finally boxingBridgeMethods.clear()
276+
case dd: DefDef if dd.symbol.isLiftedMethod && !dd.symbol.isDelambdafyTarget =>
277+
// SI-9390 emit lifted methods that don't require a `this` reference as STATIC
278+
// delambdafy targets are excluded as they are made static by `transformFunction`.
279+
if (!dd.symbol.hasFlag(STATIC) && !methodReferencesThis(dd.symbol))
280+
dd.symbol.setFlag(STATIC)
281+
super.transform(tree)
282+
case Apply(fun, outer :: rest) if shouldElideOuterArg(fun.symbol, outer) =>
283+
val nullOuter = gen.mkZero(outer.tpe)
284+
treeCopy.Apply(tree, transform(fun), nullOuter :: transformTrees(rest))
275285
case _ => super.transform(tree)
276286
}
277287
} // DelambdafyTransformer
278288

289+
private def shouldElideOuterArg(fun: Symbol, outerArg: Tree): Boolean =
290+
fun.isConstructor && treeInfo.isQualifierSafeToElide(outerArg) && fun.hasAttachment[OuterArgCanBeElided.type]
279291

280292
// A traverser that finds symbols used but not defined in the given Tree
281293
// TODO freeVarTraverser in LambdaLift does a very similar task. With some
@@ -326,19 +338,28 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
326338

327339
// recursively find methods that refer to 'this' directly or indirectly via references to other methods
328340
// for each method found add it to the referrers set
329-
private def refersToThis(symbol: Symbol): Boolean =
330-
(thisReferringMethods contains symbol) ||
331-
(liftedMethodReferences(symbol) exists refersToThis) && {
332-
// add it early to memoize
333-
debuglog(s"$symbol indirectly refers to 'this'")
334-
thisReferringMethods += symbol
335-
true
341+
private def refersToThis(symbol: Symbol): Boolean = {
342+
var seen = mutable.Set[Symbol]()
343+
def loop(symbol: Symbol): Boolean = {
344+
if (seen(symbol)) false
345+
else {
346+
seen += symbol
347+
(thisReferringMethods contains symbol) ||
348+
(liftedMethodReferences(symbol) exists loop) && {
349+
// add it early to memoize
350+
debuglog(s"$symbol indirectly refers to 'this'")
351+
thisReferringMethods += symbol
352+
true
353+
}
336354
}
355+
}
356+
loop(symbol)
357+
}
337358

338359
private var currentMethod: Symbol = NoSymbol
339360

340361
override def traverse(tree: Tree) = tree match {
341-
case DefDef(_, _, _, _, _, _) if tree.symbol.isDelambdafyTarget =>
362+
case DefDef(_, _, _, _, _, _) if tree.symbol.isDelambdafyTarget || tree.symbol.isLiftedMethod =>
342363
// we don't expect defs within defs. At this phase trees should be very flat
343364
if (currentMethod.exists) devWarning("Found a def within a def at a phase where defs are expected to be flattened out.")
344365
currentMethod = tree.symbol
@@ -349,6 +370,12 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
349370
// They'll be of the form {(args...) => this.anonfun(args...)}
350371
// but we do need to make note of the lifted body method in case it refers to 'this'
351372
if (currentMethod.exists) liftedMethodReferences(currentMethod) += targetMethod(fun)
373+
case Apply(sel @ Select(This(_), _), args) if sel.symbol.isLiftedMethod =>
374+
if (currentMethod.exists) liftedMethodReferences(currentMethod) += sel.symbol
375+
super.traverseTrees(args)
376+
case Apply(fun, outer :: rest) if shouldElideOuterArg(fun.symbol, outer) =>
377+
super.traverse(fun)
378+
super.traverseTrees(rest)
352379
case This(_) =>
353380
if (currentMethod.exists && tree.symbol == currentMethod.enclClass) {
354381
debuglog(s"$currentMethod directly refers to 'this'")

‎src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ abstract class ExplicitOuter extends InfoTransform
6767
result
6868
}
6969

70-
private val innerClassConstructorParamName: TermName = newTermName("arg" + nme.OUTER)
71-
7270
class RemoveBindingsTransformer(toRemove: Set[Symbol]) extends Transformer {
7371
override def transform(tree: Tree) = tree match {
7472
case Bind(_, body) if toRemove(tree.symbol) => super.transform(body)
@@ -169,7 +167,7 @@ abstract class ExplicitOuter extends InfoTransform
169167

170168
val paramsWithOuter =
171169
if (sym.isClassConstructor && isInner(sym.owner)) // 1
172-
sym.newValueParameter(innerClassConstructorParamName, sym.pos).setInfo(sym.owner.outerClass.thisType) :: params
170+
sym.newValueParameter(nme.OUTER_ARG, sym.pos).setInfo(sym.owner.outerClass.thisType) :: params
173171
else params
174172

175173
if ((resTpTransformed ne resTp) || (paramsWithOuter ne params)) MethodType(paramsWithOuter, resTpTransformed)

‎src/reflect/scala/reflect/internal/StdAttachments.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,9 @@ trait StdAttachments {
7171
abstract class InlineAnnotatedAttachment
7272
case object NoInlineCallsiteAttachment extends InlineAnnotatedAttachment
7373
case object InlineCallsiteAttachment extends InlineAnnotatedAttachment
74+
75+
/** Attached to a local class that has its outer field elided. A `null` constant may be passed
76+
* in place of the outer parameter, can help callers to avoid capturing the outer instance.
77+
*/
78+
case object OuterArgCanBeElided extends PlainAttachment
7479
}

‎src/reflect/scala/reflect/internal/StdNames.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,7 @@ trait StdNames {
365365
val MODULE_INSTANCE_FIELD: NameType = NameTransformer.MODULE_INSTANCE_NAME // "MODULE$"
366366
val OUTER: NameType = "$outer"
367367
val OUTER_LOCAL: NameType = OUTER.localName
368+
val OUTER_ARG: NameType = "arg" + OUTER
368369
val OUTER_SYNTH: NameType = "<outer>" // emitted by virtual pattern matcher, replaced by outer accessor in explicitouter
369370
val ROOTPKG: NameType = "_root_"
370371
val SELECTOR_DUMMY: NameType = "<unapply-selector>"

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,9 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
914914
/** Is this symbol an accessor method for outer? */
915915
final def isOuterField = isArtifact && (unexpandedName == nme.OUTER_LOCAL)
916916

917+
/** Is this symbol an outer parameter in a constructor */
918+
final def isOuterParam = isParameter && owner.isConstructor && (name == nme.OUTER_ARG || name == nme.OUTER)
919+
917920
/** Does this symbol denote a stable value, ignoring volatility?
918921
*
919922
* Stability and volatility are checked separately to allow volatile paths in patterns that amount to equality checks. SI-6815

‎src/reflect/scala/reflect/runtime/JavaUniverseForce.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse =>
4545
this.SubpatternsAttachment
4646
this.NoInlineCallsiteAttachment
4747
this.InlineCallsiteAttachment
48+
this.OuterArgCanBeElided
4849
this.noPrint
4950
this.typeDebug
5051
this.Range

‎test/files/jvm/t9105.check

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,8 @@
1-
#partest -Ydelambdafy:inline
2-
(class C$$anonfun$1$A$1,class C$$anonfun$1,null)
3-
(class C$$anonfun$1$B$1,class C$$anonfun$1,private final java.lang.Object C$$anonfun$1.m$1())
4-
(class C$$anonfun$1$C$1,class C$$anonfun$1,null)
5-
(class C$$anonfun$1$$anonfun$2$D$1,class C$$anonfun$1$$anonfun$2,null)
6-
(class C$$anonfun$met$1$E$1,class C$$anonfun$met$1,null)
7-
(class C$$anonfun$met$1$F$1,class C$$anonfun$met$1,private final java.lang.Object C$$anonfun$met$1.m$2())
8-
(class C$$anonfun$met$1$G$1,class C$$anonfun$met$1,null)
9-
(class C$$anonfun$met$1$$anonfun$3$H$1,class C$$anonfun$met$1$$anonfun$3,null)
10-
#partest !-Ydelambdafy:inline
111
(class C$A$1,class C,null)
12-
(class C$B$1,class C,private final java.lang.Object C.m$1())
2+
(class C$B$1,class C,private static final java.lang.Object C.m$1())
133
(class C$C$1,class C,null)
144
(class C$D$1,class C,null)
155
(class C$E$1,class C,public scala.Function0 C.met())
16-
(class C$F$1,class C,private final java.lang.Object C.m$2())
6+
(class C$F$1,class C,private static final java.lang.Object C.m$2())
177
(class C$G$1,class C,public scala.Function0 C.met())
188
(class C$H$1,class C,public scala.Function0 C.met())

‎test/files/run/t5652.check

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
public default int T1.T1$$g$1()
21
public default int T1.f0()
32
public default void T1.$init$()
4-
public final int A1.A1$$g$2()
3+
public static int T1.T1$$g$1()
54
public int A1.f1()
6-
public final int A2.A2$$g$1()
5+
public static final int A1.A1$$g$2()
76
public int A2.f2()
7+
public static final int A2.A2$$g$1()

‎test/files/run/t5652b.check

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
private final int A1.g$1()
1+
private static final int A1.g$1()
22
public int A1.f1()
3-
private final int A2.g$1()
3+
private static final int A2.g$1()
44
public int A2.f2()

‎test/files/run/t5652c.check

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
public final int A1.A1$$g$1()
2-
public final int A1.A1$$g$2()
31
public int A1.f1()
42
public int A1.f2()
3+
public static final int A1.A1$$g$1()
4+
public static final int A1.A1$$g$2()
55
1
66
2

‎test/files/run/t9390.scala

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
class C {
2+
def methodLift1 = {
3+
def isEven(c: Int) = c % 2 == 0
4+
val f: Int => Boolean = isEven
5+
f
6+
}
7+
def methodLift2 = {
8+
def isEven(c: Int) = c % 2 == 0
9+
def isEven0(c: Int) = isEven(c)
10+
val f: Int => Boolean = isEven0
11+
f
12+
}
13+
14+
def methodLift3 = {
15+
def isEven(c: Int) = {toString; c % 2 == 0}
16+
def isEven0(c: Int) = isEven(c)
17+
val f: Int => Boolean = isEven0
18+
f
19+
}
20+
}
21+
22+
object Test {
23+
def main(args: Array[String]): Unit = {
24+
val c = new C
25+
26+
{
27+
val f = c.methodLift1
28+
assert(f(0))
29+
assert(!f(1))
30+
val f1 = serializeDeserialize(f)
31+
assert(f1(0))
32+
assert(!f1(1))
33+
}
34+
35+
36+
{
37+
val f = c.methodLift2
38+
assert(f(0))
39+
assert(!f(1))
40+
val f1 = serializeDeserialize(f)
41+
assert(f1(0))
42+
assert(!f1(1))
43+
}
44+
45+
{
46+
val f = c.methodLift3
47+
assert(f(0))
48+
assert(!f(1))
49+
try {
50+
serializeDeserialize(this)
51+
assert(false)
52+
} catch {
53+
case _: java.io.NotSerializableException =>
54+
// expected, the closure in methodLift3 must capture C which is not serializable
55+
}
56+
}
57+
}
58+
59+
def serializeDeserialize[T <: AnyRef](obj: T): T = {
60+
import java.io._
61+
val buffer = new ByteArrayOutputStream
62+
val out = new ObjectOutputStream(buffer)
63+
out.writeObject(obj)
64+
val in = new ObjectInputStream(new ByteArrayInputStream(buffer.toByteArray))
65+
in.readObject.asInstanceOf[T]
66+
}
67+
}

‎test/files/run/t9390b.scala

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
class C { // C is not serializable
2+
def foo = (x: Int) => (y: Int) => x + y
3+
def bar = (x: Int) => (y: Int) => {toString; x + y}
4+
}
5+
6+
object Test {
7+
def main(args: Array[String]): Unit = {
8+
val c = new C
9+
val f = c.foo
10+
assert(f(1)(2) == 3)
11+
val f1 = serializeDeserialize(f)
12+
assert(f1(1)(2) == 3)
13+
14+
try {
15+
serializeDeserialize(c.bar)
16+
assert(false)
17+
} catch {
18+
case _: java.io.NotSerializableException =>
19+
// expected, lambda transitively refers to this
20+
}
21+
}
22+
23+
def serializeDeserialize[T <: AnyRef](obj: T): T = {
24+
import java.io._
25+
val buffer = new ByteArrayOutputStream
26+
val out = new ObjectOutputStream(buffer)
27+
out.writeObject(obj)
28+
val in = new ObjectInputStream(new ByteArrayInputStream(buffer.toByteArray))
29+
in.readObject.asInstanceOf[T]
30+
}
31+
}

‎test/files/run/t9390c.scala

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
class C { // C is not serializable
2+
def foo = {
3+
{ (x: Any) => new Object {} }
4+
}
5+
}
6+
object Test {
7+
def main(args: Array[String]): Unit = {
8+
val c = new C
9+
val f = c.foo
10+
val f1 = serializeDeserialize(f)
11+
}
12+
13+
def serializeDeserialize[T <: AnyRef](obj: T): T = {
14+
import java.io._
15+
val buffer = new ByteArrayOutputStream
16+
val out = new ObjectOutputStream(buffer)
17+
out.writeObject(obj)
18+
val in = new ObjectInputStream(new ByteArrayInputStream(buffer.toByteArray))
19+
in.readObject.asInstanceOf[T]
20+
}
21+
}

‎test/files/run/t9390d.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
class C { // C is not serializable
2+
def foo: () => Any = {
3+
{ () => class UseOuterInConstructor { C.this.toString }; new UseOuterInConstructor : Any}
4+
}
5+
}
6+
object Test {
7+
def main(args: Array[String]): Unit = {
8+
val c = new C
9+
val f = c.foo
10+
f() // Doesn't NPE, as we didn't elide the outer instance in the constructor call.
11+
}
12+
}

‎test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class InlineWarningTest extends BytecodeTesting {
113113

114114
val warn =
115115
"""M::f()I is annotated @inline but could not be inlined:
116-
|The callee M::f()I contains the instruction INVOKESPECIAL M.nested$1 ()I
116+
|The callee M::f()I contains the instruction INVOKESTATIC M.nested$1 ()I
117117
|that would cause an IllegalAccessError when inlined into class N""".stripMargin
118118

119119
var c = 0
@@ -140,7 +140,7 @@ class InlineWarningTest extends BytecodeTesting {
140140

141141
val warn =
142142
"""M::f(Lscala/Function1;)I could not be inlined:
143-
|The callee M::f(Lscala/Function1;)I contains the instruction INVOKESPECIAL M.nested$1 ()I
143+
|The callee M::f(Lscala/Function1;)I contains the instruction INVOKESTATIC M.nested$1 ()I
144144
|that would cause an IllegalAccessError when inlined into class N""".stripMargin
145145

146146
var c = 0

‎test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1141,7 +1141,7 @@ class InlinerTest extends BytecodeTesting {
11411141

11421142
val warn =
11431143
"""C::h()I is annotated @inline but could not be inlined:
1144-
|The callee C::h()I contains the instruction INVOKESPECIAL C.f$1 ()I
1144+
|The callee C::h()I contains the instruction INVOKESTATIC C.f$1 ()I
11451145
|that would cause an IllegalAccessError when inlined into class D.""".stripMargin
11461146

11471147
val List(c, d) = compile(code, allowMessage = _.msg contains warn)

0 commit comments

Comments
 (0)