<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Dec 3, 2014 at 6:52 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On Tue, Dec 2, 2014 at 8:05 PM, Richard Smith<br>
<<a href="mailto:richard-llvm@metafoo.co.uk">richard-llvm@metafoo.co.uk</a>> wrote:<br>
> Author: rsmith<br>
> Date: Tue Dec  2 19:05:50 2014<br>
> New Revision: 223184<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=223184&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=223184&view=rev</a><br>
> Log:<br>
> PR21706: -Wunsequenced was missing warnings when leaving a sequenced region that contained side effects.<br>
<br>
</span>This doesn't seem to account for unevaluated contexts.</blockquote><div><br></div><div>This is a pre-existing issue that was merely exposed in more cases by this change; we used to warn on, for instance,</div><div><br></div><div>  int n;</div><div>  int k = ++n + _Generic(++n, default: 0);<br></div><div><br></div><div>Fixed in r223266.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">For instance, I<br>
now get a false-positive with code like:<br>
<br>
void blah(int a);<br>
<br>
#define GenTest(x) _Generic(x, default : blah)(x)<br>
<br>
void f(void) {<br>
  int i = 0;<br>
  GenTest(++i);<br>
}<br>
<span class=""><font color="#888888"><br>
~Aaron<br>
</font></span><div class=""><div class="h5"><br>
><br>
> Modified:<br>
>     cfe/trunk/lib/Sema/SemaChecking.cpp<br>
>     cfe/trunk/test/Sema/warn-unsequenced.c<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=223184&r1=223183&r2=223184&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=223184&r1=223183&r2=223184&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Dec  2 19:05:50 2014<br>
> @@ -7014,11 +7014,12 @@ class SequenceChecker : public Evaluated<br>
>        Self.ModAsSideEffect = &ModAsSideEffect;<br>
>      }<br>
>      ~SequencedSubexpression() {<br>
> -      for (unsigned I = 0, E = ModAsSideEffect.size(); I != E; ++I) {<br>
> -        UsageInfo &U = Self.UsageMap[ModAsSideEffect[I].first];<br>
> -        U.Uses[UK_ModAsSideEffect] = ModAsSideEffect[I].second;<br>
> -        Self.addUsage(U, ModAsSideEffect[I].first,<br>
> -                      ModAsSideEffect[I].second.Use, UK_ModAsValue);<br>
> +      for (auto MI = ModAsSideEffect.rbegin(), ME = ModAsSideEffect.rend();<br>
> +           MI != ME; ++MI) {<br>
> +        UsageInfo &U = Self.UsageMap[MI->first];<br>
> +        auto &SideEffectUsage = U.Uses[UK_ModAsSideEffect];<br>
> +        Self.addUsage(U, MI->first, SideEffectUsage.Use, UK_ModAsValue);<br>
> +        SideEffectUsage = MI->second;<br>
>        }<br>
>        Self.ModAsSideEffect = OldModAsSideEffect;<br>
>      }<br>
><br>
> Modified: cfe/trunk/test/Sema/warn-unsequenced.c<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-unsequenced.c?rev=223184&r1=223183&r2=223184&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-unsequenced.c?rev=223184&r1=223183&r2=223184&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Sema/warn-unsequenced.c (original)<br>
> +++ cfe/trunk/test/Sema/warn-unsequenced.c Tue Dec  2 19:05:50 2014<br>
> @@ -29,6 +29,11 @@ void test() {<br>
>    a = f(a++, 0); // ok<br>
>    a = f(++a, a++); // expected-warning {{multiple unsequenced modifications}}<br>
><br>
> +  ++a + f(++a, 0); // expected-warning {{multiple unsequenced modifications}}<br>
> +  f(++a, 0) + ++a; // expected-warning {{multiple unsequenced modifications}}<br>
> +  a++ + f(a++, 0); // expected-warning {{multiple unsequenced modifications}}<br>
> +  f(a++, 0) + a++; // expected-warning {{multiple unsequenced modifications}}<br>
> +<br>
>    a = ++a; // expected-warning {{multiple unsequenced modifications}}<br>
>    a += ++a; // expected-warning {{unsequenced modification and access}}<br>
><br>
> @@ -48,7 +53,7 @@ void test() {<br>
>    (1 ? a : ++a) + a; // ok<br>
>    (xs[5] ? ++a : ++a) + a; // FIXME: warn here<br>
><br>
> -  (++a, xs[6] ? ++a : 0) + a; // FIXME: warn here<br>
> +  (++a, xs[6] ? ++a : 0) + a; // expected-warning {{unsequenced modification and access}}<br>
><br>
>    // Here, the read of the fourth 'a' might happen before or after the write to<br>
>    // the second 'a'.<br>
> @@ -84,5 +89,5 @@ void test() {<br>
><br>
>    (__builtin_classify_type(++a) ? 1 : 0) + ++a; // ok<br>
>    (__builtin_constant_p(++a) ? 1 : 0) + ++a; // ok<br>
> -  (__builtin_expect(++a, 0) ? 1 : 0) + ++a; // FIXME: warn here<br>
> +  (__builtin_expect(++a, 0) ? 1 : 0) + ++a; // expected-warning {{multiple unsequenced modifications}}<br>
>  }<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>