From: John McCall <rjmccall@apple.com>
Date: Fri, 7 Oct 2011 02:39:22 +0000 (+0000)
Subject: Record layout requires not just a definition, but a complete
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6595935602f57b4f2115785bb39dabc83e4232dc;p=clang

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 8e71588768..369ebec3dd 100644
--- 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 61c1581798..31aed99825 100644
--- 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 9569f476dd..dac0a0ae54 100644
--- 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;
+}