[clang] [clang-tools-extra] [clang-tidy] New performance linter: performance-inefficient-copy-assign (PR #179467)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 3 06:06:32 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: None (serge-sans-paille)
<details>
<summary>Changes</summary>
This linter suggests call std::move when a costly copy would happen otherwise. It does not try to suggest std::move when they are valid but obviously not profitable (e.g. for trivially movable types)
This is a very simple version that only considers terminating basic blocks. Further work will extend the approach through the control flow graph.
It has already been used successfully on llvm/lib to submit bugs #<!-- -->178174,
#<!-- -->178169, #<!-- -->178176, #<!-- -->178172, #<!-- -->178175, #<!-- -->178180, #<!-- -->178178, #<!-- -->178177, #<!-- -->178179,
#<!-- -->178173 and #<!-- -->178167, and on the firefox codebase to submit most of the
dependencies of bug https://bugzilla.mozilla.org/show_bug.cgi?id=2012658
---
Full diff: https://github.com/llvm/llvm-project/pull/179467.diff
6 Files Affected:
- (modified) clang-tools-extra/clang-tidy/performance/CMakeLists.txt (+1)
- (added) clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp (+103)
- (added) clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h (+35)
- (modified) clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp (+3)
- (added) clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-copy-assign.cpp (+75)
- (modified) clang/docs/ReleaseNotes.rst (+6)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index 4dba117e1ee54..7b25e5946ddba 100644
--- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -10,6 +10,7 @@ add_clang_library(clangTidyPerformanceModule STATIC
ForRangeCopyCheck.cpp
ImplicitConversionInLoopCheck.cpp
InefficientAlgorithmCheck.cpp
+ InefficientCopyAssignCheck.cpp
InefficientStringConcatenationCheck.cpp
InefficientVectorOperationCheck.cpp
MoveConstArgCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp
new file mode 100644
index 0000000000000..e3c6cecf82553
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp
@@ -0,0 +1,103 @@
+//===--- InefficientCopyAssignCheck.cpp - clang-tidy
+//-------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "InefficientCopyAssignCheck.h"
+
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
+
+#include "../utils/ExprSequence.h"
+#include "../utils/Matchers.h"
+#include <optional>
+
+using namespace clang::ast_matchers;
+using namespace clang::tidy::utils;
+
+namespace clang::tidy::performance {
+
+void InefficientCopyAssignCheck::registerMatchers(MatchFinder *Finder) {
+ auto AssignOperatorExpr =
+ cxxOperatorCallExpr(
+ hasOperatorName("="),
+ hasArgument(
+ 0, hasType(cxxRecordDecl(hasMethod(isMoveAssignmentOperator()))
+ .bind("assign-target-type"))),
+ hasArgument(1, declRefExpr(to(varDecl().bind("assign-value-decl")))
+ .bind("assign-value")),
+ hasAncestor(functionDecl().bind("within-func")))
+ .bind("assign");
+ Finder->addMatcher(AssignOperatorExpr, this);
+}
+
+CFG *InefficientCopyAssignCheck::getCFG(const FunctionDecl *FD,
+ ASTContext *Context) {
+ std::unique_ptr<CFG> &TheCFG = CFGCache[FD];
+ if (!TheCFG) {
+ CFG::BuildOptions Options;
+ std::unique_ptr<CFG> FCFG =
+ CFG::buildCFG(nullptr, FD->getBody(), Context, Options);
+ if (!FCFG)
+ return nullptr;
+ TheCFG.swap(FCFG);
+ }
+ return TheCFG.get();
+}
+
+void InefficientCopyAssignCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *AssignExpr = Result.Nodes.getNodeAs<Expr>("assign");
+ const auto *AssignValue = Result.Nodes.getNodeAs<DeclRefExpr>("assign-value");
+ const auto *AssignValueDecl =
+ Result.Nodes.getNodeAs<VarDecl>("assign-value-decl");
+ const auto *AssignTargetType =
+ Result.Nodes.getNodeAs<CXXRecordDecl>("assign-target-type");
+ const auto *WithinFunctionDecl =
+ Result.Nodes.getNodeAs<FunctionDecl>("within-func");
+
+ QualType AssignValueQual = AssignValueDecl->getType();
+ if (AssignValueQual->isReferenceType() ||
+ AssignValueQual.isConstQualified() || AssignValueQual->isPointerType() ||
+ AssignValueQual->isScalarType())
+ return;
+
+ if (AssignTargetType->hasTrivialMoveAssignment())
+ return;
+
+ if (CFG *TheCFG = getCFG(WithinFunctionDecl, Result.Context)) {
+ // Walk the CFG bottom-up, starting with the exit node.
+ // TODO: traverse the whole CFG instead of only considering terminator
+ // nodes.
+
+ CFGBlock &TheExit = TheCFG->getExit();
+ for (auto &Pred : TheExit.preds()) {
+ for (const CFGElement &Elt : llvm::reverse(*Pred)) {
+ if (Elt.getKind() == CFGElement::Kind::Statement) {
+ const Stmt *EltStmt = Elt.castAs<CFGStmt>().getStmt();
+ if (EltStmt == AssignExpr) {
+ diag(AssignValue->getBeginLoc(), "'%0' could be moved here")
+ << AssignValue->getDecl()->getName();
+ break;
+ }
+ // The reference is being referenced before the assignment, bail out.
+ auto DeclRefMatcher =
+ declRefExpr(hasDeclaration(equalsNode(AssignValue->getDecl())));
+ if (!match(findAll(DeclRefMatcher), *EltStmt, *Result.Context)
+ .empty())
+ break;
+ }
+ }
+ }
+ }
+}
+
+} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h
new file mode 100644
index 0000000000000..57cc237152cb6
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h
@@ -0,0 +1,35 @@
+//===--- InefficientCopyAssign.h - clang-tidy
+//---------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTCOPYASSIGN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTCOPYASSIGN_H
+
+#include "../ClangTidyCheck.h"
+
+#include "clang/Analysis/CFG.h"
+
+namespace clang::tidy::performance {
+
+class InefficientCopyAssignCheck : public ClangTidyCheck {
+ llvm::DenseMap<const FunctionDecl *, std::unique_ptr<CFG>> CFGCache;
+ CFG *getCFG(const FunctionDecl *, ASTContext *);
+
+public:
+ InefficientCopyAssignCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus11;
+ }
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::performance
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTCOPYASSIGN_H
diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
index 294a209e4c602..164de2bcba568 100644
--- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -14,6 +14,7 @@
#include "ForRangeCopyCheck.h"
#include "ImplicitConversionInLoopCheck.h"
#include "InefficientAlgorithmCheck.h"
+#include "InefficientCopyAssignCheck.h"
#include "InefficientStringConcatenationCheck.h"
#include "InefficientVectorOperationCheck.h"
#include "MoveConstArgCheck.h"
@@ -46,6 +47,8 @@ class PerformanceModule : public ClangTidyModule {
"performance-implicit-conversion-in-loop");
CheckFactories.registerCheck<InefficientAlgorithmCheck>(
"performance-inefficient-algorithm");
+ CheckFactories.registerCheck<InefficientCopyAssignCheck>(
+ "performance-inefficient-copy-assign");
CheckFactories.registerCheck<InefficientStringConcatenationCheck>(
"performance-inefficient-string-concatenation");
CheckFactories.registerCheck<InefficientVectorOperationCheck>(
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-copy-assign.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-copy-assign.cpp
new file mode 100644
index 0000000000000..ee958bb3479c1
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-copy-assign.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s performance-inefficient-copy-assign %t
+
+struct NonTrivialMoveAssign {
+ NonTrivialMoveAssign& operator=(const NonTrivialMoveAssign&);
+ NonTrivialMoveAssign& operator=(NonTrivialMoveAssign&&);
+ NonTrivialMoveAssign& operator+=(const NonTrivialMoveAssign&);
+ NonTrivialMoveAssign& operator+=(NonTrivialMoveAssign&&);
+ void stuff();
+};
+
+struct TrivialMoveAssign {
+ TrivialMoveAssign& operator=(const TrivialMoveAssign&);
+ TrivialMoveAssign& operator=(TrivialMoveAssign&&) = default;
+};
+
+struct NoMoveAssign {
+ NoMoveAssign& operator=(const NoMoveAssign&);
+ NoMoveAssign& operator=(NoMoveAssign&&) = delete;
+};
+
+void ConvertibleNonTrivialMoveAssign(NonTrivialMoveAssign& target, NonTrivialMoveAssign source) {
+ // CHECK-MESSAGES: [[@LINE+1]]:{{[0-9]*}}: warning: 'source' could be moved here [performance-inefficient-copy-assign]
+ target = source;
+}
+
+void NonProfitableTrivialMoveAssign(TrivialMoveAssign& target, TrivialMoveAssign source) {
+ // No message expected, moving is possible but pedantic.
+ target = source;
+}
+
+void ConvertibleNonTrivialMoveAssignWithBranching(bool cond, NonTrivialMoveAssign& target, NonTrivialMoveAssign source) {
+ if(cond) {
+ // CHECK-MESSAGES: [[@LINE+1]]:{{[0-9]*}}: warning: 'source' could be moved here [performance-inefficient-copy-assign]
+ target = source;
+ }
+}
+
+void ConvertibleNonTrivialMoveAssignBothBranches(bool cond, NonTrivialMoveAssign& target, NonTrivialMoveAssign source) {
+ if(cond) {
+ // CHECK-MESSAGES: [[@LINE+1]]:{{[0-9]*}}: warning: 'source' could be moved here [performance-inefficient-copy-assign]
+ target = source;
+ }
+ else {
+ source.stuff();
+ // CHECK-MESSAGES: [[@LINE+1]]:{{[0-9]*}}: warning: 'source' could be moved here [performance-inefficient-copy-assign]
+ target = source;
+ }
+}
+
+void NonConvertibleNonTrivialMoveAssignWithExtraUse(NonTrivialMoveAssign& target, NonTrivialMoveAssign source) {
+ // No message expected, moving would make the call to `stuff' invalid.
+ target = source;
+ source.stuff();
+}
+
+void NonConvertibleNonTrivialMoveAssignInLoop(NonTrivialMoveAssign& target, NonTrivialMoveAssign source) {
+ // No message expected, moving would make the next iteration invalid.
+ for(int i = 0; i < 10; ++i)
+ target = source;
+}
+
+void NonConvertibleNonTrivialMoveUpdateAssign(NonTrivialMoveAssign& target, NonTrivialMoveAssign source) {
+ // No message currently expected, we only consider assignment.
+ target += source;
+}
+
+void NonProfitableTrivialTypeAssign(int& target, int source) {
+ // No message needed, it's correct to move but pedantic.
+ target = source;
+}
+
+void InvalidMoveAssign(NoMoveAssign& target, NoMoveAssign source) {
+ // No message expected, moving is deleted.
+ target = source;
+}
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7dc6881ed43e6..b6273a4e4bded 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -354,6 +354,12 @@ Static Analyzer
.. _release-notes-sanitizers:
+New features
+^^^^^^^^^^^^
+
+- Added a new checker ``performance.InefficientCopyAssign`` to detect
+ *copy-assignments* that could profitably be turned into *move-assignments*.
+
Sanitizers
----------
``````````
</details>
https://github.com/llvm/llvm-project/pull/179467
More information about the cfe-commits
mailing list