<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Jun 26, 2013, at 3:36 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:</div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jun 26, 2013 at 3:31 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank" class="cremed">eli.friedman@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Wed, Jun 26, 2013 at 2:54 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank" class="cremed">rnk@google.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr">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.<div>
<br></div><div>Ultimately, we should be able to do MS C struct bitfield layout in this builder in addition to the C++ record layout.</div><div><br></div><div>This patch doesn't actually do anything with C++ record layout yet to keep the diff smaller and be more incremental.</div>
<div><br></div><div>Does that make sense to you?</div></div></blockquote><div><br></div></div><div>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.</div>
</div></div></blockquote></div><br>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.</div>
</div>
</blockquote></div><br><div>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.</div><div><br></div><div>John.</div></body></html>