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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@187639 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index 6be323a981c424116e0e513532a013c226406da3..b1fe7b4c207e7cc32f72c7901615f67b3f4bd341 100644 (file)
--- a/include/clang/AST/Decl.h
+++ b/include/clang/AST/Decl.h
@@ -3424,6 +3424,8 @@ void Redeclarable<decl_type>::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<decl_type*>(this)->IdentifierNamespace |=
       PrevDecl->getIdentifierNamespace() &
       (Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type);

diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index fff5cbca8578d425ef42a913fb0f0eb146785121..b5dfcd0b47b42f8786f2e68b6559b103d2aac15c 100644 (file)
--- a/include/clang/Serialization/ASTReader.h
+++ b/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<Stmt *, 16> StmtStack;

diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 99d6eaa3a03fb4daaee3b472701ca0f0d5904c03..122865b4686e450013dee60b120a444834983219 100644 (file)
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -2003,6 +2003,17 @@ void ASTDeclReader::attachPreviousDecl(Decl *D, Decl *previous) {
     RedeclarableTemplateDecl *TD = cast<RedeclarableTemplateDecl>(D);
     TD->RedeclLink.setNext(cast<RedeclarableTemplateDecl>(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/test/Modules/Inputs/cxx-templates-a.h b/test/Modules/Inputs/cxx-templates-a.h
index aaafc1b1bdb46f85c5afeeb959c1633b8336692b..dbf12dacbae2681fc82152b3d110e31cd87133e7 100644 (file)
--- a/test/Modules/Inputs/cxx-templates-a.h
+++ b/test/Modules/Inputs/cxx-templates-a.h
@@ -22,3 +22,5 @@ template<typename T> void PerformDelayedLookup(T &t) {
 }
 
 template<typename T> void PerformDelayedLookupInDefaultArgument(T &t, int a = (FoundByADL(T()), 0)) {}
+
+template<typename T> struct RedeclaredAsFriend {};

diff --git a/test/Modules/Inputs/cxx-templates-b.h b/test/Modules/Inputs/cxx-templates-b.h
index 36d7d78c130f88ef2ae655cec584657fb06837d5..9bc76d5bbaea48e79991c41f09f883464065859d 100644 (file)
--- a/test/Modules/Inputs/cxx-templates-b.h
+++ b/test/Modules/Inputs/cxx-templates-b.h
@@ -29,6 +29,12 @@ template<typename T> void UseDefinedInBImpl() {
 
 extern DefinedInBImpl &defined_in_b_impl;
 
+template<typename T>
+struct RedeclareTemplateAsFriend {
+  template<typename U>
+  friend struct RedeclaredAsFriend;
+};
+
 @import cxx_templates_a;
 template<typename T> void UseDefinedInBImplIndirectly(T &v) {
   PerformDelayedLookup(v);

diff --git a/test/Modules/cxx-templates.cpp b/test/Modules/cxx-templates.cpp
index 79052dd34bb19274db7f7220cfc66ca24ae9d396..0949436b5d80300020f4a6bd0d7c520441c2dc44 100644 (file)
--- a/test/Modules/cxx-templates.cpp
+++ b/test/Modules/cxx-templates.cpp
@@ -71,6 +71,10 @@ void g() {
   PerformDelayedLookup(defined_in_b_impl); // expected-note {{in instantiation of}}
 }
 
+RedeclaredAsFriend<int> raf1;
+RedeclareTemplateAsFriend<double> rtaf;
+RedeclaredAsFriend<double> raf2;
+
 @import cxx_templates_common;
 
 typedef SomeTemplate<int*> SomeTemplateIntPtr;