[PATCH] Microsoft C++ Record Layout

Reid Kleckner rnk at google.com
Thu Jul 11 06:22:16 PDT 2013


On Wed, Jul 10, 2013 at 10:01 PM, Eli Friedman <eli.friedman at gmail.com>wrote:

> Please split out the SemaDecl.cpp changes into another patch.
>
> Some of the changes to test/Sema/ms_class_layout.cpp don't look right;
> are you sure they're accurate?
>
> >   * It's still in two files and doesn't use CRTP.  The net overlap
> between the current record builder and this one is 7 member variables, 1
> typedef and 10 functions with the same names but different functionality.
>  The closest functionality is laying out fields (non-bit-fields) but it
> rounds in a slightly different place and is only a few lines of code.
>  Basically although in spirit the ms and non-ms builders achieve similar
> goals, they do so in completely different ways.  I would use an analogy of
> implementing a dictionary using a B-tree or a red-black tree.  They may
> both implement a dictionary using a tree but don't actually have much of
> any share-able code.
>
> It's also worth noting that you didn't implement -Wpadded...
>

I would argue that -Wpadded should be in Sema operating on the layouts
produced by AST, but it looks like the field sizes aren't recorded in the
layout and recomputing them is not trivial.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130711/ecc58b8b/attachment.html>


More information about the cfe-commits mailing list