[modules] When either redecl chain merging or an update record causes us to

give an exception specification to a declaration that didn't have an exception
specification in any of our imported modules, emit an update record ourselves.
Without this, code importing the current module would not see an exception
specification that we could see and might have relied on.

llvm-svn: 232870

10 files changed, 99 insertions, 64 deletions

diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 1b93702e1be..c6310595190 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -446,6 +446,12 @@ private:
   /// that we needed but hadn't loaded yet.
   llvm::DenseMap<void *, PendingFakeDefinitionKind> PendingFakeDefinitionData;
 
+  /// \brief Exception specification updates that have been loaded but not yet
+  /// propagated across the relevant redeclaration chain. The map key is the
+  /// canonical declaration (used only for deduplication) and the value is a
+  /// declaration that has an exception specification.
+  llvm::SmallMapVector<Decl *, FunctionDecl *, 4> PendingExceptionSpecUpdates;
+
   struct ReplacedDeclInfo {
     ModuleFile *Mod;
     uint64_t Offset;
@@ -950,7 +956,7 @@ private:
 
   /// \brief The set of lookup results that we have faked in order to support
   /// merging of partially deserialized decls but that we have not yet removed.
-  llvm::MapVector<IdentifierInfo *, SmallVector<NamedDecl*, 2> >
+  llvm::SmallMapVector<IdentifierInfo *, SmallVector<NamedDecl*, 2>, 16>
     PendingFakeLookupResults;
 
   /// \brief The generation number of each identifier, which keeps track of
@@ -1179,6 +1185,18 @@ private:
   /// merged into its redecl chain.
   Decl *getMostRecentExistingDecl(Decl *D);
 
+  template <typename Fn>
+  void forEachFormerlyCanonicalImportedDecl(const Decl *D, Fn Visit) {
+    D = D->getCanonicalDecl();
+    if (D->isFromASTFile())
+      Visit(D);
+
+    auto It = MergedDecls.find(const_cast<Decl*>(D));
+    if (It != MergedDecls.end())
+      for (auto ID : It->second)
+        Visit(GetExistingDecl(ID));
+  }
+
   RecordLocation DeclCursorForID(serialization::DeclID ID,
                                  unsigned &RawLocation);
   void loadDeclUpdateRecords(serialization::DeclID ID, Decl *D);
diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index 41abd49627b..51d6acebada 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -167,13 +167,13 @@ Sema::ResolveExceptionSpec(SourceLocation Loc, const FunctionProtoType *FPT) {
 void
 Sema::UpdateExceptionSpec(FunctionDecl *FD,
                           const FunctionProtoType::ExceptionSpecInfo &ESI) {
-  for (auto *Redecl : FD->redecls())
-    Context.adjustExceptionSpec(cast<FunctionDecl>(Redecl), ESI);
-
   // If we've fully resolved the exception specification, notify listeners.
   if (!isUnresolvedExceptionSpec(ESI.Type))
     if (auto *Listener = getASTMutationListener())
       Listener->ResolvedExceptionSpec(FD);
+
+  for (auto *Redecl : FD->redecls())
+    Context.adjustExceptionSpec(cast<FunctionDecl>(Redecl), ESI);
 }
 
 /// Determine whether a function has an implicitly-generated exception
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index bd54779513e..a26f251cbdb 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1,4 +1,4 @@
-//===--- ASTReader.cpp - AST File Reader ----------------------------------===//
+//===-- ASTReader.cpp - AST File Reader ----------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -8617,6 +8617,13 @@ void ASTReader::FinishedDeserializing() {
   --NumCurrentElementsDeserializing;
 
   if (NumCurrentElementsDeserializing == 0) {
+    // Propagate exception specification updates along redeclaration chains.
+    for (auto Update : PendingExceptionSpecUpdates) {
+      auto *FPT = Update.second->getType()->castAs<FunctionProtoType>();
+      SemaObj->UpdateExceptionSpec(Update.second,
+                                   FPT->getExtProtoInfo().ExceptionSpec);
+    }
+
     diagnoseOdrViolations();
 
     // We are not in recursive loading, so it's safe to pass the "interesting"
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 9c3776268d9..5c6820f4834 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -228,9 +228,11 @@ namespace clang {
 
     template <typename DeclT>
     static void attachPreviousDeclImpl(ASTReader &Reader,
-                                       Redeclarable<DeclT> *D, Decl *Previous);
+                                       Redeclarable<DeclT> *D, Decl *Previous,
+                                       Decl *Canon);
     static void attachPreviousDeclImpl(ASTReader &Reader, ...);
-    static void attachPreviousDecl(ASTReader &Reader, Decl *D, Decl *Previous);
+    static void attachPreviousDecl(ASTReader &Reader, Decl *D, Decl *Previous,
+                                   Decl *Canon);
 
     template <typename DeclT>
     static void attachLatestDeclImpl(Redeclarable<DeclT> *D, Decl *Latest);
@@ -2820,14 +2822,14 @@ Decl *ASTReader::getMostRecentExistingDecl(Decl *D) {
 template<typename DeclT>
 void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader,
                                            Redeclarable<DeclT> *D,
-                                           Decl *Previous) {
+                                           Decl *Previous, Decl *Canon) {
   D->RedeclLink.setPrevious(cast<DeclT>(Previous));
 }
 namespace clang {
 template<>
 void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader,
                                            Redeclarable<FunctionDecl> *D,
-                                           Decl *Previous) {
+                                           Decl *Previous, Decl *Canon) {
   FunctionDecl *FD = static_cast<FunctionDecl*>(D);
   FunctionDecl *PrevFD = cast<FunctionDecl>(Previous);
 
@@ -2854,25 +2856,17 @@ void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader,
     FD->IsInline = true;
   }
 
-  // If this declaration has an unresolved exception specification but the
-  // previous declaration had a resolved one, resolve the exception
-  // specification now. If this declaration has a resolved exception
-  // specification but the previous declarations did not, apply our exception
-  // specification to all prior ones now.
+  // If we need to propagate an exception specification along the redecl
+  // chain, make a note of that so that we can do so later.
   auto *FPT = FD->getType()->getAs<FunctionProtoType>();
   auto *PrevFPT = PrevFD->getType()->getAs<FunctionProtoType>();
   if (FPT && PrevFPT) {
-    bool WasUnresolved = isUnresolvedExceptionSpec(FPT->getExceptionSpecType());
-    bool IsUnresolved = isUnresolvedExceptionSpec(PrevFPT->getExceptionSpecType());
-    if (WasUnresolved && !IsUnresolved) {
-      Reader.Context.adjustExceptionSpec(
-          FD, PrevFPT->getExtProtoInfo().ExceptionSpec);
-    } else if (!WasUnresolved && IsUnresolved) {
-      FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
-      for (FunctionDecl *PrevFDToUpdate = PrevFD; PrevFDToUpdate;
-           PrevFDToUpdate = PrevFDToUpdate->getPreviousDecl())
-         Reader.Context.adjustExceptionSpec(PrevFDToUpdate, EPI.ExceptionSpec);
-    }
+    bool IsUnresolved = isUnresolvedExceptionSpec(FPT->getExceptionSpecType());
+    bool WasUnresolved =
+        isUnresolvedExceptionSpec(PrevFPT->getExceptionSpecType());
+    if (IsUnresolved != WasUnresolved)
+      Reader.PendingExceptionSpecUpdates.insert(
+          std::make_pair(Canon, IsUnresolved ? PrevFD : FD));
   }
 }
 }
@@ -2881,14 +2875,14 @@ void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader, ...) {
 }
 
 void ASTDeclReader::attachPreviousDecl(ASTReader &Reader, Decl *D,
-                                       Decl *Previous) {
+                                       Decl *Previous, Decl *Canon) {
   assert(D && Previous);
 
   switch (D->getKind()) {
 #define ABSTRACT_DECL(TYPE)
-#define DECL(TYPE, BASE)                                           \
-  case Decl::TYPE:                                                 \
-    attachPreviousDeclImpl(Reader, cast<TYPE##Decl>(D), Previous); \
+#define DECL(TYPE, BASE)                                                  \
+  case Decl::TYPE:                                                        \
+    attachPreviousDeclImpl(Reader, cast<TYPE##Decl>(D), Previous, Canon); \
     break;
 #include "clang/AST/DeclNodes.inc"
   }
@@ -3392,7 +3386,7 @@ void ASTReader::loadPendingDeclChain(Decl *CanonDecl) {
     if (Chain[I] == CanonDecl)
       continue;
 
-    ASTDeclReader::attachPreviousDecl(*this, Chain[I], MostRecent);
+    ASTDeclReader::attachPreviousDecl(*this, Chain[I], MostRecent, CanonDecl);
     MostRecent = Chain[I];
   }
   ASTDeclReader::attachLatestDecl(CanonDecl, MostRecent);
@@ -3736,23 +3730,24 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile,
     }
 
     case UPD_CXX_RESOLVED_EXCEPTION_SPEC: {
-      // FIXME: This doesn't send the right notifications if there are
-      // ASTMutationListeners other than an ASTWriter.
       FunctionProtoType::ExceptionSpecInfo ESI;
       SmallVector<QualType, 8> ExceptionStorage;
       Reader.readExceptionSpec(ModuleFile, ExceptionStorage, ESI, Record, Idx);
-      for (auto *Redecl : merged_redecls(D)) {
-        auto *FD = cast<FunctionDecl>(Redecl);
-        auto *FPT = FD->getType()->castAs<FunctionProtoType>();
-        if (!isUnresolvedExceptionSpec(FPT->getExceptionSpecType())) {
-          // AST invariant: if any exception spec in the redecl chain is
-          // resolved, all are resolved. We don't need to go any further.
-          // FIXME: If the exception spec is resolved, check that it matches.
-          break;
-        }
+
+      // Update this declaration's exception specification, if needed.
+      auto *FD = cast<FunctionDecl>(D);
+      auto *FPT = FD->getType()->castAs<FunctionProtoType>();
+      // FIXME: If the exception specification is already present, check that it
+      // matches.
+      if (isUnresolvedExceptionSpec(FPT->getExceptionSpecType())) {
         FD->setType(Reader.Context.getFunctionType(
             FPT->getReturnType(), FPT->getParamTypes(),
             FPT->getExtProtoInfo().withExceptionSpec(ESI)));
+
+        // When we get to the end of deserializing, see if there are other decls
+        // that we need to propagate this exception specification onto.
+        Reader.PendingExceptionSpecUpdates.insert(
+            std::make_pair(FD->getCanonicalDecl(), FD));
       }
       break;
     }
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index a8e92b8151c..a1fadb258ac 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5804,37 +5804,36 @@ void ASTWriter::AddedCXXTemplateSpecialization(const FunctionTemplateDecl *TD,
 }
 
 void ASTWriter::ResolvedExceptionSpec(const FunctionDecl *FD) {
-  assert(!WritingAST && "Already writing the AST!");
-  FD = FD->getCanonicalDecl();
-  if (!FD->isFromASTFile())
-    return; // Not a function declared in PCH and defined outside.
-
-  DeclUpdates[FD].push_back(UPD_CXX_RESOLVED_EXCEPTION_SPEC);
+  assert(!DoneWritingDeclsAndTypes && "Already done writing updates!");
+  if (!Chain) return;
+  Chain->forEachFormerlyCanonicalImportedDecl(FD, [&](const Decl *D) {
+    // If we don't already know the exception specification for this redecl
+    // chain, add an update record for it.
+    if (isUnresolvedExceptionSpec(cast<FunctionDecl>(D)
+                                      ->getType()
+                                      ->castAs<FunctionProtoType>()
+                                      ->getExceptionSpecType()))
+      DeclUpdates[D].push_back(UPD_CXX_RESOLVED_EXCEPTION_SPEC);
+  });
 }
 
 void ASTWriter::DeducedReturnType(const FunctionDecl *FD, QualType ReturnType) {
   assert(!WritingAST && "Already writing the AST!");
-  FD = FD->getCanonicalDecl();
-  if (!FD->isFromASTFile())
-    return; // Not a function declared in PCH and defined outside.
-
-  DeclUpdates[FD].push_back(DeclUpdate(UPD_CXX_DEDUCED_RETURN_TYPE, ReturnType));
+  if (!Chain) return;
+  Chain->forEachFormerlyCanonicalImportedDecl(FD, [&](const Decl *D) {
+    DeclUpdates[D].push_back(
+        DeclUpdate(UPD_CXX_DEDUCED_RETURN_TYPE, ReturnType));
+  });
 }
 
 void ASTWriter::ResolvedOperatorDelete(const CXXDestructorDecl *DD,
                                        const FunctionDecl *Delete) {
   assert(!WritingAST && "Already writing the AST!");
   assert(Delete && "Not given an operator delete");
-  for (auto *D : DD->redecls()) {
-    if (D->isFromASTFile()) {
-      // We added an operator delete that some imported destructor didn't
-      // know about. Add an update record to let importers of us and that
-      // declaration know about it.
-      DeclUpdates[DD].push_back(
-          DeclUpdate(UPD_CXX_RESOLVED_DTOR_DELETE, Delete));
-      return;
-    }
-  }
+  if (!Chain) return;
+  Chain->forEachFormerlyCanonicalImportedDecl(DD, [&](const Decl *D) {
+    DeclUpdates[D].push_back(DeclUpdate(UPD_CXX_RESOLVED_DTOR_DELETE, Delete));
+  });
 }
 
 void ASTWriter::CompletedImplicitDefinition(const FunctionDecl *D) {
@@ -5851,8 +5850,7 @@ void ASTWriter::FunctionDefinitionInstantiated(const FunctionDecl *D) {
   if (!D->isFromASTFile())
     return;
 
-  DeclUpdates[D].push_back(
-      DeclUpdate(UPD_CXX_ADDED_FUNCTION_DEFINITION));
+  DeclUpdates[D].push_back(DeclUpdate(UPD_CXX_ADDED_FUNCTION_DEFINITION));
 }
 
 void ASTWriter::StaticDataMemberInstantiated(const VarDecl *D) {
diff --git a/clang/test/Modules/Inputs/update-exception-spec/a.h b/clang/test/Modules/Inputs/update-exception-spec/a.h
new file mode 100644
index 00000000000..078ebf9aecf
--- /dev/null
+++ b/clang/test/Modules/Inputs/update-exception-spec/a.h
@@ -0,0 +1,2 @@
+struct A { ~A() throw(int); };
+struct B { A a; };
diff --git a/clang/test/Modules/Inputs/update-exception-spec/b.h b/clang/test/Modules/Inputs/update-exception-spec/b.h
new file mode 100644
index 00000000000..f75b559bee4
--- /dev/null
+++ b/clang/test/Modules/Inputs/update-exception-spec/b.h
@@ -0,0 +1,3 @@
+struct A { ~A() throw(int); };
+struct B { A a; };
+inline void f(B *p) { p->~B(); }
diff --git a/clang/test/Modules/Inputs/update-exception-spec/c.h b/clang/test/Modules/Inputs/update-exception-spec/c.h
new file mode 100644
index 00000000000..067dbb65055
--- /dev/null
+++ b/clang/test/Modules/Inputs/update-exception-spec/c.h
@@ -0,0 +1,3 @@
+#include "a.h"
+#include "b.h"
+inline void g(B *p) { p->~B(); }
diff --git a/clang/test/Modules/Inputs/update-exception-spec/module.modulemap b/clang/test/Modules/Inputs/update-exception-spec/module.modulemap
new file mode 100644
index 00000000000..880ae38b97a
--- /dev/null
+++ b/clang/test/Modules/Inputs/update-exception-spec/module.modulemap
@@ -0,0 +1,3 @@
+module a { header "a.h" }
+module b { header "b.h" }
+module c { header "c.h" }
diff --git a/clang/test/Modules/update-exception-spec.cpp b/clang/test/Modules/update-exception-spec.cpp
new file mode 100644
index 00000000000..bccdddc9c09
--- /dev/null
+++ b/clang/test/Modules/update-exception-spec.cpp
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fexceptions -fcxx-exceptions -fmodules -fmodules-cache-path=%t -I%S/Inputs/update-exception-spec -emit-llvm-only %s
+#include "a.h"
+void use(B *p);
+#include "c.h"
+void use(B *p) { g(p); }