[clang-tools-extra] [clang-tidy] Improve performance-use-std-move in presence of control-flow (PR #184136)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 2 13:50:33 PST 2026
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 <sguelton at mozilla.com>
Date: Mon, 2 Mar 2026 12:40:10 +0100
Subject: [PATCH 1/3] [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 <sguelton at mozilla.com>
Date: Mon, 2 Mar 2026 16:13:24 +0100
Subject: [PATCH 2/3] 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 <sguelton at mozilla.com>
Date: Mon, 2 Mar 2026 22:49:56 +0100
Subject: [PATCH 3/3] 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
// --------------------------------
More information about the cfe-commits
mailing list