[cfe-commits] [Patch][Review] Bug 13329 - Incorrect code generated for automatic assignment operator

[Jonathan Sauer]

Hello,

> Checking isPODType here is not correct. The relevant term in the ABI document is "POD for the purpose of layout", not "POD", and means something subtly different (critically, it does not depend on whether we are in C++98 or C++11 mode). This term is defined in the ABI document.

You're correct, although my patch is *almost* correct in the context of clang's current implementation
of 'POD for the purpose of layout' ;-). In AST/RecordLayoutBuilder.cpp[*]:

const ASTRecordLayout & ASTContext::getASTRecordLayout(const RecordDecl *D) const {
   [...]
   // 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.
   // FIXME: IsPODForThePurposeOfLayout should be stored in the record layout.
   // This does not affect the calculations of MSVC layouts
   bool IsPODForThePurposeOfLayout = 
     (!Builder.isMicrosoftCXXABI() && cast<CXXRecordDecl>(D)->isPOD());

   // FIXME: This should be done in FinalizeLayout.
   CharUnits DataSize =
     IsPODForThePurposeOfLayout ? Builder.getSize() : Builder.getDataSize();
   [...]


I propose I do one of the following:

1. Introduce a method QualType#isPODForThePurposeOfLayout which as of now delegates to isPOD and
   includes the above FIXME.

2. Introduce a method ASTContext#getTypeInfoForThePurposeOfLayout that returns either the complete
   size of a type or the size without tail padding (plus the alignment, of course) and includes the
   FIXME as well.

Personally I prefer the second solution as it avoids duplicating the "select correct size" logic.

What do you (plural, even though I only replied to Richard's mail) think?


Jonathan

[*] Does anyone know why this method is not in ASTContext.cpp?