[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 23:23:00 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/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 <sguelton at mozilla.com>
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 <sguelton at mozilla.com>
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 <sguelton at mozilla.com>
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 <sguelton at mozilla.com>
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};
More information about the cfe-commits
mailing list