[PATCH] Microsoft C Record Layout

[John McCall]

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130627/2b4f2617/attachment.html>