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 <[email protected]> wrote: > 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 > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
