r186040 - If we friend a declaration twice, that should not make it visible to name
Alexander Potapenko
glider at google.com
Thu Jul 11 06:31:22 PDT 2013
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
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
More information about the cfe-commits
mailing list