https://github.com/serge-sans-paille created https://github.com/llvm/llvm-project/pull/184136
…flow >From 65cad83b138151c3ad38b07468e4397949f52696 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Mon, 2 Mar 2026 12:40:10 +0100 Subject: [PATCH] [clang-tidy] Improve performance-use-std-move in presence of control-flow --- .../performance/UseStdMoveCheck.cpp | 47 +++++++++++--- .../checkers/performance/use-std-move.cpp | 65 +++++++++++++++++++ 2 files changed, 104 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..a71badf341cfc 100644 --- a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp @@ -93,14 +93,27 @@ 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(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 +125,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(); + } + } +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
