[PATCH] Microsoft C Record Layout

[Warren Hunt]

The C differences are, to my knowledge, only in bit-fields.  One of the
reasons that this C code has been broken into a separate file is because
I'm implementing C++ as an extension to the C code and it's going in the
same file.  The current state is that I should have a full MS-32-ABI C++
layout patch up late this week.  I presented a C patch first so that we
could start the code review process rather than some belief that compiling
C code in a MS-compatible way was particularly important.  The C++ patch
presently assumes the current structure where there's one file with C and
C++ semantics for MS.  It's been helpful for me to have all of the MS stuff
isolated so I can turn it off/on easily and gives me more confidence I'm
not breaking anything in the interim (it will also certainly make merging
with Eli's patch trivial).  I'm happy to have a discussion about the
ultimate organization of all of the layout code.  The MS C++ layout
algorithm is *completely different* than the Itanium one so I would suggest
keeping them separate.  Breaking C out from C++ can certainly be done and
may provide some benefit, presently my MicrosoftCXXRecordLayoutBuilder
inherits from the MicrosoftRecordLayoutBuilder.

-Warren


On Thu, Jun 27, 2013 at 11:14 AM, John McCall <rjmccall at apple.com> wrote:

> On Jun 26, 2013, at 3:36 PM, Chandler Carruth <chandlerc at google.com>
> wrote:
>
> On Wed, Jun 26, 2013 at 3:31 PM, Eli Friedman <eli.friedman at gmail.com>wrote:
>
>> On Wed, Jun 26, 2013 at 2:54 PM, Reid Kleckner <rnk at google.com> wrote:
>>
>>> The motivation is that C++ record layout for the Microsoft C++ ABI is
>>> sufficiently different to warrant its own builder.  There's already lots of
>>> code in there for virtual bases and vtordisps and other fun things that
>>> don't concern Itanium.
>>>
>>> Ultimately, we should be able to do MS C struct bitfield layout in this
>>> builder in addition to the C++ record layout.
>>>
>>> This patch doesn't actually do anything with C++ record layout yet to
>>> keep the diff smaller and be more incremental.
>>>
>>> Does that make sense to you?
>>>
>>
>> If you think the current MS C++ ABI support in RecordLayoutBuilder.cpp is
>> unmaintainable, then yes, it makes sense to do something to separate it
>> out.  That said, I still don't understand why duplicating the field layout
>> routines helps us in any way; the current IsMsStruct checks are much more
>> concise than duplicating the routines.  If you want to keep the MS and
>> Itanium C++ code in separate files, we can introduce a common base class or
>> something like that.
>>
>
> I'd like to at least get John's input here as well before we do more
> refactoring as he gave some early direction which is why Warren ended up
> heading in this general direction.
>
>
> The C++-specific parts passed the point of getting any useful re-use a
> long time ago.  The differences on the C side are just bit-field
> differences, right?  I think our lives may be better keeping things in the
> same file, though, unless that's really prohibitively large;  in
> particular, that makes it easier to use CRTP for the things that are
> common, like adding member fields to a layout.
>
> John.
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130627/3561ba3d/attachment.html>