r185035 - PR16467: Teach -Wunsequenced that in C11 (unlike C++11), an assignment's

Richard Smith richard-llvm at metafoo.co.uk
Wed Jun 26 16:16:51 PDT 2013


Author: rsmith
Date: Wed Jun 26 18:16:51 2013
New Revision: 185035

URL: http://llvm.org/viewvc/llvm-project?rev=185035&view=rev
Log:
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.

Added:
    cfe/trunk/test/Sema/warn-unsequenced.c
Modified:
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/SemaCXX/warn-unsequenced.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=185035&r1=185034&r2=185035&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Jun 26 18:16:51 2013
@@ -5347,7 +5347,7 @@ class SequenceChecker : public Evaluated
     /// 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))

Added: cfe/trunk/test/Sema/warn-unsequenced.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-unsequenced.c?rev=185035&view=auto
==============================================================================
--- cfe/trunk/test/Sema/warn-unsequenced.c (added)
+++ cfe/trunk/test/Sema/warn-unsequenced.c Wed Jun 26 18:16:51 2013
@@ -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
+}

Modified: cfe/trunk/test/SemaCXX/warn-unsequenced.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unsequenced.cpp?rev=185035&r1=185034&r2=185035&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-unsequenced.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unsequenced.cpp Wed Jun 26 18:16:51 2013
@@ -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}}





More information about the cfe-commits mailing list