[PATCH] Fixing Alias-Avoidance Padding for MS-ABI Layout
Richard Smith
richard at metafoo.co.uk
Sat Nov 23 11:07:58 PST 2013
Code changes LGTM (other than minor issues below). Tests seem to be missing from the patch, though.
================
Comment at: lib/AST/RecordLayout.cpp:57-60
@@ -56,3 +56,6 @@
const CXXRecordDecl *BaseSharingVBPtr,
- bool AlignAfterVBases,
+ bool alignAfterVBases,
+ bool hasZeroSizedMemberOrBase,
+ bool leftMostBaseIsZeroSized,
+ bool isEmpty,
const BaseOffsetsMapTy& BaseOffsets,
----------------
If you're going to fix case here, please capitalize rather than lowercasing (to move towards the coding style).
================
Comment at: lib/AST/RecordLayoutBuilder.cpp:2025
@@ +2024,3 @@
+// additional padding between two bases is that the first base is zero sized
+// or has a zero sized element as a member or base or any member or base has
+// that property and the second base is zero sized or has a zero sized base
----------------
"as a subobject" would seem more accurate than "as a member or base", since we also look at the element type for an array.
================
Comment at: lib/AST/RecordLayoutBuilder.cpp:2149
@@ -2144,1 +2148,3 @@
+ /// has been laid out.
+ const ASTRecordLayout* PreviousBaseLayout;
/// \brief Lets us know if we're in 64-bit mode
----------------
" *"
================
Comment at: lib/AST/RecordLayoutBuilder.cpp:2370
@@ -2361,1 +2369,3 @@
+ if (Layout->leftMostBaseIsZeroSized() && PreviousBaseLayout &&
+ PreviousBaseLayout->hasZeroSizedMemberOrBase())
----------------
A comment on this would be useful.
http://llvm-reviews.chandlerc.com/D2258
More information about the cfe-commits
mailing list