r223184 - PR21706: -Wunsequenced was missing warnings when leaving a sequenced region that contained side effects.

[Aaron Ballman]

On Wed, Dec 3, 2014 at 4:00 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Wed, Dec 3, 2014 at 6:52 AM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> On Tue, Dec 2, 2014 at 8:05 PM, Richard Smith
>> <richard-llvm at metafoo.co.uk> wrote:
>> > Author: rsmith
>> > Date: Tue Dec  2 19:05:50 2014
>> > New Revision: 223184
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=223184&view=rev
>> > Log:
>> > PR21706: -Wunsequenced was missing warnings when leaving a sequenced
>> > region that contained side effects.
>>
>> This doesn't seem to account for unevaluated contexts.
>
>
> This is a pre-existing issue that was merely exposed in more cases by this
> change; we used to warn on, for instance,
>
>   int n;
>   int k = ++n + _Generic(++n, default: 0);
>
> Fixed in r223266.

Ah, thanks!

~Aaron

>
>> For instance, I
>> now get a false-positive with code like:
>>
>> void blah(int a);
>>
>> #define GenTest(x) _Generic(x, default : blah)(x)
>>
>> void f(void) {
>>   int i = 0;
>>   GenTest(++i);
>> }
>>
>> ~Aaron
>>
>> >
>> > Modified:
>> >     cfe/trunk/lib/Sema/SemaChecking.cpp
>> >     cfe/trunk/test/Sema/warn-unsequenced.c
>> >
>> > Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=223184&r1=223183&r2=223184&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> > +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Dec  2 19:05:50 2014
>> > @@ -7014,11 +7014,12 @@ class SequenceChecker : public Evaluated
>> >        Self.ModAsSideEffect = &ModAsSideEffect;
>> >      }
>> >      ~SequencedSubexpression() {
>> > -      for (unsigned I = 0, E = ModAsSideEffect.size(); I != E; ++I) {
>> > -        UsageInfo &U = Self.UsageMap[ModAsSideEffect[I].first];
>> > -        U.Uses[UK_ModAsSideEffect] = ModAsSideEffect[I].second;
>> > -        Self.addUsage(U, ModAsSideEffect[I].first,
>> > -                      ModAsSideEffect[I].second.Use, UK_ModAsValue);
>> > +      for (auto MI = ModAsSideEffect.rbegin(), ME =
>> > ModAsSideEffect.rend();
>> > +           MI != ME; ++MI) {
>> > +        UsageInfo &U = Self.UsageMap[MI->first];
>> > +        auto &SideEffectUsage = U.Uses[UK_ModAsSideEffect];
>> > +        Self.addUsage(U, MI->first, SideEffectUsage.Use,
>> > UK_ModAsValue);
>> > +        SideEffectUsage = MI->second;
>> >        }
>> >        Self.ModAsSideEffect = OldModAsSideEffect;
>> >      }
>> >
>> > Modified: cfe/trunk/test/Sema/warn-unsequenced.c
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-unsequenced.c?rev=223184&r1=223183&r2=223184&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/test/Sema/warn-unsequenced.c (original)
>> > +++ cfe/trunk/test/Sema/warn-unsequenced.c Tue Dec  2 19:05:50 2014
>> > @@ -29,6 +29,11 @@ void test() {
>> >    a = f(a++, 0); // ok
>> >    a = f(++a, a++); // expected-warning {{multiple unsequenced
>> > modifications}}
>> >
>> > +  ++a + f(++a, 0); // expected-warning {{multiple unsequenced
>> > modifications}}
>> > +  f(++a, 0) + ++a; // expected-warning {{multiple unsequenced
>> > modifications}}
>> > +  a++ + f(a++, 0); // expected-warning {{multiple unsequenced
>> > modifications}}
>> > +  f(a++, 0) + a++; // expected-warning {{multiple unsequenced
>> > modifications}}
>> > +
>> >    a = ++a; // expected-warning {{multiple unsequenced modifications}}
>> >    a += ++a; // expected-warning {{unsequenced modification and access}}
>> >
>> > @@ -48,7 +53,7 @@ void test() {
>> >    (1 ? a : ++a) + a; // ok
>> >    (xs[5] ? ++a : ++a) + a; // FIXME: warn here
>> >
>> > -  (++a, xs[6] ? ++a : 0) + a; // FIXME: warn here
>> > +  (++a, xs[6] ? ++a : 0) + a; // expected-warning {{unsequenced
>> > modification and access}}
>> >
>> >    // Here, the read of the fourth 'a' might happen before or after the
>> > write to
>> >    // the second 'a'.
>> > @@ -84,5 +89,5 @@ void test() {
>> >
>> >    (__builtin_classify_type(++a) ? 1 : 0) + ++a; // ok
>> >    (__builtin_constant_p(++a) ? 1 : 0) + ++a; // ok
>> > -  (__builtin_expect(++a, 0) ? 1 : 0) + ++a; // FIXME: warn here
>> > +  (__builtin_expect(++a, 0) ? 1 : 0) + ++a; // expected-warning
>> > {{multiple unsequenced modifications}}
>> >  }
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>