[PATCH] D36386: [clang] Remove unit test which uses reverse-iterate and fix a PointerLikeTypeTrait specialization
Grang, Mandeep Singh via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 9 01:44:52 PDT 2017
In D35043 I have removed the llvm tests which use -reverse-iterate. This
patch removes the clang tests.
Should I post a later patch to change all "class PointerLikeTypeTraits"
to "struct PointerLikeTypeTraits"?
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 <mailto: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
> <http://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/1ddea00c/attachment.html>
More information about the cfe-commits
mailing list