<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Aug 7, 2017 at 12:08 PM Mandeep Singh Grang via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">mgrang added a comment.<br>
<br>
This patch does 3 things:<br>
<br>
1. Get rid of the unit test <a href="http://objc-modern-metadata-visibility2.mm" rel="noreferrer" target="_blank">objc-modern-metadata-visibility2.mm</a> because this test check uses flag -reverse-iterate. This flag will be removed in <a href="https://reviews.llvm.org/D35043" rel="noreferrer" target="_blank">https://reviews.llvm.org/D35043</a>.<br></blockquote><div><br>Sure - please commit that separately (probably once D35043 is approved - probably best to include that removal in D35043, or a separate patch that /only/ removes the -reverse-iterate flag (& any tests that use it) as a standalone change?).<br><br>Does this test need a replacement? If this test is removed and the underlying issue it was testing regresses, will one of the buildbots (reverse or normal) catch the problem?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">2. <a href="https://reviews.llvm.org/D35043" rel="noreferrer" target="_blank">https://reviews.llvm.org/D35043</a> gets rid of the empty base definition for PointerLikeTypeTraits. This results in a compiler warning because PointerLikeTypeTrait has been defined as struct here while in the header it is a class. So I have changed struct to class.<br></blockquote><div><br>I'd probably go the other way - traits classes like this make more sense as structs, I think - it only has public members & no implementation really has any need for supporting private members.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">3. Since I changed struct PointerLikeTypeTrait to class PointerLikeTypeTrait here, the member functions are no longer public now. This results in a compiler error. So I explicitly marked them as public here.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D36386" rel="noreferrer" target="_blank">https://reviews.llvm.org/D36386</a><br>
<br>
<br>
<br>
</blockquote></div></div>