<div dir="ltr">I still think this is completely busted:<div><br></div><div><div>% cat x.cc</div><div>namespace N {</div><div> struct X {</div><div> friend void f();</div><div> };</div><div> extern void f();</div><div>
struct Y {</div><div> friend void f();</div><div> };</div><div>}</div><div><br></div><div>int main() {</div><div> N::f();</div><div>}</div></div><div><br></div><div><div>% ./bin/clang -fsyntax-only x.cc</div><div>x.cc:12:6: error: no member named 'f' in namespace 'N'</div>
<div> N::f();</div><div> ~~~^</div><div>1 error generated.</div></div><div><br></div><div>Huh?</div><div><br></div><div>Commenting out the friend on line 7 fixes this. Old clang and GCC don't behave this way, and I can't come up with a reason why the new behavior would be correct, but maybe I'm missing something?</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jul 12, 2013 at 1:38 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard-llvm@metafoo.co.uk" target="_blank">richard-llvm@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rsmith<br>
Date: Fri Jul 12 15:38:49 2013<br>
New Revision: 186199<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=186199&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=186199&view=rev</a><br>
Log:<br>
Unrevert r186040, reverted in r186185, with fix for PR16597.<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=186199&r1=186198&r2=186199&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=186199&r1=186198&r2=186199&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/Decl.h (original)<br>
+++ cfe/trunk/include/clang/AST/Decl.h Fri Jul 12 15:38:49 2013<br>
@@ -3402,6 +3402,12 @@ 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=186199&r1=186198&r2=186199&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=186199&r1=186198&r2=186199&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/DeclBase.h (original)<br>
+++ cfe/trunk/include/clang/AST/DeclBase.h Fri Jul 12 15:38:49 2013<br>
@@ -295,6 +295,8 @@ 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>
@@ -824,7 +826,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 PreviouslyDeclared) {<br>
+ void setObjectOfFriendDecl(bool PerformFriendInjection = false) {<br>
unsigned OldNS = IdentifierNamespace;<br>
assert((OldNS & (IDNS_Tag | IDNS_Ordinary |<br>
IDNS_TagFriend | IDNS_OrdinaryFriend)) &&<br>
@@ -833,15 +835,20 @@ 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 (PreviouslyDeclared) IdentifierNamespace |= IDNS_Tag | IDNS_Type;<br>
+ if (PerformFriendInjection ||<br>
+ (Prev && Prev->getIdentifierNamespace() & IDNS_Tag))<br>
+ IdentifierNamespace |= IDNS_Tag | IDNS_Type;<br>
}<br>
<br>
if (OldNS & (IDNS_Ordinary | IDNS_OrdinaryFriend)) {<br>
IdentifierNamespace |= IDNS_OrdinaryFriend;<br>
- if (PreviouslyDeclared) IdentifierNamespace |= IDNS_Ordinary;<br>
+ if (PerformFriendInjection ||<br>
+ (Prev && Prev->getIdentifierNamespace() & IDNS_Ordinary))<br>
+ 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=186199&r1=186198&r2=186199&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=186199&r1=186198&r2=186199&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Jul 12 15:38:49 2013<br>
@@ -6328,12 +6328,11 @@ 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(false);<br>
+ FunctionTemplate->setObjectOfFriendDecl();<br>
FunctionTemplate->setAccess(AS_public);<br>
}<br>
- NewFD->setObjectOfFriendDecl(false);<br>
+ NewFD->setObjectOfFriendDecl();<br>
NewFD->setAccess(AS_public);<br>
}<br>
<br>
@@ -6652,8 +6651,6 @@ 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>
@@ -10384,9 +10381,8 @@ 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(/* PreviouslyDeclared = */ !Previous.empty() ||<br>
- (!FriendSawTagOutsideEnclosingNamespace &&<br>
- getLangOpts().MicrosoftExt));<br>
+ New->setObjectOfFriendDecl(!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=186199&r1=186198&r2=186199&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=186199&r1=186198&r2=186199&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Fri Jul 12 15:38:49 2013<br>
@@ -2757,8 +2757,15 @@ 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>
- DeclContext *LexDC = D->getLexicalDeclContext();<br>
- if (!AssociatedClasses.count(cast<CXXRecordDecl>(LexDC)))<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>
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=186199&r1=186198&r2=186199&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=186199&r1=186198&r2=186199&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Fri Jul 12 15:38:49 2013<br>
@@ -1120,8 +1120,7 @@ Sema::CheckClassTemplate(Scope *S, unsig<br>
NewClass->setAccess(PrevClassTemplate->getAccess());<br>
}<br>
<br>
- NewTemplate->setObjectOfFriendDecl(/* PreviouslyDeclared = */<br>
- PrevClassTemplate != NULL);<br>
+ NewTemplate->setObjectOfFriendDecl();<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=186199&r1=186198&r2=186199&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=186199&r1=186198&r2=186199&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Fri Jul 12 15:38:49 2013<br>
@@ -960,7 +960,7 @@ Decl *TemplateDeclInstantiator::VisitCla<br>
else<br>
Inst->setAccess(D->getAccess());<br>
<br>
- Inst->setObjectOfFriendDecl(PrevClassTemplate != 0);<br>
+ Inst->setObjectOfFriendDecl();<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 (Decl::FriendObjectKind FOK = D->getFriendObjectKind())<br>
- Record->setObjectOfFriendDecl(FOK == Decl::FOK_Declared);<br>
+ if (D->getFriendObjectKind())<br>
+ Record->setObjectOfFriendDecl();<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(/*HasPrevious*/ true);<br>
+ Function->setObjectOfFriendDecl();<br>
<br>
// Instantiate the explicit template arguments.<br>
TemplateArgumentListInfo ExplicitArgs(Info->getLAngleLoc(),<br>
@@ -1365,13 +1365,7 @@ 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>
- NamedDecl *PrevDecl;<br>
- if (TemplateParams)<br>
- PrevDecl = FunctionTemplate->getPreviousDecl();<br>
- else<br>
- PrevDecl = Function->getPreviousDecl();<br>
-<br>
- PrincipalDecl->setObjectOfFriendDecl(PrevDecl != 0);<br>
+ PrincipalDecl->setObjectOfFriendDecl();<br>
DC->makeDeclVisibleInContext(PrincipalDecl);<br>
<br>
bool queuedInstantiation = false;<br>
@@ -1639,7 +1633,7 @@ TemplateDeclInstantiator::VisitCXXMethod<br>
TemplateParams, Method);<br>
if (isFriend) {<br>
FunctionTemplate->setLexicalDeclContext(Owner);<br>
- FunctionTemplate->setObjectOfFriendDecl(true);<br>
+ FunctionTemplate->setObjectOfFriendDecl();<br>
} else if (D->isOutOfLine())<br>
FunctionTemplate->setLexicalDeclContext(D->getLexicalDeclContext());<br>
Method->setDescribedFunctionTemplate(FunctionTemplate);<br>
@@ -1666,7 +1660,7 @@ TemplateDeclInstantiator::VisitCXXMethod<br>
TempParamLists.data());<br>
<br>
Method->setLexicalDeclContext(Owner);<br>
- Method->setObjectOfFriendDecl(true);<br>
+ Method->setObjectOfFriendDecl();<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=186199&r1=186198&r2=186199&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p4.cpp?rev=186199&r1=186198&r2=186199&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 Fri Jul 12 15:38:49 2013<br>
@@ -26,3 +26,20 @@ 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=186199&r1=186198&r2=186199&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/friend.cpp?rev=186199&r1=186198&r2=186199&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/friend.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/friend.cpp Fri Jul 12 15:38:49 2013<br>
@@ -163,3 +163,27 @@ namespace test9 {<br>
friend void C::f(int, int, int) {} // expected-error {{no function named 'f' with type 'void (int, int, int)' was found in the specified scope}}<br>
};<br>
}<br>
+<br>
+namespace test10 {<br>
+ struct A {<br>
+ friend void f10();<br>
+ };<br>
+ struct B {<br>
+ friend void f10();<br>
+ };<br>
+ void g() {<br>
+ f10(); // expected-error {{undeclared identifier}}<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></div>