[PATCH] D50766: Fix false positive unsequenced access and modification warning in array subscript expression.

[Mateusz Janek via Phabricator via cfe-commits]

stryku updated this revision to Diff 161943.
stryku added a comment.

@Rakete1111 Thank you for the comments. You're perfectly right, there was an issue in my code. I've fixed it and also E1 and E2 will be sequenced only in C++ >= 17.


https://reviews.llvm.org/D50766

Files:
  lib/Sema/SemaChecking.cpp


Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -11658,30 +11658,42 @@
       notePostUse(O, E);
   }
 
-  void VisitBinComma(BinaryOperator *BO) {
-    // C++11 [expr.comma]p1:
-    //   Every value computation and side effect associated with the left
-    //   expression is sequenced before every value computation and side
-    //   effect associated with the right expression.
-    SequenceTree::Seq LHS = Tree.allocate(Region);
-    SequenceTree::Seq RHS = Tree.allocate(Region);
+  void VisitSequencedExpressions(Expr *SequencedBefore, Expr *SequencedAfter) {
+    SequenceTree::Seq BeforeRegion = Tree.allocate(Region);
+    SequenceTree::Seq AfterRegion = Tree.allocate(Region);
     SequenceTree::Seq OldRegion = Region;
 
     {
-      SequencedSubexpression SeqLHS(*this);
-      Region = LHS;
-      Visit(BO->getLHS());
+      SequencedSubexpression SeqBefore(*this);
+      Region = BeforeRegion;
+      Visit(SequencedBefore);
     }
 
-    Region = RHS;
-    Visit(BO->getRHS());
+    Region = AfterRegion;
+    Visit(SequencedAfter);
 
     Region = OldRegion;
 
-    // Forget that LHS and RHS are sequenced. They are both unsequenced
-    // with respect to other stuff.
-    Tree.merge(LHS);
-    Tree.merge(RHS);
+    Tree.merge(BeforeRegion);
+    Tree.merge(AfterRegion);
+  }
+
+  void VisitArraySubscriptExpr(ArraySubscriptExpr *ASE) {
+    // C++17 [expr.sub]p1:
+    //   The expression E1[E2] is identical (by definition) to *((E1)+(E2)). The
+    //   expression E1 is sequenced before the expression E2.
+    if (SemaRef.getLangOpts().CPlusPlus17)
+      VisitSequencedExpressions(ASE->getLHS(), ASE->getRHS());
+    else
+      Base::VisitStmt(ASE);
+  }
+
+  void VisitBinComma(BinaryOperator *BO) {
+    // C++11 [expr.comma]p1:
+    //   Every value computation and side effect associated with the left
+    //   expression is sequenced before every value computation and side
+    //   effect associated with the right expression.
+    VisitSequencedExpressions(BO->getLHS(), BO->getRHS());
   }
 
   void VisitBinAssign(BinaryOperator *BO) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50766.161943.patch
Type: text/x-patch
Size: 2195 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180822/f54c2fc3/attachment.bin>