Permalink
Browse files

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)
  • Loading branch information...
1 parent d99a491 commit cca4d51dbf3f8478cb338e6d53e34003e9a3fa45 @retronym retronym committed Nov 27, 2013
@@ -100,23 +100,36 @@ abstract class Flatten extends InfoTransform {
/** Buffers for lifted out classes */
private val liftedDefs = perRunCaches.newMap[Symbol, ListBuffer[Tree]]()
- override def transform(tree: Tree): Tree = {
+ override def transform(tree: Tree): Tree = postTransform {
tree match {
case PackageDef(_, _) =>
liftedDefs(tree.symbol.moduleClass) = new ListBuffer
+ super.transform(tree)
case Template(_, _, _) if tree.symbol.isDefinedInPackage =>
liftedDefs(tree.symbol.owner) = new ListBuffer
+ super.transform(tree)
+ case ClassDef(_, _, _, _) if tree.symbol.isNestedClass =>
+ // SI-5508 Ordering important. In `object O { trait A { trait B } }`, we want `B` to appear after `A` in
+ // the sequence of lifted trees in the enclosing package. Why does this matter? Currently, mixin
+ // needs to transform `A` first to a chance to create accessors for private[this] trait fields
+ // *before* it transforms inner classes that refer to them. This also fixes SI-6231.
+ //
+ // Alternative solutions
+ // - create the private[this] accessors eagerly in Namer (but would this cover private[this] fields
+ // added later phases in compilation?)
+ // - move the accessor creation to the Mixin info transformer
+ val liftedBuffer = liftedDefs(tree.symbol.enclosingTopLevelClass.owner)
+ val index = liftedBuffer.length
+ liftedBuffer.insert(index, super.transform(tree))
+ EmptyTree
case _ =>
+ super.transform(tree)
}
- postTransform(super.transform(tree))
}
private def postTransform(tree: Tree): Tree = {
val sym = tree.symbol
val tree1 = tree match {
- case ClassDef(_, _, _, _) if sym.isNestedClass =>
- liftedDefs(sym.enclosingTopLevelClass.owner) += tree
- EmptyTree
case Select(qual, name) if sym.isStaticModule && !sym.isTopLevel =>
exitingFlatten {
atPos(tree.pos) {
@@ -134,7 +147,10 @@ abstract class Flatten extends InfoTransform {
/** Transform statements and add lifted definitions to them. */
override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = {
val stats1 = super.transformStats(stats, exprOwner)
- if (currentOwner.isPackageClass) stats1 ::: liftedDefs(currentOwner).toList
+ if (currentOwner.isPackageClass) {
+ val lifted = liftedDefs(currentOwner).toList
+ stats1 ::: lifted
+ }
else stats1
}
}
@@ -1,6 +0,0 @@
-t6231.scala:4: error: Implementation restriction: local trait Bug$X$1 is unable to automatically capture the
-free variable value ev$1 on behalf of <$anon: Function0>. You can manually assign it to a val inside the trait,
-and refer to that val in <$anon: Function0>. For more details, see SI-6231.
- def qux = { () => ev }
- ^
-one error found
@@ -1 +0,0 @@
--Ydelambdafy:inline
@@ -0,0 +1,6 @@
+object Test {
+ trait NestedTrait { // must be nested and a trait
+ private val _st : Int = 0 // crashes if changed to private[this]
+ val escape = { () => _st }
+ }
+}
@@ -0,0 +1,4 @@
+trait TopTrait { // must be nested and a trait
+ private[this] val _st : Int = 0 // crashes if TopTrait is not top level
+ val escape = { () => _st }
+}
@@ -0,0 +1,6 @@
+object Test {
+ trait NestedTrait { // must be nested and a trait
+ private[this] val _st : Int = 0 // must be private[this]
+ val escape = { () => _st }
+ }
+}
View
@@ -0,0 +1,83 @@
+package TestTestters
+
+trait Test1 {
+ private[this] var _st : Int = 0
+ def close : PartialFunction[Any,Any] = {
+ case x : Int =>
+ _st = identity(_st)
+ }
+}
+
+object Base1 {
+ trait Test2 {
+ private[this] var _st : Int = 0
+ def close : PartialFunction[Any,Any] = {
+ case x : Int =>
+ _st = identity(_st)
+ }
+ }
+}
+
+class Test3 {
+ private[this] var _st : Int = 0
+ def close : PartialFunction[Any,Any] = {
+ case x : Int =>
+ _st = 1
+ }
+}
+
+object Base2 {
+ class Test4 {
+ private[this] var _st : Int = 0
+ def close : PartialFunction[Any,Any] = {
+ case x : Int =>
+ _st = 1
+ }
+ }
+}
+
+class Base3 {
+ trait Test5 {
+ private[this] var _st : Int = 0
+ def close : PartialFunction[Any,Any] = {
+ case x : Int =>
+ _st = 1
+ }
+ }
+}
+
+object Base4 {
+ trait Test6 {
+ private[this] var _st : Int = 0
+ def close : PartialFunction[Any,Any] = {
+ case x : Int => ()
+ }
+ }
+}
+
+object Base5 {
+ trait Test7 {
+ private[this] var _st : Int = 0
+ def close = () => {
+ _st = 1
+ }
+ }
+}
+
+object Base6 {
+ class Test8 {
+ private[this] var _st : Int = 0
+ def close = () => {
+ _st = 1
+ }
+ }
+}
+
+object Base7 {
+ trait Test9 {
+ var st : Int = 0
+ def close = () => {
+ st = 1
+ }
+ }
+}
File renamed without changes.
@@ -0,0 +1,8 @@
+class Test {
+ def f1(t: String) = {
+ trait T {
+ def xs = Nil map (_ => t)
+ }
+ ()
+ }
+}

0 comments on commit cca4d51

Please sign in to comment.