Record layout requires not just a definition, but a complete

definition.  Assert this.  Change IR generation to not try to
aggressively emit the IR translation of a record during its
own definition.  Fixes PR10912.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@141350 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp
index 8e715887680bddc08824d0e13bc155edda10b907..369ebec3ddf748e011ec9256bdb08682abf78cdf 100644 (file)
--- a/lib/AST/RecordLayoutBuilder.cpp
+++ b/lib/AST/RecordLayoutBuilder.cpp
@@ -2007,8 +2007,13 @@ RecordLayoutBuilder::Diag(SourceLocation Loc, unsigned DiagID) {
 /// position information.
 const ASTRecordLayout &
 ASTContext::getASTRecordLayout(const RecordDecl *D) const {
+  // These asserts test different things.  A record has a definition
+  // as soon as we begin to parse the definition.  That definition is
+  // not a complete definition (which is what isDefinition() tests)
+  // until we *finish* parsing the definition.
   D = D->getDefinition();
   assert(D && "Cannot get layout of forward declarations!");
+  assert(D->isDefinition() && "Cannot layout type before complete!");
 
   // Look up this layout, if already laid out, return what we have.
   // Note that we can't save a reference to the entry because this function

diff --git a/lib/CodeGen/CodeGenTypes.cpp b/lib/CodeGen/CodeGenTypes.cpp
index 61c15817982be2502280fba47b1977524b3fc5fd..31aed99825a9233ec5506b7fe4ef5dfd91022699 100644 (file)
--- a/lib/CodeGen/CodeGenTypes.cpp
+++ b/lib/CodeGen/CodeGenTypes.cpp
@@ -579,7 +579,7 @@ llvm::StructType *CodeGenTypes::ConvertRecordDeclType(const RecordDecl *RD) {
   // If this is still a forward declaration, or the LLVM type is already
   // complete, there's nothing more to do.
   RD = RD->getDefinition();
-  if (RD == 0 || !Ty->isOpaque())
+  if (RD == 0 || !RD->isDefinition() || !Ty->isOpaque())
     return Ty;
   
   // If converting this type would cause us to infinitely loop, don't do it!

diff --git a/test/CodeGenCXX/class-layout.cpp b/test/CodeGenCXX/class-layout.cpp
index 9569f476dd8702c5d565240f2872289500c43951..dac0a0ae5467b7ab15c0985747a8c1c5cd3485c0 100644 (file)
--- a/test/CodeGenCXX/class-layout.cpp
+++ b/test/CodeGenCXX/class-layout.cpp
@@ -45,3 +45,35 @@ namespace Test5 {
     char c;
   } *b;
 }
+
+// PR10912: don't crash
+namespace Test6 {
+  template <typename T> class A {
+    // If T is complete, IR-gen will want to translate it recursively
+    // when translating T*.
+    T *foo;
+  };
+
+  class B;
+
+  // This causes IR-gen to have an incomplete translation of A<B>
+  // sitting around.
+  A<B> *a;
+
+  class C {};
+  class B : public C {
+    // This forces Sema to instantiate A<B>, which triggers a callback
+    // to IR-gen.  Because of the previous, incomplete translation,
+    // IR-gen actually cares, and it immediately tries to complete
+    // A<B>'s IR type.  That, in turn, causes the translation of B*.
+    // B isn't complete yet, but it has a definition, and if we try to
+    // compute a record layout for that definition then we'll really
+    // regret it later.
+    A<B> a;
+  };
+
+  // The derived class E and empty base class C are required to
+  // provoke the original assertion.
+  class E : public B {};
+  E *e;
+}