Fix false positive unsequenced access and modification warning in array subscript expression.
Summary: In the [expr.sub] p1, we can read that for a given E1[E2], E1 is sequenced before E2.
Patch by Mateusz Janek.
Reviewers: rsmith, Rakete1111
Reviewed By: rsmith, Rakete1111
Subscribers: riccibruno, lebedev.ri, Rakete1111, hiraditya, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D50766
llvm-svn: 350874
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index cd96200..a47c01d 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11908,30 +11908,42 @@
notePostUse(O, E);
}
+ void VisitSequencedExpressions(Expr *SequencedBefore, Expr *SequencedAfter) {
+ SequenceTree::Seq BeforeRegion = Tree.allocate(Region);
+ SequenceTree::Seq AfterRegion = Tree.allocate(Region);
+ SequenceTree::Seq OldRegion = Region;
+
+ {
+ SequencedSubexpression SeqBefore(*this);
+ Region = BeforeRegion;
+ Visit(SequencedBefore);
+ }
+
+ Region = AfterRegion;
+ Visit(SequencedAfter);
+
+ Region = OldRegion;
+
+ 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.
- SequenceTree::Seq LHS = Tree.allocate(Region);
- SequenceTree::Seq RHS = Tree.allocate(Region);
- SequenceTree::Seq OldRegion = Region;
-
- {
- SequencedSubexpression SeqLHS(*this);
- Region = LHS;
- Visit(BO->getLHS());
- }
-
- Region = RHS;
- Visit(BO->getRHS());
-
- Region = OldRegion;
-
- // Forget that LHS and RHS are sequenced. They are both unsequenced
- // with respect to other stuff.
- Tree.merge(LHS);
- Tree.merge(RHS);
+ VisitSequencedExpressions(BO->getLHS(), BO->getRHS());
}
void VisitBinAssign(BinaryOperator *BO) {
diff --git a/clang/test/SemaCXX/warn-unsequenced-cxx17.cpp b/clang/test/SemaCXX/warn-unsequenced-cxx17.cpp
new file mode 100644
index 0000000..3c221fb
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsequenced-cxx17.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wno-unused %s
+
+void test() {
+ int xs[10];
+ int *p = xs;
+ // expected-no-diagnostics
+ p[(long long unsigned)(p = 0)]; // ok
+}
diff --git a/clang/test/SemaCXX/warn-unsequenced.cpp b/clang/test/SemaCXX/warn-unsequenced.cpp
index 9e8a5b4..6d4468c 100644
--- a/clang/test/SemaCXX/warn-unsequenced.cpp
+++ b/clang/test/SemaCXX/warn-unsequenced.cpp
@@ -81,6 +81,7 @@
int *p = xs;
a = *(a++, p); // ok
a = a++ && a; // ok
+ p[(long long unsigned)(p = 0)]; // expected-warning {{unsequenced modification and access to 'p'}}
A *q = &agg1;
(q = &agg2)->y = q->x; // expected-warning {{unsequenced modification and access to 'q'}}