Skip to content
Permalink
Browse files

[clang-tidy] Fix bug in bugprone-use-after-move check

Summary:
The bugprone-use-after-move check exhibits false positives for certain uses of
the C++17 if/switch init statements. These false positives are caused by a bug
in the ExprSequence calculations.

This revision adds tests for the false positives and fixes the corresponding
sequence calculation.

Reviewers: gribozavr

Subscribers: xazax.hun, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D67292

llvm-svn: 371396
  • Loading branch information...
ymand committed Sep 9, 2019
1 parent 1ad508e commit f9ce864558ab4110233e86f6391ced320dd1e07b
@@ -145,17 +145,24 @@ const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const {
return ForRange->getBody();
} else if (const auto *TheIfStmt = dyn_cast<IfStmt>(Parent)) {
// If statement:
// - Sequence init statement before variable declaration.
// - Sequence init statement before variable declaration, if present;
// before condition evaluation, otherwise.
// - Sequence variable declaration (along with the expression used to
// initialize it) before the evaluation of the condition.
if (S == TheIfStmt->getInit())
return TheIfStmt->getConditionVariableDeclStmt();
if (S == TheIfStmt->getInit()) {
if (TheIfStmt->getConditionVariableDeclStmt() != nullptr)
return TheIfStmt->getConditionVariableDeclStmt();
return TheIfStmt->getCond();
}
if (S == TheIfStmt->getConditionVariableDeclStmt())
return TheIfStmt->getCond();
} else if (const auto *TheSwitchStmt = dyn_cast<SwitchStmt>(Parent)) {
// Ditto for switch statements.
if (S == TheSwitchStmt->getInit())
return TheSwitchStmt->getConditionVariableDeclStmt();
if (S == TheSwitchStmt->getInit()) {
if (TheSwitchStmt->getConditionVariableDeclStmt() != nullptr)
return TheSwitchStmt->getConditionVariableDeclStmt();
return TheSwitchStmt->getCond();
}
if (S == TheSwitchStmt->getConditionVariableDeclStmt())
return TheSwitchStmt->getCond();
} else if (const auto *TheWhileStmt = dyn_cast<WhileStmt>(Parent)) {
@@ -1182,23 +1182,55 @@ void forRangeSequences() {

// If a variable is declared in an if, while or switch statement, the init
// statement (for if and switch) is sequenced before the variable declaration,
// which in turn is sequenced before the evaluation of the condition.
// which in turn is sequenced before the evaluation of the condition. We place
// all tests inside a for loop to ensure that the checker understands the
// sequencing. If it didn't, then the loop would trigger the "moved twice"
// logic.
void ifWhileAndSwitchSequenceInitDeclAndCondition() {
for (int i = 0; i < 10; ++i) {
A a1;
if (A a2 = std::move(a1)) {
std::move(a2);
}
}
for (int i = 0; i < 10; ++i) {
A a1;
if (A a2 = std::move(a1); a2) {
std::move(a2);
}
}
for (int i = 0; i < 10; ++i) {
A a1;
if (A a2 = std::move(a1); A a3 = std::move(a2)) {
std::move(a3);
}
}
for (int i = 0; i < 10; ++i) {
// init followed by condition with move, but without variable declaration.
if (A a1; A(std::move(a1)).getInt() > 0) {}
}
for (int i = 0; i < 10; ++i) {
if (A a1; A(std::move(a1)).getInt() > a1.getInt()) {}
// CHECK-NOTES: [[@LINE-1]]:43: warning: 'a1' used after it was moved
// CHECK-NOTES: [[@LINE-2]]:15: note: move occurred here
// CHECK-NOTES: [[@LINE-3]]:43: note: the use and move are unsequenced
}
for (int i = 0; i < 10; ++i) {
A a1;
if (A a2 = std::move(a1); A(a1) > 0) {}
// CHECK-NOTES: [[@LINE-1]]:33: warning: 'a1' used after it was moved
// CHECK-NOTES: [[@LINE-2]]:16: note: move occurred here
}
while (A a = A()) {
std::move(a);
}
for (int i = 0; i < 10; ++i) {
A a1;
switch (A a2 = std::move(a1); a2) {
case true:
std::move(a2);
}
}
for (int i = 0; i < 10; ++i) {
A a1;
switch (A a2 = a1; A a3 = std::move(a2)) {

0 comments on commit f9ce864

Please sign in to comment.
You can’t perform that action at this time.