[PATCH] D53497: [AST] Do not align virtual bases in `MicrosoftRecordLayoutBuilder` when an external layout is used

Aleksandr Urakov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 22 06:49:10 PDT 2018


aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: rnk, rsmith, zturner, mstorsjo, majnemer.
aleksandr.urakov added a project: clang.
Herald added a subscriber: cfe-commits.

The patch removes alignment of virtual bases when an external layout is used. We have two cases:

- the external layout source has an information about virtual bases offsets, so we just use them;
- the external source has no information about virtual bases offsets. In this case we can't predict where the base will be located. If we will align it but there will be something like `#pragma pack(push, 1)` really, then likely our layout will not fit into the real structure size, and then some asserts will hit. The asserts look reasonable, so I don't think that we need to remove them. May be it would be better instead don't align fields / bases etc. (so treat it always as `#pragma pack(push, 1)`) when an external layout source is used but no info about a field location is presented.

This one is related to https://reviews.llvm.org/D49871

Also it seems that `MicrosoftRecordLayoutBuilder` with an external source and without one differ considerably, may be it is worth to split this and to create two different builders? I think that it would make the logic simpler. What do you think about it?


Repository:
  rC Clang

https://reviews.llvm.org/D53497

Files:
  lib/AST/RecordLayoutBuilder.cpp
  test/CodeGenCXX/Inputs/override-layout-packed-base.layout
  test/CodeGenCXX/override-layout-packed-base.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53497.170399.patch
Type: text/x-patch
Size: 2847 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181022/5c92d0c1/attachment.bin>


More information about the cfe-commits mailing list