summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Phillips <paulp@improving.org>2012-07-25 15:21:16 -0700
committerPaul Phillips <paulp@improving.org>2012-07-26 10:04:20 -0700
commitadeffda25e94ed0206d35bdb9b42523227a89f8c (patch)
tree1c07a172eadb99b5854ccee6c288097df4e24fe0
parentfe513d1f147e60988414b01e5928811bb6f78714 (diff)
downloadscala-adeffda25e94ed0206d35bdb9b42523227a89f8c.tar.gz
scala-adeffda25e94ed0206d35bdb9b42523227a89f8c.tar.bz2
scala-adeffda25e94ed0206d35bdb9b42523227a89f8c.zip
Refined isEffectivelyFinal logic for sealedness.
If the enclosing class of a method is sealed and has only final subclasses, then the method is effectively final in the sealed class if none of the subclasses overrides it. This makes it possible to inline more methods without explicitly marking them final. Note that the test doesn't fail before this patch due to SI-6142, a bug in the optimizer, but here's a bytecode diff to prove it: @@ -16,8 +16,10 @@ public final class Test$ { Code: : getstatic // Field Foo$.MODULE$:LFoo$; : invokevirtual // Method Foo$.mkFoo:()LFoo; +: pop : bipush -: invokevirtual // Method Foo.bar:(I)I +: iconst_1 +: iadd : ireturn And the test in neg, which is manually made to fail due to the absence of inline warnings, correctly refuses to inline the methods. Review by @dragos.
-rw-r--r--src/reflect/scala/reflect/internal/Symbols.scala16
-rw-r--r--test/files/neg/sealed-final-neg.check4
-rw-r--r--test/files/neg/sealed-final-neg.flags1
-rw-r--r--test/files/neg/sealed-final-neg.scala41
-rw-r--r--test/files/pos/sealed-final.flags1
-rw-r--r--test/files/pos/sealed-final.scala14
-rw-r--r--test/files/run/t2886.check10
7 files changed, 78 insertions, 9 deletions
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index 636f1e2f01..7e6de14295 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -191,7 +191,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
} else None
}
-
+
// Begin Reflection Helpers
// Replaces a repeated parameter type at the end of the parameter list
@@ -354,7 +354,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
val selection = select(alts, defaultFilteringOps)
val knownApplicable = applicable(selection)
-
+
if (knownApplicable.size == 1) knownApplicable.head
else NoSymbol
}
@@ -1016,6 +1016,14 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
def isTopLevelModule = hasFlag(MODULE) && owner.isPackageClass
+ /** A helper function for isEffectivelyFinal. */
+ private def isNotOverridden = (
+ owner.isClass && (
+ owner.isEffectivelyFinal
+ || owner.isSealed && owner.children.forall(c => c.isEffectivelyFinal && (overridingSymbol(c) == NoSymbol))
+ )
+ )
+
/** Is this symbol effectively final? I.e, it cannot be overridden */
final def isEffectivelyFinal: Boolean = (
(this hasFlag FINAL | PACKAGE)
@@ -1023,8 +1031,8 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
|| isTerm && (
isPrivate
|| isLocal
- || owner.isClass && owner.isEffectivelyFinal
- )
+ || isNotOverridden
+ )
)
/** Is this symbol locally defined? I.e. not accessed from outside `this` instance */
diff --git a/test/files/neg/sealed-final-neg.check b/test/files/neg/sealed-final-neg.check
new file mode 100644
index 0000000000..500d23f49a
--- /dev/null
+++ b/test/files/neg/sealed-final-neg.check
@@ -0,0 +1,4 @@
+sealed-final-neg.scala:41: error: expected class or object definition
+"Due to SI-6142 this emits no warnings, so we'll just break it until that's fixed."
+^
+one error found
diff --git a/test/files/neg/sealed-final-neg.flags b/test/files/neg/sealed-final-neg.flags
new file mode 100644
index 0000000000..cfabf7a5b4
--- /dev/null
+++ b/test/files/neg/sealed-final-neg.flags
@@ -0,0 +1 @@
+-Xfatal-warnings -Yinline-warnings -optimise \ No newline at end of file
diff --git a/test/files/neg/sealed-final-neg.scala b/test/files/neg/sealed-final-neg.scala
new file mode 100644
index 0000000000..bc25330e13
--- /dev/null
+++ b/test/files/neg/sealed-final-neg.scala
@@ -0,0 +1,41 @@
+package neg1 {
+ sealed abstract class Foo {
+ @inline def bar(x: Int) = x + 1
+ }
+ object Foo {
+ def mkFoo(): Foo = new Baz2
+ }
+
+ object Baz1 extends Foo
+ final class Baz2 extends Foo
+ final class Baz3 extends Foo {
+ override def bar(x: Int) = x - 1
+ }
+
+ object Test {
+ // bar can't be inlined - it is overridden in Baz3
+ def f = Foo.mkFoo() bar 10
+ }
+}
+
+package neg2 {
+ sealed abstract class Foo {
+ @inline def bar(x: Int) = x + 1
+ }
+ object Foo {
+ def mkFoo(): Foo = new Baz2
+ }
+
+ object Baz1 extends Foo
+ final class Baz2 extends Foo
+ class Baz3 extends Foo {
+ override def bar(x: Int) = x - 1
+ }
+
+ object Test {
+ // bar can't be inlined - Baz3 is not final
+ def f = Foo.mkFoo() bar 10
+ }
+}
+
+"Due to SI-6142 this emits no warnings, so we'll just break it until that's fixed."
diff --git a/test/files/pos/sealed-final.flags b/test/files/pos/sealed-final.flags
new file mode 100644
index 0000000000..cfabf7a5b4
--- /dev/null
+++ b/test/files/pos/sealed-final.flags
@@ -0,0 +1 @@
+-Xfatal-warnings -Yinline-warnings -optimise \ No newline at end of file
diff --git a/test/files/pos/sealed-final.scala b/test/files/pos/sealed-final.scala
new file mode 100644
index 0000000000..bdedb5c1f6
--- /dev/null
+++ b/test/files/pos/sealed-final.scala
@@ -0,0 +1,14 @@
+sealed abstract class Foo {
+ @inline def bar(x: Int) = x + 1
+}
+object Foo {
+ def mkFoo(): Foo = new Baz2
+}
+
+object Baz1 extends Foo
+final class Baz2 extends Foo
+
+object Test {
+ // bar should be inlined now
+ def f = Foo.mkFoo() bar 10
+}
diff --git a/test/files/run/t2886.check b/test/files/run/t2886.check
index 8d97a82799..b093815562 100644
--- a/test/files/run/t2886.check
+++ b/test/files/run/t2886.check
@@ -1,5 +1,5 @@
-((x: String) => {
- val x$1 = x;
- val x$2 = x;
- Test.this.test(x$2, x$1)
-})
+((x: String) => {
+ <hidden> val x$1 = x;
+ <hidden> val x$2 = x;
+ Test.this.test(x$2, x$1)
+})