If two sibling modules declare the same entity, and we indirectly pull a

declaration of that entity in from one of those modules, keep track of the fact
that we've not completed the redeclaration chain yet so that we can pull the
remaining declarations in from the other module if they're needed.

llvm-svn: 209161

clang/include/clang/AST/DeclCXX.h

@@ -564,11 +564,13 @@ class CXXRecordDecl : public RecordDecl {
   }
 
   struct LambdaDefinitionData &getLambdaData() const {
-    auto &DD = data();
-    assert(DD.IsLambda && "queried lambda property of non-lambda class");
-    return static_cast<LambdaDefinitionData&>(DD);
+    // No update required: a merged definition cannot change any lambda
+    // properties.
+    auto *DD = DefinitionData.getNotUpdated();
+    assert(DD && DD->IsLambda && "queried lambda property of non-lambda class");
+    return static_cast<LambdaDefinitionData&>(*DD);
   }
-  
+
   /// \brief The template or declaration that this declaration
   /// describes or was instantiated from, respectively.
   ///
@@ -977,7 +979,11 @@ public:
   }
 
   /// \brief Determine whether this class describes a lambda function object.
-  bool isLambda() const { return hasDefinition() && data().IsLambda; }
+  bool isLambda() const {
+    // An update record can't turn a non-lambda into a lambda.
+    auto *DD = DefinitionData.getNotUpdated();
+    return DD && DD->IsLambda;
+  }
 
   /// \brief Determine whether this class describes a generic 
   /// lambda function object (i.e. function call operator is

clang/include/clang/AST/ExternalASTSource.h

