diff options
author | Lukas Rytz <lukas.rytz@typesafe.com> | 2015-07-23 13:52:11 +0200 |
---|---|---|
committer | Lukas Rytz <lukas.rytz@typesafe.com> | 2015-07-23 13:52:11 +0200 |
commit | 823fb0ffb256590305a44897acfa80bb99f6d19b (patch) | |
tree | 9e584cda7434fd280256a744950f27197079b07c | |
parent | e1f7cca3379d4ee9ba8be7e9e6b1cd88cd1913ed (diff) | |
parent | 93bee55e68522a46e501229dd0d5f2af4b72ac4a (diff) | |
download | scala-823fb0ffb256590305a44897acfa80bb99f6d19b.tar.gz scala-823fb0ffb256590305a44897acfa80bb99f6d19b.tar.bz2 scala-823fb0ffb256590305a44897acfa80bb99f6d19b.zip |
Merge pull request #4652 from retronym/ticket/9408
SI-9408 Avoid capturing outer class in local classes.
6 files changed, 82 insertions, 9 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/Constructors.scala b/src/compiler/scala/tools/nsc/transform/Constructors.scala index 86685d46de..7c66bda46b 100644 --- a/src/compiler/scala/tools/nsc/transform/Constructors.scala +++ b/src/compiler/scala/tools/nsc/transform/Constructors.scala @@ -165,11 +165,19 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL { return } + // Note: elision of outer reference is based on a class-wise analysis, if a class might have subclasses, + // it doesn't work. For example, `LocalParent` retains the outer reference in: + // + // class Outer { def test = {class LocalParent; class LocalChild extends LocalParent } } + // + // See run/t9408.scala for related test cases. + val isEffectivelyFinal = clazz.isEffectivelyFinal def isParamCandidateForElision(sym: Symbol) = (sym.isParamAccessor && sym.isPrivateLocal) - def isOuterCandidateForElision(sym: Symbol) = (sym.isOuterAccessor && sym.owner.isEffectivelyFinal && !sym.isOverridingSymbol) + def isOuterCandidateForElision(sym: Symbol) = (sym.isOuterAccessor && isEffectivelyFinal && !sym.isOverridingSymbol) - val paramCandidatesForElision: Set[ /*Field*/ Symbol] = (clazz.info.decls.toSet filter isParamCandidateForElision) - val outerCandidatesForElision: Set[ /*Method*/ Symbol] = (clazz.info.decls.toSet filter isOuterCandidateForElision) + val decls = clazz.info.decls.toSet + val paramCandidatesForElision: Set[ /*Field*/ Symbol] = (decls filter isParamCandidateForElision) + val outerCandidatesForElision: Set[ /*Method*/ Symbol] = (decls filter isOuterCandidateForElision) omittables ++= paramCandidatesForElision omittables ++= outerCandidatesForElision diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala index a11906ace1..00de77a8d4 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala @@ -138,7 +138,7 @@ trait TreeAndTypeAnalysis extends Debugging { if(grouped) { def enumerateChildren(sym: Symbol) = { - sym.children.toList + sym.sealedChildren.toList .sortBy(_.sealedSortName) .filterNot(x => x.isSealed && x.isAbstractClass && !isPrimitiveValueClass(x)) } diff --git a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala index fc632e0d0d..8bcd5e6f12 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala @@ -212,8 +212,8 @@ trait Checkable { ) /** Are all children of these symbols pairwise irreconcilable? */ def allChildrenAreIrreconcilable(sym1: Symbol, sym2: Symbol) = ( - sym1.children.toList forall (c1 => - sym2.children.toList forall (c2 => + sym1.sealedChildren.toList forall (c1 => + sym2.sealedChildren.toList forall (c2 => areIrreconcilableAsParents(c1, c2) ) ) diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 777ff388b6..cc15a2485f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -1694,6 +1694,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper psym addChild context.owner else pending += ParentSealedInheritanceError(parent, psym) + if (psym.isLocalToBlock && !phase.erasedTypes) + psym addChild context.owner val parentTypeOfThis = parent.tpe.dealias.typeOfThis if (!(selfType <:< parentTypeOfThis) && diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 1f06d91062..7740dbdd75 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -980,7 +980,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => private def isNotOverridden = ( owner.isClass && ( owner.isEffectivelyFinal - || owner.isSealed && owner.children.forall(c => c.isEffectivelyFinal && (overridingSymbol(c) == NoSymbol)) + || (owner.isSealed && owner.sealedChildren.forall(c => c.isEffectivelyFinal && (overridingSymbol(c) == NoSymbol))) ) ) @@ -992,6 +992,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => isPrivate || isLocalToBlock ) + || isClass && originalOwner.isTerm && children.isEmpty // we track known subclasses of term-owned classes, use that infer finality ) /** Is this symbol effectively final or a concrete term member of sealed class whose children do not override it */ final def isEffectivelyFinalOrNotOverridden: Boolean = isEffectivelyFinal || (isTerm && !isDeferred && isNotOverridden) @@ -2495,14 +2496,15 @@ trait Symbols extends api.Symbols { self: SymbolTable => def associatedFile: AbstractFile = enclosingTopLevelClass.associatedFile def associatedFile_=(f: AbstractFile) { abort("associatedFile_= inapplicable for " + this) } - /** If this is a sealed class, its known direct subclasses. + /** If this is a sealed or local class, its known direct subclasses. * Otherwise, the empty set. */ def children: Set[Symbol] = Set() + final def sealedChildren: Set[Symbol] = if (!isSealed) Set.empty else children /** Recursively assemble all children of this symbol. */ - def sealedDescendants: Set[Symbol] = children.flatMap(_.sealedDescendants) + this + final def sealedDescendants: Set[Symbol] = if (!isSealed) Set(this) else children.flatMap(_.sealedDescendants) + this @inline final def orElse(alt: => Symbol): Symbol = if (this ne NoSymbol) this else alt @inline final def andAlso(f: Symbol => Unit): Symbol = { if (this ne NoSymbol) f(this) ; this } diff --git a/test/files/run/t9408.scala b/test/files/run/t9408.scala new file mode 100644 index 0000000000..231dca4ce7 --- /dev/null +++ b/test/files/run/t9408.scala @@ -0,0 +1,61 @@ +class Outer { + def assertNoFields(c: Class[_]) { + assert(c.getDeclaredFields.isEmpty) + } + def assertHasOuter(c: Class[_]) { + assert(c.getDeclaredFields.exists(_.getName.contains("outer"))) + } + class Member + final class FinalMember + + def test { + assertHasOuter(classOf[Member]) + assertNoFields(classOf[FinalMember]) + final class C + assertNoFields(classOf[C]) + class D + assertNoFields(classOf[D]) + (() => {class E; assertNoFields(classOf[E])}).apply() + + // The outer reference elision currently runs on a class-by-class basis. If it cannot rule out that a class has + // subclasses, it will not remove the outer reference. A smarter analysis here could detect if no members of + // a sealed (or effectively sealed) hierarchy use the outer reference, the optimization could be performed. + class Parent + class Child extends Parent + assertHasOuter(classOf[Parent]) + + // Note: outer references (if they haven't been elided) are used in pattern matching as follows. + // This isn't relevant to term-owned classes, as you can't refer to them with a prefix that includes + // the outer class. + val outer1 = new Outer + val outer2 = new Outer + (new outer1.Member: Any) match { + case _: outer2.Member => sys.error("wrong match!") + case _: outer1.Member => // okay + } + + // ... continuing on that theme, note that `Member` isn't considered as a local class, it is owned by a the class + // `LocalOuter`, which itself happens to be term-owned. So we expect that it has an outer reference, and that this + // is respected in type tests. + class LocalOuter { + class Member + final class FinalMember + } + assertNoFields(classOf[LocalOuter]) + assertHasOuter(classOf[LocalOuter#Member]) + val localOuter1 = new LocalOuter + val localOuter2 = new LocalOuter + (new localOuter1.Member: Any) match { + case _: localOuter2.Member => sys.error("wrong match!") + case _: localOuter1.Member => // okay + } + // Final member classes still lose the outer reference. + assertNoFields(classOf[LocalOuter#FinalMember]) + } +} + +object Test { + def main(args: Array[String]): Unit = { + new Outer().test + } +} |