When elevating access along an inheritance path, initialize the computed

access to the (elevated) access of the accessed declaration, if applicable,
rather than plunking that access onto the end after we've calculated the
inheritance access.

Also, being a friend of a derived class gives you public access to its
members (subject to later modification by further inheritance);  it does
not simply ignore a single location of restricted inheritance.

Also, when computing the best unprivileged path to a subobject, preserve
the information that the worst path might be AS_none (forbidden) rather
than a minimum of AS_private.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@98899 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 80 insertions, 16 deletions

diff --git a/lib/Sema/SemaAccess.cpp b/lib/Sema/SemaAccess.cpp
index 3737c50602..171ed3783e 100644
--- a/lib/Sema/SemaAccess.cpp
+++ b/lib/Sema/SemaAccess.cpp
@@ -212,11 +212,14 @@ static Sema::AccessResult GetFriendKind(Sema &S,
 /// Finds the best path from the naming class to the declaring class,
 /// taking friend declarations into account.
 ///
+/// \param FinalAccess the access of the "final step", or AS_none if
+///   there is no final step.
 /// \return null if friendship is dependent
 static CXXBasePath *FindBestPath(Sema &S,
                                  const EffectiveContext &EC,
                                  CXXRecordDecl *Derived,
                                  CXXRecordDecl *Base,
+                                 AccessSpecifier FinalAccess,
                                  CXXBasePaths &Paths) {
   // Derive the paths to the desired base.
   bool isDerived = Derived->isDerivedFrom(Base, Paths);
@@ -225,28 +228,43 @@ static CXXBasePath *FindBestPath(Sema &S,
 
   CXXBasePath *BestPath = 0;
 
+  assert(FinalAccess != AS_none && "forbidden access after declaring class");
+
   // Derive the friend-modified access along each path.
   for (CXXBasePaths::paths_iterator PI = Paths.begin(), PE = Paths.end();
          PI != PE; ++PI) {
 
     // Walk through the path backwards.
-    AccessSpecifier PathAccess = AS_public;
+    AccessSpecifier PathAccess = FinalAccess;
     CXXBasePath::iterator I = PI->end(), E = PI->begin();
     while (I != E) {
       --I;
 
+      assert(PathAccess != AS_none);
+
+      // If the declaration is a private member of a base class, there
+      // is no level of friendship in derived classes that can make it
+      // accessible.
+      if (PathAccess == AS_private) {
+        PathAccess = AS_none;
+        break;
+      }
+
       AccessSpecifier BaseAccess = I->Base->getAccessSpecifier();
       if (BaseAccess != AS_public) {
         switch (GetFriendKind(S, EC, I->Class)) {
-        case Sema::AR_inaccessible: break;
-        case Sema::AR_accessible: BaseAccess = AS_public; break;
-        case Sema::AR_dependent: return 0;
+        case Sema::AR_inaccessible:
+          PathAccess = CXXRecordDecl::MergeAccess(BaseAccess, PathAccess);
+          break;
+        case Sema::AR_accessible:
+          PathAccess = AS_public;
+          break;
+        case Sema::AR_dependent:
+          return 0;
         case Sema::AR_delayed:
           llvm_unreachable("friend resolution is never delayed"); break;
         }
       }
-
-      PathAccess = CXXRecordDecl::MergeAccess(BaseAccess, PathAccess);
     }
 
     // Note that we modify the path's Access field to the
@@ -291,7 +309,8 @@ static void DiagnoseAccessPath(Sema &S,
   }
 
   CXXBasePaths Paths;
-  CXXBasePath &Path = *FindBestPath(S, EC, NamingClass, DeclaringClass, Paths);
+  CXXBasePath &Path = *FindBestPath(S, EC, NamingClass, DeclaringClass,
+                                    AS_public, Paths);
 
   CXXBasePath::iterator I = Path.end(), E = Path.begin();
   while (I != E) {
@@ -396,7 +415,7 @@ static void TryElevateAccess(Sema &S,
   CXXRecordDecl *NamingClass = Entity.getNamingClass();
 
   // Adjust the declaration of the referred entity.
-  AccessSpecifier DeclAccess = AS_none;
+  AccessSpecifier DeclAccess = AS_public;
   if (Entity.isMemberAccess()) {
     NamedDecl *Target = Entity.getTargetDecl();
 
@@ -421,17 +440,15 @@ static void TryElevateAccess(Sema &S,
   // Append the declaration's access if applicable.
   CXXBasePaths Paths;
   CXXBasePath *Path = FindBestPath(S, EC, Entity.getNamingClass(),
-                                   DeclaringClass, Paths);
+                                   DeclaringClass, DeclAccess, Paths);
   if (!Path) {
     // FIXME: delay dependent friendship
     return;
   }
 
-  // Grab the access along the best path.
+  // Grab the access along the best path (note that this includes the
+  // final-step access).
   AccessSpecifier NewAccess = Path->Access;
-  if (Entity.isMemberAccess())
-    NewAccess = CXXRecordDecl::MergeAccess(NewAccess, DeclAccess);
-  
   assert(NewAccess <= Access && "access along best path worse than direct?");
   Access = NewAccess;
 }
diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index 6caeec620d..9ae520d27a 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -1176,7 +1176,7 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
   // FIXME: support using declarations!
   QualType SubobjectType;
   int SubobjectNumber = 0;
-  AccessSpecifier SubobjectAccess = AS_private;
+  AccessSpecifier SubobjectAccess = AS_none;
   for (CXXBasePaths::paths_iterator Path = Paths.begin(), PathEnd = Paths.end();
        Path != PathEnd; ++Path) {
     const CXXBasePathElement &PathElement = Path->back();
diff --git a/test/CXX/class.access/class.friend/p1.cpp b/test/CXX/class.access/class.friend/p1.cpp
index 83b4227aa3..851cd3d008 100644
--- a/test/CXX/class.access/class.friend/p1.cpp
+++ b/test/CXX/class.access/class.friend/p1.cpp
@@ -115,3 +115,37 @@ namespace test0 {
     }
   };
 }
+
+// Make sure that friends have access to inherited protected members.
+namespace test2 {
+  struct X;
+
+  class ilist_half_node {
+    friend struct ilist_walker_bad;
+    X *Prev;
+  protected:
+    X *getPrev() { return Prev; }
+  };
+
+  class ilist_node : private ilist_half_node { // expected-note {{declared private here}} expected-note {{constrained by private inheritance here}}
+    friend struct ilist_walker;
+    X *Next;
+    X *getNext() { return Next; } // expected-note {{declared private here}}
+  };
+
+  struct X : ilist_node {};
+
+  struct ilist_walker {
+    static X *getPrev(X *N) { return N->getPrev(); }
+    static X *getNext(X *N) { return N->getNext(); }
+  };  
+
+  struct ilist_walker_bad {
+    static X *getPrev(X *N) { return N->getPrev(); } // \
+    // expected-error {{'getPrev' is a private member of 'test2::ilist_half_node'}} \
+    // expected-error {{cannot cast 'test2::X' to its private base class 'test2::ilist_half_node'}}
+
+    static X *getNext(X *N) { return N->getNext(); } // \
+    // expected-error {{'getNext' is a private member of 'test2::ilist_node'}}
+  };  
+}
diff --git a/test/CXX/class.access/p4.cpp b/test/CXX/class.access/p4.cpp
index d101dcb12c..bc69bee657 100644
--- a/test/CXX/class.access/p4.cpp
+++ b/test/CXX/class.access/p4.cpp
@@ -250,3 +250,15 @@ namespace test8 {
     new (2) A();
   }
 }
+
+// Don't silently upgrade forbidden-access paths to private.
+namespace test9 {
+  class A {
+    public: static int x;
+  };
+  class B : private A { // expected-note {{constrained by private inheritance here}}
+  };
+  class C : public B {
+    static int getX() { return x; } // expected-error {{'x' is a private member of 'test9::A'}}
+  };
+}
diff --git a/test/SemaCXX/access-base-class.cpp b/test/SemaCXX/access-base-class.cpp
index eeb5f1c86f..25fd9e52aa 100644
--- a/test/SemaCXX/access-base-class.cpp
+++ b/test/SemaCXX/access-base-class.cpp
@@ -63,13 +63,14 @@ namespace T6 {
   
   class A {};
   
-  class B : private A { // expected-note {{declared private here}}
+  class B : private A { // expected-note {{declared private here}} expected-note {{constrained by private inheritance here}}
     void f(C* c);
   };
   
   class C : public B { 
     void f(C *c) {
-      A* a = c; // expected-error {{cannot cast 'T6::C' to its private base class 'T6::A'}}
+      A* a = c; // expected-error {{cannot cast 'T6::C' to its private base class 'T6::A'}} \
+                // expected-error {{'A' is a private member of 'T6::A'}}
     }
   };