From 0c0b3909d11de7440d77556089516918b9c04cef Mon Sep 17 00:00:00 2001
From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Sun, 30 Jun 2013 10:40:20 +0000
Subject: [PATCH] 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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@185282 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Sema/SemaChecking.cpp         | 25 +++++++++++++++++++++----
 test/Sema/warn-unsequenced.c      |  3 +++
 test/SemaCXX/warn-unsequenced.cpp | 12 +++++++++++-
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 5afb653c10..f5296ce70f 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -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 EvaluatedExprVisitor<SequenceChecker> {
 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);
 
diff --git a/test/Sema/warn-unsequenced.c b/test/Sema/warn-unsequenced.c
index 49c7acc2d3..a14d328166 100644
--- a/test/Sema/warn-unsequenced.c
+++ b/test/Sema/warn-unsequenced.c
@@ -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}}
diff --git a/test/SemaCXX/warn-unsequenced.cpp b/test/SemaCXX/warn-unsequenced.cpp
index 26bd92ec9e..54e16a5e6d 100644
--- a/test/SemaCXX/warn-unsequenced.cpp
+++ b/test/SemaCXX/warn-unsequenced.cpp
@@ -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}}
-- 
2.40.0