Thank you!<br><br><div class="gmail_quote">On Mon, Jul 15, 2013 at 10:27 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: chandlerc<br>
Date: Mon Jul 15 12:27:42 2013<br>
New Revision: 186331<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=186331&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=186331&view=rev</a><br>
Log:<br>
Re-revert r86040, which was un-reverted in r186199.<br>
<br>
This breaks the build of basic patterns with repeated friend<br>
declarations. See the added test case in SemaCXX/friend.cpp or the test<br>
case reported to the original commit log.<br>
<br>
Original commit log:<br>
If we friend a declaration twice, that should not make it visible to<br>
name lookup in the surrounding context. Slightly rework how we handle<br>
friend declarations to inherit the visibility of the prior<br>
declaration, rather than setting a friend declaration to be visible<br>
whenever there was a prior declaration.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/AST/Decl.h<br>
cfe/trunk/include/clang/AST/DeclBase.h<br>
cfe/trunk/lib/Sema/SemaDecl.cpp<br>
cfe/trunk/lib/Sema/SemaLookup.cpp<br>
cfe/trunk/lib/Sema/SemaTemplate.cpp<br>
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp<br>
cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp<br>
cfe/trunk/test/SemaCXX/friend.cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/Decl.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=186331&r1=186330&r2=186331&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=186331&r1=186330&r2=186331&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/Decl.h (original)<br>
+++ cfe/trunk/include/clang/AST/Decl.h Mon Jul 15 12:27:42 2013<br>
@@ -3415,12 +3415,6 @@ void Redeclarable<decl_type>::setPreviou<br>
First->RedeclLink = LatestDeclLink(static_cast<decl_type*>(this));<br>
assert(!isa<NamedDecl>(static_cast<decl_type*>(this)) ||<br>
cast<NamedDecl>(static_cast<decl_type*>(this))->isLinkageValid());<br>
-<br>
- // If the declaration was previously visible, a redeclaration of it remains<br>
- // visible even if it wouldn't be visible by itself.<br>
- static_cast<decl_type*>(this)->IdentifierNamespace |=<br>
- First->getIdentifierNamespace() &<br>
- (Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type);<br>
}<br>
<br>
// Inline function definitions.<br>
<br>
Modified: cfe/trunk/include/clang/AST/DeclBase.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=186331&r1=186330&r2=186331&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=186331&r1=186330&r2=186331&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/DeclBase.h (original)<br>
+++ cfe/trunk/include/clang/AST/DeclBase.h Mon Jul 15 12:27:42 2013<br>
@@ -295,8 +295,6 @@ protected:<br>
friend class ASTReader;<br>
friend class LinkageComputer;<br>
<br>
- template<typename decl_type> friend class Redeclarable;<br>
-<br>
private:<br>
void CheckAccessDeclContext() const;<br>
<br>
@@ -826,7 +824,7 @@ public:<br>
/// class, but in the semantic context of the actual entity. This property<br>
/// applies only to a specific decl object; other redeclarations of the<br>
/// same entity may not (and probably don't) share this property.<br>
- void setObjectOfFriendDecl(bool PerformFriendInjection = false) {<br>
+ void setObjectOfFriendDecl(bool PreviouslyDeclared) {<br>
unsigned OldNS = IdentifierNamespace;<br>
assert((OldNS & (IDNS_Tag | IDNS_Ordinary |<br>
IDNS_TagFriend | IDNS_OrdinaryFriend)) &&<br>
@@ -835,20 +833,15 @@ public:<br>
IDNS_TagFriend | IDNS_OrdinaryFriend)) &&<br>
"namespace includes other than ordinary or tag");<br>
<br>
- Decl *Prev = getPreviousDecl();<br>
IdentifierNamespace = 0;<br>
if (OldNS & (IDNS_Tag | IDNS_TagFriend)) {<br>
IdentifierNamespace |= IDNS_TagFriend;<br>
- if (PerformFriendInjection ||<br>
- (Prev && Prev->getIdentifierNamespace() & IDNS_Tag))<br>
- IdentifierNamespace |= IDNS_Tag | IDNS_Type;<br>
+ if (PreviouslyDeclared) IdentifierNamespace |= IDNS_Tag | IDNS_Type;<br>
}<br>
<br>
if (OldNS & (IDNS_Ordinary | IDNS_OrdinaryFriend)) {<br>
IdentifierNamespace |= IDNS_OrdinaryFriend;<br>
- if (PerformFriendInjection ||<br>
- (Prev && Prev->getIdentifierNamespace() & IDNS_Ordinary))<br>
- IdentifierNamespace |= IDNS_Ordinary;<br>
+ if (PreviouslyDeclared) IdentifierNamespace |= IDNS_Ordinary;<br>
}<br>
}<br>
<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=186331&r1=186330&r2=186331&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=186331&r1=186330&r2=186331&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Jul 15 12:27:42 2013<br>
@@ -6328,11 +6328,12 @@ Sema::ActOnFunctionDeclarator(Scope *S,<br>
}<br>
<br>
if (isFriend) {<br>
+ // For now, claim that the objects have no previous declaration.<br>
if (FunctionTemplate) {<br>
- FunctionTemplate->setObjectOfFriendDecl();<br>
+ FunctionTemplate->setObjectOfFriendDecl(false);<br>
FunctionTemplate->setAccess(AS_public);<br>
}<br>
- NewFD->setObjectOfFriendDecl();<br>
+ NewFD->setObjectOfFriendDecl(false);<br>
NewFD->setAccess(AS_public);<br>
}<br>
<br>
@@ -6651,6 +6652,8 @@ Sema::ActOnFunctionDeclarator(Scope *S,<br>
<br>
NewFD->setAccess(Access);<br>
if (FunctionTemplate) FunctionTemplate->setAccess(Access);<br>
+<br>
+ PrincipalDecl->setObjectOfFriendDecl(true);<br>
}<br>
<br>
if (NewFD->isOverloadedOperator() && !DC->isRecord() &&<br>
@@ -10381,8 +10384,9 @@ CreateNewDecl:<br>
// declaration so we always pass true to setObjectOfFriendDecl to make<br>
// the tag name visible.<br>
if (TUK == TUK_Friend)<br>
- New->setObjectOfFriendDecl(!FriendSawTagOutsideEnclosingNamespace &&<br>
- getLangOpts().MicrosoftExt);<br>
+ New->setObjectOfFriendDecl(/* PreviouslyDeclared = */ !Previous.empty() ||<br>
+ (!FriendSawTagOutsideEnclosingNamespace &&<br>
+ getLangOpts().MicrosoftExt));<br>
<br>
// Set the access specifier.<br>
if (!Invalid && SearchDC->isRecord())<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=186331&r1=186330&r2=186331&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=186331&r1=186330&r2=186331&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Mon Jul 15 12:27:42 2013<br>
@@ -2757,15 +2757,8 @@ void Sema::ArgumentDependentLookup(Decla<br>
// If the only declaration here is an ordinary friend, consider<br>
// it only if it was declared in an associated classes.<br>
if (D->getIdentifierNamespace() == Decl::IDNS_OrdinaryFriend) {<br>
- bool DeclaredInAssociatedClass = false;<br>
- for (Decl *DI = D; DI; DI = DI->getPreviousDecl()) {<br>
- DeclContext *LexDC = DI->getLexicalDeclContext();<br>
- if (AssociatedClasses.count(cast<CXXRecordDecl>(LexDC))) {<br>
- DeclaredInAssociatedClass = true;<br>
- break;<br>
- }<br>
- }<br>
- if (!DeclaredInAssociatedClass)<br>
+ DeclContext *LexDC = D->getLexicalDeclContext();<br>
+ if (!AssociatedClasses.count(cast<CXXRecordDecl>(LexDC)))<br>
continue;<br>
}<br>
<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=186331&r1=186330&r2=186331&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=186331&r1=186330&r2=186331&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Mon Jul 15 12:27:42 2013<br>
@@ -1120,7 +1120,8 @@ Sema::CheckClassTemplate(Scope *S, unsig<br>
NewClass->setAccess(PrevClassTemplate->getAccess());<br>
}<br>
<br>
- NewTemplate->setObjectOfFriendDecl();<br>
+ NewTemplate->setObjectOfFriendDecl(/* PreviouslyDeclared = */<br>
+ PrevClassTemplate != NULL);<br>
<br>
// Friend templates are visible in fairly strange ways.<br>
if (!CurContext->isDependentContext()) {<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=186331&r1=186330&r2=186331&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=186331&r1=186330&r2=186331&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Mon Jul 15 12:27:42 2013<br>
@@ -960,7 +960,7 @@ Decl *TemplateDeclInstantiator::VisitCla<br>
else<br>
Inst->setAccess(D->getAccess());<br>
<br>
- Inst->setObjectOfFriendDecl();<br>
+ Inst->setObjectOfFriendDecl(PrevClassTemplate != 0);<br>
// TODO: do we want to track the instantiation progeny of this<br>
// friend target decl?<br>
} else {<br>
@@ -1110,8 +1110,8 @@ Decl *TemplateDeclInstantiator::VisitCXX<br>
<br>
// If the original function was part of a friend declaration,<br>
// inherit its namespace state.<br>
- if (D->getFriendObjectKind())<br>
- Record->setObjectOfFriendDecl();<br>
+ if (Decl::FriendObjectKind FOK = D->getFriendObjectKind())<br>
+ Record->setObjectOfFriendDecl(FOK == Decl::FOK_Declared);<br>
<br>
// Make sure that anonymous structs and unions are recorded.<br>
if (D->isAnonymousStructOrUnion()) {<br>
@@ -1315,7 +1315,7 @@ Decl *TemplateDeclInstantiator::VisitFun<br>
assert(isFriend && "non-friend has dependent specialization info?");<br>
<br>
// This needs to be set now for future sanity.<br>
- Function->setObjectOfFriendDecl();<br>
+ Function->setObjectOfFriendDecl(/*HasPrevious*/ true);<br>
<br>
// Instantiate the explicit template arguments.<br>
TemplateArgumentListInfo ExplicitArgs(Info->getLAngleLoc(),<br>
@@ -1365,7 +1365,13 @@ Decl *TemplateDeclInstantiator::VisitFun<br>
// If the original function was part of a friend declaration,<br>
// inherit its namespace state and add it to the owner.<br>
if (isFriend) {<br>
- PrincipalDecl->setObjectOfFriendDecl();<br>
+ NamedDecl *PrevDecl;<br>
+ if (TemplateParams)<br>
+ PrevDecl = FunctionTemplate->getPreviousDecl();<br>
+ else<br>
+ PrevDecl = Function->getPreviousDecl();<br>
+<br>
+ PrincipalDecl->setObjectOfFriendDecl(PrevDecl != 0);<br>
DC->makeDeclVisibleInContext(PrincipalDecl);<br>
<br>
bool queuedInstantiation = false;<br>
@@ -1633,7 +1639,7 @@ TemplateDeclInstantiator::VisitCXXMethod<br>
TemplateParams, Method);<br>
if (isFriend) {<br>
FunctionTemplate->setLexicalDeclContext(Owner);<br>
- FunctionTemplate->setObjectOfFriendDecl();<br>
+ FunctionTemplate->setObjectOfFriendDecl(true);<br>
} else if (D->isOutOfLine())<br>
FunctionTemplate->setLexicalDeclContext(D->getLexicalDeclContext());<br>
Method->setDescribedFunctionTemplate(FunctionTemplate);<br>
@@ -1660,7 +1666,7 @@ TemplateDeclInstantiator::VisitCXXMethod<br>
TempParamLists.data());<br>
<br>
Method->setLexicalDeclContext(Owner);<br>
- Method->setObjectOfFriendDecl();<br>
+ Method->setObjectOfFriendDecl(true);<br>
} else if (D->isOutOfLine())<br>
Method->setLexicalDeclContext(D->getLexicalDeclContext());<br>
<br>
<br>
Modified: cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp?rev=186331&r1=186330&r2=186331&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp?rev=186331&r1=186330&r2=186331&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp (original)<br>
+++ cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp Mon Jul 15 12:27:42 2013<br>
@@ -26,20 +26,3 @@ void g() {<br>
X2<float> xf;<br>
f(xf);<br>
}<br>
-<br>
-template<typename T><br>
-struct X3 {<br>
- operator int();<br>
-<br>
- friend void h(int x);<br>
-};<br>
-<br>
-int array2[sizeof(X3<int>)];<br>
-int array3[sizeof(X3<float>)];<br>
-<br>
-void i() {<br>
- X3<int> xi;<br>
- h(xi);<br>
- X3<float> xf;<br>
- h(xf);<br>
-}<br>
<br>
Modified: cfe/trunk/test/SemaCXX/friend.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/friend.cpp?rev=186331&r1=186330&r2=186331&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/friend.cpp?rev=186331&r1=186330&r2=186331&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/friend.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/friend.cpp Mon Jul 15 12:27:42 2013<br>
@@ -166,24 +166,13 @@ namespace test9 {<br>
<br>
namespace test10 {<br>
struct A {<br>
- friend void f10();<br>
+ friend void f();<br>
};<br>
+ extern void f();<br>
struct B {<br>
- friend void f10();<br>
+ friend void f();<br>
};<br>
void g() {<br>
- f10(); // expected-error {{undeclared identifier}}<br>
+ ::test10::f();<br>
}<br>
}<br>
-<br>
-namespace PR16597 {<br>
- struct A {<br>
- friend void f_16597();<br>
- };<br>
- struct B {<br>
- friend void f_16597();<br>
- };<br>
- struct C {<br>
- };<br>
- void g(C a) { f_16597(a); } // expected-error {{undeclared identifier}}<br>
-}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br>