Skip to content

Commit c8f81ee

Browse files
committed
[clang-tidy] bugprone-use-after-move: Ctor arguments should be sequenced if ctor call is written as list-initialization.
See https://timsong-cpp.github.io/cppwp/n4868/dcl.init#list-4 This eliminates a false positive in bugprone-use-after-move; this newly added test used to be falsely classified as a use-after-move: ``` A a; S3 s3{a.getInt(), std::move(a)}; ``` Reviewed By: PiotrZSL Differential Revision: https://reviews.llvm.org/D148110
1 parent c3f1faf commit c8f81ee

File tree

3 files changed

+62
-30
lines changed

3 files changed

+62
-30
lines changed

clang-tools-extra/clang-tidy/utils/ExprSequence.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,16 @@ const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const {
131131
}
132132
}
133133
}
134+
} else if (const auto *ConstructExpr = dyn_cast<CXXConstructExpr>(Parent)) {
135+
// Constructor arguments are sequenced if the constructor call is written
136+
// as list-initialization.
137+
if (ConstructExpr->isListInitialization()) {
138+
for (unsigned I = 1; I < ConstructExpr->getNumArgs(); ++I) {
139+
if (ConstructExpr->getArg(I - 1) == S) {
140+
return ConstructExpr->getArg(I);
141+
}
142+
}
143+
}
134144
} else if (const auto *Compound = dyn_cast<CompoundStmt>(Parent)) {
135145
// Compound statement: Each sub-statement is sequenced after the
136146
// statements that precede it.

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,8 +229,9 @@ Changes in existing checks
229229
to ``std::forward``.
230230

231231
- Improved :doc:`bugprone-use-after-move
232-
<clang-tidy/checks/bugprone/use-after-move>` check to also cover constructor
233-
initializers.
232+
<clang-tidy/checks/bugprone/use-after-move>` check. Detect uses and moves in
233+
constructor initializers. Correctly handle constructor arguments as being
234+
sequenced when constructor call is written as list-initialization.
234235

235236
- Deprecated :doc:`cert-dcl21-cpp
236237
<clang-tidy/checks/cert/dcl21-cpp>` check.

clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp

Lines changed: 49 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,42 +1160,63 @@ void commaOperatorSequences() {
11601160
}
11611161
}
11621162

1163+
namespace InitializerListSequences {
1164+
1165+
struct S1 {
1166+
int i;
1167+
A a;
1168+
};
1169+
1170+
struct S2 {
1171+
A a;
1172+
int i;
1173+
};
1174+
1175+
struct S3 {
1176+
S3() {}
1177+
S3(int, A) {}
1178+
S3(A, int) {}
1179+
};
1180+
11631181
// An initializer list sequences its initialization clauses.
11641182
void initializerListSequences() {
11651183
{
1166-
struct S1 {
1167-
int i;
1168-
A a;
1169-
};
1170-
{
1171-
A a;
1172-
S1 s1{a.getInt(), std::move(a)};
1173-
}
1174-
{
1175-
A a;
1176-
S1 s1{.i = a.getInt(), .a = std::move(a)};
1177-
}
1184+
A a;
1185+
S1 s1{a.getInt(), std::move(a)};
11781186
}
11791187
{
1180-
struct S2 {
1181-
A a;
1182-
int i;
1183-
};
1184-
{
1185-
A a;
1186-
S2 s2{std::move(a), a.getInt()};
1187-
// CHECK-NOTES: [[@LINE-1]]:27: warning: 'a' used after it was moved
1188-
// CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
1189-
}
1190-
{
1191-
A a;
1192-
S2 s2{.a = std::move(a), .i = a.getInt()};
1193-
// CHECK-NOTES: [[@LINE-1]]:37: warning: 'a' used after it was moved
1194-
// CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
1195-
}
1188+
A a;
1189+
S1 s1{.i = a.getInt(), .a = std::move(a)};
1190+
}
1191+
{
1192+
A a;
1193+
S2 s2{std::move(a), a.getInt()};
1194+
// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
1195+
// CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
1196+
}
1197+
{
1198+
A a;
1199+
S2 s2{.a = std::move(a), .i = a.getInt()};
1200+
// CHECK-NOTES: [[@LINE-1]]:35: warning: 'a' used after it was moved
1201+
// CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
1202+
}
1203+
{
1204+
// Check the case where the constructed type has a constructor and the
1205+
// initializer list therefore manifests as a `CXXConstructExpr` instead of
1206+
// an `InitListExpr`.
1207+
A a;
1208+
S3 s3{a.getInt(), std::move(a)};
1209+
}
1210+
{
1211+
A a;
1212+
S3 s3{std::move(a), a.getInt()};
1213+
// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
1214+
// CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
11961215
}
11971216
}
11981217

1218+
} // namespace InitializerListSequences
1219+
11991220
// A declaration statement containing multiple declarations sequences the
12001221
// initializer expressions.
12011222
void declarationSequences() {

0 commit comments

Comments
 (0)