Add a quick-and-dirty hack to give a better diagnostic for [class.protected]

restrictions.  The note's not really on the right place given its wording,
but putting a second note on the call site (or muddying the wording) doesn't
appeal.

There are corner cases where this can be wrong, but I'm not concerned.

llvm-svn: 112950

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -567,6 +567,8 @@ def note_access_natural : Note<
 def note_access_constrained_by_path : Note<
   "constrained by %select{|implicitly }1%select{private|protected}0"
   " inheritance here">;
+def note_access_protected_restricted : Note<
+  "object type %select{|%1 }0must derive from context type %2">;
   
 // C++ name lookup
 def err_incomplete_nested_name_spec : Error<

clang/lib/Sema/SemaAccess.cpp

@@ -930,6 +930,57 @@ static CXXBasePath *FindBestPath(Sema &S,
   return BestPath;
 }
 
+/// Given that an entity has protected natural access, check whether
+/// access might be denied because of the protected member access
+/// restriction.
+///
+/// \return true if a note was emitted
+static bool TryDiagnoseProtectedAccess(Sema &S, const EffectiveContext &EC,
+                                       AccessTarget &Target) {
+  // Only applies to instance accesses.
+  if (!Target.hasInstanceContext())
+    return false;
+  assert(Target.isMemberAccess());
+  NamedDecl *D = Target.getTargetDecl();
+
+  const CXXRecordDecl *DeclaringClass = Target.getDeclaringClass();
+  DeclaringClass = DeclaringClass->getCanonicalDecl();
+
+  for (EffectiveContext::record_iterator
+         I = EC.Records.begin(), E = EC.Records.end(); I != E; ++I) {
+    const CXXRecordDecl *ECRecord = *I;
+    switch (IsDerivedFromInclusive(ECRecord, DeclaringClass)) {
+    case AR_accessible: break;
+    case AR_inaccessible: continue;
+    case AR_dependent: continue;
+    }
+
+    // The effective context is a subclass of the declaring class.
+    // If that class isn't a superclass of the instance context,
+    // then the [class.protected] restriction applies.
+
+    // To get this exactly right, this might need to be checked more
+    // holistically;  it's not necessarily the case that gaining
+    // access here would grant us access overall.
+
+    const CXXRecordDecl *InstanceContext = Target.resolveInstanceContext(S);
+    assert(InstanceContext && "diagnosing dependent access");
+
+    switch (IsDerivedFromInclusive(InstanceContext, ECRecord)) {
+    case AR_accessible: continue;
+    case AR_dependent: continue;
+    case AR_inaccessible:
+      S.Diag(D->getLocation(), diag::note_access_protected_restricted)
+        << (InstanceContext != Target.getNamingClass()->getCanonicalDecl())
+        << S.Context.getTypeDeclType(InstanceContext)
+        << S.Context.getTypeDeclType(ECRecord);
+      return true;
+    }
+  }
+
+  return false;
+}
+
 /// Diagnose the path which caused the given declaration or base class
 /// to become inaccessible.
 static void DiagnoseAccessPath(Sema &S,
@@ -948,6 +999,10 @@ static void DiagnoseAccessPath(Sema &S,
   if (D && (Access == D->getAccess() || D->getAccess() == AS_private)) {
     switch (HasAccess(S, EC, DeclaringClass, D->getAccess(), Entity)) {
     case AR_inaccessible: {
+      if (Access == AS_protected &&
+          TryDiagnoseProtectedAccess(S, EC, Entity))
+        return;
+
       S.Diag(D->getLocation(), diag::note_access_natural)
         << (unsigned) (Access == AS_protected)
         << /*FIXME: not implicitly*/ 0;

clang/test/CXX/class.access/class.protected/p1.cpp

@@ -68,7 +68,7 @@ namespace test1 {
 
 namespace test2 {
   class A {
-    protected: int x; // expected-note 3 {{declared}}
+    protected: int x; // expected-note 3 {{object type must derive}}
     static int sx;
     static void test(A&);
   };
@@ -103,7 +103,7 @@ namespace test2 {
 namespace test3 {
   class B;
   class A {
-    protected: int x; // expected-note {{declared}}
+    protected: int x; // expected-note {{object type must derive}}
     static int sx;
     static void test(B&);
   };
@@ -138,7 +138,7 @@ namespace test3 {
 namespace test4 {
   class C;
   class A {
-    protected: int x; // expected-note 3 {{declared}}
+    protected: int x; // expected-note {{declared}} expected-note 2 {{object type must derive}}
     static int sx;    // expected-note 3{{member is declared here}}
     static void test(C&);
   };
@@ -215,7 +215,7 @@ namespace test6 {
   class Static {};
   class A {
   protected:
-    void foo(int); // expected-note 3 {{declared}}
+    void foo(int); // expected-note 3 {{object type must derive}}
     void foo(long);
     static void foo(Static);
 
@@ -253,7 +253,7 @@ namespace test7 {
   class Static {};
   class A {
     protected:
-    void foo(int); // expected-note 3 {{declared}}
+    void foo(int); // expected-note 3 {{object type must derive}}
     void foo(long);
     static void foo(Static);
 
@@ -291,7 +291,7 @@ namespace test8 {
   class Static {};
   class A {
     protected:
-    void foo(int); // expected-note 3 {{declared}}
+    void foo(int); // expected-note 3 {{object type must derive}}
     void foo(long);
     static void foo(Static);
 
@@ -329,7 +329,7 @@ namespace test8 {
 
 namespace test9 {
   class A { // expected-note {{member is declared here}}
-  protected: int foo(); // expected-note 7 {{declared}}
+  protected: int foo(); // expected-note 4 {{declared}} expected-note 2 {{object type must derive}} expected-note {{object type 'test9::A' must derive}}
   };
 
   class B : public A { // expected-note {{member is declared here}}

clang/test/CXX/class.access/p4.cpp

@@ -372,7 +372,7 @@ namespace test15 {
     int private_foo; // expected-note {{declared private here}}
     static int private_sfoo; // expected-note {{declared private here}}
   protected:
-    int protected_foo; // expected-note 4 {{declared protected here}}
+    int protected_foo; // expected-note 3 {{declared protected here}} // expected-note {{object type must derive from context type 'test15::B<int>'}}
     static int protected_sfoo; // expected-note 3 {{declared protected here}}
 
     int test1(A<int> &a) {