[PATCH] D58579: [Sema] SequenceChecker: C++17 sequencing rule for call expression.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 190671.
riccibruno added a comment.

Don't visit the pre-argument expression(s). A pre-argument expression is only present for `CUDAKernelCallExpr`, and it is just a `CallExpr` for the configuration call, which I don't think we need to visit. This makes the usage of CallExpr::arguments() possible and results in cleaner code.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58579/new/

https://reviews.llvm.org/D58579

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


Index: test/SemaCXX/warn-unsequenced.cpp
===================================================================
--- test/SemaCXX/warn-unsequenced.cpp
+++ test/SemaCXX/warn-unsequenced.cpp
@@ -15,7 +15,6 @@
   int n;
 };
 
-// TODO: Implement the C++17 sequencing rules.
 void test() {
   int a;
   int xs[10];
@@ -256,6 +255,17 @@
   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 members {
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -12529,6 +12529,11 @@
   }
 
   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
@@ -12536,9 +12541,40 @@
     //   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.
+    // 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;
+
+    // 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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58579.190671.patch
Type: text/x-patch
Size: 3535 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190314/6ea48363/attachment-0001.bin>