mboehme created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
As designated initializers show up only in the syntactic form of the
InitListExpr, we need to make sure we're searching both forms of the
InitListExpr when determining successors in the evaluation order.
This fixes a bug in bugprone-use-after-move where previously we erroneously
concluded that two designated initializers were unsequenced. The newly added
tests fail without the fix.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D145906
Files:
clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
@@ -1155,18 +1155,32 @@
int i;
A a;
};
- A a;
- S1 s1{a.getInt(), std::move(a)};
+ {
+ A a;
+ S1 s1{a.getInt(), std::move(a)};
+ }
+ {
+ A a;
+ S1 s1{.i = a.getInt(), .a = std::move(a)};
+ }
}
{
struct S2 {
A a;
int i;
};
- A a;
- S2 s2{std::move(a), a.getInt()};
- // CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
- // CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
+ {
+ A a;
+ S2 s2{std::move(a), a.getInt()};
+ // CHECK-NOTES: [[@LINE-1]]:27: warning: 'a' used after it was moved
+ // CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
+ }
+ {
+ A a;
+ S2 s2{.a = std::move(a), .i = a.getInt()};
+ // CHECK-NOTES: [[@LINE-1]]:37: warning: 'a' used after it was moved
+ // CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
+ }
}
}
Index: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
+++ clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
@@ -8,6 +8,7 @@
#include "ExprSequence.h"
#include "clang/AST/ParentMapContext.h"
+#include "llvm/ADT/SmallVector.h"
#include <optional>
namespace clang::tidy::utils {
@@ -49,6 +50,7 @@
}
namespace {
+
bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor,
ASTContext *Context) {
if (Descendant == Ancestor)
@@ -60,6 +62,17 @@
return false;
}
+
+llvm::SmallVector<const InitListExpr *>
+allInitListExprForms(const InitListExpr *InitList) {
+ llvm::SmallVector<const InitListExpr *> result = {InitList};
+ if (InitList->isSemanticForm() && InitList->getSyntacticForm())
+ result.push_back(InitList->getSyntacticForm());
+ if (InitList->isSyntacticForm() && InitList->getSemanticForm())
+ result.push_back(InitList->getSemanticForm());
+ return result;
+}
+
} // namespace
ExprSequence::ExprSequence(const CFG *TheCFG, const Stmt *Root,
@@ -111,9 +124,12 @@
} else if (const auto *InitList = dyn_cast<InitListExpr>(Parent)) {
// Initializer list: Each initializer clause is sequenced after the
// clauses that precede it.
- for (unsigned I = 1; I < InitList->getNumInits(); ++I) {
- if (InitList->getInit(I - 1) == S)
- return InitList->getInit(I);
+ for (const InitListExpr *Form : allInitListExprForms(InitList)) {
+ for (unsigned I = 1; I < Form->getNumInits(); ++I) {
+ if (Form->getInit(I - 1) == S) {
+ return Form->getInit(I);
+ }
+ }
}
} else if (const auto *Compound = dyn_cast<CompoundStmt>(Parent)) {
// Compound statement: Each sub-statement is sequenced after the
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
@@ -1155,18 +1155,32 @@
int i;
A a;
};
- A a;
- S1 s1{a.getInt(), std::move(a)};
+ {
+ A a;
+ S1 s1{a.getInt(), std::move(a)};
+ }
+ {
+ A a;
+ S1 s1{.i = a.getInt(), .a = std::move(a)};
+ }
}
{
struct S2 {
A a;
int i;
};
- A a;
- S2 s2{std::move(a), a.getInt()};
- // CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
- // CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
+ {
+ A a;
+ S2 s2{std::move(a), a.getInt()};
+ // CHECK-NOTES: [[@LINE-1]]:27: warning: 'a' used after it was moved
+ // CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
+ }
+ {
+ A a;
+ S2 s2{.a = std::move(a), .i = a.getInt()};
+ // CHECK-NOTES: [[@LINE-1]]:37: warning: 'a' used after it was moved
+ // CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
+ }
}
}
Index: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
+++ clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
@@ -8,6 +8,7 @@
#include "ExprSequence.h"
#include "clang/AST/ParentMapContext.h"
+#include "llvm/ADT/SmallVector.h"
#include <optional>
namespace clang::tidy::utils {
@@ -49,6 +50,7 @@
}
namespace {
+
bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor,
ASTContext *Context) {
if (Descendant == Ancestor)
@@ -60,6 +62,17 @@
return false;
}
+
+llvm::SmallVector<const InitListExpr *>
+allInitListExprForms(const InitListExpr *InitList) {
+ llvm::SmallVector<const InitListExpr *> result = {InitList};
+ if (InitList->isSemanticForm() && InitList->getSyntacticForm())
+ result.push_back(InitList->getSyntacticForm());
+ if (InitList->isSyntacticForm() && InitList->getSemanticForm())
+ result.push_back(InitList->getSemanticForm());
+ return result;
+}
+
} // namespace
ExprSequence::ExprSequence(const CFG *TheCFG, const Stmt *Root,
@@ -111,9 +124,12 @@
} else if (const auto *InitList = dyn_cast<InitListExpr>(Parent)) {
// Initializer list: Each initializer clause is sequenced after the
// clauses that precede it.
- for (unsigned I = 1; I < InitList->getNumInits(); ++I) {
- if (InitList->getInit(I - 1) == S)
- return InitList->getInit(I);
+ for (const InitListExpr *Form : allInitListExprForms(InitList)) {
+ for (unsigned I = 1; I < Form->getNumInits(); ++I) {
+ if (Form->getInit(I - 1) == S) {
+ return Form->getInit(I);
+ }
+ }
}
} else if (const auto *Compound = dyn_cast<CompoundStmt>(Parent)) {
// Compound statement: Each sub-statement is sequenced after the
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
