[clang] 7394c15 - [Sema] SequenceChecker: C++17 sequencing rules for built-in operators <<, >>, .*, ->*, =, op=
Bruno Ricci via cfe-commits
cfe-commits at lists.llvm.org
Sun Dec 22 04:50:30 PST 2019
Author: Bruno Ricci
Date: 2019-12-22T12:41:14Z
New Revision: 7394c15178ed9cb7bd04585526a1e73396e60e17
URL: https://github.com/llvm/llvm-project/commit/7394c15178ed9cb7bd04585526a1e73396e60e17
DIFF: https://github.com/llvm/llvm-project/commit/7394c15178ed9cb7bd04585526a1e73396e60e17.diff
LOG: [Sema] SequenceChecker: C++17 sequencing rules for built-in operators <<, >>, .*, ->*, =, op=
Implement the C++17 sequencing rules for the built-in operators <<, >>, .*,
->*, = and op=.
Differential Revision: https://reviews.llvm.org/D58297
Reviewed By: rsmith
Added:
Modified:
clang/lib/Sema/SemaChecking.cpp
clang/test/CXX/drs/dr2xx.cpp
clang/test/CXX/drs/dr6xx.cpp
clang/test/SemaCXX/warn-unsequenced.cpp
Removed:
################################################################################
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2cbe3632be3a..b8295bc741eb 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12832,8 +12832,37 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> {
// expression E1 is sequenced before the expression E2.
if (SemaRef.getLangOpts().CPlusPlus17)
VisitSequencedExpressions(ASE->getLHS(), ASE->getRHS());
- else
- Base::VisitStmt(ASE);
+ else {
+ Visit(ASE->getLHS());
+ Visit(ASE->getRHS());
+ }
+ }
+
+ void VisitBinPtrMemD(const BinaryOperator *BO) { VisitBinPtrMem(BO); }
+ void VisitBinPtrMemI(const BinaryOperator *BO) { VisitBinPtrMem(BO); }
+ void VisitBinPtrMem(const BinaryOperator *BO) {
+ // C++17 [expr.mptr.oper]p4:
+ // Abbreviating pm-expression.*cast-expression as E1.*E2, [...]
+ // the expression E1 is sequenced before the expression E2.
+ if (SemaRef.getLangOpts().CPlusPlus17)
+ VisitSequencedExpressions(BO->getLHS(), BO->getRHS());
+ else {
+ Visit(BO->getLHS());
+ Visit(BO->getRHS());
+ }
+ }
+
+ void VisitBinShl(const BinaryOperator *BO) { VisitBinShlShr(BO); }
+ void VisitBinShr(const BinaryOperator *BO) { VisitBinShlShr(BO); }
+ void VisitBinShlShr(const BinaryOperator *BO) {
+ // C++17 [expr.shift]p4:
+ // The expression E1 is sequenced before the expression E2.
+ if (SemaRef.getLangOpts().CPlusPlus17)
+ VisitSequencedExpressions(BO->getLHS(), BO->getRHS());
+ else {
+ Visit(BO->getLHS());
+ Visit(BO->getRHS());
+ }
}
void VisitBinComma(const BinaryOperator *BO) {
@@ -12845,38 +12874,67 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> {
}
void VisitBinAssign(const BinaryOperator *BO) {
- // The modification is sequenced after the value computation of the LHS
- // and RHS, so check it before inspecting the operands and update the
+ SequenceTree::Seq RHSRegion;
+ SequenceTree::Seq LHSRegion;
+ if (SemaRef.getLangOpts().CPlusPlus17) {
+ RHSRegion = Tree.allocate(Region);
+ LHSRegion = Tree.allocate(Region);
+ } else {
+ RHSRegion = Region;
+ LHSRegion = Region;
+ }
+ SequenceTree::Seq OldRegion = Region;
+
+ // C++11 [expr.ass]p1:
+ // [...] the assignment is sequenced after the value computation
+ // of the right and left operands, [...]
+ //
+ // so check it before inspecting the operands and update the
// map afterwards.
- Object O = getObject(BO->getLHS(), true);
- if (!O)
- return VisitExpr(BO);
+ Object O = getObject(BO->getLHS(), /*Mod=*/true);
+ if (O)
+ notePreMod(O, BO);
+
+ if (SemaRef.getLangOpts().CPlusPlus17) {
+ // C++17 [expr.ass]p1:
+ // [...] The right operand is sequenced before the left operand. [...]
+ {
+ SequencedSubexpression SeqBefore(*this);
+ Region = RHSRegion;
+ Visit(BO->getRHS());
+ }
- notePreMod(O, BO);
+ Region = LHSRegion;
+ Visit(BO->getLHS());
- // C++11 [expr.ass]p7:
- // E1 op= E2 is equivalent to E1 = E1 op E2, except that E1 is evaluated
- // only once.
- //
- // Therefore, for a compound assignment operator, O is considered used
- // everywhere except within the evaluation of E1 itself.
- if (isa<CompoundAssignOperator>(BO))
- notePreUse(O, BO);
+ if (O && isa<CompoundAssignOperator>(BO))
+ notePostUse(O, BO);
- Visit(BO->getLHS());
+ } else {
+ // C++11 does not specify any sequencing between the LHS and RHS.
+ Region = LHSRegion;
+ Visit(BO->getLHS());
- if (isa<CompoundAssignOperator>(BO))
- notePostUse(O, BO);
+ if (O && isa<CompoundAssignOperator>(BO))
+ notePostUse(O, BO);
- Visit(BO->getRHS());
+ Region = RHSRegion;
+ Visit(BO->getRHS());
+ }
// C++11 [expr.ass]p1:
- // the assignment is sequenced [...] before the value computation of the
- // assignment expression.
+ // the assignment is sequenced [...] before the value computation of the
+ // assignment expression.
// C11 6.5.16/3 has no such rule.
- notePostMod(O, BO,
- SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue
- : UK_ModAsSideEffect);
+ Region = OldRegion;
+ if (O)
+ notePostMod(O, BO,
+ SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue
+ : UK_ModAsSideEffect);
+ if (SemaRef.getLangOpts().CPlusPlus17) {
+ Tree.merge(RHSRegion);
+ Tree.merge(LHSRegion);
+ }
}
void VisitCompoundAssignOperator(const CompoundAssignOperator *CAO) {
diff --git a/clang/test/CXX/drs/dr2xx.cpp b/clang/test/CXX/drs/dr2xx.cpp
index 89e13367a474..1f625efe2b55 100644
--- a/clang/test/CXX/drs/dr2xx.cpp
+++ b/clang/test/CXX/drs/dr2xx.cpp
@@ -225,11 +225,17 @@ namespace dr222 { // dr222: dup 637
void((a += b) += c);
void((a += b) + (a += c)); // expected-warning {{multiple unsequenced modifications to 'a'}}
- x[a++] = a; // expected-warning {{unsequenced modification and access to 'a'}}
+ x[a++] = a;
+#if __cplusplus < 201703L
+ // expected-warning at -2 {{unsequenced modification and access to 'a'}}
+#endif
a = b = 0; // ok, read and write of 'b' are sequenced
- a = (b = a++); // expected-warning {{multiple unsequenced modifications to 'a'}}
+ a = (b = a++);
+#if __cplusplus < 201703L
+ // expected-warning at -2 {{multiple unsequenced modifications to 'a'}}
+#endif
a = (b = ++a);
#pragma clang diagnostic pop
}
diff --git a/clang/test/CXX/drs/dr6xx.cpp b/clang/test/CXX/drs/dr6xx.cpp
index 31e3571f500a..6ff162545826 100644
--- a/clang/test/CXX/drs/dr6xx.cpp
+++ b/clang/test/CXX/drs/dr6xx.cpp
@@ -317,7 +317,10 @@ namespace dr635 { // dr635: yes
namespace dr637 { // dr637: yes
void f(int i) {
i = ++i + 1;
- i = i++ + 1; // expected-warning {{unsequenced}}
+ i = i++ + 1;
+#if __cplusplus < 201703L
+ // expected-warning at -2 {{unsequenced}}
+#endif
}
}
diff --git a/clang/test/SemaCXX/warn-unsequenced.cpp b/clang/test/SemaCXX/warn-unsequenced.cpp
index 43b36009fe30..62f725bd0712 100644
--- a/clang/test/SemaCXX/warn-unsequenced.cpp
+++ b/clang/test/SemaCXX/warn-unsequenced.cpp
@@ -26,7 +26,6 @@ void test() {
a + a++; // cxx11-warning {{unsequenced modification and access to 'a'}}
// cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
a = a++; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // TODO cxx17-warning at -1 {{multiple unsequenced modifications to 'a'}}
++ ++a; // ok
(a++, a++); // ok
++a + ++a; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
@@ -36,13 +35,10 @@ void test() {
(a++, a) = 0; // ok, increment is sequenced before value computation of LHS
a = xs[++a]; // ok
a = xs[a++]; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // TODO cxx17-warning at -1 {{multiple unsequenced modifications to 'a'}}
(a ? xs[0] : xs[1]) = ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
a = (++a, ++a); // ok
a = (a++, ++a); // ok
a = (a++, a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // TODO cxx17-warning at -1 {{multiple unsequenced modifications to 'a'}}
f(a, a); // ok
f(a = 0, a); // cxx11-warning {{unsequenced modification and access to 'a'}}
// cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
@@ -61,7 +57,6 @@ void test() {
(++a, a) += 1; // ok
a = ++a; // ok
a += ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
A agg1 = { a++, a++ }; // ok
A agg2 = { a++ + a, a++ }; // cxx11-warning {{unsequenced modification and access to 'a'}}
@@ -77,7 +72,6 @@ void test() {
a = S { ++a, a++ }.n; // ok
A { ++a, a++ }.x; // ok
a = A { ++a, a++ }.x; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // TODO cxx17-warning at -1 {{multiple unsequenced modifications to 'a'}}
A { ++a, a++ }.x + A { ++a, a++ }.y; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
// cxx17-warning at -1 {{multiple unsequenced modifications to 'a'}}
@@ -112,14 +106,10 @@ void test() {
a += (a++, a) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
// cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
- int *p = xs;
- a = *(a++, p); // ok
a = a++ && a; // ok
- p[(long long unsigned)(p = 0)]; // cxx11-warning {{unsequenced modification and access to 'p'}}
A *q = &agg1;
(q = &agg2)->y = q->x; // cxx11-warning {{unsequenced modification and access to 'q'}}
- // TODO cxx17-warning at -1 {{unsequenced modification and access to 'q'}}
// This has undefined behavior if a == 0; otherwise, the side-effect of the
// increment is sequenced before the value computation of 'f(a, a)', which is
@@ -147,20 +137,14 @@ void test() {
xs[8] ? ++a : a++; // no-warning
xs[8] ? a+=1 : a+= 2; // no-warning
(xs[8] ? a+=1 : a+= 2) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
(xs[8] ? a+=1 : a) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
(xs[8] ? a : a+= 2) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
a = (xs[8] ? a+=1 : a+= 2); // no-warning
a += (xs[8] ? a+=1 : a+= 2); // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
(false ? a+=1 : a) = a; // no-warning
(true ? a+=1 : a) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
(false ? a : a+=2) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
(true ? a : a+=2) = a; // no-warning
xs[8] && (++a + a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
@@ -180,19 +164,15 @@ void test() {
a = ((a++, false) || (a++, false) || (a++, false) || (a++, false)); // no-warning
a = ((a++, true) && (a++, true) && (a++, true) && (a++, true)); // no-warning
a = ((a++, false) || (a++, false) || (a++, false) || a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // TODO cxx17-warning at -1 {{multiple unsequenced modifications to 'a'}}
a = ((a++, true) && (a++, true) && (a++, true) && a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // TODO cxx17-warning at -1 {{multiple unsequenced modifications to 'a'}}
a = ((a++, false) || (a++, false) || (a++, false) || (a + a, false)); // no-warning
a = ((a++, true) && (a++, true) && (a++, true) && (a + a, true)); // no-warning
a = (false && a++); // no-warning
a = (true && a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // TODO cxx17-warning at -1 {{multiple unsequenced modifications to 'a'}}
a = (true && ++a); // no-warning
a = (true || a++); // no-warning
a = (false || a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // TODO cxx17-warning at -1 {{multiple unsequenced modifications to 'a'}}
a = (false || ++a); // no-warning
(a++) | (a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
@@ -207,6 +187,75 @@ void test() {
(__builtin_object_size(&(++a, a), 0) ? 1 : 0) + ++a; // ok
(__builtin_expect(++a, 0) ? 1 : 0) + ++a; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
// cxx17-warning at -1 {{multiple unsequenced modifications to 'a'}}
+
+
+ int *p = xs;
+ a = *(a++, p); // no-warning
+ p[(long long unsigned)(p = 0)]; // cxx11-warning {{unsequenced modification and access to 'p'}}
+ (i++, xs)[i++]; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ (++i, xs)[++i]; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ (i, xs)[++i + ++i]; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ // cxx17-warning at -1 {{multiple unsequenced modifications to 'i'}}
+ p++[p == xs]; // cxx11-warning {{unsequenced modification and access to 'p'}}
+ ++p[p++ == xs]; // cxx11-warning {{unsequenced modification and access to 'p'}}
+
+ struct S { int x; } s, *ps = &s;
+ int (S::*PtrMem);
+ (PtrMem = &S::x ,s).*(PtrMem); // cxx11-warning {{unsequenced modification and access to 'PtrMem'}}
+ (PtrMem = &S::x ,s).*(PtrMem = &S::x); // cxx11-warning {{multiple unsequenced modifications to 'PtrMem'}}
+ (PtrMem = &S::x ,ps)->*(PtrMem); // cxx11-warning {{unsequenced modification and access to 'PtrMem'}}
+ (PtrMem = &S::x ,ps)->*(PtrMem = &S::x); // cxx11-warning {{multiple unsequenced modifications to 'PtrMem'}}
+ (PtrMem = nullptr) == (PtrMem = nullptr); // cxx11-warning {{multiple unsequenced modifications to 'PtrMem'}}
+ // cxx17-warning at -1 {{multiple unsequenced modifications to 'PtrMem'}}
+ (PtrMem = nullptr) == PtrMem; // cxx11-warning {{unsequenced modification and access to 'PtrMem'}}
+ // cxx17-warning at -1 {{unsequenced modification and access to 'PtrMem'}}
+
+ i++ << i++; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ ++i << ++i; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ i++ << i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i << i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i++ >> i++; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ ++i >> ++i; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ i++ >> i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i >> i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ (i++ << i) + i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ // cxx17-warning at -1 {{unsequenced modification and access to 'i'}}
+ (i++ << i) << i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+
+ ++i = i++; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ i = i+= 1; // no-warning
+ i = i++ + ++i; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ // cxx17-warning at -1 {{multiple unsequenced modifications to 'i'}}
+ ++i += ++i; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ ++i += i++; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ (i++, i) += ++i; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ (i++, i) += i++; // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+ i += i+= 1; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i += i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i += ++i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i -= i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i -= ++i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i *= i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i *= ++i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i /= i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i /= ++i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i %= i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i %= ++i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i ^= i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i ^= ++i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i |= i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i |= ++i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i &= i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i &= ++i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i <<= i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i <<= ++i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i >>= i++; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ i >>= ++i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+
+ p[i++] = i; // cxx11-warning {{unsequenced modification and access to 'i'}}
+ 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'}}
}
namespace members {
More information about the cfe-commits
mailing list