[cfe-commits] r141350 - in /cfe/trunk: lib/AST/RecordLayoutBuilder.cpp lib/CodeGen/CodeGenTypes.cpp test/CodeGenCXX/class-layout.cpp

[John McCall]

Author: rjmccall
Date: Thu Oct  6 21:39:22 2011
New Revision: 141350

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


Modified:
    cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
    cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
    cfe/trunk/test/CodeGenCXX/class-layout.cpp

Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=141350&r1=141349&r2=141350&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Thu Oct  6 21:39:22 2011
@@ -2007,8 +2007,13 @@
 /// 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

Modified: cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenTypes.cpp?rev=141350&r1=141349&r2=141350&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenTypes.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenTypes.cpp Thu Oct  6 21:39:22 2011
@@ -579,7 +579,7 @@
   // 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!

Modified: cfe/trunk/test/CodeGenCXX/class-layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/class-layout.cpp?rev=141350&r1=141349&r2=141350&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/class-layout.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/class-layout.cpp Thu Oct  6 21:39:22 2011
@@ -45,3 +45,35 @@
     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;
+}