Only suppress instance context if a member is actually

accessible in its declaring class;  otherwise we might
fail to apply [class.protected] when considering
accessibility in derived classes.

Noticed by inspection; <rdar://13270329>.

I had an existing test wrong.  Here's why it's wrong:

Follow the rules (and notation) of [class.access]p5.
The naming class (N) is B and the context (R) is D::getX.
- 'x' as a member of B is protected, but R does not occur
  in a member or friend of a class derived from B.
- There does exist a base class of B, A, which is accessible
  from R, and 'x' is accessible at R when named in A because
  'x' as a member of A is protected and R occurs in a member
  of a class, D, that is derived from A;  however, by
  [class.protected], the class of the object expression must
  be equal to or derived from that class, and A does not
  derive from D.

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

2 files changed, 10 insertions, 5 deletions

diff --git a/lib/Sema/SemaAccess.cpp b/lib/Sema/SemaAccess.cpp
index 229e6f4b8f..55bd76b737 100644
--- a/lib/Sema/SemaAccess.cpp
+++ b/lib/Sema/SemaAccess.cpp
@@ -1317,7 +1317,13 @@ static AccessResult IsAccessible(Sema &S,
     FinalAccess = Target->getAccess();
     switch (HasAccess(S, EC, DeclaringClass, FinalAccess, Entity)) {
     case AR_accessible:
+      // Target is accessible at EC when named in its declaring class.
+      // We can now hill-climb and simply check whether the declaring
+      // class is accessible as a base of the naming class.  This is
+      // equivalent to checking the access of a notional public
+      // member with no instance context.
       FinalAccess = AS_public;
+      Entity.suppressInstanceContext();
       break;
     case AR_inaccessible: break;
     case AR_dependent: return AR_dependent; // see above
@@ -1325,8 +1331,6 @@ static AccessResult IsAccessible(Sema &S,
 
     if (DeclaringClass == NamingClass)
       return (FinalAccess == AS_public ? AR_accessible : AR_inaccessible);
-
-    Entity.suppressInstanceContext();
   } else {
     FinalAccess = AS_public;
   }
diff --git a/test/CXX/class.access/class.protected/p1.cpp b/test/CXX/class.access/class.protected/p1.cpp
index 132ff6176c..825447ef16 100644
--- a/test/CXX/class.access/class.protected/p1.cpp
+++ b/test/CXX/class.access/class.protected/p1.cpp
@@ -329,7 +329,7 @@ namespace test8 {
 
 namespace test9 {
   class A { // expected-note {{member is declared here}}
-  protected: int foo(); // expected-note 4 {{declared}} expected-note 2 {{can only access this member on an object of type}} expected-note {{member is declared here}}
+  protected: int foo(); // expected-note 4 {{declared}} expected-note 3 {{can only access this member on an object of type}} expected-note 2 {{member is declared here}}
   };
 
   class B : public A { // expected-note {{member is declared here}}
@@ -344,14 +344,15 @@ namespace test9 {
     static void test(A &a) {
       a.foo(); // expected-error {{'foo' is a protected member}}
       a.A::foo(); // expected-error {{'foo' is a protected member}}
-      a.B::foo();
+      a.B::foo(); // expected-error {{'foo' is a protected member}}
       a.C::foo(); // expected-error {{'foo' is a protected member}}
+      a.D::foo(); // expected-error {{'foo' is a protected member}}
     }
 
     static void test(B &b) {
       b.foo();
       b.A::foo();
-      b.B::foo();
+      b.B::foo(); // accessible as named in A
       b.C::foo(); // expected-error {{'foo' is a protected member}}
     }