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

Richard Smith richard-llvm at metafoo.co.uk
Thu Jul 31 16:46:45 PDT 2014


Author: rsmith
Date: Thu Jul 31 18:46:44 2014
New Revision: 214458

URL: http://llvm.org/viewvc/llvm-project?rev=214458&view=rev
Log:
[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.

Modified:
    cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
    cfe/trunk/test/Modules/Inputs/cxx-irgen-left.h
    cfe/trunk/test/Modules/Inputs/cxx-irgen-right.h
    cfe/trunk/test/Modules/Inputs/cxx-irgen-top.h
    cfe/trunk/test/Modules/cxx-irgen.cpp

Modified: cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExceptionSpec.cpp?rev=214458&r1=214457&r2=214458&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExceptionSpec.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExceptionSpec.cpp Thu Jul 31 18:46:44 2014
@@ -135,13 +135,16 @@ Sema::ResolveExceptionSpec(SourceLocatio
 void
 Sema::UpdateExceptionSpec(FunctionDecl *FD,
                           const FunctionProtoType::ExceptionSpecInfo &ESI) {
-  const FunctionProtoType *Proto =
-      FD->getType()->castAs<FunctionProtoType>();
+  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.
-  FD->setType(Context.getFunctionType(
-      Proto->getReturnType(), Proto->getParamTypes(),
-      Proto->getExtProtoInfo().withExceptionSpec(ESI)));
+    // 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))

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=214458&r1=214457&r2=214458&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Thu Jul 31 18:46:44 2014
@@ -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 ASTDec
 }
 
 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(D
   // 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(ser
     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(seria
   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,
     }
 
     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;
     }
 

Modified: cfe/trunk/test/Modules/Inputs/cxx-irgen-left.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-irgen-left.h?rev=214458&r1=214457&r2=214458&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-irgen-left.h (original)
+++ cfe/trunk/test/Modules/Inputs/cxx-irgen-left.h Thu Jul 31 18:46:44 2014
@@ -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);
+  }
+}

Modified: cfe/trunk/test/Modules/Inputs/cxx-irgen-right.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-irgen-right.h?rev=214458&r1=214457&r2=214458&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-irgen-right.h (original)
+++ cfe/trunk/test/Modules/Inputs/cxx-irgen-right.h Thu Jul 31 18:46:44 2014
@@ -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));
+  }
+}

Modified: cfe/trunk/test/Modules/Inputs/cxx-irgen-top.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-irgen-top.h?rev=214458&r1=214457&r2=214458&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-irgen-top.h (original)
+++ cfe/trunk/test/Modules/Inputs/cxx-irgen-top.h Thu Jul 31 18:46:44 2014
@@ -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);
+  };
+}

Modified: cfe/trunk/test/Modules/cxx-irgen.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-irgen.cpp?rev=214458&r1=214457&r2=214458&view=diff
==============================================================================
--- cfe/trunk/test/Modules/cxx-irgen.cpp (original)
+++ cfe/trunk/test/Modules/cxx-irgen.cpp Thu Jul 31 18:46:44 2014
@@ -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





More information about the cfe-commits mailing list