<div dir="ltr">Yes, I should have noted there are two warnings that I haven't implemented.  It's much easier to drop them into the builder than it is to add them to Sema, although it makes more logical sense to put them in Sema (which would be a different patch).  I'm okay with either, are there any strong votes?</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 11, 2013 at 6:22 AM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div><div class="h5">On Wed, Jul 10, 2013 at 10:01 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>></span> wrote:<br></div></div><div class="gmail_extra">
<div class="gmail_quote"><div><div class="h5">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Please split out the SemaDecl.cpp changes into another patch.<br>
<br>
Some of the changes to test/Sema/ms_class_layout.cpp don't look right;<br>
are you sure they're accurate?<br>
<div><br>
>   * It's still in two files and doesn't use CRTP.  The net overlap between the current record builder and this one is 7 member variables, 1 typedef and 10 functions with the same names but different functionality.  The closest functionality is laying out fields (non-bit-fields) but it rounds in a slightly different place and is only a few lines of code.  Basically although in spirit the ms and non-ms builders achieve similar goals, they do so in completely different ways.  I would use an analogy of implementing a dictionary using a B-tree or a red-black tree.  They may both implement a dictionary using a tree but don't actually have much of any share-able code.<br>


<br>
</div>It's also worth noting that you didn't implement -Wpadded...<br></blockquote><div><br></div></div></div><div>I would argue that -Wpadded should be in Sema operating on the layouts produced by AST, but it looks like the field sizes aren't recorded in the layout and recomputing them is not trivial.</div>

</div></div></div>
</blockquote></div><br></div>