[clang] a2f32bf - [clang][Sema] SequenceChecker: C++17 sequencing rule for call expressions.

[Bruno Ricci via cfe-commits]

Author: Bruno Ricci
Date: 2020-06-03T12:35:12+01:00
New Revision: a2f32bfcc7cd6828785011ce90f6e0314d096e89

URL: https://github.com/llvm/llvm-project/commit/a2f32bfcc7cd6828785011ce90f6e0314d096e89
DIFF: https://github.com/llvm/llvm-project/commit/a2f32bfcc7cd6828785011ce90f6e0314d096e89.diff

LOG: [clang][Sema] SequenceChecker: C++17 sequencing rule for call expressions.

In C++17 the postfix-expression of a call expression is sequenced before
each expression in the expression-list and any default argument.

Differential Revision: https://reviews.llvm.org/D58579

Reviewed By: rsmith

Added: 
    

Modified: 
    clang/lib/Sema/SemaChecking.cpp
    clang/test/SemaCXX/warn-unsequenced.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index c9fa2b993c1e..64df2740145b 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12928,6 +12928,11 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> {
   }
 
   void VisitCallExpr(const CallExpr *CE) {
+    // FIXME: CXXNewExpr and CXXDeleteExpr implicitly call functions.
+
+    if (CE->isUnevaluatedBuiltinCall(Context))
+      return;
+
     // 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
@@ -12935,10 +12940,41 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> {
     //   expression or statement in the body of the function [and thus before
     //   the value computation of its result].
     SequencedSubexpression Sequenced(*this);
-    SemaRef.runWithSufficientStackSpace(CE->getExprLoc(),
-                                        [&] { Base::VisitCallExpr(CE); });
+    SemaRef.runWithSufficientStackSpace(CE->getExprLoc(), [&] {
+      // C++17 [expr.call]p5
+      //   The postfix-expression is sequenced before each expression in the
+      //   expression-list and any default argument. [...]
+      SequenceTree::Seq CalleeRegion;
+      SequenceTree::Seq OtherRegion;
+      if (SemaRef.getLangOpts().CPlusPlus17) {
+        CalleeRegion = Tree.allocate(Region);
+        OtherRegion = Tree.allocate(Region);
+      } else {
+        CalleeRegion = Region;
+        OtherRegion = Region;
+      }
+      SequenceTree::Seq OldRegion = Region;
 
-    // FIXME: CXXNewExpr and CXXDeleteExpr implicitly call functions.
+      // Visit the callee expression first.
+      Region = CalleeRegion;
+      if (SemaRef.getLangOpts().CPlusPlus17) {
+        SequencedSubexpression Sequenced(*this);
+        Visit(CE->getCallee());
+      } else {
+        Visit(CE->getCallee());
+      }
+
+      // Then visit the argument expressions.
+      Region = OtherRegion;
+      for (const Expr *Argument : CE->arguments())
+        Visit(Argument);
+
+      Region = OldRegion;
+      if (SemaRef.getLangOpts().CPlusPlus17) {
+        Tree.merge(CalleeRegion);
+        Tree.merge(OtherRegion);
+      }
+    });
   }
 
   void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {

diff  --git a/clang/test/SemaCXX/warn-unsequenced.cpp b/clang/test/SemaCXX/warn-unsequenced.cpp
index 62f725bd0712..84b0f9fd5b2f 100644
--- a/clang/test/SemaCXX/warn-unsequenced.cpp
+++ b/clang/test/SemaCXX/warn-unsequenced.cpp
@@ -15,7 +15,6 @@ struct S {
   int n;
 };
 
-// TODO: Implement the C++17 sequencing rules.
 void test() {
   int a;
   int xs[10];
@@ -256,6 +255,27 @@ void test() {
   p[i++] = (i = 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
   p++[i++] = (i = p ? i++ : i++); // cxx11-warning {{unsequenced modification and access to 'p'}}
                                   // cxx11-warning at -1 {{multiple unsequenced modifications to 'i'}}
+
+  (i++, f)(i++, 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+  (i++ + i++, f)(42, 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+                          // cxx17-warning at -1 {{multiple unsequenced modifications to 'i'}}
+  int (*pf)(int, int);
+  (pf = f)(pf != nullptr, pf != nullptr); // cxx11-warning {{unsequenced modification and access to 'pf'}}
+  pf((pf = f) != nullptr, 42); // cxx11-warning {{unsequenced modification and access to 'pf'}}
+  f((pf = f, 42), (pf = f, 42)); // cxx11-warning {{multiple unsequenced modifications to 'pf'}}
+                                 // cxx17-warning at -1 {{multiple unsequenced modifications to 'pf'}}
+  pf((pf = f) != nullptr, pf == nullptr); // cxx11-warning {{unsequenced modification and access to 'pf'}}
+                                          // cxx17-warning at -1 {{unsequenced modification and access to 'pf'}}
+}
+
+namespace PR20819 {
+  struct foo { void bar(int); };
+  foo get_foo(int);
+
+  void g() {
+    int a = 0;
+    get_foo(a).bar(a++);  // cxx11-warning {{unsequenced modification and access to 'a'}}
+  }
 }
 
 namespace members {