[modules] Maintain an AST invariant across module load/save: if any declaration

of a function has a resolved exception specification, then all declarations of
the function do.

We should probably improve the AST representation to make this implicit (perhaps
only store the exception specification on the canonical declaration), but this
fixes things for now.

The testcase for this (which used to assert) also exposes the actual bug I was
trying to reduce here: we sometimes fail to emit the body of an imported
special member function definition. Fix for that to follow.

llvm-svn: 214458

6 files changed, 189 insertions, 50 deletions

diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index 192b273c7c3..e4963b13d64 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -135,13 +135,16 @@ Sema::ResolveExceptionSpec(SourceLocation Loc, const FunctionProtoType *FPT) {
 void
 Sema::UpdateExceptionSpec(FunctionDecl *FD,
                           const FunctionProtoType::ExceptionSpecInfo &ESI) {
-  const FunctionProtoType *Proto =
-      FD->getType()->castAs<FunctionProtoType>();
-
-  // Overwrite the exception spec and rebuild the function type.
-  FD->setType(Context.getFunctionType(
-      Proto->getReturnType(), Proto->getParamTypes(),
-      Proto->getExtProtoInfo().withExceptionSpec(ESI)));
+  for (auto *Redecl : FD->redecls()) {
+    auto *RedeclFD = dyn_cast<FunctionDecl>(Redecl);
+    const FunctionProtoType *Proto =
+        RedeclFD->getType()->castAs<FunctionProtoType>();
+
+    // Overwrite the exception spec and rebuild the function type.
+    RedeclFD->setType(Context.getFunctionType(
+        Proto->getReturnType(), Proto->getParamTypes(),
+        Proto->getExtProtoInfo().withExceptionSpec(ESI)));
+  }
 
   // If we've fully resolved the exception specification, notify listeners.
   if (!isUnresolvedExceptionSpec(ESI.Type))
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index f68284b9dde..d1906e0d005 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -199,9 +199,10 @@ namespace clang {
         TypeIDForTypeDecl(0), HasPendingBody(false) { }
 
     template <typename DeclT>
-    static void attachPreviousDeclImpl(Redeclarable<DeclT> *D, Decl *Previous);
-    static void attachPreviousDeclImpl(...);
-    static void attachPreviousDecl(Decl *D, Decl *previous);
+    static void attachPreviousDeclImpl(ASTReader &Reader,
+                                       Redeclarable<DeclT> *D, Decl *Previous);
+    static void attachPreviousDeclImpl(ASTReader &Reader, ...);
+    static void attachPreviousDecl(ASTReader &Reader, Decl *D, Decl *Previous);
 
     template <typename DeclT>
     static void attachLatestDeclImpl(Redeclarable<DeclT> *D, Decl *Latest);
@@ -2484,22 +2485,68 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) {
 }
 
 template<typename DeclT>
-void ASTDeclReader::attachPreviousDeclImpl(Redeclarable<DeclT> *D,
+void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader,
+                                           Redeclarable<DeclT> *D,
                                            Decl *Previous) {
   D->RedeclLink.setPrevious(cast<DeclT>(Previous));
 }
-void ASTDeclReader::attachPreviousDeclImpl(...) {
+template<>
+void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader,
+                                           Redeclarable<FunctionDecl> *D,
+                                           Decl *Previous) {
+  FunctionDecl *FD = static_cast<FunctionDecl*>(D);
+  FunctionDecl *PrevFD = cast<FunctionDecl>(Previous);
+
+  FD->RedeclLink.setPrevious(PrevFD);
+
+  // If the previous declaration is an inline function declaration, then this
+  // declaration is too.
+  if (PrevFD->IsInline != FD->IsInline) {
+    // FIXME: [dcl.fct.spec]p4:
+    //   If a function with external linkage is declared inline in one
+    //   translation unit, it shall be declared inline in all translation
+    //   units in which it appears.
+    //
+    // Be careful of this case:
+    //
+    // module A:
+    //   template<typename T> struct X { void f(); };
+    //   template<typename T> inline void X<T>::f() {}
+    //
+    // module B instantiates the declaration of X<int>::f
+    // module C instantiates the definition of X<int>::f
+    //
+    // If module B and C are merged, we do not have a violation of this rule.
+    FD->IsInline = true;
+  }
+
+  // If this declaration has an unresolved exception specification but the
+  // previous declaration had a resolved one, resolve the exception
+  // specification now.
+  auto *FPT = FD->getType()->getAs<FunctionProtoType>();
+  auto *PrevFPT = PrevFD->getType()->getAs<FunctionProtoType>();
+  if (FPT && PrevFPT &&
+      isUnresolvedExceptionSpec(FPT->getExceptionSpecType()) &&
+      !isUnresolvedExceptionSpec(PrevFPT->getExceptionSpecType())) {
+    FunctionProtoType::ExtProtoInfo EPI = PrevFPT->getExtProtoInfo();
+    FD->setType(Reader.Context.getFunctionType(
+        FPT->getReturnType(), FPT->getParamTypes(),
+        FPT->getExtProtoInfo().withExceptionSpec(EPI.ExceptionSpec)));
+  }
+}
+void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader, ...) {
   llvm_unreachable("attachPreviousDecl on non-redeclarable declaration");
 }
 
-void ASTDeclReader::attachPreviousDecl(Decl *D, Decl *Previous) {
+void ASTDeclReader::attachPreviousDecl(ASTReader &Reader, Decl *D,
+                                       Decl *Previous) {
   assert(D && Previous);
 
   switch (D->getKind()) {
 #define ABSTRACT_DECL(TYPE)
-#define DECL(TYPE, BASE)                                   \
-  case Decl::TYPE:                                         \
-    attachPreviousDeclImpl(cast<TYPE##Decl>(D), Previous); \
+#define DECL(TYPE, BASE)                                           \
+  case Decl::TYPE:                                                 \
+    attachPreviousDeclImpl(Reader, cast<TYPE##Decl>(D), Previous); \
     break;
 #include "clang/AST/DeclNodes.inc"
   }
@@ -2517,32 +2564,6 @@ void ASTDeclReader::attachPreviousDecl(Decl *D, Decl *Previous) {
   // be too.
   if (Previous->Used)
     D->Used = true;
-
-  // If the previous declaration is an inline function declaration, then this
-  // declaration is too.
-  if (auto *FD = dyn_cast<FunctionDecl>(D)) {
-    if (cast<FunctionDecl>(Previous)->IsInline != FD->IsInline) {
-      // FIXME: [dcl.fct.spec]p4:
-      //   If a function with external linkage is declared inline in one
-      //   translation unit, it shall be declared inline in all translation
-      //   units in which it appears.
-      //
-      // Be careful of this case:
-      //
-      // module A:
-      //   template<typename T> struct X { void f(); };
-      //   template<typename T> inline void X<T>::f() {}
-      //
-      // module B instantiates the declaration of X<int>::f
-      // module C instantiates the definition of X<int>::f
-      //
-      // If module B and C are merged, we do not have a violation of this rule.
-      //
-      //if (!FD->IsInline || Previous->getOwningModule())
-      //  Diag(FD->getLocation(), diag::err_odr_differing_inline);
-      FD->IsInline = true;
-    }
-  }
 }
 
 template<typename DeclT>
@@ -3041,7 +3062,7 @@ void ASTReader::loadPendingDeclChain(serialization::GlobalDeclID ID) {
     if (Chain[I] == CanonDecl)
       continue;
 
-    ASTDeclReader::attachPreviousDecl(Chain[I], MostRecent);
+    ASTDeclReader::attachPreviousDecl(*this, Chain[I], MostRecent);
     MostRecent = Chain[I];
   }
   
@@ -3171,6 +3192,46 @@ void ASTReader::loadObjCCategories(serialization::GlobalDeclID ID,
   ModuleMgr.visit(ObjCCategoriesVisitor::visit, &Visitor);
 }
 
+namespace {
+/// Iterator over the redeclarations of a declaration that have already
+/// been merged into the same redeclaration chain.
+template<typename DeclT>
+class MergedRedeclIterator {
+  DeclT *Start, *Canonical, *Current;
+public:
+  MergedRedeclIterator() : Current(nullptr) {}
+  MergedRedeclIterator(DeclT *Start)
+      : Start(Start), Canonical(nullptr), Current(Start) {}
+
+  DeclT *operator*() { return Current; }
+
+  MergedRedeclIterator &operator++() {
+    if (Current->isFirstDecl())
+      Canonical = Current;
+    Current = Current->getPreviousDecl();
+
+    // If we started in the merged portion, we'll reach our start position
+    // eventually. Otherwise, we'll never reach it, but the second declaration
+    // we reached was the canonical declaration, so stop when we see that one
+    // again.
+    if (Current == Start || Current == Canonical)
+      Current = nullptr;
+    return *this;
+  }
+
+  friend bool operator!=(const MergedRedeclIterator &A,
+                         const MergedRedeclIterator &B) {
+    return A.Current != B.Current;
+  }
+};
+}
+template<typename DeclT>
+llvm::iterator_range<MergedRedeclIterator<DeclT>> merged_redecls(DeclT *D) {
+  return llvm::iterator_range<MergedRedeclIterator<DeclT>>(
+      MergedRedeclIterator<DeclT>(D),
+      MergedRedeclIterator<DeclT>());
+}
+
 void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile,
                                const RecordData &Record) {
   while (Idx < Record.size()) {
@@ -3288,14 +3349,24 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile,
     }
 
     case UPD_CXX_RESOLVED_EXCEPTION_SPEC: {
-      auto *FD = cast<FunctionDecl>(D);
-      auto *FPT = FD->getType()->castAs<FunctionProtoType>();
-      SmallVector<QualType, 8> ExceptionStorage;
+      // 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);
-      FD->setType(Reader.Context.getFunctionType(
-          FPT->getReturnType(), FPT->getParamTypes(),
-          FPT->getExtProtoInfo().withExceptionSpec(ESI)));
+      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;
+        }
+        FD->setType(Reader.Context.getFunctionType(
+            FPT->getReturnType(), FPT->getParamTypes(),
+            FPT->getExtProtoInfo().withExceptionSpec(ESI)));
+      }
       break;
     }
 
diff --git a/clang/test/Modules/Inputs/cxx-irgen-left.h b/clang/test/Modules/Inputs/cxx-irgen-left.h
index ceb50846bbf..fb36b218a49 100644
--- a/clang/test/Modules/Inputs/cxx-irgen-left.h
+++ b/clang/test/Modules/Inputs/cxx-irgen-left.h
@@ -9,3 +9,12 @@ inline int instantiate_min() {
 inline int instantiate_CtorInit(CtorInit<int> i = CtorInit<int>()) {
   return i.a;
 }
+
+namespace ImplicitSpecialMembers {
+  inline void create_left() {
+    // Trigger declaration, but not definition, of special members.
+    B b(0); C c(0); D d(0);
+    // Trigger definition of copy constructor.
+    C c2(c); D d2(d);
+  }
+}
diff --git a/clang/test/Modules/Inputs/cxx-irgen-right.h b/clang/test/Modules/Inputs/cxx-irgen-right.h
index 4400c760f07..30686a12479 100644
--- a/clang/test/Modules/Inputs/cxx-irgen-right.h
+++ b/clang/test/Modules/Inputs/cxx-irgen-right.h
@@ -1,3 +1,13 @@
 #include "cxx-irgen-top.h"
 
 inline int h() { return S<int>::f(); }
+
+namespace ImplicitSpecialMembers {
+  inline void create_right() {
+    // Trigger declaration, but not definition, of special members.
+    B b(0); C c(0); D d(0);
+    // Trigger definition of move constructor.
+    B b2(static_cast<B&&>(b));
+    D d2(static_cast<D&&>(d));
+  }
+}
diff --git a/clang/test/Modules/Inputs/cxx-irgen-top.h b/clang/test/Modules/Inputs/cxx-irgen-top.h
index 8753d8daa3d..0ab586f7ae1 100644
--- a/clang/test/Modules/Inputs/cxx-irgen-top.h
+++ b/clang/test/Modules/Inputs/cxx-irgen-top.h
@@ -14,3 +14,21 @@ template<typename T> struct CtorInit {
   int a;
   CtorInit() : a(f()) {}
 };
+
+namespace ImplicitSpecialMembers {
+  struct A {
+    A(const A&);
+  };
+  struct B {
+    A a;
+    B(int);
+  };
+  struct C {
+    A a;
+    C(int);
+  };
+  struct D {
+    A a;
+    D(int);
+  };
+}
diff --git a/clang/test/Modules/cxx-irgen.cpp b/clang/test/Modules/cxx-irgen.cpp
index 4c61a3a3583..eea6b316e3f 100644
--- a/clang/test/Modules/cxx-irgen.cpp
+++ b/clang/test/Modules/cxx-irgen.cpp
@@ -13,10 +13,38 @@ CtorInit<int> x;
 // CHECK-DAG: define available_externally hidden {{signext i32|i32}} @_ZN1SIiE1gEv({{.*}} #[[ALWAYS_INLINE:.*]] align
 int a = S<int>::g();
 
-// CHECK-DAG: define available_externally {{signext i32|i32}} @_ZN1SIiE1fEv({{.*}} #[[ALWAYS_INLINE]] align
 int b = h();
 
 // CHECK-DAG: define linkonce_odr {{signext i32|i32}} @_Z3minIiET_S0_S0_(i32
 int c = min(1, 2);
 
+namespace ImplicitSpecialMembers {
+  // CHECK-LABEL: define {{.*}} @_ZN22ImplicitSpecialMembers1DC2EOS0_(
+  // CHECK: call {{.*}} @_ZN22ImplicitSpecialMembers1AC1ERKS0_(
+  // CHECK-LABEL: define {{.*}} @_ZN22ImplicitSpecialMembers1DC2ERKS0_(
+  // CHECK: call {{.*}} @_ZN22ImplicitSpecialMembers1AC1ERKS0_(
+  // CHECK-LABEL: define {{.*}} @_ZN22ImplicitSpecialMembers1CC2EOS0_(
+  // CHECK: call {{.*}} @_ZN22ImplicitSpecialMembers1AC1ERKS0_(
+  // CHECK-LABEL: define {{.*}} @_ZN22ImplicitSpecialMembers1CC2ERKS0_(
+  // CHECK: call {{.*}} @_ZN22ImplicitSpecialMembers1AC1ERKS0_(
+  // CHECK-LABEL: define {{.*}} @_ZN22ImplicitSpecialMembers1BC2EOS0_(
+  // CHECK: call {{.*}} @_ZN22ImplicitSpecialMembers1AC1ERKS0_(
+  // CHECK-LABEL: define {{.*}} @_ZN22ImplicitSpecialMembers1BC2ERKS0_(
+  // FIXME CHECK-NOT: call {{.*}} @_ZN22ImplicitSpecialMembers1AC1ERKS0_(
+
+  extern B b1;
+  B b2(b1);
+  B b3(static_cast<B&&>(b1));
+
+  extern C c1;
+  C c2(c1);
+  C c3(static_cast<C&&>(c1));
+
+  extern D d1;
+  D d2(d1);
+  D d3(static_cast<D&&>(d1));
+}
+
+// CHECK: define available_externally {{signext i32|i32}} @_ZN1SIiE1fEv({{.*}} #[[ALWAYS_INLINE]] align
+
 // CHECK: attributes #[[ALWAYS_INLINE]] = {{.*}} alwaysinline