[cfe-commits] r161652 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaAccess.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaTemplateInstantiate.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp test/CXX/class.access/class.friend/p1.cpp test/CXX/class.access/class.friend/p9-cxx0x.cpp test/CXX/temp/temp.decls/temp.friend/p1.cpp
John McCall
rjmccall at apple.com
Thu Aug 9 20:15:35 PDT 2012
Author: rjmccall
Date: Thu Aug 9 22:15:35 2012
New Revision: 161652
URL: http://llvm.org/viewvc/llvm-project?rev=161652&view=rev
Log:
Check access to friend declarations. There's a number of different
things going on here that were problematic:
- We were missing the actual access check, or rather, it was suppressed
on account of being a redeclaration lookup.
- The access check would naturally happen during delay, which isn't
appropriate in this case.
- We weren't actually emitting dependent diagnostics associated with
class templates, which was unfortunate.
- Access was being propagated incorrectly for friend method declarations
that couldn't be matched at parse-time.
Added:
cfe/trunk/test/CXX/class.access/class.friend/p9-cxx0x.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaAccess.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/CXX/class.access/class.friend/p1.cpp
cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p1.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=161652&r1=161651&r2=161652&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Aug 9 22:15:35 2012
@@ -909,6 +909,9 @@
"field of type %0 has %select{private|protected}2 "
"%select{default |copy |move |*ERROR* |*ERROR* |*ERROR* |}1constructor">,
AccessControl;
+def err_access_friend_function : Error<
+ "friend function %1 is a %select{private|protected}0 member of %3">,
+ AccessControl;
def err_access_dtor : Error<
"calling a %select{private|protected}1 destructor of class %0">,
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=161652&r1=161651&r2=161652&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Aug 9 22:15:35 2012
@@ -4427,9 +4427,7 @@
CXXDestructorDecl *Dtor,
const PartialDiagnostic &PDiag,
QualType objectType = QualType());
- AccessResult CheckDirectMemberAccess(SourceLocation Loc,
- NamedDecl *D,
- const PartialDiagnostic &PDiag);
+ AccessResult CheckFriendAccess(NamedDecl *D);
AccessResult CheckMemberOperatorAccess(SourceLocation Loc,
Expr *ObjectExpr,
Expr *ArgExpr,
Modified: cfe/trunk/lib/Sema/SemaAccess.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAccess.cpp?rev=161652&r1=161651&r2=161652&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaAccess.cpp (original)
+++ cfe/trunk/lib/Sema/SemaAccess.cpp Thu Aug 9 22:15:35 2012
@@ -1632,25 +1632,6 @@
return CheckAccess(*this, UseLoc, AccessEntity);
}
-/// Checks direct (i.e. non-inherited) access to an arbitrary class
-/// member.
-Sema::AccessResult Sema::CheckDirectMemberAccess(SourceLocation UseLoc,
- NamedDecl *Target,
- const PartialDiagnostic &Diag) {
- AccessSpecifier Access = Target->getAccess();
- if (!getLangOpts().AccessControl ||
- Access == AS_public)
- return AR_accessible;
-
- CXXRecordDecl *NamingClass = cast<CXXRecordDecl>(Target->getDeclContext());
- AccessTarget Entity(Context, AccessTarget::Member, NamingClass,
- DeclAccessPair::make(Target, Access),
- QualType());
- Entity.setDiag(Diag);
- return CheckAccess(*this, UseLoc, Entity);
-}
-
-
/// Checks access to an overloaded operator new or delete.
Sema::AccessResult Sema::CheckAllocationAccess(SourceLocation OpLoc,
SourceRange PlacementRange,
@@ -1693,6 +1674,44 @@
return CheckAccess(*this, OpLoc, Entity);
}
+/// Checks access to the target of a friend declaration.
+Sema::AccessResult Sema::CheckFriendAccess(NamedDecl *target) {
+ assert(isa<CXXMethodDecl>(target) ||
+ (isa<FunctionTemplateDecl>(target) &&
+ isa<CXXMethodDecl>(cast<FunctionTemplateDecl>(target)
+ ->getTemplatedDecl())));
+
+ // Friendship lookup is a redeclaration lookup, so there's never an
+ // inheritance path modifying access.
+ AccessSpecifier access = target->getAccess();
+
+ if (!getLangOpts().AccessControl || access == AS_public)
+ return AR_accessible;
+
+ CXXMethodDecl *method = dyn_cast<CXXMethodDecl>(target);
+ if (!method)
+ method = cast<CXXMethodDecl>(
+ cast<FunctionTemplateDecl>(target)->getTemplatedDecl());
+ assert(method->getQualifier());
+
+ AccessTarget entity(Context, AccessTarget::Member,
+ cast<CXXRecordDecl>(target->getDeclContext()),
+ DeclAccessPair::make(target, access),
+ /*no instance context*/ QualType());
+ entity.setDiag(diag::err_access_friend_function)
+ << method->getQualifierLoc().getSourceRange();
+
+ // We need to bypass delayed-diagnostics because we might be called
+ // while the ParsingDeclarator is active.
+ EffectiveContext EC(CurContext);
+ switch (CheckEffectiveAccess(*this, EC, target->getLocation(), entity)) {
+ case AR_accessible: return Sema::AR_accessible;
+ case AR_inaccessible: return Sema::AR_inaccessible;
+ case AR_dependent: return Sema::AR_dependent;
+ }
+ llvm_unreachable("falling off end");
+}
+
Sema::AccessResult Sema::CheckAddressOfMemberAccess(Expr *OvlExpr,
DeclAccessPair Found) {
if (!getLangOpts().AccessControl ||
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=161652&r1=161651&r2=161652&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Aug 9 22:15:35 2012
@@ -10342,9 +10342,11 @@
FrD->setAccess(AS_public);
CurContext->addDecl(FrD);
- if (ND->isInvalidDecl())
+ if (ND->isInvalidDecl()) {
FrD->setInvalidDecl();
- else {
+ } else {
+ if (DC->isRecord()) CheckFriendAccess(ND);
+
FunctionDecl *FD;
if (FunctionTemplateDecl *FTD = dyn_cast<FunctionTemplateDecl>(ND))
FD = FTD->getTemplatedDecl();
Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp?rev=161652&r1=161651&r2=161652&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp Thu Aug 9 22:15:35 2012
@@ -2005,6 +2005,9 @@
}
if (!Instantiation->isInvalidDecl()) {
+ // Perform any dependent diagnostics from the pattern.
+ PerformDependentDiagnostics(Pattern, TemplateArgs);
+
// Instantiate any out-of-line class template partial
// specializations now.
for (TemplateDeclInstantiator::delayed_partial_spec_iterator
Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=161652&r1=161651&r2=161652&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Thu Aug 9 22:15:35 2012
@@ -935,8 +935,6 @@
if (!Instantiated)
return 0;
- Instantiated->setAccess(D->getAccess());
-
// Link the instantiated function template declaration to the function
// template from which it was instantiated.
FunctionTemplateDecl *InstTemplate
@@ -954,8 +952,12 @@
InstTemplate->setInstantiatedFromMemberTemplate(D);
// Make declarations visible in the appropriate context.
- if (!isFriend)
+ if (!isFriend) {
Owner->addDecl(InstTemplate);
+ } else if (InstTemplate->getDeclContext()->isRecord() &&
+ !D->getPreviousDecl()) {
+ SemaRef.CheckFriendAccess(InstTemplate);
+ }
return InstTemplate;
}
@@ -1526,7 +1528,15 @@
if (D->isPure())
SemaRef.CheckPureMethod(Method, SourceRange());
- Method->setAccess(D->getAccess());
+ // Propagate access. For a non-friend declaration, the access is
+ // whatever we're propagating from. For a friend, it should be the
+ // previous declaration we just found.
+ if (isFriend && Method->getPreviousDecl())
+ Method->setAccess(Method->getPreviousDecl()->getAccess());
+ else
+ Method->setAccess(D->getAccess());
+ if (FunctionTemplate)
+ FunctionTemplate->setAccess(Method->getAccess());
SemaRef.CheckOverrideControl(Method);
@@ -1536,18 +1546,28 @@
if (D->isDeletedAsWritten())
Method->setDeletedAsWritten();
+ // If there's a function template, let our caller handle it.
if (FunctionTemplate) {
- // If there's a function template, let our caller handle it.
+ // do nothing
+
+ // Don't hide a (potentially) valid declaration with an invalid one.
} else if (Method->isInvalidDecl() && !Previous.empty()) {
- // Don't hide a (potentially) valid declaration with an invalid one.
- } else {
- NamedDecl *DeclToAdd = (TemplateParams
- ? cast<NamedDecl>(FunctionTemplate)
- : Method);
- if (isFriend)
- Record->makeDeclVisibleInContext(DeclToAdd);
- else if (!IsClassScopeSpecialization)
- Owner->addDecl(DeclToAdd);
+ // do nothing
+
+ // Otherwise, check access to friends and make them visible.
+ } else if (isFriend) {
+ // We only need to re-check access for methods which we didn't
+ // manage to match during parsing.
+ if (!D->getPreviousDecl())
+ SemaRef.CheckFriendAccess(Method);
+
+ Record->makeDeclVisibleInContext(Method);
+
+ // Otherwise, add the declaration. We don't need to do this for
+ // class-scope specializations because we'll have matched them with
+ // the appropriate template.
+ } else if (!IsClassScopeSpecialization) {
+ Owner->addDecl(Method);
}
if (D->isExplicitlyDefaulted()) {
Modified: cfe/trunk/test/CXX/class.access/class.friend/p1.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.access/class.friend/p1.cpp?rev=161652&r1=161651&r2=161652&view=diff
==============================================================================
--- cfe/trunk/test/CXX/class.access/class.friend/p1.cpp (original)
+++ cfe/trunk/test/CXX/class.access/class.friend/p1.cpp Thu Aug 9 22:15:35 2012
@@ -64,6 +64,7 @@
};
class MemberFriend {
+ public:
void test();
};
@@ -309,6 +310,7 @@
// PR8705
namespace test11 {
class A {
+ public:
void test0(int);
void test1(int);
void test2(int);
Added: cfe/trunk/test/CXX/class.access/class.friend/p9-cxx0x.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.access/class.friend/p9-cxx0x.cpp?rev=161652&view=auto
==============================================================================
--- cfe/trunk/test/CXX/class.access/class.friend/p9-cxx0x.cpp (added)
+++ cfe/trunk/test/CXX/class.access/class.friend/p9-cxx0x.cpp Thu Aug 9 22:15:35 2012
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// C++98 [class.friend]p7:
+// C++11 [class.friend]p9:
+// A name nominated by a friend declaration shall be accessible in
+// the scope of the class containing the friend declaration.
+
+// PR12328
+// Simple, non-templated case.
+namespace test0 {
+ class X {
+ void f(); // expected-note {{implicitly declared private here}}
+ };
+
+ class Y {
+ friend void X::f(); // expected-error {{friend function 'f' is a private member of 'test0::X'}}
+ };
+}
+
+// Templated but non-dependent.
+namespace test1 {
+ class X {
+ void f(); // expected-note {{implicitly declared private here}}
+ };
+
+ template <class T> class Y {
+ friend void X::f(); // expected-error {{friend function 'f' is a private member of 'test1::X'}}
+ };
+}
+
+// Dependent but instantiated at the right type.
+namespace test2 {
+ template <class T> class Y;
+
+ class X {
+ void f();
+ friend class Y<int>;
+ };
+
+ template <class T> class Y {
+ friend void X::f();
+ };
+
+ template class Y<int>;
+}
+
+// Dependent and instantiated at the wrong type.
+namespace test3 {
+ template <class T> class Y;
+
+ class X {
+ void f(); // expected-note {{implicitly declared private here}}
+ friend class Y<int>;
+ };
+
+ template <class T> class Y {
+ friend void X::f(); // expected-error {{friend function 'f' is a private member of 'test3::X'}}
+ };
+
+ template class Y<float>; // expected-note {{in instantiation}}
+}
+
+// Dependent because dependently-scoped.
+namespace test4 {
+ template <class T> class X {
+ void f();
+ };
+
+ template <class T> class Y {
+ friend void X<T>::f();
+ };
+}
+
+// Dependently-scoped, no friends.
+namespace test5 {
+ template <class T> class X {
+ void f(); // expected-note {{implicitly declared private here}}
+ };
+
+ template <class T> class Y {
+ friend void X<T>::f(); // expected-error {{friend function 'f' is a private member of 'test5::X<int>'}}
+ };
+
+ template class Y<int>; // expected-note {{in instantiation}}
+}
+
+// Dependently-scoped, wrong friend.
+namespace test6 {
+ template <class T> class Y;
+
+ template <class T> class X {
+ void f(); // expected-note {{implicitly declared private here}}
+ friend class Y<float>;
+ };
+
+ template <class T> class Y {
+ friend void X<T>::f(); // expected-error {{friend function 'f' is a private member of 'test6::X<int>'}}
+ };
+
+ template class Y<int>; // expected-note {{in instantiation}}
+}
+
+// Dependently-scoped, right friend.
+namespace test7 {
+ template <class T> class Y;
+
+ template <class T> class X {
+ void f();
+ friend class Y<int>;
+ };
+
+ template <class T> class Y {
+ friend void X<T>::f();
+ };
+
+ template class Y<int>;
+}
Modified: cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p1.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p1.cpp?rev=161652&r1=161651&r2=161652&view=diff
==============================================================================
--- cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p1.cpp (original)
+++ cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p1.cpp Thu Aug 9 22:15:35 2012
@@ -302,6 +302,7 @@
};
template <class T> class B {
+ public:
void foo() { return A<long>::foo(); } // expected-error {{'foo' is a private member of 'test14::A<long>'}}
};
@@ -320,10 +321,12 @@
};
template <class T> class B {
+ public:
void foo() { return A<long>::foo(); } // expected-error {{'foo' is a private member of 'test15::A<long>'}}
};
template <> class B<float> {
+ public:
void foo() { return A<float>::foo(); }
template <class U> void bar(U u) {
(void) A<float>::foo();
More information about the cfe-commits
mailing list