On Jun 26, 2013, at 3:36 PM, Chandler Carruth <[email protected]> wrote: > On Wed, Jun 26, 2013 at 3:31 PM, Eli Friedman <[email protected]> wrote: > On Wed, Jun 26, 2013 at 2:54 PM, Reid Kleckner <[email protected]> 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 [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
