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

[Nico Weber]

Hi John,

On Thu, Oct 6, 2011 at 7:39 PM, John McCall <rjmccall at apple.com> wrote:
> 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!");

^ this fails for some invalid inputs, see PR13096

>
>   // 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;
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits