diff options
author | Nicolas Lesser <blitzrakete@gmail.com> | 2019-01-10 19:03:33 +0000 |
---|---|---|
committer | Nicolas Lesser <blitzrakete@gmail.com> | 2019-01-10 19:03:33 +0000 |
commit | b18291aadb475c37d3317eef14b5917168643958 (patch) | |
tree | 81b4d5be9fc72810b864a6e40f50c990bd319c6e | |
parent | 3bdcb479cea4860f693fe95a95ebc8e8c4dc3656 (diff) |
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
-rw-r--r-- | clang/lib/Sema/SemaChecking.cpp | 44 | ||||
-rw-r--r-- | clang/test/SemaCXX/warn-unsequenced-cxx17.cpp | 8 | ||||
-rw-r--r-- | clang/test/SemaCXX/warn-unsequenced.cpp | 1 |
3 files changed, 37 insertions, 16 deletions
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index cd96200b817..a47c01daf0a 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -11908,30 +11908,42 @@ public: 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) { diff --git a/clang/test/SemaCXX/warn-unsequenced-cxx17.cpp b/clang/test/SemaCXX/warn-unsequenced-cxx17.cpp new file mode 100644 index 00000000000..3c221fb8d65 --- /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 9e8a5b46c21..6d4468cabfe 100644 --- a/clang/test/SemaCXX/warn-unsequenced.cpp +++ b/clang/test/SemaCXX/warn-unsequenced.cpp @@ -81,6 +81,7 @@ void test() { 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'}} |