[cfe-commits] r76348 - in /cfe/trunk: lib/AST/RecordLayoutBuilder.cpp lib/AST/RecordLayoutBuilder.h test/SemaCXX/class-layout.cpp

[Douglas Gregor]

On Jul 18, 2009, at 5:18 PM, Anders Carlsson wrote:

> Author: andersca
> Date: Sat Jul 18 19:18:47 2009
> New Revision: 76348
>
> URL: http://llvm.org/viewvc/llvm-project?rev=76348&view=rev
> Log:
> Handle layout of non-virtual base classes.

Cool :)

> +void ASTRecordLayoutBuilder::LayoutNonVirtualBase(const  
> CXXRecordDecl *RD) {
> +  const ASTRecordLayout &BaseInfo = Ctx.getASTRecordLayout(RD);
> +    assert(BaseInfo.getDataSize() > 0 &&
> +           "FIXME: Handle empty classes.");

Yeah, that's a good way to start out. Empty base classes cause a ruckus.

> +  // FIXME: Should get the non-virtual alignment of the base.
> +  unsigned BaseAlign = BaseInfo.getAlignment();
> +
> +  // FIXME: Should get the non-virtual size of the base.
> +  uint64_t BaseSize = BaseInfo.getDataSize();
> +
> +  // Round up the current record size to the base's alignment  
> boundary.
> +  Size = (Size + (BaseAlign-1)) & ~(BaseAlign-1);
> +
> +  // Reserve space for this base.
> +  Size += BaseSize;
> +
> +  // Remember the next available offset.
> +  NextOffset = Size;
> +
> +  // Remember max struct/class alignment.
> +  UpdateAlignment(BaseAlign);
> +}

Looks fine for now. Those three FIXME's imply a bunch of work that  
will make this code a bit more complicated in the future.

I think ASTRecordLayout store some information mapping direct base  
classes to their offsets? We'll need that for derived-to-base and base- 
to-derived cases.

> void ASTRecordLayoutBuilder::Layout(const RecordDecl *D) {
>   IsUnion = D->isUnion();
>
> @@ -31,7 +71,11 @@
>
>   if (const AlignedAttr *AA = D->getAttr<AlignedAttr>())
>     UpdateAlignment(AA->getAlignment());
> -
> +
> +  // If this is a C++ class, lay out the nonvirtual bases.
> +  if (Ctx.getLangOptions().CPlusPlus)
> +    LayoutNonVirtualBases(cast<CXXRecordDecl>(D));
> +
>   LayoutFields(D);
>
>   // Finally, round the size of the total struct up to the alignment  
> of the
> @@ -191,8 +235,20 @@
>
>   Builder.Layout(D);
>
> -  return new ASTRecordLayout(Builder.Size, Builder.Alignment,
> -                             Builder.NextOffset,
> +  bool IsPODForThePurposeOfLayout;
> +  if (!Ctx.getLangOptions().CPlusPlus) {
> +    // In C, all record types are POD.
> +    IsPODForThePurposeOfLayout = true;
> +  } else {
> +    // FIXME: This is not always correct. See the part about  
> bitfields at
> +    // http://www.codesourcery.com/public/cxx-abi/abi.html#POD for  
> more info.
> +    IsPODForThePurposeOfLayout = cast<CXXRecordDecl>(D)->isPOD();
> +  }

Oh, yuck. I guess we'll have to add an  
IsPODForThePurposeOfItaniumABILayout bit to ASTRecordLayout to track  
this, because we certainly don't want to perform this (recursive)  
computation for each record layout.

   - Doug