r185282 - Teach -Wunsequenced that the side-effects of a function evaluation are sequenced

[Richard Smith]

Author: rsmith
Date: Sun Jun 30 05:40:20 2013
New Revision: 185282

URL: http://llvm.org/viewvc/llvm-project?rev=185282&view=rev
Log:
Teach -Wunsequenced that the side-effects of a function evaluation are sequenced
before the value computation of the result. In C, this is implied by there being
a sequence point after their evaluation, and in C++, it's implied by the
side-effects being sequenced before the expressions and statements in the
function body.

Modified:
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/Sema/warn-unsequenced.c
    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=185282&r1=185281&r2=185282&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Sun Jun 30 05:40:20 2013
@@ -5274,6 +5274,8 @@ namespace {
 /// \brief Visitor for expressions which looks for unsequenced operations on the
 /// same object.
 class SequenceChecker : public EvaluatedExprVisitor<SequenceChecker> {
+  typedef EvaluatedExprVisitor<SequenceChecker> Base;
+
   /// \brief A tree of sequenced regions within an expression. Two regions are
   /// unsequenced if one is an ancestor or a descendent of the other. When we
   /// finish processing an expression with sequencing, such as a comma
@@ -5518,9 +5520,8 @@ class SequenceChecker : public Evaluated
 public:
   SequenceChecker(Sema &S, Expr *E,
                   llvm::SmallVectorImpl<Expr*> &WorkList)
-    : EvaluatedExprVisitor<SequenceChecker>(S.Context), SemaRef(S),
-      Region(Tree.root()), ModAsSideEffect(0), WorkList(WorkList),
-      EvalTracker(0) {
+    : Base(S.Context), SemaRef(S), Region(Tree.root()),
+      ModAsSideEffect(0), WorkList(WorkList), EvalTracker(0) {
     Visit(E);
   }
 
@@ -5530,7 +5531,7 @@ public:
 
   void VisitExpr(Expr *E) {
     // By default, just recurse to evaluated subexpressions.
-    EvaluatedExprVisitor<SequenceChecker>::VisitStmt(E);
+    Base::VisitStmt(E);
   }
 
   void VisitCastExpr(CastExpr *E) {
@@ -5695,7 +5696,23 @@ public:
     }
   }
 
+  void VisitCallExpr(CallExpr *CE) {
+    // C++11 [intro.execution]p15:
+    //   When calling a function [...], every value computation and side effect
+    //   associated with any argument expression, or with the postfix expression
+    //   designating the called function, is sequenced before execution of every
+    //   expression or statement in the body of the function [and thus before
+    //   the value computation of its result].
+    SequencedSubexpression Sequenced(*this);
+    Base::VisitCallExpr(CE);
+
+    // FIXME: CXXNewExpr and CXXDeleteExpr implicitly call functions.
+  }
+
   void VisitCXXConstructExpr(CXXConstructExpr *CCE) {
+    // This is a call, so all subexpressions are sequenced before the result.
+    SequencedSubexpression Sequenced(*this);
+
     if (!CCE->isListInitialization())
       return VisitExpr(CCE);
 

Modified: cfe/trunk/test/Sema/warn-unsequenced.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-unsequenced.c?rev=185282&r1=185281&r2=185282&view=diff
==============================================================================
--- cfe/trunk/test/Sema/warn-unsequenced.c (original)
+++ cfe/trunk/test/Sema/warn-unsequenced.c Sun Jun 30 05:40:20 2013
@@ -25,6 +25,9 @@ void test() {
   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 = f(++a, 0); // ok
+  a = f(a++, 0); // ok
+  a = f(++a, a++); // expected-warning {{multiple unsequenced modifications}}
 
   a = ++a; // expected-warning {{multiple unsequenced modifications}}
   a += ++a; // expected-warning {{unsequenced modification and access}}

Modified: cfe/trunk/test/SemaCXX/warn-unsequenced.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unsequenced.cpp?rev=185282&r1=185281&r2=185282&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-unsequenced.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unsequenced.cpp Sun Jun 30 05:40:20 2013
@@ -1,12 +1,13 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wno-unused %s
 
-int f(int, int);
+int f(int, int = 0);
 
 struct A {
   int x, y;
 };
 struct S {
   S(int, int);
+  int n;
 };
 
 void test() {
@@ -32,6 +33,9 @@ void test() {
   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 = f(++a); // ok
+  a = f(a++); // ok
+  a = f(++a, a++); // expected-warning {{multiple unsequenced modifications}}
 
   // Compound assignment "A OP= B" is equivalent to "A = A OP B" except that A
   // is evaluated only once.
@@ -47,6 +51,12 @@ void test() {
   S str2 = { a++, a++ }; // ok
   S str3 = { a++ + a, a++ }; // expected-warning {{unsequenced modification and access}}
 
+  struct Z { A a; S s; } z = { { ++a, ++a }, { ++a, ++a } }; // ok
+  a = S { ++a, a++ }.n; // ok
+  A { ++a, a++ }.x; // ok
+  a = A { ++a, a++ }.x; // expected-warning {{unsequenced modifications}}
+  A { ++a, a++ }.x + A { ++a, a++ }.y; // expected-warning {{unsequenced modifications}}
+
   (xs[2] && (a = 0)) + a; // ok
   (0 && (a = 0)) + a; // ok
   (1 && (a = 0)) + a; // expected-warning {{unsequenced modification and access}}