[PATCH] Lexical scope of friend function should not be taken into account for access checking

[Ismail Pazarbasi]

Hi rsmith, doug.gregor,

PR9103 is concerning access to a static protected member of a base class member from a friend function body defined in derived class.

PR11515 is concerning accessing a private member of another class that is befriending class of a friend function.

The patch for PR9103 used lexical context of the befriended class for checking access from a friend function definition. Per standard, a friend function is in the lexical scope of the class iff it's defined in the class. Using lexical scope of friend function as its declcontext for access checking leads to PR11515.

I think EffectiveContext should not consider lexical scope of friend function as declcontext. Access to protected static members of a base class should be handled differently.

http://llvm-reviews.chandlerc.com/D849

Files:
  lib/Sema/SemaAccess.cpp
  test/SemaCXX/access.cpp

Index: lib/Sema/SemaAccess.cpp
===================================================================
--- lib/Sema/SemaAccess.cpp
+++ lib/Sema/SemaAccess.cpp
@@ -110,10 +110,7 @@
       } else if (isa<FunctionDecl>(DC)) {
         FunctionDecl *Function = cast<FunctionDecl>(DC);
         Functions.push_back(Function->getCanonicalDecl());
-        if (Function->getFriendObjectKind())
-          DC = Function->getLexicalDeclContext();
-        else
-          DC = Function->getDeclContext();
+        DC = Function->getDeclContext();
       } else if (DC->isFileContext()) {
         break;
       } else {
@@ -850,15 +847,26 @@
   // *unless* the [class.protected] restriction applies.  If it does,
   // however, we should ignore whether the naming class is a friend,
   // and instead rely on whether any potential P is a friend.
-  if (Access == AS_protected && Target.isInstanceMember()) {
+  if (Access == AS_protected) {
+    const CXXRecordDecl *Context = 0;
     // Compute the instance context if possible.
-    const CXXRecordDecl *InstanceContext = 0;
-    if (Target.hasInstanceContext()) {
-      InstanceContext = Target.resolveInstanceContext(S);
-      if (!InstanceContext) return AR_dependent;
+    if (Target.isInstanceMember()) {
+      if (Target.hasInstanceContext()) {
+        Context = Target.resolveInstanceContext(S);
+        if (!Context)
+          return AR_dependent;
+      }
+    } else {
+      if (EC.getInnerContext()->isFunctionOrMethod()) {
+        assert(!EC.Functions.empty()
+               && "EffectiveContext should contain a function declaration");
+        const FunctionDecl *FD = EC.Functions[0];
+        if (FD->getFriendObjectKind())
+          Context = cast<CXXRecordDecl>(FD->getLexicalDeclContext());
+      }
     }
 
-    switch (GetProtectedFriendKind(S, EC, InstanceContext, NamingClass)) {
+    switch (GetProtectedFriendKind(S, EC, Context, NamingClass)) {
     case AR_accessible: return AR_accessible;
     case AR_inaccessible: return OnFailure;
     case AR_dependent: return AR_dependent;
Index: test/SemaCXX/access.cpp
===================================================================
--- test/SemaCXX/access.cpp
+++ test/SemaCXX/access.cpp
@@ -106,3 +106,76 @@
     }
   }
 }
+
+namespace PR11515 {
+  class A {
+    int n; // expected-note {{implicitly declared private here}}
+    friend struct B;
+  };
+
+  struct B {
+    friend int get(A &a) {
+      return a.n;  // expected-error {{'n' is a private member of 'PR11515::A'}}
+    }
+  };
+}
+
+// test for PR9103, and example from the standard class.protected/p1
+namespace class_protected {
+  struct base
+  {
+  protected:
+    static void foo(void) {}
+  };
+
+  struct cls: base
+  {
+    friend void bar(void)
+    {
+      base::foo();
+    }
+  };
+
+  class B {
+  protected:
+    int i1;  // expected-note {{declared protected here}}
+    int i2;  // expected-note {{declared protected here}}
+    int i3;
+    int i4;  // expected-note {{declared protected here}}
+    int i5;  // expected-note {{can only access this member on an object of type 'class_protected::D2'}}
+    int i6;  // expected-note {{declared protected here}}
+    int i7;  // expected-note {{must name member using the type of the current context 'class_protected::D2'}}
+    static int j;
+  };
+
+  class D1 : public B {
+  };
+
+  class D2 : public B {
+    friend void fr(B* pb,D1* p1,D2* p2);
+    void mem(B*,D1*);
+  };
+
+  void fr(B* pb, D1* p1, D2* p2)
+  {
+    pb->i1 = 1; // expected-error {{'i1' is a protected member of 'class_protected::B'}}
+    p1->i2 = 2; // expected-error {{'i2' is a protected member of 'class_protected::B'}}
+    p2->i3 = 3;
+    p2->B::i3 = 4;
+    int B::* pmi_B = &B::i4;  // expected-error {{'i4' is a protected member of 'class_protected::B'}}
+    int B::* pmi_B2 = &D2::i3;
+    B::j = 5;
+    D2::j = 6;
+  }
+
+  void D2::mem(B* pb, D1* p1) {
+    pb->i5 = 1;  // expected-error {{'i5' is a protected member of 'class_protected::B'}}
+    p1->i6 = 2;  // expected-error {{'i6' is a protected member of 'class_protected::B'}}
+    i3 = 3;
+    B::i4 = 4;
+    int B::* pmi_B = &B::i7;  // expected-error {{'i7' is a protected member of 'class_protected::B'}}
+    int B::* pmi_B2 = &D2::i3;
+    j = 5;
+    B::j = 6;
+  }
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D849.1.patch
Type: text/x-patch
Size: 4285 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130523/bd16918e/attachment.bin>