r186040 - If we friend a declaration twice, that should not make it visible to name
Richard Smith
richard at metafoo.co.uk
Thu Jul 11 14:29:03 PDT 2013
On Thu, Jul 11, 2013 at 6:31 AM, Alexander Potapenko <glider at google.com> wrote:
> This change has seriously affected the compilation time of some Chrome parts.
> I couldn't build a single file within several minutes.
> http://llvm.org/bugs/show_bug.cgi?id=16597
The testcase is missing from that bug. Does the attached patch help at all?
> On Thu, Jul 11, 2013 at 2:04 AM, Richard Smith
> <richard-llvm at metafoo.co.uk> wrote:
>> Author: rsmith
>> Date: Wed Jul 10 17:04:13 2013
>> New Revision: 186040
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=186040&view=rev
>> 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=186040&r1=186039&r2=186040&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/AST/Decl.h (original)
>> +++ cfe/trunk/include/clang/AST/Decl.h Wed Jul 10 17:04:13 2013
>> @@ -3402,6 +3402,12 @@ void Redeclarable<decl_type>::setPreviou
>> First->RedeclLink = LatestDeclLink(static_cast<decl_type*>(this));
>> assert(!isa<NamedDecl>(static_cast<decl_type*>(this)) ||
>> cast<NamedDecl>(static_cast<decl_type*>(this))->isLinkageValid());
>> +
>> + // 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 |=
>> + First->getIdentifierNamespace() &
>> + (Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type);
>> }
>>
>> // Inline function definitions.
>>
>> Modified: cfe/trunk/include/clang/AST/DeclBase.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=186040&r1=186039&r2=186040&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclBase.h Wed Jul 10 17:04:13 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=186040&r1=186039&r2=186040&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jul 10 17:04:13 2013
>> @@ -6328,12 +6328,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);
>> }
>>
>> @@ -6652,8 +6651,6 @@ Sema::ActOnFunctionDeclarator(Scope *S,
>>
>> NewFD->setAccess(Access);
>> if (FunctionTemplate) FunctionTemplate->setAccess(Access);
>> -
>> - PrincipalDecl->setObjectOfFriendDecl(true);
>> }
>>
>> if (NewFD->isOverloadedOperator() && !DC->isRecord() &&
>> @@ -10384,9 +10381,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=186040&r1=186039&r2=186040&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Wed Jul 10 17:04:13 2013
>> @@ -2757,8 +2757,15 @@ 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 = D->getPreviousDecl()) {
>> + DeclContext *LexDC = DI->getLexicalDeclContext();
>> + if (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=186040&r1=186039&r2=186040&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Jul 10 17:04:13 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=186040&r1=186039&r2=186040&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Jul 10 17:04:13 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=186040&r1=186039&r2=186040&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 10 17:04:13 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=186040&r1=186039&r2=186040&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/friend.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/friend.cpp Wed Jul 10 17:04:13 2013
>> @@ -163,3 +163,15 @@ namespace test9 {
>> 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}}
>> };
>> }
>> +
>> +namespace test10 {
>> + struct A {
>> + friend void f10();
>> + };
>> + struct B {
>> + friend void f10();
>> + };
>> + void g() {
>> + f10(); // expected-error {{undeclared identifier}}
>> + }
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
> --
> Alexander Potapenko
> Software Engineer
> Google Moscow
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr16597.diff
Type: application/octet-stream
Size: 990 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130711/02f58beb/attachment.obj>
More information about the cfe-commits
mailing list