From 7ecc31b0324c3bb99a41a12a7f19aada14c45012 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Fri, 2 Aug 2013 01:09:12 +0000 Subject: [PATCH] When merging redeclaration chains across modules, if a declaration is visible in one module but is only declared as a friend in another module, keep it visible in the result of the merge. This is incomplete on two axes: 1) Our handling of local extern declarations is basically broken (we put them in the wrong decl context, and don't find them in redeclaration lookup, unless they've previously been declared), and this results in them making friends visible after a merge. 2) Eventually we'll need to mark that this has happened, and more carefully check whether a declaration should be visible if it was only visible in some of the modules in which it was declared. Fortunately it's rare for the identifier namespace of a declaration to change along its redeclaration chain. llvm-svn: 187639 --- clang/include/clang/AST/Decl.h | 2 ++ clang/include/clang/Serialization/ASTReader.h | 3 --- clang/lib/Serialization/ASTReaderDecl.cpp | 18 ++++++++++++------ clang/test/Modules/Inputs/cxx-templates-a.h | 2 ++ clang/test/Modules/Inputs/cxx-templates-b.h | 6 ++++++ clang/test/Modules/cxx-templates.cpp | 4 ++++ 6 files changed, 26 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 6be323a..b1fe7b4 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -3424,6 +3424,8 @@ void Redeclarable::setPreviousDeclaration(decl_type *PrevDecl) { // If the declaration was previously visible, a redeclaration of it remains // visible even if it wouldn't be visible by itself. + // FIXME: Once we handle local extern decls properly, this should inherit + // the visibility from MostRecent, not from PrevDecl. static_cast(this)->IdentifierNamespace |= PrevDecl->getIdentifierNamespace() & (Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type); diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index fff5cbc..b5dfcd0 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -907,9 +907,6 @@ private: /// the given canonical declaration. MergedDeclsMap::iterator combineStoredMergedDecls(Decl *Canon, serialization::GlobalDeclID CanonID); - - /// \brief Ready to load the previous declaration of the given Decl. - void loadAndAttachPreviousDecl(Decl *D, serialization::DeclID ID); /// \brief When reading a Stmt tree, Stmt operands are placed in this stack. SmallVector StmtStack; diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 99d6eaa..122865b 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -2003,6 +2003,17 @@ void ASTDeclReader::attachPreviousDecl(Decl *D, Decl *previous) { RedeclarableTemplateDecl *TD = cast(D); TD->RedeclLink.setNext(cast(previous)); } + + // If the declaration was visible in one module, a redeclaration of it in + // another module remains visible even if it wouldn't be visible by itself. + // + // FIXME: In this case, the declaration should only be visible if a module + // that makes it visible has been imported. + // FIXME: This is not correct in the case where previous is a local extern + // declaration and D is a friend declaraton. + D->IdentifierNamespace |= + previous->IdentifierNamespace & + (Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type); } void ASTDeclReader::attachLatestDecl(Decl *D, Decl *Latest) { @@ -2063,11 +2074,6 @@ ASTReader::combineStoredMergedDecls(Decl *Canon, GlobalDeclID CanonID) { return Pos; } -void ASTReader::loadAndAttachPreviousDecl(Decl *D, serialization::DeclID ID) { - Decl *previous = GetDecl(ID); - ASTDeclReader::attachPreviousDecl(D, previous); -} - /// \brief Read the declaration at the given offset from the AST file. Decl *ASTReader::ReadDeclRecord(DeclID ID) { unsigned Index = ID - NUM_PREDEF_DECL_IDS; @@ -2499,7 +2505,7 @@ void ASTReader::loadPendingDeclChain(serialization::GlobalDeclID ID) { for (unsigned I = 0, N = Chain.size(); I != N; ++I) { if (Chain[I] == CanonDecl) continue; - + ASTDeclReader::attachPreviousDecl(Chain[I], MostRecent); MostRecent = Chain[I]; } diff --git a/clang/test/Modules/Inputs/cxx-templates-a.h b/clang/test/Modules/Inputs/cxx-templates-a.h index aaafc1b..dbf12da 100644 --- a/clang/test/Modules/Inputs/cxx-templates-a.h +++ b/clang/test/Modules/Inputs/cxx-templates-a.h @@ -22,3 +22,5 @@ template void PerformDelayedLookup(T &t) { } template void PerformDelayedLookupInDefaultArgument(T &t, int a = (FoundByADL(T()), 0)) {} + +template struct RedeclaredAsFriend {}; diff --git a/clang/test/Modules/Inputs/cxx-templates-b.h b/clang/test/Modules/Inputs/cxx-templates-b.h index 36d7d78..9bc76d5 100644 --- a/clang/test/Modules/Inputs/cxx-templates-b.h +++ b/clang/test/Modules/Inputs/cxx-templates-b.h @@ -29,6 +29,12 @@ template void UseDefinedInBImpl() { extern DefinedInBImpl &defined_in_b_impl; +template +struct RedeclareTemplateAsFriend { + template + friend struct RedeclaredAsFriend; +}; + @import cxx_templates_a; template void UseDefinedInBImplIndirectly(T &v) { PerformDelayedLookup(v); diff --git a/clang/test/Modules/cxx-templates.cpp b/clang/test/Modules/cxx-templates.cpp index 79052dd..0949436b 100644 --- a/clang/test/Modules/cxx-templates.cpp +++ b/clang/test/Modules/cxx-templates.cpp @@ -71,6 +71,10 @@ void g() { PerformDelayedLookup(defined_in_b_impl); // expected-note {{in instantiation of}} } +RedeclaredAsFriend raf1; +RedeclareTemplateAsFriend rtaf; +RedeclaredAsFriend raf2; + @import cxx_templates_common; typedef SomeTemplate SomeTemplateIntPtr; -- 2.7.4