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