r186546 - Reinstate r186040, with additional fixes and more test coverage (reverted in
Richard Smith
richard-llvm at metafoo.co.uk
Wed Jul 17 16:53:16 PDT 2013
Author: rsmith
Date: Wed Jul 17 18:53:16 2013
New Revision: 186546
URL: http://llvm.org/viewvc/llvm-project?rev=186546&view=rev
Log:
Reinstate r186040, with additional fixes and more test coverage (reverted in
r186331).
Original commit log:
If we friend a declaration twice, that should not make it visible to
name lookup in the surrounding context. Slightly rework how we handle
friend declarations to inherit the visibility of the prior
declaration, rather than setting a friend declaration to be visible
whenever there was a prior declaration.
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp
cfe/trunk/test/SemaCXX/friend.cpp
Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=186546&r1=186545&r2=186546&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Wed Jul 17 18:53:16 2013
@@ -3406,6 +3406,12 @@ void Redeclarable<decl_type>::setPreviou
assert(First->RedeclLink.NextIsLatest() && "Expected first");
decl_type *MostRecent = First->RedeclLink.getNext();
RedeclLink = PreviousDeclLink(cast<decl_type>(MostRecent));
+
+ // If the declaration was previously visible, a redeclaration of it remains
+ // visible even if it wouldn't be visible by itself.
+ static_cast<decl_type*>(this)->IdentifierNamespace |=
+ PrevDecl->getIdentifierNamespace() &
+ (Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type);
} else {
// Make this first.
First = static_cast<decl_type*>(this);
Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=186546&r1=186545&r2=186546&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Wed Jul 17 18:53:16 2013
@@ -295,6 +295,8 @@ protected:
friend class ASTReader;
friend class LinkageComputer;
+ template<typename decl_type> friend class Redeclarable;
+
private:
void CheckAccessDeclContext() const;
@@ -824,7 +826,7 @@ public:
/// class, but in the semantic context of the actual entity. This property
/// applies only to a specific decl object; other redeclarations of the
/// same entity may not (and probably don't) share this property.
- void setObjectOfFriendDecl(bool PreviouslyDeclared) {
+ void setObjectOfFriendDecl(bool PerformFriendInjection = false) {
unsigned OldNS = IdentifierNamespace;
assert((OldNS & (IDNS_Tag | IDNS_Ordinary |
IDNS_TagFriend | IDNS_OrdinaryFriend)) &&
@@ -833,15 +835,20 @@ public:
IDNS_TagFriend | IDNS_OrdinaryFriend)) &&
"namespace includes other than ordinary or tag");
+ Decl *Prev = getPreviousDecl();
IdentifierNamespace = 0;
if (OldNS & (IDNS_Tag | IDNS_TagFriend)) {
IdentifierNamespace |= IDNS_TagFriend;
- if (PreviouslyDeclared) IdentifierNamespace |= IDNS_Tag | IDNS_Type;
+ if (PerformFriendInjection ||
+ (Prev && Prev->getIdentifierNamespace() & IDNS_Tag))
+ IdentifierNamespace |= IDNS_Tag | IDNS_Type;
}
if (OldNS & (IDNS_Ordinary | IDNS_OrdinaryFriend)) {
IdentifierNamespace |= IDNS_OrdinaryFriend;
- if (PreviouslyDeclared) IdentifierNamespace |= IDNS_Ordinary;
+ if (PerformFriendInjection ||
+ (Prev && Prev->getIdentifierNamespace() & IDNS_Ordinary))
+ IdentifierNamespace |= IDNS_Ordinary;
}
}
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=186546&r1=186545&r2=186546&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jul 17 18:53:16 2013
@@ -6330,12 +6330,11 @@ Sema::ActOnFunctionDeclarator(Scope *S,
}
if (isFriend) {
- // For now, claim that the objects have no previous declaration.
if (FunctionTemplate) {
- FunctionTemplate->setObjectOfFriendDecl(false);
+ FunctionTemplate->setObjectOfFriendDecl();
FunctionTemplate->setAccess(AS_public);
}
- NewFD->setObjectOfFriendDecl(false);
+ NewFD->setObjectOfFriendDecl();
NewFD->setAccess(AS_public);
}
@@ -6654,8 +6653,6 @@ Sema::ActOnFunctionDeclarator(Scope *S,
NewFD->setAccess(Access);
if (FunctionTemplate) FunctionTemplate->setAccess(Access);
-
- PrincipalDecl->setObjectOfFriendDecl(true);
}
if (NewFD->isOverloadedOperator() && !DC->isRecord() &&
@@ -10392,9 +10389,8 @@ CreateNewDecl:
// declaration so we always pass true to setObjectOfFriendDecl to make
// the tag name visible.
if (TUK == TUK_Friend)
- New->setObjectOfFriendDecl(/* PreviouslyDeclared = */ !Previous.empty() ||
- (!FriendSawTagOutsideEnclosingNamespace &&
- getLangOpts().MicrosoftExt));
+ New->setObjectOfFriendDecl(!FriendSawTagOutsideEnclosingNamespace &&
+ getLangOpts().MicrosoftExt);
// Set the access specifier.
if (!Invalid && SearchDC->isRecord())
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=186546&r1=186545&r2=186546&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Wed Jul 17 18:53:16 2013
@@ -2757,8 +2757,16 @@ void Sema::ArgumentDependentLookup(Decla
// If the only declaration here is an ordinary friend, consider
// it only if it was declared in an associated classes.
if (D->getIdentifierNamespace() == Decl::IDNS_OrdinaryFriend) {
- DeclContext *LexDC = D->getLexicalDeclContext();
- if (!AssociatedClasses.count(cast<CXXRecordDecl>(LexDC)))
+ bool DeclaredInAssociatedClass = false;
+ for (Decl *DI = D; DI; DI = DI->getPreviousDecl()) {
+ DeclContext *LexDC = DI->getLexicalDeclContext();
+ if (isa<CXXRecordDecl>(LexDC) &&
+ AssociatedClasses.count(cast<CXXRecordDecl>(LexDC))) {
+ DeclaredInAssociatedClass = true;
+ break;
+ }
+ }
+ if (!DeclaredInAssociatedClass)
continue;
}
Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=186546&r1=186545&r2=186546&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Jul 17 18:53:16 2013
@@ -1120,8 +1120,7 @@ Sema::CheckClassTemplate(Scope *S, unsig
NewClass->setAccess(PrevClassTemplate->getAccess());
}
- NewTemplate->setObjectOfFriendDecl(/* PreviouslyDeclared = */
- PrevClassTemplate != NULL);
+ NewTemplate->setObjectOfFriendDecl();
// Friend templates are visible in fairly strange ways.
if (!CurContext->isDependentContext()) {
Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=186546&r1=186545&r2=186546&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Jul 17 18:53:16 2013
@@ -960,7 +960,7 @@ Decl *TemplateDeclInstantiator::VisitCla
else
Inst->setAccess(D->getAccess());
- Inst->setObjectOfFriendDecl(PrevClassTemplate != 0);
+ Inst->setObjectOfFriendDecl();
// TODO: do we want to track the instantiation progeny of this
// friend target decl?
} else {
@@ -1110,8 +1110,8 @@ Decl *TemplateDeclInstantiator::VisitCXX
// If the original function was part of a friend declaration,
// inherit its namespace state.
- if (Decl::FriendObjectKind FOK = D->getFriendObjectKind())
- Record->setObjectOfFriendDecl(FOK == Decl::FOK_Declared);
+ if (D->getFriendObjectKind())
+ Record->setObjectOfFriendDecl();
// Make sure that anonymous structs and unions are recorded.
if (D->isAnonymousStructOrUnion()) {
@@ -1315,7 +1315,7 @@ Decl *TemplateDeclInstantiator::VisitFun
assert(isFriend && "non-friend has dependent specialization info?");
// This needs to be set now for future sanity.
- Function->setObjectOfFriendDecl(/*HasPrevious*/ true);
+ Function->setObjectOfFriendDecl();
// Instantiate the explicit template arguments.
TemplateArgumentListInfo ExplicitArgs(Info->getLAngleLoc(),
@@ -1365,13 +1365,7 @@ Decl *TemplateDeclInstantiator::VisitFun
// If the original function was part of a friend declaration,
// inherit its namespace state and add it to the owner.
if (isFriend) {
- NamedDecl *PrevDecl;
- if (TemplateParams)
- PrevDecl = FunctionTemplate->getPreviousDecl();
- else
- PrevDecl = Function->getPreviousDecl();
-
- PrincipalDecl->setObjectOfFriendDecl(PrevDecl != 0);
+ PrincipalDecl->setObjectOfFriendDecl();
DC->makeDeclVisibleInContext(PrincipalDecl);
bool queuedInstantiation = false;
@@ -1639,7 +1633,7 @@ TemplateDeclInstantiator::VisitCXXMethod
TemplateParams, Method);
if (isFriend) {
FunctionTemplate->setLexicalDeclContext(Owner);
- FunctionTemplate->setObjectOfFriendDecl(true);
+ FunctionTemplate->setObjectOfFriendDecl();
} else if (D->isOutOfLine())
FunctionTemplate->setLexicalDeclContext(D->getLexicalDeclContext());
Method->setDescribedFunctionTemplate(FunctionTemplate);
@@ -1666,7 +1660,7 @@ TemplateDeclInstantiator::VisitCXXMethod
TempParamLists.data());
Method->setLexicalDeclContext(Owner);
- Method->setObjectOfFriendDecl(true);
+ Method->setObjectOfFriendDecl();
} else if (D->isOutOfLine())
Method->setLexicalDeclContext(D->getLexicalDeclContext());
Modified: cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp?rev=186546&r1=186545&r2=186546&view=diff
==============================================================================
--- cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp (original)
+++ cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp Wed Jul 17 18:53:16 2013
@@ -26,3 +26,20 @@ void g() {
X2<float> xf;
f(xf);
}
+
+template<typename T>
+struct X3 {
+ operator int();
+
+ friend void h(int x);
+};
+
+int array2[sizeof(X3<int>)];
+int array3[sizeof(X3<float>)];
+
+void i() {
+ X3<int> xi;
+ h(xi);
+ X3<float> xf;
+ h(xf);
+}
Modified: cfe/trunk/test/SemaCXX/friend.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/friend.cpp?rev=186546&r1=186545&r2=186546&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/friend.cpp (original)
+++ cfe/trunk/test/SemaCXX/friend.cpp Wed Jul 17 18:53:16 2013
@@ -165,14 +165,126 @@ namespace test9 {
}
namespace test10 {
+ struct X {};
+ extern void f10_a();
+ extern void f10_a(X);
struct A {
- friend void f();
+ friend void f10_a();
+ friend void f10_b();
+ friend void f10_c();
+ friend void f10_d();
+ friend void f10_a(X);
+ friend void f10_b(X);
+ friend void f10_c(X);
+ friend void f10_d(X);
};
- extern void f();
+ extern void f10_b();
+ extern void f10_b(X);
struct B {
- friend void f();
+ friend void f10_a();
+ friend void f10_b();
+ friend void f10_c();
+ friend void f10_d();
+ friend void f10_a(X);
+ friend void f10_b(X);
+ friend void f10_c(X);
+ friend void f10_d(X);
};
- void g() {
- ::test10::f();
+ extern void f10_c();
+ extern void f10_c(X);
+
+ // FIXME: Give a better diagnostic for the case where a function exists but is
+ // not visible.
+ void g(X x) {
+ f10_a();
+ f10_b();
+ f10_c();
+ f10_d(); // expected-error {{undeclared identifier}}
+
+ ::test10::f10_a();
+ ::test10::f10_b();
+ ::test10::f10_c();
+ ::test10::f10_d(); // expected-error {{no member named 'f10_d'}}
+
+ f10_a(x);
+ f10_b(x);
+ f10_c(x);
+ f10_d(x); // PR16597: expected-error {{undeclared identifier}}
+
+ ::test10::f10_a(x);
+ ::test10::f10_b(x);
+ ::test10::f10_c(x);
+ ::test10::f10_d(x); // expected-error {{no type named 'f10_d'}}
+ }
+
+ struct Y : X {
+ friend void f10_d();
+ friend void f10_d(X);
+ };
+
+ struct Z {
+ operator X();
+ friend void f10_d();
+ friend void f10_d(X);
+ };
+
+ void g(X x, Y y, Z z) {
+ f10_d(); // expected-error {{undeclared identifier}}
+ ::test10::f10_d(); // expected-error {{no member named 'f10_d'}}
+
+ // f10_d is visible to ADL in the second and third cases.
+ f10_d(x); // expected-error {{undeclared identifier}}
+ f10_d(y);
+ f10_d(z);
+
+ // No ADL here.
+ ::test10::f10_d(x); // expected-error {{no type named 'f10_d'}}
+ ::test10::f10_d(y); // expected-error {{no type named 'f10_d'}}
+ ::test10::f10_d(z); // expected-error {{no type named 'f10_d'}}
+ }
+
+ void local_externs(X x, Y y) {
+ extern void f10_d();
+ extern void f10_d(X);
+ f10_d();
+ f10_d(x);
+ // FIXME: This lookup should fail, because the local extern declaration
+ // should suppress ADL.
+ f10_d(y);
+ {
+ int f10_d;
+ f10_d(); // expected-error {{not a function}}
+ f10_d(x); // expected-error {{not a function}}
+ f10_d(y); // expected-error {{not a function}}
+ }
+ }
+
+ void i(X x, Y y) {
+ f10_d(); // expected-error {{undeclared identifier}}
+ f10_d(x); // expected-error {{undeclared identifier}}
+ f10_d(y);
+ }
+
+ struct C {
+ friend void f10_d();
+ friend void f10_d(X);
+ };
+
+ void j(X x, Y y) {
+ f10_d(); // expected-error {{undeclared identifier}}
+ f10_d(x); // expected-error {{undeclared identifier}}
+ f10_d(y);
+ }
+
+ extern void f10_d();
+ extern void f10_d(X);
+ void k(X x, Y y, Z z) {
+ // All OK now.
+ f10_d();
+ f10_d(x);
+ ::test10::f10_d();
+ ::test10::f10_d(x);
+ ::test10::f10_d(y);
+ ::test10::f10_d(z);
}
}
More information about the cfe-commits
mailing list