riccibruno created this revision.
riccibruno added reviewers: rsmith, rjmccall, aaron.ballman.
riccibruno added a project: clang.
Herald added subscribers: cfe-commits, jdoerfert.
Implement the C++17 sequencing rules for the built-in operators `<<`, `>>`,
`.*`, `->*`, `=` and `op=`.
Repository:
rC Clang
https://reviews.llvm.org/D58297
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
@@ -24,7 +24,6 @@
a + a++; // cxx11-warning {{unsequenced modification and access to 'a'}}
// cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
a = a++; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // TODO cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
++ ++a; // ok
(a++, a++); // ok
++a + ++a; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
@@ -34,13 +33,10 @@
(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@-1 {{multiple unsequenced modifications to 'a'}}
(a ? xs[0] : xs[1]) = ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning@-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@-1 {{multiple unsequenced modifications to 'a'}}
f(a, a); // ok
f(a = 0, a); // cxx11-warning {{unsequenced modification and access to 'a'}}
// cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
@@ -59,7 +55,6 @@
(++a, a) += 1; // ok
a = ++a; // ok
a += ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning@-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'}}
@@ -75,7 +70,6 @@
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@-1 {{multiple unsequenced modifications to 'a'}}
A { ++a, a++ }.x + A { ++a, a++ }.y; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
// cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
@@ -110,14 +104,10 @@
a += (a++, a) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
// cxx17-warning@-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@-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
@@ -145,20 +135,14 @@
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@-1 {{unsequenced modification and access to 'a'}}
(xs[8] ? a+=1 : a) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
(xs[8] ? a : a+= 2) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning@-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@-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@-1 {{unsequenced modification and access to 'a'}}
(false ? a : a+=2) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning@-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'}}
@@ -178,19 +162,15 @@
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@-1 {{multiple unsequenced modifications to 'a'}}
a = ((a++, true) && (a++, true) && (a++, true) && a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // TODO cxx17-warning@-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@-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@-1 {{multiple unsequenced modifications to 'a'}}
a = (false || ++a); // no-warning
(a++) | (a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
@@ -205,6 +185,75 @@
(__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@-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@-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@-1 {{multiple unsequenced modifications to 'PtrMem'}}
+ (PtrMem = nullptr) == PtrMem; // cxx11-warning {{unsequenced modification and access to 'PtrMem'}}
+ // cxx17-warning@-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@-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@-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@-1 {{multiple unsequenced modifications to 'i'}}
}
namespace members {
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -12130,8 +12130,37 @@
// 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(BinaryOperator *BO) { VisitBinPtrMem(BO); }
+ void VisitBinPtrMemI(BinaryOperator *BO) { VisitBinPtrMem(BO); }
+ void VisitBinPtrMem(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(BinaryOperator *BO) { VisitBinShlShr(BO); }
+ void VisitBinShr(BinaryOperator *BO) { VisitBinShlShr(BO); }
+ void VisitBinShlShr(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(BinaryOperator *BO) {
@@ -12143,38 +12172,64 @@
}
void VisitBinAssign(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.
MemoryLocation MemoryLoc = getMemoryLocation(BO->getLHS(), /*Mod=*/true);
- if (!MemoryLoc)
- return VisitExpr(BO);
-
notePreMod(MemoryLoc, BO);
- // 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(MemoryLoc, 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());
+ }
- Visit(BO->getLHS());
+ Region = LHSRegion;
+ Visit(BO->getLHS());
- if (isa<CompoundAssignOperator>(BO))
- notePostUse(MemoryLoc, BO);
+ if (isa<CompoundAssignOperator>(BO))
+ notePostUse(MemoryLoc, BO);
+ } else {
+ // C++11 does not specify any sequencing between the LHS and RHS.
+ Region = LHSRegion;
+ Visit(BO->getLHS());
- Visit(BO->getRHS());
+ if (isa<CompoundAssignOperator>(BO))
+ notePostUse(MemoryLoc, BO);
+
+ 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.
+ Region = OldRegion;
notePostMod(MemoryLoc, BO,
SemaRef.getLangOpts().CPlusPlus ? UK_ModAsValue
: UK_ModAsSideEffect);
+ if (SemaRef.getLangOpts().CPlusPlus17) {
+ Tree.merge(RHSRegion);
+ Tree.merge(LHSRegion);
+ }
}
void VisitCompoundAssignOperator(CompoundAssignOperator *CAO) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
