diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt @@ -18,6 +18,7 @@ PerformanceTidyModule.cpp TriviallyDestructibleCheck.cpp TypePromotionInMathFnCheck.cpp + UnnecessaryCopyOnLastUseCheck.cpp UnnecessaryCopyInitialization.cpp UnnecessaryValueParamCheck.cpp diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp --- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp +++ b/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( "performance-unnecessary-copy-initialization"); + CheckFactories.registerCheck( + "performance-unnecessary-copy-on-last-use"); CheckFactories.registerCheck( "performance-unnecessary-value-param"); } diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.h new file mode 100644 --- /dev/null +++ b/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(const FunctionDecl *FD, ASTContext *C); + + utils::IncludeInserter Inserter; + const std::vector BlockedTypes; + const std::vector BlockedFunctions; + + // cfg cache + llvm::DenseMap> CFGs; +}; + +} // namespace tidy::performance +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_COPY_ON_LAST_USE_H diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.cpp new file mode 100644 --- /dev/null +++ b/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 +#include +#include +#include + +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 { + const CFGBlock *DeclRefBlock = nullptr; + CFGBlock::const_iterator StartElement{}; +}; + +enum class Usage { + Error = -1, + Usage = 0, + DefiniteLastUse, + LikelyDefiniteLastUse, +}; + +} // namespace + +static FindDeclRefBlockReturn findDeclRefBlock(CFG const *TheCFG, + const DeclRefExpr *DeclRef) { + for (CFGBlock *Block : *TheCFG) { + auto Iter = + llvm::find_if(Block->Elements, [&, DeclRef](const CFGElement &Element) { + if (Element.getKind() == CFGElement::Statement) { + return Element.template castAs().getStmt() == DeclRef; + } + return false; + }); + if (Iter != Block->Elements.end()) { + return {Block, ++Iter}; + } + } + return {nullptr, {}}; +} + +static const clang::CFGElement * +nextUsageInCurrentBlock(const FindDeclRefBlockReturn &StartBlockElement, + const DeclRefExpr *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, [&](const CFGElement &Element) { + if (Element.getKind() == CFGElement::Statement) { + if (auto *Stmt = Element.template castAs().getStmt()) { + if (auto *DRE = dyn_cast(Stmt)) { + if (DRE->getDecl() == DeclRef->getDecl()) { + return true; + } + } + } + } + return false; + }); + return Iter != Endi ? &*Iter : nullptr; +} + +static bool isLHSOfAssignment(const DeclRefExpr *DeclRef, ASTContext &Context) { + const TraversalKindScope 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 { + const DeclRefExpr *Ref{}; + + IsLHSOfAssignment(const DeclRefExpr *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(BO->getArg(0)->IgnoreParenImpCasts())) { + if (DRE && DRE == Ref) { + return false; + } + } + } + return true; + } + }; + return !IsLHSOfAssignment{DeclRef, Context}.TraverseAST(Context); +} + +static bool isInLambdaCapture(const DeclRefExpr *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 { + const DeclRefExpr *Ref{}; + + IsInLambdaCapture(const DeclRefExpr *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(Inits); + llvm::SmallVector Childs; + while (S != nullptr) { + if (auto *DeclRef = dyn_cast(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, + const DeclRefExpr *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 VisitedBlocks; + llvm::SmallVector Worklist; + + auto HandleInternal = [&](FindDeclRefBlockReturn const &BlockElement) { + CFGElement const *NextUsageE = + nextUsageInCurrentBlock(BlockElement, DeclRef); + if (NextUsageE) { + if (bool const IsLastUsage = + isLHSOfAssignment(llvm::cast( + NextUsageE->castAs().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) { + const auto 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"); + + const auto IsMoveAssignable = cxxOperatorCallExpr( + hasDeclaration(cxxMethodDecl( + isCopyAssignmentOperator(), + ofClass(hasMethod(cxxMethodDecl(isMoveAssignmentOperator(), + unless(isDeleted())))))), + hasRHS(ValueParameter)); + + const auto 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("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(FunctionOfDeclContext); + + const auto *const VarDeclVal = llvm::dyn_cast(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("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()), + ""); + } 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(const FunctionDecl *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 diff --git a/clang-tools-extra/clangd/TidyProvider.cpp b/clang-tools-extra/clangd/TidyProvider.cpp --- a/clang-tools-extra/clangd/TidyProvider.cpp +++ b/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", diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -115,6 +115,13 @@ Warns when using ``do-while`` loops. +- New :doc:`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 ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -318,6 +318,7 @@ `performance-trivially-destructible `_, "Yes" `performance-type-promotion-in-math-fn `_, "Yes" `performance-unnecessary-copy-initialization `_, "Yes" + `performance-unnecessary-copy-on-last-use `_, "Yes" `performance-unnecessary-value-param `_, "Yes" `portability-restrict-system-includes `_, "Yes" `portability-simd-intrinsics `_, diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-copy-on-last-use.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-copy-on-last-use.rst new file mode 100644 --- /dev/null +++ b/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 v); + void Function() { + std::vector 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>, int& i); + + void f(std::shared_ptr> 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. diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-on-last-use.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-on-last-use.cpp new file mode 100644 --- /dev/null +++ b/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 + +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 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 +struct RemoveRef{ + using type = T; +}; + +template +struct RemoveRef{ + using type = T; +}; + +template +struct RemoveRef{ + using type = T; +}; + +template +using RemoveRefT = typename RemoveRef::type; + +template +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{}; + 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{Mov}); +} + +void initSomethingRValRef(Movable const& Mov) { + initSomething(Movable{Mov}); +} + +void initSomethingRef(Movable& Mov) { + initSomething(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>; + +void shareMutex(NoMove Nmov); + +void testNoMove(std::mutex& M, int& I) { + NoMove Nmov = std::make_shared>(M); + shareMutex(Nmov); + I = 42; +} +*/