[llvm] r275717 - Fix warnings in ImmutableSetTest and SequenceTest.

[David Blaikie via llvm-commits]

Generally we just try to avoid the false positives - even if it means
dropping a few true positives, so I'd just suggest doing the first part of
that check & ignoring the second part.

On Mon, Jul 18, 2016 at 10:42 AM Justin Lebar <jlebar at google.com> wrote:

> It is a clang diagnostic.
>
> > Should we fix the diagnostic rather than the code (the code seems pretty
> reasonable)?
>
> Maybe...  If in a macro expansion a side-effecting expression comes
> from a macro param and is used in both an evaluated and unevaluated
> context, then don't warn?  That seems awful complicated.
>
> On Mon, Jul 18, 2016 at 10:37 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
> > Is that a Clang diagnostic? Should we fix the diagnostic rather than the
> > code (the code seems pretty reasonable)?
> >
> > On Sun, Jul 17, 2016 at 11:17 AM Justin Lebar via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >>
> >> Author: jlebar
> >> Date: Sun Jul 17 13:10:30 2016
> >> New Revision: 275717
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=275717&view=rev
> >> Log:
> >> Fix warnings in ImmutableSetTest and SequenceTest.
> >>
> >> Doing "I++" inside of an EXPECT_* triggers
> >>
> >>   warning: expression with side effects has no effect in an unevaluated
> >> context
> >>
> >> because EXPECT_* partially expands to
> >>
> >>   EqHelper<(sizeof(::testing::internal::IsNullLiteralHelper(i++)) == 1)>
> >>
> >> which is an unevaluated context.
> >>
> >> Modified:
> >>     llvm/trunk/unittests/ADT/ImmutableSetTest.cpp
> >>     llvm/trunk/unittests/ADT/SequenceTest.cpp
> >>
> >> Modified: llvm/trunk/unittests/ADT/ImmutableSetTest.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/ImmutableSetTest.cpp?rev=275717&r1=275716&r2=275717&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/unittests/ADT/ImmutableSetTest.cpp (original)
> >> +++ llvm/trunk/unittests/ADT/ImmutableSetTest.cpp Sun Jul 17 13:10:30
> 2016
> >> @@ -181,19 +181,22 @@ TEST_F(ImmutableSetTest, IterLongSetTest
> >>
> >>    int i = 0;
> >>    for (ImmutableSet<long>::iterator I = S.begin(), E = S.end(); I != E;
> >> ++I) {
> >> -    ASSERT_EQ(i++, *I);
> >> +    ASSERT_EQ(i, *I);
> >> +    i++;
> >>    }
> >>    ASSERT_EQ(0, i);
> >>
> >>    i = 0;
> >>    for (ImmutableSet<long>::iterator I = S2.begin(), E = S2.end(); I !=
> E;
> >> ++I) {
> >> -    ASSERT_EQ(i++, *I);
> >> +    ASSERT_EQ(i, *I);
> >> +    i++;
> >>    }
> >>    ASSERT_EQ(3, i);
> >>
> >>    i = 0;
> >>    for (ImmutableSet<long>::iterator I = S3.begin(), E = S3.end(); I !=
> E;
> >> I++) {
> >> -    ASSERT_EQ(i++, *I);
> >> +    ASSERT_EQ(i, *I);
> >> +    i++;
> >>    }
> >>    ASSERT_EQ(6, i);
> >>  }
> >>
> >> Modified: llvm/trunk/unittests/ADT/SequenceTest.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/SequenceTest.cpp?rev=275717&r1=275716&r2=275717&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/unittests/ADT/SequenceTest.cpp (original)
> >> +++ llvm/trunk/unittests/ADT/SequenceTest.cpp Sun Jul 17 13:10:30 2016
> >> @@ -18,8 +18,10 @@ namespace {
> >>
> >>  TEST(SequenceTest, Basic) {
> >>    int x = 0;
> >> -  for (int i : seq(0, 10))
> >> -    EXPECT_EQ(x++, i);
> >> +  for (int i : seq(0, 10)) {
> >> +    EXPECT_EQ(x, i);
> >> +    x++;
> >> +  }
> >>    EXPECT_EQ(10, x);
> >>
> >>    auto my_seq = seq(0, 4);
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160718/ac255db9/attachment.html>