OK - go ahead and remove the tests then, if the functionality is now covered by the buildbot+existing tests.
On Wed, Aug 9, 2017 at 12:04 PM Grang, Mandeep Singh <mgr...@codeaurora.org> wrote: > > 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? > > Sorry ... I missed that. Yes, the reverse iteration buildbot > http://lab.llvm.org:8011/builders/reverse-iteration would test these. > These tests were a stopgap solution anyway while the reverse builtbot was > being setup. > > > I'll just do that (r310506 & r310508) - done! :) > Thanks! > > > --Mandeep > > On 8/9/2017 11:35 AM, David Blaikie wrote: > > > > On Wed, Aug 9, 2017 at 1:44 AM Grang, Mandeep Singh <mgr...@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 < >> revi...@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 >>> >>> >>> >>> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
