From 8d96380caeb1f03da8916d494e878f57a363f459 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 31 Jan 2014 14:29:19 +0100 Subject: SI-7475 Private members are not inheritable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It turns out `findMembers` has been a bit sloppy in recent years and has returned private members from *anywhere* up the base class sequence. Access checks usually pick up the slack and eliminate the unwanted privates. But, in concert with the "concrete beats abstract" rule in `findMember`, the following mishap appeared: scala> :paste // Entering paste mode (ctrl-D to finish) trait T { def a: Int } trait B { private def a: Int = 0 } trait C extends T with B { a } // Exiting paste mode, now interpreting. :9: error: method a in trait B cannot be accessed in C trait C extends T with B { a } ^ I noticed this when compiling Akka against JDK 8; a new private method in the bowels of the JDK was enough to break the build! It turns out that some finesse in needed to interpret SLS 5.2: > The private modifier can be used with any definition or declaration > in a template. They are not inherited by subclasses [...] So, can we simply exclude privates from all but the first base class? No, as that might be a refinement class! The following must be allowed: trait A { private def foo = 0; trait T { self: A => this.foo } } This commit: - tracks when the walk up the base class sequence passes the first non-refinement class, and excludes private members - ... except, if we are at a direct parent of a refinement class itself - Makes a corresponding change to OverridingPairs, to only consider private members if they are owned by the `base` Symbol under consideration. We don't need to deal with the subtleties of refinements there as that code is only used for bona-fide classes. - replaces use of `hasTransOwner` when considering whether a private[this] symbol is a member. The last condition was not grounded in the spec at all. The change is visible in cases like: // Old scala> trait A { private[this] val x = 0; class B extends A { this.x } } :7: error: value x in trait A cannot be accessed in A.this.B trait A { private[this] val x = 0; class B extends A { this.x } } ^ // New scala> trait A { private[this] val x = 0; class B extends A { this.x } } :8: error: value x is not a member of A.this.B trait A { private[this] val x = 0; class B extends A { this.x } } ^ Furthermore, we no longer give a `private[this]` member a free pass if it is sourced from the very first base class. trait Cake extends Slice { private[this] val bippy = () } trait Slice { self: Cake => bippy // BCS: Cake, Slice, AnyRef, Any } The different handling between `private` and `private[this]` still seems a bit dubious. The spec says: > An different form of qualification is private[this]. A member M > marked with this modifier can be accessed only from within the > object in which it is defined. That is, a selection p.M is only > legal if the prefix is this or O.this, for some class O enclosing > the reference. In addition, the restrictions for unqualified > private apply. This sounds like a question of access, not membership. If so, we should admit `private[this]` members from parents of refined types in `FindMember`. AFAICT, not too much rests on the distinction: do we get a "no such member", or "member foo inaccessible" error? I welcome scrutinee of the checkfile of `neg/t7475f.scala` to help put this last piece into the puzzle. One more thing: findMember does not have *any* code the corresponds to the last sentence of: > SLS 5.2 The modifier can be qualified with an identifier C > (e.g. private[C]) that must denote a class or package enclosing > the definition. Members labeled with such a modifier are accessible > respectively only from code inside the package C or only from code > inside the class C and its companion module (ยง5.4). > Such members are also inherited only from templates inside C. When I showed Martin this, he suggested it was an error in the spec, and we should leave the access checking to callers of that inherited qualified-private member. --- src/compiler/scala/tools/nsc/transform/OverridingPairs.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src/compiler') diff --git a/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala b/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala index 870eafbf20..bbd11efa7e 100644 --- a/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala +++ b/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala @@ -24,7 +24,12 @@ abstract class OverridingPairs extends SymbolPairs { /** Symbols to exclude: Here these are constructors and private/artifact symbols, * including bridges. But it may be refined in subclasses. */ - override protected def exclude(sym: Symbol) = sym.isPrivateLocal || sym.isArtifact || sym.isConstructor + override protected def exclude(sym: Symbol) = ( + sym.isPrivateLocal + || sym.isArtifact + || sym.isConstructor + || (sym.isPrivate && sym.owner != base) // Privates aren't inherited. Needed for pos/t7475a.scala + ) /** Types always match. Term symbols match if their member types * relative to `self` match. -- cgit v1.2.3