[PATCH] D36386: [clang] Remove unit test which uses reverse-iterate and fix a PointerLikeTypeTrait specialization
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 9 11:35:32 PDT 2017
On Wed, Aug 9, 2017 at 1:44 AM Grang, Mandeep Singh <mgrang at codeaurora.org>
wrote:
> In D35043 I have removed the llvm tests which use -reverse-iterate. This
> patch removes the clang tests.
>
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?
> Should I post a later patch to change all "class PointerLikeTypeTraits" to
> "struct PointerLikeTypeTraits"?
>
I'll just do that (r310506 & r310508) - done! :)
>
>
> On 8/7/2017 2:50 PM, David Blaikie wrote:
>
>
>
> On Mon, Aug 7, 2017 at 12:08 PM Mandeep Singh Grang via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> mgrang added a comment.
>>
>> This patch does 3 things:
>>
>> 1. Get rid of the unit test objc-modern-metadata-visibility2.mm because
>> this test check uses flag -reverse-iterate. This flag will be removed in
>> https://reviews.llvm.org/D35043.
>>
>
> 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?).
>
> 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?
>
>
>> 2. https://reviews.llvm.org/D35043 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.
>>
>
> 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.
>
>
>> 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.
>>
>>
>> https://reviews.llvm.org/D36386
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170809/98570526/attachment.html>
More information about the cfe-commits
mailing list