https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/184136
>From 2488384822cf67cb7cd316ee8bd3e51f41f64a88 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Mon, 2 Mar 2026 12:40:10 +0100 Subject: [PATCH 1/5] [clang-tidy] Improve performance-use-std-move in presence of control-flow --- .../performance/UseStdMoveCheck.cpp | 48 +++++++++++--- .../checkers/performance/use-std-move.cpp | 65 +++++++++++++++++++ 2 files changed, 105 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp index ecd6a33aa722d..bd32aeefd5b02 100644 --- a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp @@ -93,14 +93,28 @@ void UseStdMoveCheck::check(const MatchFinder::MatchResult &Result) { if (!TheCFG) return; - // Walk the CFG bottom-up, starting with the exit node. - // TODO: traverse the whole CFG instead of only considering terminator - // nodes. + struct BlockState { + bool Ready; + unsigned RemainingSuccessors; + }; + std::unordered_map<const CFGBlock *, BlockState> CFGState; + for (const auto *B : *TheCFG) + CFGState.emplace(B, BlockState{true, B->succ_size()}); + const CFGBlock &TheExit = TheCFG->getExit(); - for (auto &Pred : TheExit.preds()) { - if (!Pred.isReachable()) + std::vector<const CFGBlock *> ToVisit = {&TheExit}; + + // Walk the CFG bottom-up, starting with the exit node. + while (!ToVisit.empty()) { + const CFGBlock *B = ToVisit.back(); + ToVisit.pop_back(); + if (!CFGState.find(B)->second.Ready) continue; - for (const CFGElement &Elt : llvm::reverse(*Pred)) { + + assert(CFGState.find(B)->second.RemainingSuccessors == 0 && + "All successors have been processed."); + bool ReferencesAssignedValue = false; + for (const CFGElement &Elt : llvm::reverse(*B)) { if (Elt.getKind() != CFGElement::Kind::Statement) continue; @@ -112,13 +126,31 @@ void UseStdMoveCheck::check(const MatchFinder::MatchResult &Result) { << FixItHint::CreateReplacement( AssignValue->getLocation(), ("std::move(" + AssignValueName + ")").str()); + ReferencesAssignedValue = true; break; } - // The reference is being referenced after the assignment, bail out. + + // The reference is being referenced after the assignment. if (!allDeclRefExprs(*cast<VarDecl>(AssignValue->getDecl()), *EltStmt, *Result.Context) - .empty()) + .empty()) { + ReferencesAssignedValue = true; break; + } + } + if (ReferencesAssignedValue) { + // Cancel all predecessors. + for (const auto &S : B->preds()) + CFGState.find(&*S)->second.Ready = false; + } else { + // Or process the ready ones. + for (const auto &S : B->preds()) { + auto &W = CFGState.find(&*S)->second; + if (W.Ready) { + if (--W.RemainingSuccessors == 0 && S.isReachable()) + ToVisit.push_back(&*S); + } + } } } } diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp index 4492d976c37bd..1a6c31feca3d4 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp @@ -300,3 +300,68 @@ void InvalidMoveAssign(NoMoveAssign& target, NoMoveAssign source) { // No message expected, moving is deleted. target = source; } + +// Check moving within control-flow +// -------------------------------- + +void ConvertibleNonTrivialMoveAssignWithinTestPred(bool pred, NonTrivialMoveAssign& target, NonTrivialMoveAssign source) { + // CHECK-MESSAGES: [[@LINE+1]]:12: warning: 'source' could be moved here [performance-use-std-move] + target = source; + // CHECK-FIXES: target = std::move(source); + if (pred) + target = target; +} + +void NonConvertibleNonTrivialMoveAssignWithinTestPred(bool pred, NonTrivialMoveAssign& target, NonTrivialMoveAssign source) { + target = source; + if (pred) + source.stuff(); +} + +void ConvertibleNonTrivialMoveAssignWithinTestBothPred(bool pred, NonTrivialMoveAssign& target, NonTrivialMoveAssign source) { + // CHECK-MESSAGES: [[@LINE+1]]:12: warning: 'source' could be moved here [performance-use-std-move] + target = source; + // CHECK-FIXES: target = std::move(source); + if (pred) + target = target; + else + target.stuff(); +} + +void NonConvertibleNonTrivialMoveAssignWithinLoop(unsigned count, NonTrivialMoveAssign& target, NonTrivialMoveAssign source) { + while(--count) + target = source; +} + +void ConvertibleNonTrivialMoveAssignWithinTestMultiplePred(bool pred0, bool pred1, NonTrivialMoveAssign& target, NonTrivialMoveAssign source) { + if(pred0) { + // CHECK-MESSAGES: [[@LINE+1]]:14: warning: 'source' could be moved here [performance-use-std-move] + target = source; + // CHECK-FIXES: target = std::move(source); + } + else { + if (pred1) { + // CHECK-MESSAGES: [[@LINE+1]]:16: warning: 'source' could be moved here [performance-use-std-move] + target = source; + // CHECK-FIXES: target = std::move(source); + } + else { + target.stuff(); + } + } +} + +void NonConvertibleNonTrivialMoveAssignWithinTestMultiplePred(bool pred0, bool pred1, NonTrivialMoveAssign& target, NonTrivialMoveAssign source) { + target = source; + if(pred0) { + target.stuff(); + } + else { + // CHECK-MESSAGES: [[@LINE+1]]:14: warning: 'source' could be moved here [performance-use-std-move] + target = source; + // CHECK-FIXES: target = std::move(source); + if (pred1) { + target.stuff(); + } + } +} >From 71c8211b422bb8310a8285d6b7152c4eded487d7 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Mon, 2 Mar 2026 16:13:24 +0100 Subject: [PATCH 2/5] fixup! [clang-tidy] Improve performance-use-std-move in presence of control-flow --- clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp index bd32aeefd5b02..a17bff69f066e 100644 --- a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp @@ -108,10 +108,11 @@ void UseStdMoveCheck::check(const MatchFinder::MatchResult &Result) { while (!ToVisit.empty()) { const CFGBlock *B = ToVisit.back(); ToVisit.pop_back(); - if (!CFGState.find(B)->second.Ready) + const BlockState &BS = CFGState.find(B)->second; + if (!BS.Ready) continue; - assert(CFGState.find(B)->second.RemainingSuccessors == 0 && + assert(BS.RemainingSuccessors == 0 && "All successors have been processed."); bool ReferencesAssignedValue = false; for (const CFGElement &Elt : llvm::reverse(*B)) { >From 56c3e71327889d4d00cb1188c3a73ac7368f04a9 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Mon, 2 Mar 2026 22:49:56 +0100 Subject: [PATCH 3/5] fixup! fixup! [clang-tidy] Improve performance-use-std-move in presence of control-flow --- .../performance/UseStdMoveCheck.cpp | 19 ++++++++++++---- .../checkers/performance/use-std-move.cpp | 22 +++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp index a17bff69f066e..5bd81b3d752f2 100644 --- a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp @@ -22,8 +22,13 @@ using namespace clang::ast_matchers; namespace clang::tidy::performance { namespace { -AST_MATCHER(CXXRecordDecl, hasNonTrivialMoveAssignment) { - return Node.hasNonTrivialMoveAssignment(); +AST_MATCHER(CXXRecordDecl, hasAccessibleNonTrivialMoveAssignment) { + if (!Node.hasNonTrivialMoveAssignment()) + return false; + for (const auto *CM : Node.methods()) + if (CM->isMoveAssignmentOperator()) + return !CM->isDeleted() && CM->getAccess() == AS_public; + llvm_unreachable("Move Assignment Operaotr Not Found"); } AST_MATCHER(QualType, isLValueReferenceType) { @@ -53,7 +58,8 @@ void UseStdMoveCheck::registerMatchers(MatchFinder *Finder) { auto AssignOperatorExpr = cxxOperatorCallExpr( isCopyAssignmentOperator(), - hasArgument(0, hasType(cxxRecordDecl(hasNonTrivialMoveAssignment()))), + hasArgument(0, hasType(cxxRecordDecl( + hasAccessibleNonTrivialMoveAssignment()))), hasArgument( 1, declRefExpr( to(varDecl( @@ -141,11 +147,16 @@ void UseStdMoveCheck::check(const MatchFinder::MatchResult &Result) { } if (ReferencesAssignedValue) { // Cancel all predecessors. - for (const auto &S : B->preds()) + for (const auto &S : B->preds()) { + if (!S.isReachable()) + continue; CFGState.find(&*S)->second.Ready = false; + } } else { // Or process the ready ones. for (const auto &S : B->preds()) { + if (!S.isReachable()) + continue; auto &W = CFGState.find(&*S)->second; if (W.Ready) { if (--W.RemainingSuccessors == 0 && S.isReachable()) diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp index 1a6c31feca3d4..1b011f2fb4f3c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp @@ -33,6 +33,18 @@ struct NoMoveAssign { NoMoveAssign& operator=(NoMoveAssign&&) = delete; }; +struct NoDefaultedMoveAssign { + NoDefaultedMoveAssign& operator=(const NoDefaultedMoveAssign&); + NoDefaultedMoveAssign& operator=(NoDefaultedMoveAssign&&) = default; + NoMoveAssign Field; +}; + +struct PrivateMoveAssign { + PrivateMoveAssign& operator=(const PrivateMoveAssign&); + private: + PrivateMoveAssign& operator=(PrivateMoveAssign&&); +}; + template<class T> void use(T&) {} @@ -301,6 +313,16 @@ void InvalidMoveAssign(NoMoveAssign& target, NoMoveAssign source) { target = source; } +void InvalidPrivateMoveAssign(PrivateMoveAssign& target, PrivateMoveAssign source) { + // No message expected, moving is private. + target = source; +} + +void DeletedDefaultMoveAssign(NoDefaultedMoveAssign& target, NoDefaultedMoveAssign source) { + // No message expected, default move is invalid. + target = source; +} + // Check moving within control-flow // -------------------------------- >From 84a19ba5a7bab40ceb7f14f092170b2f2db341a2 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Mon, 2 Mar 2026 23:06:31 +0100 Subject: [PATCH 4/5] fixup! fixup! fixup! [clang-tidy] Improve performance-use-std-move in presence of control-flow --- .../performance/UseStdMoveCheck.cpp | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp index 5bd81b3d752f2..09dad27394475 100644 --- a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp @@ -99,6 +99,21 @@ void UseStdMoveCheck::check(const MatchFinder::MatchResult &Result) { if (!TheCFG) return; + // The algorithm to look for a convertible move-assign operator is the + // following: each node starts in the `Ready` state, with a number of + // `RemainingSuccessors` equal to its number of successors. + // + // Starting from the exit node, we walk the CFG backward. Whenever + // we meet a new block, we check if it either: + // 1. touches the `AssignValue`, in which case we stop the search, and mark + // each + // predecessor as not `Ready`. No predecessor walk. + // 2. contains a convertible copy-assign operator, in which case we generate a + // fix, and mark each predecessor as not Ready. No predecessor walk. + // 3. does not interact with `AssignValue`, in which case we decrement the + // `RemainingSuccessors` of each predecessor. And if it happens to turn to + // 0 while still being `Ready`, we add it to the `WorkList`. + struct BlockState { bool Ready; unsigned RemainingSuccessors; @@ -108,12 +123,11 @@ void UseStdMoveCheck::check(const MatchFinder::MatchResult &Result) { CFGState.emplace(B, BlockState{true, B->succ_size()}); const CFGBlock &TheExit = TheCFG->getExit(); - std::vector<const CFGBlock *> ToVisit = {&TheExit}; + std::vector<const CFGBlock *> WorkList = {&TheExit}; - // Walk the CFG bottom-up, starting with the exit node. - while (!ToVisit.empty()) { - const CFGBlock *B = ToVisit.back(); - ToVisit.pop_back(); + while (!WorkList.empty()) { + const CFGBlock *B = WorkList.back(); + WorkList.pop_back(); const BlockState &BS = CFGState.find(B)->second; if (!BS.Ready) continue; @@ -160,7 +174,7 @@ void UseStdMoveCheck::check(const MatchFinder::MatchResult &Result) { auto &W = CFGState.find(&*S)->second; if (W.Ready) { if (--W.RemainingSuccessors == 0 && S.isReachable()) - ToVisit.push_back(&*S); + WorkList.push_back(&*S); } } } >From 5652d63b8cd424db8986b826f62c79676de47b69 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Tue, 3 Mar 2026 08:22:33 +0100 Subject: [PATCH 5/5] fixup! fixup! fixup! fixup! [clang-tidy] Improve performance-use-std-move in presence of control-flow --- .../clang-tidy/performance/UseStdMoveCheck.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp index 09dad27394475..b15a40b780511 100644 --- a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp @@ -15,6 +15,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h" #include "clang/Lex/Lexer.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/STLExtras.h" using namespace clang::ast_matchers; @@ -28,7 +29,7 @@ AST_MATCHER(CXXRecordDecl, hasAccessibleNonTrivialMoveAssignment) { for (const auto *CM : Node.methods()) if (CM->isMoveAssignmentOperator()) return !CM->isDeleted() && CM->getAccess() == AS_public; - llvm_unreachable("Move Assignment Operaotr Not Found"); + llvm_unreachable("Move Assignment Operator Not Found"); } AST_MATCHER(QualType, isLValueReferenceType) { @@ -118,9 +119,9 @@ void UseStdMoveCheck::check(const MatchFinder::MatchResult &Result) { bool Ready; unsigned RemainingSuccessors; }; - std::unordered_map<const CFGBlock *, BlockState> CFGState; + llvm::DenseMap<const CFGBlock *, BlockState> CFGState; for (const auto *B : *TheCFG) - CFGState.emplace(B, BlockState{true, B->succ_size()}); + CFGState.try_emplace(B, BlockState{true, B->succ_size()}); const CFGBlock &TheExit = TheCFG->getExit(); std::vector<const CFGBlock *> WorkList = {&TheExit}; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
