Skip to content

Commit cca4d51

Browse files
committed
SI-5508 Fix crasher with private[this] in nested traits
Currently, accessors for private local trait fields are added very late in the game when the `Mixin` tree transformer treats the trait. By contrast, fields with weaker access have accessors created eagerly in `Namers`. // Mixin#addLateInterfaceMembers val getter = member.getter(clazz) if (getter == NoSymbol) addMember(clazz, newGetter(member)) `addMember` mutates the type of the interface to add the getter. (This seems like a pretty poor design: usually if a phase changes types, it should do in an `InfoTransformer`.) However, if an inner class or anonymous function of the trait has been flattened to a spot where it precedes the trait in the enclosing packages info, this code hasn't had a chance to run, and the lookup of the getter crashes as mixins `postTransform` runs over a selection of the not-yet-materialized getter. // Mixin#postTransform case Select(qual, name) if sym.owner.isImplClass && !isStaticOnly(sym) => val iface = toInterface(sym.owner.tpe).typeSymbol val ifaceGetter = sym getter iface This commit ensures that `Flatten` lifts inner classes to a position *after* the enclosing class in the stats of the enclosing package. Bonus fix: SI-7012 (the followup ticket to SI-6231 / SI-2897)
1 parent d99a491 commit cca4d51

File tree

9 files changed

+129
-13
lines changed

9 files changed

+129
-13
lines changed

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

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,23 +100,36 @@ abstract class Flatten extends InfoTransform {
100100
/** Buffers for lifted out classes */
101101
private val liftedDefs = perRunCaches.newMap[Symbol, ListBuffer[Tree]]()
102102

103-
override def transform(tree: Tree): Tree = {
103+
override def transform(tree: Tree): Tree = postTransform {
104104
tree match {
105105
case PackageDef(_, _) =>
106106
liftedDefs(tree.symbol.moduleClass) = new ListBuffer
107+
super.transform(tree)
107108
case Template(_, _, _) if tree.symbol.isDefinedInPackage =>
108109
liftedDefs(tree.symbol.owner) = new ListBuffer
110+
super.transform(tree)
111+
case ClassDef(_, _, _, _) if tree.symbol.isNestedClass =>
112+
// SI-5508 Ordering important. In `object O { trait A { trait B } }`, we want `B` to appear after `A` in
113+
// the sequence of lifted trees in the enclosing package. Why does this matter? Currently, mixin
114+
// needs to transform `A` first to a chance to create accessors for private[this] trait fields
115+
// *before* it transforms inner classes that refer to them. This also fixes SI-6231.
116+
//
117+
// Alternative solutions
118+
// - create the private[this] accessors eagerly in Namer (but would this cover private[this] fields
119+
// added later phases in compilation?)
120+
// - move the accessor creation to the Mixin info transformer
121+
val liftedBuffer = liftedDefs(tree.symbol.enclosingTopLevelClass.owner)
122+
val index = liftedBuffer.length
123+
liftedBuffer.insert(index, super.transform(tree))
124+
EmptyTree
109125
case _ =>
126+
super.transform(tree)
110127
}
111-
postTransform(super.transform(tree))
112128
}
113129

114130
private def postTransform(tree: Tree): Tree = {
115131
val sym = tree.symbol
116132
val tree1 = tree match {
117-
case ClassDef(_, _, _, _) if sym.isNestedClass =>
118-
liftedDefs(sym.enclosingTopLevelClass.owner) += tree
119-
EmptyTree
120133
case Select(qual, name) if sym.isStaticModule && !sym.isTopLevel =>
121134
exitingFlatten {
122135
atPos(tree.pos) {
@@ -134,7 +147,10 @@ abstract class Flatten extends InfoTransform {
134147
/** Transform statements and add lifted definitions to them. */
135148
override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = {
136149
val stats1 = super.transformStats(stats, exprOwner)
137-
if (currentOwner.isPackageClass) stats1 ::: liftedDefs(currentOwner).toList
150+
if (currentOwner.isPackageClass) {
151+
val lifted = liftedDefs(currentOwner).toList
152+
stats1 ::: lifted
153+
}
138154
else stats1
139155
}
140156
}

test/files/neg/t6231.check

Lines changed: 0 additions & 6 deletions
This file was deleted.

test/files/neg/t6231.flags

Lines changed: 0 additions & 1 deletion
This file was deleted.

test/files/pos/t5508-min-okay.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
object Test {
2+
trait NestedTrait { // must be nested and a trait
3+
private val _st : Int = 0 // crashes if changed to private[this]
4+
val escape = { () => _st }
5+
}
6+
}

test/files/pos/t5508-min-okay2.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
trait TopTrait { // must be nested and a trait
2+
private[this] val _st : Int = 0 // crashes if TopTrait is not top level
3+
val escape = { () => _st }
4+
}

test/files/pos/t5508-min.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
object Test {
2+
trait NestedTrait { // must be nested and a trait
3+
private[this] val _st : Int = 0 // must be private[this]
4+
val escape = { () => _st }
5+
}
6+
}

test/files/pos/t5508.scala

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package TestTestters
2+
3+
trait Test1 {
4+
private[this] var _st : Int = 0
5+
def close : PartialFunction[Any,Any] = {
6+
case x : Int =>
7+
_st = identity(_st)
8+
}
9+
}
10+
11+
object Base1 {
12+
trait Test2 {
13+
private[this] var _st : Int = 0
14+
def close : PartialFunction[Any,Any] = {
15+
case x : Int =>
16+
_st = identity(_st)
17+
}
18+
}
19+
}
20+
21+
class Test3 {
22+
private[this] var _st : Int = 0
23+
def close : PartialFunction[Any,Any] = {
24+
case x : Int =>
25+
_st = 1
26+
}
27+
}
28+
29+
object Base2 {
30+
class Test4 {
31+
private[this] var _st : Int = 0
32+
def close : PartialFunction[Any,Any] = {
33+
case x : Int =>
34+
_st = 1
35+
}
36+
}
37+
}
38+
39+
class Base3 {
40+
trait Test5 {
41+
private[this] var _st : Int = 0
42+
def close : PartialFunction[Any,Any] = {
43+
case x : Int =>
44+
_st = 1
45+
}
46+
}
47+
}
48+
49+
object Base4 {
50+
trait Test6 {
51+
private[this] var _st : Int = 0
52+
def close : PartialFunction[Any,Any] = {
53+
case x : Int => ()
54+
}
55+
}
56+
}
57+
58+
object Base5 {
59+
trait Test7 {
60+
private[this] var _st : Int = 0
61+
def close = () => {
62+
_st = 1
63+
}
64+
}
65+
}
66+
67+
object Base6 {
68+
class Test8 {
69+
private[this] var _st : Int = 0
70+
def close = () => {
71+
_st = 1
72+
}
73+
}
74+
}
75+
76+
object Base7 {
77+
trait Test9 {
78+
var st : Int = 0
79+
def close = () => {
80+
st = 1
81+
}
82+
}
83+
}
File renamed without changes.

test/files/pos/t6231b.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
class Test {
2+
def f1(t: String) = {
3+
trait T {
4+
def xs = Nil map (_ => t)
5+
}
6+
()
7+
}
8+
}

0 commit comments

Comments
 (0)