From: Richard Smith Date: Wed, 26 Jun 2013 23:16:51 +0000 (+0000) Subject: PR16467: Teach -Wunsequenced that in C11 (unlike C++11), an assignment's X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=418dd3eb3e813235af089b5c88182941f8a03d20;p=clang PR16467: Teach -Wunsequenced that in C11 (unlike C++11), an assignment's side-effect is not sequenced before its value computation. Also fix a mishandling of ?: expressions where the condition is constant that was exposed by the tests for this. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@185035 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 996f20cc91..5afb653c10 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -5347,7 +5347,7 @@ class SequenceChecker : public EvaluatedExprVisitor { /// A read of an object. Multiple unsequenced reads are OK. UK_Use, /// A modification of an object which is sequenced before the value - /// computation of the expression, such as ++n. + /// computation of the expression, such as ++n in C++. UK_ModAsValue, /// A modification of an object which is not sequenced before the value /// computation of the expression, such as n++. @@ -5597,7 +5597,12 @@ public: Visit(BO->getRHS()); - notePostMod(O, BO, UK_ModAsValue); + // C++11 [expr.ass]p1: + // the assignment is sequenced [...] before the value computation of the + // assignment expression. + // C11 6.5.16/3 has no such rule. + notePostMod(O, BO, SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue + : UK_ModAsSideEffect); } void VisitCompoundAssignOperator(CompoundAssignOperator *CAO) { VisitBinAssign(CAO); @@ -5612,7 +5617,10 @@ public: notePreMod(O, UO); Visit(UO->getSubExpr()); - notePostMod(O, UO, UK_ModAsValue); + // C++11 [expr.pre.incr]p1: + // the expression ++x is equivalent to x+=1 + notePostMod(O, UO, SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue + : UK_ModAsSideEffect); } void VisitUnaryPostInc(UnaryOperator *UO) { VisitUnaryPostIncDec(UO); } @@ -5673,8 +5681,10 @@ public: // be chosen. void VisitAbstractConditionalOperator(AbstractConditionalOperator *CO) { EvaluationTracker Eval(*this); - SequencedSubexpression Sequenced(*this); - Visit(CO->getCond()); + { + SequencedSubexpression Sequenced(*this); + Visit(CO->getCond()); + } bool Result; if (Eval.evaluate(CO->getCond(), Result)) diff --git a/test/Sema/warn-unsequenced.c b/test/Sema/warn-unsequenced.c new file mode 100644 index 0000000000..49c7acc2d3 --- /dev/null +++ b/test/Sema/warn-unsequenced.c @@ -0,0 +1,85 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c11 -Wno-unused %s + +int f(int, int); + +typedef struct A { + int x, y; +} A; + +void test() { + int a; + int xs[10]; + a + ++a; // expected-warning {{unsequenced modification and access to 'a'}} + a = ++a; // expected-warning {{multiple unsequenced modifications to 'a'}} + a + a++; // expected-warning {{unsequenced modification and access to 'a'}} + a = a++; // expected-warning {{multiple unsequenced modifications to 'a'}} + (a++, a++); // ok + ++a + ++a; // expected-warning {{multiple unsequenced modifications}} + a++ + a++; // expected-warning {{multiple unsequenced modifications}} + a = xs[++a]; // expected-warning {{multiple unsequenced modifications}} + a = xs[a++]; // expected-warning {{multiple unsequenced modifications}} + a = (++a, ++a); // expected-warning {{multiple unsequenced modifications}} + a = (a++, ++a); // expected-warning {{multiple unsequenced modifications}} + a = (a++, a++); // expected-warning {{multiple unsequenced modifications}} + f(a, a); // ok + f(a = 0, a); // expected-warning {{unsequenced modification and access}} + f(a, a += 0); // expected-warning {{unsequenced modification and access}} + f(a = 0, a = 0); // expected-warning {{multiple unsequenced modifications}} + + a = ++a; // expected-warning {{multiple unsequenced modifications}} + a += ++a; // expected-warning {{unsequenced modification and access}} + + A agg1 = { a++, a++ }; // expected-warning {{multiple unsequenced modifications}} + A agg2 = { a++ + a, a++ }; // expected-warning {{unsequenced modification and access}} + + (xs[2] && (a = 0)) + a; // ok + (0 && (a = 0)) + a; // ok + (1 && (a = 0)) + a; // expected-warning {{unsequenced modification and access}} + + (xs[3] || (a = 0)) + a; // ok + (0 || (a = 0)) + a; // expected-warning {{unsequenced modification and access}} + (1 || (a = 0)) + a; // ok + + (xs[4] ? a : ++a) + a; // ok + (0 ? a : ++a) + a; // expected-warning {{unsequenced modification and access}} + (1 ? a : ++a) + a; // ok + (xs[5] ? ++a : ++a) + a; // FIXME: warn here + + (++a, xs[6] ? ++a : 0) + a; // FIXME: warn here + + // Here, the read of the fourth 'a' might happen before or after the write to + // the second 'a'. + a += (a++, a) + a; // expected-warning {{unsequenced modification and access}} + + int *p = xs; + a = *(a++, p); // ok + a = a++ && a; // ok + + A *q = &agg1; + (q = &agg2)->y = q->x; // expected-warning {{unsequenced modification and access to 'q'}} + + // This has undefined behavior if a == 0; otherwise, the side-effect of the + // increment is sequenced before the value computation of 'f(a, a)', which is + // sequenced before the value computation of the '&&', which is sequenced + // before the assignment. We treat the sequencing in '&&' as being + // unconditional. + a = a++ && f(a, a); + + // This has undefined behavior if a != 0. FIXME: We should diagnose this. + (a && a++) + a; + + (xs[7] && ++a) * (!xs[7] && ++a); // ok + + xs[0] = (a = 1, a); // ok + + xs[8] ? ++a + a++ : 0; // expected-warning {{multiple unsequenced modifications}} + xs[8] ? 0 : ++a + a++; // expected-warning {{multiple unsequenced modifications}} + xs[8] ? ++a : a++; // ok + + xs[8] && (++a + a++); // expected-warning {{multiple unsequenced modifications}} + xs[8] || (++a + a++); // expected-warning {{multiple unsequenced modifications}} + + (__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 +} diff --git a/test/SemaCXX/warn-unsequenced.cpp b/test/SemaCXX/warn-unsequenced.cpp index c7acfca6db..26bd92ec9e 100644 --- a/test/SemaCXX/warn-unsequenced.cpp +++ b/test/SemaCXX/warn-unsequenced.cpp @@ -58,6 +58,8 @@ void test() { (xs[4] ? a : ++a) + a; // ok (0 ? a : ++a) + a; // expected-warning {{unsequenced modification and access}} (1 ? a : ++a) + a; // ok + (0 ? a : a++) + a; // expected-warning {{unsequenced modification and access}} + (1 ? a : a++) + a; // ok (xs[5] ? ++a : ++a) + a; // FIXME: warn here (++a, xs[6] ? ++a : 0) + a; // expected-warning {{unsequenced modification and access}}