Febbe updated this revision to Diff 477997.
Febbe marked 3 inline comments as done.
Febbe added a comment.
Fixed typo
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137205/new/
https://reviews.llvm.org/D137205
Files:
clang-tools-extra/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.h
clang-tools-extra/clangd/TidyProvider.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-copy-on-last-use.rst
clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-on-last-use.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-on-last-use.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-on-last-use.cpp
@@ -0,0 +1,270 @@
+// RUN: %check_clang_tidy %s -std=c++17 performance-unnecessary-copy-on-last-use %t
+// RUN: %check_clang_tidy %s -std=c++11 performance-unnecessary-copy-on-last-use %t
+// CHECK-FIXES: #include <utility>
+
+struct Movable {
+ Movable() = default;
+ Movable(Movable const &) = default;
+ Movable(Movable &&) = default;
+ Movable &operator=(Movable const &) = default;
+ Movable &operator=(Movable &&) = default;
+ ~Movable();
+
+ void memberUse() {}
+};
+
+struct Copyable {
+ Copyable() = default;
+ Copyable(Copyable const &) = default;
+ Copyable(Copyable &&) = default;
+ Copyable &operator=(Copyable const &) = default;
+ Copyable &operator=(Copyable &&) = default;
+ ~Copyable() = default;
+
+ void memberUse() {}
+};
+// static_assert(!std::is_trivially_copyable_v<Movable>, "Movable must not be trivially copyable");
+
+void valueReceiver(Movable Mov);
+void constRefReceiver(Movable const &Mov);
+
+void valueTester() {
+ Movable Mov{};
+ valueReceiver(Mov);
+ valueReceiver(Mov);
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: valueReceiver(std::move(Mov));
+ Mov = Movable{};
+ valueReceiver(Mov);
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: valueReceiver(std::move(Mov));
+}
+
+void testUsageInBranch(bool Splitter) {
+ Movable Mov{};
+
+ valueReceiver(Mov);
+ if(Splitter){
+ Mov.memberUse();
+ } else {
+ Mov = Movable{};
+ }
+ valueReceiver(Mov);
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: valueReceiver(std::move(Mov));
+
+ if(Splitter){
+ Mov = Movable{};
+ } else {
+ Mov = Movable{};
+ }
+ valueReceiver(Mov);
+ Mov.memberUse();
+}
+
+void testExplicitCopy() {
+ Movable Mov{};
+ constRefReceiver(Movable{Mov});
+ // CHECK-MESSAGES: [[@LINE-1]]:28: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: constRefReceiver(Movable{std::move(Mov)});
+}
+
+Movable testReturn() {
+ Movable Mov{};
+ return Mov; // no warning, copy elision
+}
+
+Movable testReturn2(Movable && Mov, bool F) {
+ return F? Mov: Movable{};
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: return F? std::move(Mov): Movable{};
+}
+
+void rValReferenceTester(Movable&& Mov) {
+ valueReceiver(Mov);
+ valueReceiver(Mov);
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: valueReceiver(std::move(Mov));
+ Mov = Movable{};
+ valueReceiver(Mov);
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: valueReceiver(std::move(Mov));
+}
+
+void referenceTester(Movable& Mov) {
+ valueReceiver(Mov);
+ valueReceiver(Mov);
+ Mov = Movable{};
+ valueReceiver(Mov);
+}
+
+void pointerTester(Movable* Mov) {
+ valueReceiver(*Mov);
+ valueReceiver(*Mov);
+ *Mov = Movable{};
+ valueReceiver(*Mov);
+}
+
+// Replacements in expansions from macros or of their parameters are buggy, so we don't fix them.
+// Todo (future): The source location of macro parameters might be fixed in the future
+#define FUN(Mov) valueReceiver((Mov))
+void falseMacroExpansion() {
+ Movable Mov;
+ FUN(Mov);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: FUN(Mov);
+}
+
+template <class T>
+struct RemoveRef{
+ using type = T;
+};
+
+template <class T>
+struct RemoveRef<T&&>{
+ using type = T;
+};
+
+template <class T>
+struct RemoveRef<T&>{
+ using type = T;
+};
+
+template<class T>
+using RemoveRefT = typename RemoveRef<T>::type;
+
+template <class Movable>
+void initSomething(Movable&& Mov) {
+ valueReceiver(Mov);
+ valueReceiver(Mov);
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' may be copied on last use, consider forwarding it instead. [performance-unnecessary-copy-on-last-use]
+ Mov = RemoveRefT<Movable>{};
+ valueReceiver(Mov);
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' may be copied on last use, consider forwarding it instead. [performance-unnecessary-copy-on-last-use]
+}
+
+void initSomethingVal(Movable const& Mov) {
+ initSomething<Movable>(Movable{Mov});
+}
+
+void initSomethingRValRef(Movable const& Mov) {
+ initSomething<Movable&&>(Movable{Mov});
+}
+
+void initSomethingRef(Movable& Mov) {
+ initSomething<Movable&>(Mov);
+}
+
+// no "automatic - storage" tests:
+
+void staticTester() {
+ static Movable MovStatic;
+ valueReceiver(MovStatic);
+ valueReceiver(MovStatic);
+ MovStatic = Movable{};
+ valueReceiver(MovStatic);
+}
+
+void threadLocalTester() {
+ thread_local Movable MovThreadLocal;
+ valueReceiver(MovThreadLocal);
+ valueReceiver(MovThreadLocal);
+ MovThreadLocal = Movable{};
+ valueReceiver(MovThreadLocal);
+}
+
+void externTester() {
+ extern Movable MovExtern;
+ valueReceiver(MovExtern);
+ valueReceiver(MovExtern);
+ MovExtern = Movable{};
+ valueReceiver(MovExtern);
+}
+
+Movable MovGlobal;
+void globalTester() {
+ valueReceiver(MovGlobal);
+ valueReceiver(MovGlobal);
+ MovGlobal = Movable{};
+ valueReceiver(MovGlobal);
+}
+
+// lambda tests:
+void lambdaCaptureRefTester() {
+ Movable Mov{};
+ auto Lambda = [&Mov](){
+ Mov.memberUse();
+ };
+ Lambda();
+}
+
+void lambdaCaptureValueTester() {
+ Movable Mov{};
+ auto Lambda = [Mov]() mutable {
+ // CHECK-MESSAGES: [[@LINE-1]]:18: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: auto Lambda = [Mov]() mutable {
+ // Note: No fix, because a fix requires c++14.
+ Mov.memberUse();
+ };
+ Lambda();
+}
+
+/* Todo (improvement): currently the following is not fixed.
+ A differentiation between init-params in lambdas is required.
+*/
+void lambdaCaptureValueTester2() {
+ Movable Mov{};
+ auto Lambda = [Mov = Mov]() mutable {
+ // CHECK-MESSAGES: [[@LINE-1]]:24: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // NOCHECK-FIXES: auto Lambda = [Mov = std::move(Mov)]() mutable {
+ // Note: No fix, because a fix requires c++14.
+ Mov.memberUse();
+ };
+ Lambda();
+}
+
+void lambdaCaptureValueTester3() {
+ Movable Mov{};
+ auto Lambda = [=]() mutable {
+ // CHECK-MESSAGES: [[@LINE-1]]:18: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: auto Lambda = [=]() mutable {
+ // NOCHECK-FIXES: auto Lambda = [=, Mov = std::move(Mov)]() mutable {
+ // Note: No fix, because a fix requires c++14.
+ Mov.memberUse();
+ };
+ Lambda();
+}
+
+/*
+Todo (improvement):
+Currently the check does not find last usages of fields of local objects.
+
+struct MoveOwner{
+ Movable Mov{};
+};
+
+void testFieldMove(){
+ MoveOwner Owner{};
+ valueReceiver(Owner.Mov);
+ Owner.Mov = Movable{};
+ valueReceiver(Owner.Mov);
+ // DISABLED-CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Owner.Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // DISABLED-CHECK-FIXES: valueReceiver(std::move(Owner.Mov));
+}
+*/
+
+/*
+Todo (improvement):
+Currently a heuristic detection of guards is not implemented, so this test is disabled
+But before this is done, the check should not be used for automatic fixing
+
+using NoMove = std::shared_ptr<std::lock_guard<std::mutex>>;
+
+void shareMutex(NoMove Nmov);
+
+void testNoMove(std::mutex& M, int& I) {
+ NoMove Nmov = std::make_shared<std::lock_guard<std::mutex>>(M);
+ shareMutex(Nmov);
+ I = 42;
+}
+*/
Index: clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-copy-on-last-use.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-copy-on-last-use.rst
@@ -0,0 +1,77 @@
+.. title:: clang-tidy - performance-unnecessary-copy-on-last-use
+
+performance-unnecessary-copy-on-last-use
+========================================
+
+Finds variables of non-trivially-copyable types, that are used in a copy
+construction on their last usage and suggest to wrap the usage in a
+``std::move``. The usage just before an assignment is interpreted as last usage.
+
+Many programmers tend to forget ``std::move`` for variables that can be moved.
+Instead, the compiler creates implicit copies, which can significantly decrease
+performance.
+
+Example
+--------
+
+.. code-block:: c++
+
+ void acceptor(std::vector<int> v);
+ void Function() {
+ std::vector<int> v;
+ v.emplace_back(20);
+ // The warning will suggest passing this as a rvalue-reference
+ acceptor(v);
+ }
+
+Options
+-------
+
+.. option:: BlockedTypes
+
+ A semicolon-separated list of names of types allowed to be copied on last
+ usage. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches
+ every type with suffix `Ref`, `ref`, `Reference` and `reference`.
+ The default is empty. If a name in the list contains the sequence `::` it
+ is matched against the qualified typename (i.e. `namespace::Type`,
+ otherwise it is matched against only the type name (i.e. `Type`).
+
+.. option:: BlockedFunctions
+
+ A semicolon-separated list of names of functions who's parameters do not
+ participate in the resolution.
+ Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches
+ every type with suffix `Ref`, `ref`, `Reference` and `reference`.
+ The default is empty. If a name in the list contains the sequence `::` it
+ is matched against the qualified typename (i.e. `namespace::Type`,
+ otherwise it is matched against only the type name (i.e. `Type`).
+
+.. option:: IncludeStyle
+
+ A string specifying which include-style is used, `llvm` or `google`. Default
+ is `llvm`.
+
+Limitations
+-----------
+
+This check does not implement a heuristic, if a variable is used as guard until
+the end of it's scope. It also does not check, if a variable has any side
+effects in the destructor, which must be applied at the end of the scope.
+Therefore, it will suggest to use ``std::move`` in the
+following case, where `guard` protects `i`:
+
+.. code-block:: cpp
+
+ void acceptor(std::shared_ptr<std::unique_lock<std::mutex>>, int& i);
+
+ void f(std::shared_ptr<std::unique_lock<std::mutex>> guard, int& i) {
+ acceptor(guard, i);
+ i++;
+ }
+
+This check is also unable to detect last usages for fields, if the parent
+class is destructed after the last usage.
+
+Implicit copies in lambda-captures are detected, but no fixit is provided.
+Also, the check will warn for c++11 even if it is not possible to fix it without
+an language upgrade to at least c++14.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -318,6 +318,7 @@
`performance-trivially-destructible <performance/trivially-destructible.html>`_, "Yes"
`performance-type-promotion-in-math-fn <performance/type-promotion-in-math-fn.html>`_, "Yes"
`performance-unnecessary-copy-initialization <performance/unnecessary-copy-initialization.html>`_, "Yes"
+ `performance-unnecessary-copy-on-last-use <performance/unnecessary-copy-on-last-use.html>`_, "Yes"
`performance-unnecessary-value-param <performance/unnecessary-value-param.html>`_, "Yes"
`portability-restrict-system-includes <portability/restrict-system-includes.html>`_, "Yes"
`portability-simd-intrinsics <portability/simd-intrinsics.html>`_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,13 @@
Warns when using ``do-while`` loops.
+- New :doc:`performance-unnecessary-copy-on-last-use
+ <clang-tidy/checks/performance/unnecessary-copy-on-last-use>` check.
+
+ Finds variables of non-trivially-copyable types, that are used in a copy
+ construction on their last usage and suggest to wrap the usage in a
+ ``std::move``.
+
New check aliases
^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clangd/TidyProvider.cpp
===================================================================
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -212,6 +212,8 @@
// code, which is often the case when clangd
// tries to build an AST.
"-bugprone-use-after-move",
+ // Using an CFG might crash on invalid code:
+ "-performance-unnecessary-copy-on-last-use",
// Alias for bugprone-use-after-move.
"-hicpp-invalid-access-moved",
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.h
@@ -0,0 +1,62 @@
+//===--- UnnecessaryCopyOnLastUseCheck.h - clang-tidy ------------*- C++-*-===//
+//
+// 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_UNNECESSARY_COPY_ON_LAST_USE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_COPY_ON_LAST_USE_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include "llvm/ADT/DenseMap.h"
+
+namespace clang {
+
+class CFG;
+namespace tidy::performance {
+
+/// A check that flags value parameters on last usages that can safely be moved,
+/// because they are copied anyway.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance/unnecessary-copy-on-last-use.html
+
+class UnnecessaryCopyOnLastUseCheck : public ClangTidyCheck {
+public:
+ UnnecessaryCopyOnLastUseCheck(StringRef Name, ClangTidyContext *Context);
+ UnnecessaryCopyOnLastUseCheck(UnnecessaryCopyOnLastUseCheck &&) = delete;
+ UnnecessaryCopyOnLastUseCheck(const UnnecessaryCopyOnLastUseCheck &) = delete;
+ UnnecessaryCopyOnLastUseCheck &
+ operator=(UnnecessaryCopyOnLastUseCheck &&) = delete;
+ UnnecessaryCopyOnLastUseCheck &
+ operator=(const UnnecessaryCopyOnLastUseCheck &) = delete;
+ ~UnnecessaryCopyOnLastUseCheck() override;
+
+ 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;
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void onEndOfTranslationUnit() override;
+
+private:
+ CFG *getOrCreateCFG(FunctionDecl const *FD, ASTContext *C);
+
+ utils::IncludeInserter Inserter;
+ std::vector<StringRef> const BlockedTypes;
+ std::vector<StringRef> const BlockedFunctions;
+
+ // cfg cache
+ llvm::DenseMap<FunctionDecl const *, std::unique_ptr<CFG>> CFGs;
+};
+
+} // namespace tidy::performance
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_COPY_ON_LAST_USE_H
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.cpp
@@ -0,0 +1,357 @@
+//===--- UnnecessaryCopyOnLastUseCheck.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 "UnnecessaryCopyOnLastUseCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallVector.h"
+#include <algorithm>
+#include <cassert>
+#include <cstddef>
+#include <utility>
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::tidy::performance;
+using namespace clang::ast_matchers;
+
+static constexpr auto BlockedTypesOption = "BlockedTypes";
+static constexpr auto BlockedFunctionsOption = "BlockedFunctions";
+
+namespace {
+struct FindDeclRefBlockReturn {
+ CFGBlock const *DeclRefBlock = nullptr;
+ CFGBlock::const_iterator StartElement{};
+};
+
+enum class Usage {
+ Error = -1,
+ Usage = 0,
+ DefiniteLastUse,
+ LikelyDefiniteLastUse,
+};
+
+} // namespace
+
+static FindDeclRefBlockReturn findDeclRefBlock(CFG const *TheCFG,
+ DeclRefExpr const *DeclRef) {
+ for (CFGBlock *Block : *TheCFG) {
+ auto Iter =
+ llvm::find_if(Block->Elements, [&, DeclRef](CFGElement const &Element) {
+ if (Element.getKind() == CFGElement::Statement) {
+ return Element.template castAs<CFGStmt>().getStmt() == DeclRef;
+ }
+ return false;
+ });
+ if (Iter != Block->Elements.end()) {
+ return {Block, ++Iter};
+ }
+ }
+ return {nullptr, {}};
+}
+
+static clang::CFGElement const *
+nextUsageInCurrentBlock(FindDeclRefBlockReturn const &StartBlockElement,
+ DeclRefExpr const *DeclRef) {
+ // Search for uses in the current block
+ auto Begi = StartBlockElement.StartElement;
+ auto Endi = StartBlockElement.DeclRefBlock->Elements.end();
+ auto Iter = std::find_if(Begi, Endi, [&](CFGElement const &Element) {
+ if (Element.getKind() == CFGElement::Statement) {
+ if (auto *Stmt = Element.template castAs<CFGStmt>().getStmt()) {
+ if (auto *DRE = dyn_cast<DeclRefExpr>(Stmt)) {
+ if (DRE->getDecl() == DeclRef->getDecl()) {
+ return true;
+ }
+ }
+ }
+ }
+ return false;
+ });
+ return Iter != Endi ? &*Iter : nullptr;
+}
+
+static bool isLHSOfAssignment(DeclRefExpr const *DeclRef, ASTContext &Context) {
+ TraversalKindScope const RAII(Context, TK_IgnoreUnlessSpelledInSource);
+ // Todo (performance): While this is faster than a match expression,
+ // it would be faster to start from the DeclRefExpr directly
+ struct IsLHSOfAssignment : RecursiveASTVisitor<IsLHSOfAssignment> {
+ DeclRefExpr const *Ref{};
+
+ IsLHSOfAssignment(DeclRefExpr const *Ref, ASTContext &Context) : Ref(Ref) {}
+
+ bool shouldWalkTypesOfTypeLocs() const { return false; }
+ bool shouldVisitTemplateInstantiations() const { return true; }
+
+ bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr *BO) {
+ if (BO->isAssignmentOp()) {
+ if (auto *DRE =
+ dyn_cast<DeclRefExpr>(BO->getArg(0)->IgnoreParenImpCasts())) {
+ if (DRE && DRE == Ref) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+ };
+ return !IsLHSOfAssignment{DeclRef, Context}.TraverseAST(Context);
+}
+
+static bool isInLambdaCapture(DeclRefExpr const *MyDeclRef,
+ ASTContext &Context) {
+
+ // Todo (improvement): Expand this Visitor to also determine, if an explicit
+ // CaptureInitExpr is used or if a DeclRefExpr is used implicitelly via a
+ // cature default
+ struct IsInLambdaCapture : RecursiveASTVisitor<IsInLambdaCapture> {
+ DeclRefExpr const *Ref{};
+
+ IsInLambdaCapture(DeclRefExpr const *Ref) : Ref(Ref) {}
+
+ bool shouldWalkTypesOfTypeLocs() const { return false; }
+ bool shouldVisitTemplateInstantiations() const { return true; }
+
+ bool VisitLambdaExpr(LambdaExpr *Lambda) {
+ for (Expr *Inits : Lambda->capture_inits()) {
+ auto *S = cast<Stmt>(Inits);
+ llvm::SmallVector<Stmt *, 6> Childs;
+ while (S != nullptr) {
+ if (auto *DeclRef = dyn_cast<DeclRefExpr>(S);
+ DeclRef != nullptr && Ref == DeclRef) {
+ return false;
+ }
+ Childs.append(S->child_begin(), S->child_end());
+ S = Childs.empty() ? nullptr : Childs.pop_back_val();
+ }
+ }
+ return true;
+ }
+ };
+ return !IsInLambdaCapture{MyDeclRef}.TraverseAST(Context);
+}
+
+static Usage definiteLastUse(ASTContext *Context, CFG *const TheCFG,
+ DeclRefExpr const *DeclRef) {
+ if (TheCFG == nullptr) {
+ return Usage::Error;
+ }
+
+ // Find the CFGBlock containing the DeclRefExpr
+ auto StartBlockElement = findDeclRefBlock(TheCFG, DeclRef);
+ if (StartBlockElement.DeclRefBlock == nullptr) {
+ return Usage::Error; // Todo(unexpected): DeclRefExpr not found in CFG
+ }
+
+ // Find next uses of the DeclRefExpr
+
+ auto TraverseCFGForUsage = [&]() -> Usage {
+ llvm::SmallPtrSet<CFGBlock const *, 8> VisitedBlocks;
+ llvm::SmallVector<CFGBlock const *, 8> Worklist;
+
+ auto HandleInternal = [&](FindDeclRefBlockReturn const &BlockElement) {
+ CFGElement const *NextUsageE =
+ nextUsageInCurrentBlock(BlockElement, DeclRef);
+ if (NextUsageE) {
+ if (bool const IsLastUsage =
+ isLHSOfAssignment(llvm::cast<DeclRefExpr>(
+ NextUsageE->castAs<CFGStmt>().getStmt()),
+ *Context);
+ !IsLastUsage) {
+ return Usage::Usage;
+ }
+ return Usage::DefiniteLastUse;
+ }
+ assert(BlockElement.DeclRefBlock);
+ // No successing DeclRefExpr found, appending successors
+ for (CFGBlock const *Succ : BlockElement.DeclRefBlock->succs()) {
+ if (Succ) { // Succ can be nullptr, if a block is unreachable
+ Worklist.push_back(Succ);
+ }
+ }
+ return Usage::DefiniteLastUse; // No usage found, assume last use
+ };
+
+ if (auto FoundUsage = HandleInternal(StartBlockElement);
+ FoundUsage == Usage::Usage) { // Usage found
+ return FoundUsage;
+ }
+ while (!Worklist.empty()) {
+ CFGBlock const *Block = Worklist.pop_back_val();
+ if (!VisitedBlocks.insert(Block).second) {
+ continue;
+ }
+ if (auto FoundUsage = HandleInternal({Block, Block->Elements.begin()});
+ FoundUsage == Usage::Usage) {
+ return FoundUsage;
+ }
+ }
+ return Usage::DefiniteLastUse;
+ };
+
+ return TraverseCFGForUsage();
+}
+
+namespace clang::tidy::performance {
+
+UnnecessaryCopyOnLastUseCheck::~UnnecessaryCopyOnLastUseCheck() = default;
+
+UnnecessaryCopyOnLastUseCheck::UnnecessaryCopyOnLastUseCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()),
+ BlockedTypes(
+ utils::options::parseStringList(Options.get(BlockedTypesOption, ""))),
+ BlockedFunctions(utils::options::parseStringList(
+ Options.get(BlockedFunctionsOption, ""))),
+ CFGs() {}
+
+void UnnecessaryCopyOnLastUseCheck::registerMatchers(MatchFinder *Finder) {
+ auto const ValueParameter =
+ declRefExpr(
+ to(valueDecl(
+ unless(varDecl(unless(hasAutomaticStorageDuration()))),
+ hasType(qualType(
+ hasCanonicalType(qualType(
+ matchers::isExpensiveToCopy(),
+ unless(anyOf(isConstQualified(), lValueReferenceType(),
+ pointerType())))),
+ unless(hasDeclaration(namedDecl(
+ matchers::matchesAnyListedName(BlockedTypes))) //
+ ))))))
+ .bind("param");
+
+ auto const IsMoveAssignable = cxxOperatorCallExpr(
+ hasDeclaration(cxxMethodDecl(
+ isCopyAssignmentOperator(),
+ ofClass(hasMethod(cxxMethodDecl(isMoveAssignmentOperator(),
+ unless(isDeleted())))))),
+ hasRHS(ValueParameter));
+
+ auto const IsMoveConstructible =
+ ignoringElidableConstructorCall(ignoringParenImpCasts(
+ cxxConstructExpr(
+ unless(hasParent(callExpr(hasDeclaration(namedDecl(
+ matchers::matchesAnyListedName(BlockedFunctions)))))),
+ hasDeclaration(cxxConstructorDecl(
+ isCopyConstructor(),
+ ofClass(hasMethod(cxxConstructorDecl(isMoveConstructor(),
+ unless(isDeleted())))))),
+ hasArgument(0, ValueParameter))
+ .bind("constructExpr")));
+
+ Finder->addMatcher(stmt(anyOf(IsMoveAssignable, expr(IsMoveConstructible))),
+ this);
+}
+
+void UnnecessaryCopyOnLastUseCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *Param = Result.Nodes.getNodeAs<DeclRefExpr>("param");
+ const ValueDecl *const DeclOfParam = Param->getDecl();
+ const DeclContext *const FunctionOfDeclContext =
+ DeclOfParam->getParentFunctionOrMethod();
+
+ if (!FunctionOfDeclContext) {
+ // The parameter is not defined in a function, therefore it is not
+ // possible to check if it is the last use via CFG analysis
+ // Todo (improvement): Add a flag to show unanalyzable cases
+ return;
+ }
+
+ const auto *const FunctionOfDecl =
+ llvm::cast<FunctionDecl>(FunctionOfDeclContext);
+
+ const auto *const VarDeclVal = llvm::dyn_cast<VarDecl>(DeclOfParam);
+ if (!VarDeclVal) {
+ return;
+ }
+
+ Usage DefiniteLastUse = definiteLastUse(
+ Result.Context, getOrCreateCFG(FunctionOfDecl, Result.Context), Param);
+
+ if (DefiniteLastUse == Usage::Usage || DefiniteLastUse == Usage::Error) {
+ return;
+ }
+
+ // Template code cant be fixed currently
+ if (!FunctionOfDecl->isTemplateInstantiation()) {
+ clang::SourceManager &SM = *Result.SourceManager;
+ auto Diag =
+ diag(
+ Param->getExprLoc(),
+ "Parameter '%0' is copied on last use, consider moving it instead.")
+ << Param->getDecl()->getNameAsString();
+
+ if (auto *CExpr = Result.Nodes.getNodeAs<CXXConstructExpr>("constructExpr");
+ isInLambdaCapture(Param, *Result.Context) ||
+ (CExpr && CExpr->getExprLoc().isMacroID())) {
+ // Lambda captures should not be fixed.
+ // They also require at least c++14
+ return;
+ }
+ auto MVStmt = "std::move(" + Param->getDecl()->getNameAsString() + ")";
+ Diag << FixItHint::CreateReplacement(Param->getSourceRange(), MVStmt)
+ << Param->getDecl()->getNameAsString()
+ << Inserter.createIncludeInsertion(SM.getFileID(Param->getBeginLoc()),
+ "<utility>");
+ } else { // Template code can't be fixed currently, also a std::forward may be
+ // more appropriate
+ auto Diag =
+ diag(Param->getExprLoc(), "Parameter '%0' may be copied on last use, "
+ "consider forwarding it instead.")
+ << Param->getDecl()->getNameAsString();
+ }
+}
+
+void UnnecessaryCopyOnLastUseCheck::registerPPCallbacks(
+ const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+ Inserter.registerPreprocessor(PP);
+}
+
+void UnnecessaryCopyOnLastUseCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+ Options.store(Opts, BlockedTypesOption,
+ utils::options::serializeStringList(BlockedTypes));
+ Options.store(Opts, BlockedFunctionsOption,
+ utils::options::serializeStringList(BlockedFunctions));
+}
+
+void UnnecessaryCopyOnLastUseCheck::onEndOfTranslationUnit() {}
+
+static CFG::BuildOptions createBuildOptions() {
+ CFG::BuildOptions Options;
+ Options.setAlwaysAdd(DeclRefExpr::DeclRefExprClass);
+ Options.AddImplicitDtors = true;
+ Options.AddTemporaryDtors = true;
+ return Options;
+}
+
+CFG *UnnecessaryCopyOnLastUseCheck::getOrCreateCFG(FunctionDecl const *FD,
+ ASTContext *C) {
+ static auto BO = createBuildOptions();
+ if (auto Iter = this->CFGs.find(FD); Iter != this->CFGs.end()) {
+ return Iter->second.get();
+ }
+
+ auto EmplaceResult =
+ this->CFGs.try_emplace(FD, CFG::buildCFG(nullptr, FD->getBody(), C, BO));
+ return EmplaceResult.first->second.get();
+}
+} // namespace clang::tidy::performance
Index: clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
+++ clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -23,6 +23,7 @@
#include "TriviallyDestructibleCheck.h"
#include "TypePromotionInMathFnCheck.h"
#include "UnnecessaryCopyInitialization.h"
+#include "UnnecessaryCopyOnLastUseCheck.h"
#include "UnnecessaryValueParamCheck.h"
namespace clang {
@@ -59,6 +60,8 @@
"performance-type-promotion-in-math-fn");
CheckFactories.registerCheck<UnnecessaryCopyInitialization>(
"performance-unnecessary-copy-initialization");
+ CheckFactories.registerCheck<UnnecessaryCopyOnLastUseCheck>(
+ "performance-unnecessary-copy-on-last-use");
CheckFactories.registerCheck<UnnecessaryValueParamCheck>(
"performance-unnecessary-value-param");
}
Index: clang-tools-extra/clang-tidy/performance/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -18,6 +18,7 @@
PerformanceTidyModule.cpp
TriviallyDestructibleCheck.cpp
TypePromotionInMathFnCheck.cpp
+ UnnecessaryCopyOnLastUseCheck.cpp
UnnecessaryCopyInitialization.cpp
UnnecessaryValueParamCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
