Author: Martin Braenne Date: 2023-05-04T12:05:39Z New Revision: c8f81ee1da325760a4bbd83c8c54ecc9645f8e20
URL: https://github.com/llvm/llvm-project/commit/c8f81ee1da325760a4bbd83c8c54ecc9645f8e20 DIFF: https://github.com/llvm/llvm-project/commit/c8f81ee1da325760a4bbd83c8c54ecc9645f8e20.diff LOG: [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 Added: Modified: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp b/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp index f9555be57e738..50df451ecfa26 100644 --- a/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp +++ b/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp @@ -131,6 +131,16 @@ const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const { } } } + } else if (const auto *ConstructExpr = dyn_cast<CXXConstructExpr>(Parent)) { + // Constructor arguments are sequenced if the constructor call is written + // as list-initialization. + if (ConstructExpr->isListInitialization()) { + for (unsigned I = 1; I < ConstructExpr->getNumArgs(); ++I) { + if (ConstructExpr->getArg(I - 1) == S) { + return ConstructExpr->getArg(I); + } + } + } } else if (const auto *Compound = dyn_cast<CompoundStmt>(Parent)) { // Compound statement: Each sub-statement is sequenced after the // statements that precede it. diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e4220b3134714..98f18cc539faf 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -229,8 +229,9 @@ Changes in existing checks to ``std::forward``. - Improved :doc:`bugprone-use-after-move - <clang-tidy/checks/bugprone/use-after-move>` check to also cover constructor - initializers. + <clang-tidy/checks/bugprone/use-after-move>` check. Detect uses and moves in + constructor initializers. Correctly handle constructor arguments as being + sequenced when constructor call is written as list-initialization. - Deprecated :doc:`cert-dcl21-cpp <clang-tidy/checks/cert/dcl21-cpp>` check. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp index 1e0831048dbd4..00b1da1e727e4 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp @@ -1160,42 +1160,63 @@ void commaOperatorSequences() { } } +namespace InitializerListSequences { + +struct S1 { + int i; + A a; +}; + +struct S2 { + A a; + int i; +}; + +struct S3 { + S3() {} + S3(int, A) {} + S3(A, int) {} +}; + // An initializer list sequences its initialization clauses. void initializerListSequences() { { - struct S1 { - int i; - A a; - }; - { - A a; - S1 s1{a.getInt(), std::move(a)}; - } - { - A a; - S1 s1{.i = a.getInt(), .a = std::move(a)}; - } + A a; + S1 s1{a.getInt(), std::move(a)}; } { - struct S2 { - A a; - int i; - }; - { - 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 - } + A a; + S1 s1{.i = a.getInt(), .a = std::move(a)}; + } + { + 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{.a = std::move(a), .i = a.getInt()}; + // CHECK-NOTES: [[@LINE-1]]:35: warning: 'a' used after it was moved + // CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here + } + { + // Check the case where the constructed type has a constructor and the + // initializer list therefore manifests as a `CXXConstructExpr` instead of + // an `InitListExpr`. + A a; + S3 s3{a.getInt(), std::move(a)}; + } + { + A a; + S3 s3{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 } } +} // namespace InitializerListSequences + // A declaration statement containing multiple declarations sequences the // initializer expressions. void declarationSequences() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
