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?


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to