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

6 files changed, 26 insertions, 9 deletions

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 6be323a981c..b1fe7b4c207 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/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/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index fff5cbca857..b5dfcd0b47b 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<Stmt *, 16> StmtStack;
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 99d6eaa3a03..122865b4686 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<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/clang/test/Modules/Inputs/cxx-templates-a.h b/clang/test/Modules/Inputs/cxx-templates-a.h
index aaafc1b1bdb..dbf12dacbae 100644
--- a/clang/test/Modules/Inputs/cxx-templates-a.h
+++ b/clang/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/clang/test/Modules/Inputs/cxx-templates-b.h b/clang/test/Modules/Inputs/cxx-templates-b.h
index 36d7d78c130..9bc76d5bbae 100644
--- a/clang/test/Modules/Inputs/cxx-templates-b.h
+++ b/clang/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/clang/test/Modules/cxx-templates.cpp b/clang/test/Modules/cxx-templates.cpp
index 79052dd34bb..0949436b5d8 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<int> raf1;
+RedeclareTemplateAsFriend<double> rtaf;
+RedeclaredAsFriend<double> raf2;
+
 @import cxx_templates_common;
 
 typedef SomeTemplate<int*> SomeTemplateIntPtr;