https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/179467
>From 3783a01b5771ea4cb7f890053c4400be23a6e763 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Tue, 27 Jan 2026 12:47:35 +0100 Subject: [PATCH 1/4] [clang-tidy] New performance linter: performance-inefficient-copy-assign This linter suggests call to ``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 --- .../clang-tidy/performance/CMakeLists.txt | 1 + .../InefficientCopyAssignCheck.cpp | 103 ++++++++++++++++++ .../performance/InefficientCopyAssignCheck.h | 35 ++++++ .../performance/PerformanceTidyModule.cpp | 3 + .../performance/inefficient-copy-assign.cpp | 75 +++++++++++++ clang/docs/ReleaseNotes.rst | 6 + 6 files changed, 223 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-copy-assign.cpp 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 ---------- >From e8d2c120602a94ab9a80b7646dcae217adf3e464 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Tue, 3 Feb 2026 15:14:38 +0100 Subject: [PATCH 2/4] fixup! [clang-tidy] New performance linter: performance-inefficient-copy-assign --- .../performance/InefficientCopyAssignCheck.cpp | 9 ++------- .../clang-tidy/performance/InefficientCopyAssignCheck.h | 6 +++--- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp index e3c6cecf82553..fce7ff38f9201 100644 --- a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp @@ -15,11 +15,6 @@ #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; @@ -44,7 +39,7 @@ CFG *InefficientCopyAssignCheck::getCFG(const FunctionDecl *FD, ASTContext *Context) { std::unique_ptr<CFG> &TheCFG = CFGCache[FD]; if (!TheCFG) { - CFG::BuildOptions Options; + const CFG::BuildOptions Options; std::unique_ptr<CFG> FCFG = CFG::buildCFG(nullptr, FD->getBody(), Context, Options); if (!FCFG) @@ -64,7 +59,7 @@ void InefficientCopyAssignCheck::check(const MatchFinder::MatchResult &Result) { const auto *WithinFunctionDecl = Result.Nodes.getNodeAs<FunctionDecl>("within-func"); - QualType AssignValueQual = AssignValueDecl->getType(); + const QualType AssignValueQual = AssignValueDecl->getType(); if (AssignValueQual->isReferenceType() || AssignValueQual.isConstQualified() || AssignValueQual->isPointerType() || AssignValueQual->isScalarType()) diff --git a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h index 57cc237152cb6..db45c39bb48cd 100644 --- a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h +++ b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h @@ -7,8 +7,8 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTCOPYASSIGN_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTCOPYASSIGN_H +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTCOPYASSIGNCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTCOPYASSIGNCHECK_H #include "../ClangTidyCheck.h" @@ -32,4 +32,4 @@ class InefficientCopyAssignCheck : public ClangTidyCheck { } // namespace clang::tidy::performance -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTCOPYASSIGN_H +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTCOPYASSIGNCHECK_H >From 504120b2f684342ce245da85fa651c7a1c1e3dfb Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Tue, 3 Feb 2026 15:21:29 +0100 Subject: [PATCH 3/4] fixup! fixup! [clang-tidy] New performance linter: performance-inefficient-copy-assign --- .../clang-tidy/performance/InefficientCopyAssignCheck.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp index fce7ff38f9201..6e75cc4c7b8aa 100644 --- a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp @@ -17,7 +17,6 @@ #include "llvm/ADT/STLExtras.h" using namespace clang::ast_matchers; -using namespace clang::tidy::utils; namespace clang::tidy::performance { >From 34c9e3f0114d404ff11b259a0603c0b05247def8 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Tue, 3 Feb 2026 17:49:54 +0100 Subject: [PATCH 4/4] fixup! fixup! fixup! [clang-tidy] New performance linter: performance-inefficient-copy-assign --- .../InefficientCopyAssignCheck.cpp | 8 +- .../performance/InefficientCopyAssignCheck.h | 3 +- clang-tools-extra/docs/ReleaseNotes.rst | 8 +- .../docs/clang-tidy/checks/list.rst | 1 + .../performance/inefficient-copy-assign.rst | 25 +++++ .../performance/inefficient-copy-assign.cpp | 101 +++++++++++++++++- clang/docs/ReleaseNotes.rst | 6 -- 7 files changed, 136 insertions(+), 16 deletions(-) create mode 100644 clang-tools-extra/docs/clang-tidy/checks/performance/inefficient-copy-assign.rst diff --git a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp index 6e75cc4c7b8aa..97adf212cbd7a 100644 --- a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.cpp @@ -1,5 +1,4 @@ -//===--- InefficientCopyAssignCheck.cpp - clang-tidy -//-------------------------------===// +//===--- 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. @@ -59,11 +58,14 @@ void InefficientCopyAssignCheck::check(const MatchFinder::MatchResult &Result) { Result.Nodes.getNodeAs<FunctionDecl>("within-func"); const QualType AssignValueQual = AssignValueDecl->getType(); - if (AssignValueQual->isReferenceType() || + if (AssignValueQual->isLValueReferenceType() || AssignValueQual.isConstQualified() || AssignValueQual->isPointerType() || AssignValueQual->isScalarType()) return; + if (!AssignValueDecl->hasLocalStorage()) + return; + if (AssignTargetType->hasTrivialMoveAssignment()) return; diff --git a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h index db45c39bb48cd..8684018844155 100644 --- a/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h +++ b/clang-tools-extra/clang-tidy/performance/InefficientCopyAssignCheck.h @@ -1,5 +1,4 @@ -//===--- InefficientCopyAssign.h - clang-tidy -//---------------------------------===// +//===--- 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. diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ea80502735ede..b9565a66fe1f9 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -115,13 +115,19 @@ New checks Looks for functions returning ``std::[w|u8|u16|u32]string`` and suggests to change it to ``std::[...]string_view`` for performance reasons if possible. - + - New :doc:`modernize-use-structured-binding <clang-tidy/checks/modernize/use-structured-binding>` check. Finds places where structured bindings could be used to decompose pairs and suggests replacing them. +- New :doc:`performance-inefficient-copy-assign + <clang-tidy/checks/performance/inefficient-copy-assign>` check. + + Suggests insertion of ``std::move(...)`` to turn copy assignment operator + calls into move assignment ones, when deemed valid and profitable. + - New :doc:`performance-string-view-conversions <clang-tidy/checks/performance/string-view-conversions>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 0eabd9929dc39..d1dff5db3e689 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -354,6 +354,7 @@ Clang-Tidy Checks :doc:`performance-for-range-copy <performance/for-range-copy>`, "Yes" :doc:`performance-implicit-conversion-in-loop <performance/implicit-conversion-in-loop>`, :doc:`performance-inefficient-algorithm <performance/inefficient-algorithm>`, "Yes" + :doc:`performance-inefficient-copy-assign <performance/inefficient-copy-assign>`, "No" :doc:`performance-inefficient-string-concatenation <performance/inefficient-string-concatenation>`, :doc:`performance-inefficient-vector-operation <performance/inefficient-vector-operation>`, "Yes" :doc:`performance-move-const-arg <performance/move-const-arg>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/inefficient-copy-assign.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/inefficient-copy-assign.rst new file mode 100644 index 0000000000000..10f3008459966 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/performance/inefficient-copy-assign.rst @@ -0,0 +1,25 @@ +.. title:: clang-tidy - performance-inefficient-copy-assign + +performance-inefficient-copy-assign +=================================== + + +Warns on copy assignment operator recieving an lvalue reference when a +profitable move assignment operator exist and would be used if the lvalue +reference were moved through ``std::move``. + +.. code-block:: c++ + + void assign(std::vector<int>& out) { + std::vector<int> some = make_vector(); + use_vector(some); + out = some; + } + + // becomes + + void assign(std::vector<int>& out) { + std::vector<int> some = make_vector(); + use_vector(some); + out = std::move(some); + } 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 index ee958bb3479c1..07e69b48f064c 100644 --- 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 @@ -1,5 +1,8 @@ // RUN: %check_clang_tidy %s performance-inefficient-copy-assign %t +// Definitions used in the tests +// ----------------------------- + struct NonTrivialMoveAssign { NonTrivialMoveAssign& operator=(const NonTrivialMoveAssign&); NonTrivialMoveAssign& operator=(NonTrivialMoveAssign&&); @@ -18,8 +21,84 @@ struct NoMoveAssign { NoMoveAssign& operator=(NoMoveAssign&&) = delete; }; +template<class T> +void use(T&) {} + +// Check moving from various reference/pointer type +// ------------------------------------------------ + void ConvertibleNonTrivialMoveAssign(NonTrivialMoveAssign& target, NonTrivialMoveAssign source) { - // CHECK-MESSAGES: [[@LINE+1]]:{{[0-9]*}}: warning: 'source' could be moved here [performance-inefficient-copy-assign] + // CHECK-MESSAGES: [[@LINE+1]]:12: warning: 'source' could be moved here [performance-inefficient-copy-assign] + target = source; +} + +void NonProfitableNonTrivialMoveAssignPointer(NonTrivialMoveAssign*& target, NonTrivialMoveAssign* source) { + // No message expected, moving is possible but non profitable for pointer. + target = source; +} + +void ConvertibleNonTrivialMoveAssignFromLValue(NonTrivialMoveAssign& target, NonTrivialMoveAssign&& source) { + // CHECK-MESSAGES: [[@LINE+1]]:12: warning: 'source' could be moved here [performance-inefficient-copy-assign] + target = source; +} + +// Check moving from various storage +// --------------------------------- + +void NonConvertibleNonTrivialMoveAssignFromStatic(NonTrivialMoveAssign& target) { + static NonTrivialMoveAssign source; + // No message, the lifetime of `source' does not end with the scope of the function. + target = source; +} + +void NonConvertibleNonTrivialMoveAssignFromExtern(NonTrivialMoveAssign& target) { + extern NonTrivialMoveAssign source; + // No message, the lifetime of `source' does not end with the scope of the function. + target = source; +} + +void NonConvertibleNonTrivialMoveAssignFromTLS(NonTrivialMoveAssign& target) { + thread_local NonTrivialMoveAssign source; + // No message, the lifetime of `source' does not end with the scope of the function. + target = source; +} + +NonTrivialMoveAssign global_source; +void NonConvertibleNonTrivialMoveAssignToGlobal(NonTrivialMoveAssign& target) { + // No message, the lifetime of `source' does not end with the scope of the function. + target = global_source; +} + + +// Check moving to various storage +// ------------------------------- + +void ConvertibleNonTrivialMoveAssignToStatic(NonTrivialMoveAssign source) { + static NonTrivialMoveAssign target; + // CHECK-MESSAGES: [[@LINE+1]]:12: warning: 'source' could be moved here [performance-inefficient-copy-assign] + target = source; +} + +void ConvertibleNonTrivialMoveAssignToExtern(NonTrivialMoveAssign source) { + extern NonTrivialMoveAssign target; + // CHECK-MESSAGES: [[@LINE+1]]:12: warning: 'source' could be moved here [performance-inefficient-copy-assign] + target = source; +} + +void ConvertibleNonTrivialMoveAssignToTLS(NonTrivialMoveAssign source) { + thread_local NonTrivialMoveAssign target; + // CHECK-MESSAGES: [[@LINE+1]]:12: warning: 'source' could be moved here [performance-inefficient-copy-assign] + target = source; +} + +NonTrivialMoveAssign global_target; +void ConvertibleNonTrivialMoveAssignToGlobal(NonTrivialMoveAssign source) { + // CHECK-MESSAGES: [[@LINE+1]]:19: warning: 'source' could be moved here [performance-inefficient-copy-assign] + global_target = source; +} + +void NonConvertibleNonTrivialMoveAssignRValue(NonTrivialMoveAssign& target, NonTrivialMoveAssign const& source) { + // No message expected, moving a reference is invalid there. target = source; } @@ -28,21 +107,32 @@ void NonProfitableTrivialMoveAssign(TrivialMoveAssign& target, TrivialMoveAssign target = source; } +// Check moving in presence of control flow or use +// ----------------------------------------------- + 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] + // CHECK-MESSAGES: [[@LINE+1]]:14: warning: 'source' could be moved here [performance-inefficient-copy-assign] target = source; } } +void NonConvertibleNonTrivialMoveAssignWithBranchingAndUse(bool cond, NonTrivialMoveAssign& target, NonTrivialMoveAssign source) { + if(cond) { + // No message expected, moving would make use invalid. + target = source; + } + use(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] + // CHECK-MESSAGES: [[@LINE+1]]:14: 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] + // CHECK-MESSAGES: [[@LINE+1]]:14: warning: 'source' could be moved here [performance-inefficient-copy-assign] target = source; } } @@ -59,6 +149,9 @@ void NonConvertibleNonTrivialMoveAssignInLoop(NonTrivialMoveAssign& target, NonT target = source; } +// Check moving for invalid / non profitable type or operation +// ----------------------------------------------------------- + void NonConvertibleNonTrivialMoveUpdateAssign(NonTrivialMoveAssign& target, NonTrivialMoveAssign source) { // No message currently expected, we only consider assignment. target += source; diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b6273a4e4bded..7dc6881ed43e6 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -354,12 +354,6 @@ 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 ---------- _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
