SI-7475 Private members are not inheritable

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.

    <console>: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 } }
    <console>: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 } }
    <console>: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.

1 files changed, 50 insertions, 13 deletions

diff --git a/src/reflect/scala/reflect/internal/tpe/FindMembers.scala b/src/reflect/scala/reflect/internal/tpe/FindMembers.scala
index be5a3f0983..33eccd8bbd 100644
--- a/src/reflect/scala/reflect/internal/tpe/FindMembers.scala
+++ b/src/reflect/scala/reflect/internal/tpe/FindMembers.scala
@@ -17,6 +17,10 @@ trait FindMembers {
     protected val initBaseClasses: List[Symbol] = tpe.baseClasses
 
     // The first base class, or the symbol of the ThisType
+    // e.g in:
+    // trait T { self: C => }
+    //
+    // The selector class of `T.this.type` is `T`, and *not* the first base class, `C`.
     private[this] var _selectorClass: Symbol = null
     private def selectorClass: Symbol = {
       if (_selectorClass eq null) {
@@ -68,6 +72,13 @@ trait FindMembers {
       // Has we seen a candidate deferred member?
       var deferredSeen = false
 
+      // All direct parents of refinement classes in the base class sequence
+      // from the current `walkBaseClasses`
+      var refinementParents: List[Symbol] = Nil
+
+      // Has the current `walkBaseClasses` encountered a non-refinement class?
+      var seenFirstNonRefinementClass = false
+
       while (!bcs.isEmpty) {
         val currentBaseClass = bcs.head
         val decls = currentBaseClass.info.decls
@@ -80,7 +91,7 @@ trait FindMembers {
           if (meetsRequirements) {
             val excl: Long = flags & excluded
             val isExcluded: Boolean = excl != 0L
-            if (!isExcluded && isPotentialMember(sym, flags, currentBaseClass)) {
+            if (!isExcluded && isPotentialMember(sym, flags, currentBaseClass, seenFirstNonRefinementClass, refinementParents)) {
               if (shortCircuit(sym)) return false
               else addMemberIfNew(sym)
             } else if (excl == DEFERRED) {
@@ -90,6 +101,17 @@ trait FindMembers {
           entry = if (findAll) entry.next else decls lookupNextEntry entry
         }
 
+        // SLS 5.2 The private modifier can be used with any definition or declaration in a template.
+        //         They are not inherited by subclasses [...]
+        if (currentBaseClass.isRefinementClass)
+          // SLS 3.2.7 A compound type T1 with . . . with Tn {R } represents objects with members as given in
+          //           the component types T1, ..., Tn and the refinement {R }
+          //
+          //           => private members should be included from T1, ... Tn. (SI-7475)
+          refinementParents :::= currentBaseClass.parentSymbols
+        else if (currentBaseClass.isClass)
+          seenFirstNonRefinementClass = true // only inherit privates of refinement parents after this point
+
         bcs = bcs.tail
       }
       deferredSeen
@@ -105,28 +127,31 @@ trait FindMembers {
     //
     // Q. When does a potential member fail to be a an actual member?
     // A. if it is subsumed by an member in a subclass.
-    private def isPotentialMember(sym: Symbol, flags: Long, owner: Symbol): Boolean = {
+    private def isPotentialMember(sym: Symbol, flags: Long, owner: Symbol,
+                                  seenFirstNonRefinementClass: Boolean, refinementParents: List[Symbol]): Boolean = {
       // conservatively (performance wise) doing this with flags masks rather than `sym.isPrivate`
       // to avoid multiple calls to `Symbol#flags`.
+      val isPrivate      = (flags & PRIVATE) == PRIVATE
       val isPrivateLocal = (flags & PrivateLocal) == PrivateLocal
 
-      def admitPrivateLocal(sym: Symbol): Boolean =
-        (
-             (selectorClass == owner)
-          || !isPrivateLocal
-          || selectorClass.hasTransOwner(owner)     // private[this] only a member from within the class (incorrect!)
-       )
-
-      // TODO first condition incorrect wrt self types! In neg/t7507, bippy should not be a member!
-      //      The condition, or a close variant thereof, will still be needed to prevent inheritance of constructors.
-      owner == initBaseClasses.head || admitPrivateLocal(sym) && sym.name != nme.CONSTRUCTOR
+      // TODO Is the special handling of `private[this]` vs `private` backed up by the spec?
+      def admitPrivate(sym: Symbol): Boolean =
+        (selectorClass == owner) || (
+             !isPrivateLocal // private[this] only a member from within the selector class. (Optimization only? Does the spec back this up?)
+          && (
+                  !seenFirstNonRefinementClass
+               || refinementParents.contains(owner)
+             )
+        )
+
+      (!isPrivate || admitPrivate(sym)) && (sym.name != nme.CONSTRUCTOR || owner == initBaseClasses.head)
     }
 
     // True unless the already-found member of type `memberType` matches the candidate symbol `other`.
     protected def isNewMember(member: Symbol, other: Symbol): Boolean =
       (    (other ne member)
         && (    (member.owner eq other.owner)
-             || (other.flags & PRIVATE) != 0
+             || (member.flags & PRIVATE) != 0
              || !(memberTypeLow(member) matches memberTypeHi(other))
            )
       )
@@ -148,6 +173,18 @@ trait FindMembers {
     // member type of the LHS of `matches` call. This is an extension point to enable a cache in
     // FindMember.
     protected def memberTypeLow(sym: Symbol): Type = self.memberType(sym)
+
+    /** Same as a call to narrow unless existentials are visible
+     *  after widening the type. In that case, narrow from the widened
+     *  type instead of the proxy. This gives buried existentials a
+     *  chance to make peace with the other types. See SI-5330.
+     */
+    private def narrowForFindMember(tp: Type): Type = {
+      val w = tp.widen
+      // Only narrow on widened type when we have to -- narrow is expensive unless the target is a singleton type.
+      if ((tp ne w) && containsExistential(w)) w.narrow
+      else tp.narrow
+    }
   }
 
   private[reflect] final class FindMembers(tpe: Type, excludedFlags: Long, requiredFlags: Long)