Author: Martin Braenne Date: 2023-03-16T08:15:13Z New Revision: ddbcd985602dcb5fe78fcf2246cf53922db1f3c3
URL: https://github.com/llvm/llvm-project/commit/ddbcd985602dcb5fe78fcf2246cf53922db1f3c3 DIFF: https://github.com/llvm/llvm-project/commit/ddbcd985602dcb5fe78fcf2246cf53922db1f3c3.diff LOG: [clang-tidy] Correctly handle evaluation order of designated initializers. 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. Differential Revision: https://reviews.llvm.org/D145906 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 beb4c44467a80..f9555be57e738 100644 --- a/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp +++ b/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 @@ static SmallVector<const Stmt *, 1> getParentStmts(const Stmt *S, } namespace { + bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor, ASTContext *Context) { if (Descendant == Ancestor) @@ -60,6 +62,17 @@ bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor, return false; } + +llvm::SmallVector<const InitListExpr *> +getAllInitListForms(const InitListExpr *InitList) { + llvm::SmallVector<const InitListExpr *> result = {InitList}; + if (const InitListExpr *AltForm = InitList->getSyntacticForm()) + result.push_back(AltForm); + if (const InitListExpr *AltForm = InitList->getSemanticForm()) + result.push_back(AltForm); + return result; +} + } // namespace ExprSequence::ExprSequence(const CFG *TheCFG, const Stmt *Root, @@ -111,9 +124,12 @@ const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const { } 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 : getAllInitListForms(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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2be8bfc51d675..b53516a742e2c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -219,6 +219,10 @@ Changes in existing checks <clang-tidy/checks/cppcoreguidelines/slicing>` check when warning would be emitted in constructor for virtual base class initialization. +- Improved :doc:`bugprone-use-after-move + <clang-tidy/checks/bugprone/use-after-move>` to understand that there is a + sequence point between designated initializers. + Removed checks ^^^^^^^^^^^^^^ 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 281f2083857ad..45cef8abfd1f6 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 @@ -1155,18 +1155,32 @@ void initializerListSequences() { 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 + } } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
