[cfe-commits] r172760 - in /cfe/trunk: lib/Sema/SemaChecking.cpp test/SemaCXX/warn-unsequenced.cpp

[Richard Smith]

Author: rsmith
Date: Thu Jan 17 16:06:26 2013
New Revision: 172760

URL: http://llvm.org/viewvc/llvm-project?rev=172760&view=rev
Log:
-Wunsequenced: if the LHS of an &&, || or ?: is not constant, check for
unsequenced operations in the RHS. We don't compare the RHS with the rest of
the expression yet; such checks will need care to avoid diagnosing unsequenced
operations which are both in conditionally-evaluated subexpressions which
actually can't occur together, such as in '(b && ++x) + (!b && ++x)'.

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=172760&r1=172759&r2=172760&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Jan 17 16:06:26 2013
@@ -5507,9 +5507,18 @@
 
     bool Result;
     if (!BO->getLHS()->isValueDependent() &&
-        BO->getLHS()->EvaluateAsBooleanCondition(Result, SemaRef.Context) &&
-        !Result)
-      Visit(BO->getRHS());
+        BO->getLHS()->EvaluateAsBooleanCondition(Result, SemaRef.Context)) {
+      if (!Result)
+        Visit(BO->getRHS());
+    } else {
+      // Check for unsequenced operations in the RHS, treating it as an
+      // entirely separate evaluation.
+      //
+      // FIXME: If there are operations in the RHS which are unsequenced
+      // with respect to operations outside the RHS, and those operations
+      // are unconditionally evaluated, diagnose them.
+      SequenceChecker(SemaRef, BO->getRHS());
+    }
   }
   void VisitBinLAnd(BinaryOperator *BO) {
     {
@@ -5519,9 +5528,12 @@
 
     bool Result;
     if (!BO->getLHS()->isValueDependent() &&
-        BO->getLHS()->EvaluateAsBooleanCondition(Result, SemaRef.Context) &&
-        Result)
-      Visit(BO->getRHS());
+        BO->getLHS()->EvaluateAsBooleanCondition(Result, SemaRef.Context)) {
+      if (Result)
+        Visit(BO->getRHS());
+    } else {
+      SequenceChecker(SemaRef, BO->getRHS());
+    }
   }
 
   // Only visit the condition, unless we can be sure which subexpression will
@@ -5534,6 +5546,10 @@
     if (!CO->getCond()->isValueDependent() &&
         CO->getCond()->EvaluateAsBooleanCondition(Result, SemaRef.Context))
       Visit(Result ? CO->getTrueExpr() : CO->getFalseExpr());
+    else {
+      SequenceChecker(SemaRef, CO->getTrueExpr());
+      SequenceChecker(SemaRef, CO->getFalseExpr());
+    }
   }
 
   void VisitCXXConstructExpr(CXXConstructExpr *CCE) {

Modified: cfe/trunk/test/SemaCXX/warn-unsequenced.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unsequenced.cpp?rev=172760&r1=172759&r2=172760&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-unsequenced.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unsequenced.cpp Thu Jan 17 16:06:26 2013
@@ -88,4 +88,11 @@
   xs[0] = (a = 1, a); // ok
   (a -= 128) &= 128; // ok
   ++a += 1; // 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}}
 }