@@ -381,8 +381,7 @@ struct LazyGenerationalUpdatePtr {
   /// which we queried it.
   struct LazyData {
     LazyData(ExternalASTSource *Source, T Value)
-        : ExternalSource(Source), LastGeneration(Source->getGeneration()),
-          LastValue(Value) {}
+        : ExternalSource(Source), LastGeneration(0), LastValue(Value) {}
     ExternalASTSource *ExternalSource;
     uint32_t LastGeneration;
     T LastValue;
@@ -407,11 +406,15 @@ public:
   LazyGenerationalUpdatePtr(NotUpdatedTag, T Value = T())
       : Value(Value) {}
 
+  /// Forcibly set this pointer (which must be lazy) as needing updates.
+  void markIncomplete() {
+    Value.template get<LazyData *>()->LastGeneration = 0;
+  }
+
   /// Set the value of this pointer, in the current generation.
   void set(T NewValue) {
     if (LazyData *LazyVal = Value.template dyn_cast<LazyData*>()) {
       LazyVal->LastValue = NewValue;
-      LazyVal->LastGeneration = LazyVal->ExternalSource->getGeneration();
       return;
     }
     Value = NewValue;

clang/include/clang/AST/Redeclarable.h

@@ -90,6 +90,8 @@ protected:
         Next = Latest;
       }
     }
+
+    void markIncomplete() { Next.get<KnownLatest>().markIncomplete(); }
   };
 
   static DeclLink PreviousDeclLink(decl_type *D) {

clang/include/clang/Serialization/ASTReader.h

@@ -938,6 +938,10 @@ private:
   /// \brief Keeps track of the elements added to PendingDeclChains.
   llvm::SmallSet<serialization::DeclID, 16> PendingDeclChainsKnown;
 
+  /// \brief The list of canonical declarations whose redeclaration chains
+  /// need to be marked as incomplete once we're done deserializing things.
+  SmallVector<Decl *, 16> PendingIncompleteDeclChains;
+
   /// \brief The Decl IDs for the Sema/Lexical DeclContext of a Decl that has
   /// been loaded but its DeclContext was not set yet.
   struct PendingDeclContextInfo {
@@ -1141,6 +1145,7 @@ private:
   RecordLocation TypeCursorForIndex(unsigned Index);
   void LoadedDecl(unsigned Index, Decl *D);
   Decl *ReadDeclRecord(serialization::DeclID ID);
+  void markIncompleteDeclChain(Decl *Canon);
   RecordLocation DeclCursorForID(serialization::DeclID ID,
                                  unsigned &RawLocation);
   void loadDeclUpdateRecords(serialization::DeclID ID, Decl *D);

clang/lib/Serialization/ASTReader.cpp

@@ -5940,6 +5940,15 @@ Decl *ASTReader::GetExternalDecl(uint32_t ID) {
 }
 
 void ASTReader::CompleteRedeclChain(const Decl *D) {
+  if (NumCurrentElementsDeserializing) {
+    // We arrange to not care about the complete redeclaration chain while we're
+    // deserializing. Just remember that the AST has marked this one as complete
+    // but that it's not actually complete yet, so we know we still need to
+    // complete it later.
+    PendingIncompleteDeclChains.push_back(const_cast<Decl*>(D));
+    return;
+  }
+
   const DeclContext *DC = D->getDeclContext()->getRedeclContext();
 
   // Recursively ensure that the decl context itself is complete
@@ -7983,7 +7992,8 @@ std::string ASTReader::getOwningModuleNameForDiagnostic(const Decl *D) {
 }
 
 void ASTReader::finishPendingActions() {
-  while (!PendingIdentifierInfos.empty() || !PendingDeclChains.empty() ||
+  while (!PendingIdentifierInfos.empty() ||
+         !PendingIncompleteDeclChains.empty() || !PendingDeclChains.empty() ||
          !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
          !PendingUpdateRecords.empty() || !PendingOdrMergeChecks.empty()) {
     // If any identifiers with corresponding top-level declarations have
@@ -8001,6 +8011,13 @@ void ASTReader::finishPendingActions() {
       SetGloballyVisibleDecls(II, DeclIDs, &TopLevelDecls[II]);
     }
 
+    // For each decl chain that we wanted to complete while deserializing, mark
+    // it as "still needs to be completed".
+    for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
+      markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
+    }
+    PendingIncompleteDeclChains.clear();
+
     // Load pending declaration chains.
     for (unsigned I = 0; I != PendingDeclChains.size(); ++I) {
       loadPendingDeclChain(PendingDeclChains[I]);

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -208,6 +208,10 @@ namespace clang {
     static void attachLatestDeclImpl(...);
     static void attachLatestDecl(Decl *D, Decl *latest);
 
+    template <typename DeclT>
+    static void markIncompleteDeclChainImpl(Redeclarable<DeclT> *D);
+    static void markIncompleteDeclChainImpl(...);
+
     /// \brief Determine whether this declaration has a pending body.
     bool hasPendingBody() const { return HasPendingBody; }
 
@@ -2510,6 +2514,25 @@ void ASTDeclReader::attachLatestDecl(Decl *D, Decl *Latest) {
   }
 }
 
+template<typename DeclT>
+void ASTDeclReader::markIncompleteDeclChainImpl(Redeclarable<DeclT> *D) {
+  D->RedeclLink.markIncomplete();
+}
+void ASTDeclReader::markIncompleteDeclChainImpl(...) {
+  llvm_unreachable("markIncompleteDeclChain on non-redeclarable declaration");
+}
+
+void ASTReader::markIncompleteDeclChain(Decl *D) {
+  switch (D->getKind()) {
+#define ABSTRACT_DECL(TYPE)
+#define DECL(TYPE, BASE)                                             \
+  case Decl::TYPE:                                                   \
+    ASTDeclReader::markIncompleteDeclChainImpl(cast<TYPE##Decl>(D)); \
+    break;
+#include "clang/AST/DeclNodes.inc"
+  }
+}
+
 ASTReader::MergedDeclsMap::iterator
 ASTReader::combineStoredMergedDecls(Decl *Canon, GlobalDeclID CanonID) {
   // If we don't have any stored merged declarations, just look in the

clang/test/Modules/Inputs/cxx-templates-common.h

@@ -23,3 +23,10 @@ namespace Std {
 }
 
 template<typename T> struct TemplateInstantiationVisibility { typedef int type; };
+
+template<typename T> struct Outer {
+  template<typename U> struct Inner {
+    void f();
+    void g();
+  };
+};

clang/test/Modules/Inputs/module.map

@@ -70,6 +70,7 @@ module redeclarations_right { header "redeclarations_right.h" }
 module redecl_namespaces_left { header "redecl_namespaces_left.h" }
 module redecl_namespaces_right { header "redecl_namespaces_right.h" }
 module redecl_add_after_load_top { header "redecl-add-after-load-top.h" }
+module redecl_add_after_load_decls { header "redecl-add-after-load-decls.h" }
 module redecl_add_after_load { header "redecl-add-after-load.h" }
 module load_failure { header "load_failure.h" }
 

clang/test/Modules/Inputs/redecl-add-after-load-decls.h

@@ -0,0 +1,24 @@
+typedef struct A B;
+extern const int variable;
+extern constexpr int function();
+constexpr int test(bool b) { return b ? variable : function(); }
+
+namespace N {
+  typedef struct A B;
+  extern const int variable;
+  extern constexpr int function();
+}
+typedef N::B NB;
+constexpr int N_test(bool b) { return b ? N::variable : N::function(); }
+
+@import redecl_add_after_load_top;
+typedef C::A CB;
+constexpr int C_test(bool b) { return b ? C::variable : C::function(); }
+
+struct D {
+  struct A; // expected-note {{forward}}
+  static const int variable;
+  static constexpr int function(); // expected-note {{here}}
+};
+typedef D::A DB;
+constexpr int D_test(bool b) { return b ? D::variable : D::function(); } // expected-note {{subexpression}} expected-note {{undefined}}

clang/test/Modules/redecl-add-after-load.cpp

@@ -1,6 +1,11 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11
+// RUN: %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 -DIMPORT_DECLS
 
+#ifdef IMPORT_DECLS
+// expected-no-diagnostics
+@import redecl_add_after_load_decls;
+#else
 typedef struct A B;
 extern const int variable;
 extern constexpr int function();
@@ -25,6 +30,7 @@ struct D {
 };
 typedef D::A DB;
 constexpr int D_test(bool b) { return b ? D::variable : D::function(); } // expected-note {{subexpression}} expected-note {{undefined}}
+#endif
 
 @import redecl_add_after_load;
 
@@ -43,6 +49,11 @@ constexpr int struct_function_test = C_test(false);
 // FIXME: We should accept this, but we're currently too lazy when merging class
 // definitions to determine that the definitions in redecl_add_after_load are
 // definitions of these entities.
-DB merged_struct_struct_test; // expected-error {{incomplete}}
-constexpr int merged_struct_variable_test = D_test(true); // expected-error {{constant}} expected-note {{in call to}}
-constexpr int merged_struct_function_test = D_test(false); // expected-error {{constant}} expected-note {{in call to}}
+DB merged_struct_struct_test;
+constexpr int merged_struct_variable_test = D_test(true);
+constexpr int merged_struct_function_test = D_test(false);
+#ifndef IMPORT_DECLS
+// expected-error@-4 {{incomplete}}
+// expected-error@-4 {{constant}} expected-note@-4 {{in call to}}
+// expected-error@-4 {{constant}} expected-note@-4 {{in call to}}
+#endif