<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Aug 9, 2017 at 1:44 AM Grang, Mandeep Singh <<a href="mailto:mgrang@codeaurora.org">mgrang@codeaurora.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
In D35043 I have removed the llvm tests which use -reverse-iterate.
This patch removes the clang tests.<br></div></blockquote><div><br>Ah, OK. I'm still curious about whether this results in a loss of test coverage. Without this test, would the bug it was testing still be caught by some test failure in at least one of the forward or reverse iteration modes?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>Should I post a later patch to change all "class
PointerLikeTypeTraits" to "struct PointerLikeTypeTraits"?</div></blockquote><div><br>I'll just do that (r310506 & r310508) - done! :)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br>
<br>
<div class="m_-544102029957928977moz-cite-prefix">On 8/7/2017 2:50 PM, David Blaikie
wrote:<br>
</div>
<blockquote type="cite">
<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" target="_blank">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>
</blockquote>
<br>
</div></blockquote></div></div>