summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJason Zaugg <jzaugg@gmail.com>2013-03-14 12:50:52 +0400
committerJason Zaugg <jzaugg@gmail.com>2013-03-23 15:39:57 +0100
commitcd9e03af8d61e7def5df2a0958de31ca0c163780 (patch)
tree1b5f0eaa8bbb6903c753abb70da60a9d4a7c1213
parentb7b4f877326acd6a8a24ff60fa1638cc18143c45 (diff)
downloadscala-cd9e03af8d61e7def5df2a0958de31ca0c163780.tar.gz
scala-cd9e03af8d61e7def5df2a0958de31ca0c163780.tar.bz2
scala-cd9e03af8d61e7def5df2a0958de31ca0c163780.zip
SI-7246 Make $outer pointer elision Java aware
In e0853b3, a space-saving optimization elided the outer pointer of inner classes if the the (protected) outer pointer of the immediate parent class was guaranteed to point to the same instance. But, this check failed to account for Java parent classes, which don't follow the Scala scheme. This commit disables the optimization in that case. The original test case in e0853b3 was anemic, I've fleshed it out to: - test the presense or absense of $outer pointers with Java reflection - test the optimization works in the presense of aliased and annotated aliased types. (The former worked already, the latter required a change to the implementation.) - Test the negative case when the prefixes don't line up and the subclass in fact needs its own $outer. This patch is based on work by Euguene Vigdorchik with some additions by Jason Zaugg.
-rw-r--r--src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala26
-rw-r--r--test/files/run/outertest.scala47
-rwxr-xr-xtest/files/run/t7246.check1
-rwxr-xr-xtest/files/run/t7246/Outer.java4
-rwxr-xr-xtest/files/run/t7246/Test.scala16
-rwxr-xr-xtest/files/run/t7246b.check2
-rwxr-xr-xtest/files/run/t7246b/Base.scala7
-rwxr-xr-xtest/files/run/t7246b/Outer.java4
-rwxr-xr-xtest/files/run/t7246b/Test.scala14
9 files changed, 99 insertions, 22 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
index 2f28a16416..be3150fc76 100644
--- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
+++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
@@ -45,26 +45,24 @@ abstract class ExplicitOuter extends InfoTransform
private def isInner(clazz: Symbol) =
!clazz.isPackageClass && !clazz.outerClass.isStaticOwner
- private def haveSameOuter(parent: Type, clazz: Symbol) = parent match {
- case TypeRef(pre, sym, _) =>
- val owner = clazz.owner
+ private def haveSameOuter(parent: Type, clazz: Symbol) = {
+ val owner = clazz.owner
+ val parentSym = parent.typeSymbol
- //println(s"have same outer $parent $clazz $sym ${sym.owner} $owner $pre")
-
- sym.isClass && owner.isClass &&
- (owner isSubClass sym.owner) &&
- owner.thisType =:= pre
-
- case _ => false
+ parentSym.isClass && owner.isClass &&
+ (owner isSubClass parentSym.owner) &&
+ owner.thisType =:= parent.prefix
}
/** Does given clazz define an outer field? */
def hasOuterField(clazz: Symbol) = {
- val parents = clazz.info.parents
+ val parent = clazz.info.firstParent
- isInner(clazz) && !clazz.isTrait && {
- parents.isEmpty || !haveSameOuter(parents.head, clazz)
- }
+ // space optimization: inherit the $outer pointer from the parent class if
+ // we know that it will point to the correct instance.
+ def canReuseParentOuterField = !parent.typeSymbol.isJavaDefined && haveSameOuter(parent, clazz)
+
+ isInner(clazz) && !clazz.isTrait && !canReuseParentOuterField
}
private def outerField(clazz: Symbol): Symbol = {
diff --git a/test/files/run/outertest.scala b/test/files/run/outertest.scala
index 3cc96afa5b..fa0443f669 100644
--- a/test/files/run/outertest.scala
+++ b/test/files/run/outertest.scala
@@ -1,26 +1,57 @@
// A test for the case where the outer field of class B#J should be eliminated.
-// You can verify this by running a javap on B.J
+
+import reflect.ClassTag
+
abstract class A {
+ abstract class I
- abstract class I {
+ val foo = this
+}
+class B extends A {
+ class J extends I {
+ val bar = foo
}
- val foo = "foo"
+ type II = I
+ class K extends II {
+ val bar = foo
+ }
+ class L extends (I @annotation.tailrec) {
+ val bar = foo
+ }
}
-class B extends A {
- class J extends I {
+class C extends A {
+ val c: C = this
+
+ class M extends c.I {
val bar = foo
}
-
}
-object Test extends App {
+object Test extends App {
val b = new B
- assert((new b.J).bar == b.foo)
+ val c0 = new C
+ val c = new { override val c = c0 } with C
+
+ assert((new b.J).bar eq b)
+ assert((new b.K).bar eq b)
+ assert((new b.L).bar eq b)
+ assert((new c.M).bar eq c)
+
+ def checkOuterFields[C: ClassTag](expected: Int) {
+ val cls = implicitly[ClassTag[C]].runtimeClass
+ val outerFields = cls.getDeclaredFields().filter(_.getName.contains("$outer"))
+ assert(outerFields.size == expected, outerFields.map(_.getName))
+ }
+ checkOuterFields[A#I](1) // the base class must have the $outer pointer
+ checkOuterFields[B#J](0) // reuse parent class' $outer pointer
+ checkOuterFields[B#K](0) // ... through an alias
+ checkOuterFields[B#L](0) // ... through the annotated type
+ checkOuterFields[C#M](1) // different prefix, can't share.
}
diff --git a/test/files/run/t7246.check b/test/files/run/t7246.check
new file mode 100755
index 0000000000..ce01362503
--- /dev/null
+++ b/test/files/run/t7246.check
@@ -0,0 +1 @@
+hello
diff --git a/test/files/run/t7246/Outer.java b/test/files/run/t7246/Outer.java
new file mode 100755
index 0000000000..163276fb3b
--- /dev/null
+++ b/test/files/run/t7246/Outer.java
@@ -0,0 +1,4 @@
+public class Outer {
+ public class Inner {
+ }
+} \ No newline at end of file
diff --git a/test/files/run/t7246/Test.scala b/test/files/run/t7246/Test.scala
new file mode 100755
index 0000000000..9f23ca8f3a
--- /dev/null
+++ b/test/files/run/t7246/Test.scala
@@ -0,0 +1,16 @@
+object Test extends App {
+
+ val so = new SubOuter
+ val si = new so.SubInner
+ println(si.bar)
+}
+
+class SubOuter extends Outer {
+
+ val foo = "hello"
+
+ class SubInner extends Inner {
+ def bar = foo
+ }
+
+} \ No newline at end of file
diff --git a/test/files/run/t7246b.check b/test/files/run/t7246b.check
new file mode 100755
index 0000000000..5073bd8617
--- /dev/null
+++ b/test/files/run/t7246b.check
@@ -0,0 +1,2 @@
+base
+sub
diff --git a/test/files/run/t7246b/Base.scala b/test/files/run/t7246b/Base.scala
new file mode 100755
index 0000000000..4e71d3313d
--- /dev/null
+++ b/test/files/run/t7246b/Base.scala
@@ -0,0 +1,7 @@
+class Base {
+ val baseOuter = "base"
+
+ class BaseInner {
+ val baseInner = baseOuter
+ }
+}
diff --git a/test/files/run/t7246b/Outer.java b/test/files/run/t7246b/Outer.java
new file mode 100755
index 0000000000..53a79316ef
--- /dev/null
+++ b/test/files/run/t7246b/Outer.java
@@ -0,0 +1,4 @@
+public class Outer extends Base {
+ public class Inner extends BaseInner {
+ }
+} \ No newline at end of file
diff --git a/test/files/run/t7246b/Test.scala b/test/files/run/t7246b/Test.scala
new file mode 100755
index 0000000000..f0982ea8d0
--- /dev/null
+++ b/test/files/run/t7246b/Test.scala
@@ -0,0 +1,14 @@
+object Test extends App {
+
+ val so = new SubOuter
+ val si = new so.SubInner
+ println(si.baseInner)
+ println(si.subInner)
+}
+
+class SubOuter extends Outer {
+ val subOuter = "sub"
+ class SubInner extends Inner {
+ def subInner = subOuter
+ }
+}