ImplicitCastInLoopCheck.cpp
PerformanceTidyModule.cpp
UnnecessaryCopyInitialization.cpp
+ UnnecessaryValueParamCheck.cpp
LINK_LIBS
clangAST
#include "ForRangeCopyCheck.h"
#include "ImplicitCastInLoopCheck.h"
#include "UnnecessaryCopyInitialization.h"
+#include "UnnecessaryValueParamCheck.h"
namespace clang {
namespace tidy {
"performance-implicit-cast-in-loop");
CheckFactories.registerCheck<UnnecessaryCopyInitialization>(
"performance-unnecessary-copy-initialization");
+ CheckFactories.registerCheck<UnnecessaryValueParamCheck>(
+ "performance-unnecessary-value-param");
}
};
--- /dev/null
+//===--- UnnecessaryValueParamCheck.cpp - clang-tidy-----------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UnnecessaryValueParamCheck.h"
+
+#include "../utils/DeclRefExprUtils.h"
+#include "../utils/FixItHintUtils.h"
+#include "../utils/Matchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+namespace {
+
+std::string paramNameOrIndex(StringRef Name, size_t Index) {
+ return (Name.empty() ? llvm::Twine('#') + llvm::Twine(Index + 1)
+ : llvm::Twine('\'') + Name + llvm::Twine('\''))
+ .str();
+}
+
+} // namespace
+
+void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
+ const auto ExpensiveValueParamDecl =
+ parmVarDecl(hasType(hasCanonicalType(allOf(matchers::isExpensiveToCopy(),
+ unless(referenceType())))),
+ decl().bind("param"));
+ Finder->addMatcher(
+ functionDecl(isDefinition(), unless(cxxMethodDecl(isOverride())),
+ unless(isInstantiated()),
+ has(typeLoc(forEach(ExpensiveValueParamDecl))),
+ decl().bind("functionDecl")),
+ this);
+}
+
+void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param");
+ const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("functionDecl");
+ const size_t Index = std::find(Function->parameters().begin(),
+ Function->parameters().end(), Param) -
+ Function->parameters().begin();
+ bool IsConstQualified =
+ Param->getType().getCanonicalType().isConstQualified();
+
+ // Do not trigger on non-const value parameters when:
+ // 1. they are in a constructor definition since they can likely trigger
+ // misc-move-constructor-init which will suggest to move the argument.
+ // 2. they are not only used as const.
+ if (!IsConstQualified && (llvm::isa<CXXConstructorDecl>(Function) ||
+ !Function->doesThisDeclarationHaveABody() ||
+ !decl_ref_expr_utils::isOnlyUsedAsConst(
+ *Param, *Function->getBody(), *Result.Context)))
+ return;
+ auto Diag =
+ diag(Param->getLocation(),
+ IsConstQualified ? "the const qualified parameter %0 is "
+ "copied for each invocation; consider "
+ "making it a reference"
+ : "the parameter %0 is copied for each "
+ "invocation but only used as a const reference; "
+ "consider making it a const reference")
+ << paramNameOrIndex(Param->getName(), Index);
+ // Do not propose fixes in macros since we cannot place them correctly.
+ if (Param->getLocStart().isMacroID())
+ return;
+ for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
+ FunctionDecl = FunctionDecl->getPreviousDecl()) {
+ const auto &CurrentParam = *FunctionDecl->getParamDecl(Index);
+ Diag << utils::create_fix_it::changeVarDeclToReference(CurrentParam,
+ *Result.Context);
+ if (!IsConstQualified)
+ Diag << utils::create_fix_it::changeVarDeclToConst(CurrentParam);
+ }
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
--- /dev/null
+//===--- UnnecessaryValueParamCheck.h - clang-tidy---------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_VALUE_PARAM_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_VALUE_PARAM_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// \brief A check that flags value parameters of expensive to copy types that
+/// can safely be converted to const references.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-unnecessary-value-param.html
+class UnnecessaryValueParamCheck : public ClangTidyCheck {
+public:
+ UnnecessaryValueParamCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_VALUE_PARAM_H
performance-faster-string-find
performance-for-range-copy
performance-implicit-cast-in-loop
+ performance-unnecessary-value-param
performance-unnecessary-copy-initialization
readability-braces-around-statements
readability-container-size-empty
--- /dev/null
+.. title:: clang-tidy - performance-unnecessary-value-param
+
+performance-unnecessary-value-param
+===================================
+
+Flags value parameter declarations of expensive to copy types that are copied
+for each invocation but it would suffice to pass them by const reference.
+
+The check is only applied to parameters of types that are expensive to copy
+which means they are not trivially copyable or have a non-trivial copy
+constructor or destructor.
+
+To ensure that it is safe to replace the value paramater with a const reference
+the following heuristic is employed:
+
+1. the parameter is const qualified;
+2. the parameter is not const, but only const methods or operators are invoked
+ on it, or it is used as const reference or value argument in constructors or
+ function calls.
+
+Example:
+
+.. code-block:: c++
+
+ void f(const string Value) {
+ // The warning will suggest making Value a reference.
+ }
+
+ void g(ExpensiveToCopy Value) {
+ // The warning will suggest making Value a const reference.
+ Value.ConstMethd();
+ ExpensiveToCopy Copy(Value);
+ }
--- /dev/null
+// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t
+
+struct ExpensiveToCopyType {
+ const ExpensiveToCopyType & constReference() const {
+ return *this;
+ }
+ void nonConstMethod();
+ virtual ~ExpensiveToCopyType();
+};
+
+void mutate(ExpensiveToCopyType &);
+void mutate(ExpensiveToCopyType *);
+void useAsConstReference(const ExpensiveToCopyType &);
+void useByValue(ExpensiveToCopyType);
+
+// This class simulates std::pair<>. It is trivially copy constructible
+// and trivially destructible, but not trivially copy assignable.
+class SomewhatTrivial {
+ public:
+ SomewhatTrivial();
+ SomewhatTrivial(const SomewhatTrivial&) = default;
+ ~SomewhatTrivial() = default;
+ SomewhatTrivial& operator=(const SomewhatTrivial&);
+};
+
+void positiveExpensiveConstValue(const ExpensiveToCopyType Obj);
+// CHECK-FIXES: void positiveExpensiveConstValue(const ExpensiveToCopyType& Obj);
+void positiveExpensiveConstValue(const ExpensiveToCopyType Obj) {
+ // CHECK-MESSAGES: [[@LINE-1]]:60: warning: the const qualified parameter 'Obj' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]
+ // CHECK-FIXES: void positiveExpensiveConstValue(const ExpensiveToCopyType& Obj) {
+}
+
+void positiveExpensiveValue(ExpensiveToCopyType Obj);
+// CHECK-FIXES: void positiveExpensiveValue(const ExpensiveToCopyType& Obj);
+void positiveExpensiveValue(ExpensiveToCopyType Obj) {
+ // CHECK-MESSAGES: [[@LINE-1]]:49: warning: the parameter 'Obj' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
+ // CHECK-FIXES: void positiveExpensiveValue(const ExpensiveToCopyType& Obj) {
+ Obj.constReference();
+ useAsConstReference(Obj);
+ auto Copy = Obj;
+ useByValue(Obj);
+}
+
+void positiveWithComment(const ExpensiveToCopyType /* important */ S);
+// CHECK-FIXES: void positiveWithComment(const ExpensiveToCopyType& /* important */ S);
+void positiveWithComment(const ExpensiveToCopyType /* important */ S) {
+ // CHECK-MESSAGES: [[@LINE-1]]:68: warning: the const qualified
+ // CHECK-FIXES: void positiveWithComment(const ExpensiveToCopyType& /* important */ S) {
+}
+
+void positiveUnnamedParam(const ExpensiveToCopyType) {
+ // CHECK-MESSAGES: [[@LINE-1]]:52: warning: the const qualified parameter #1
+ // CHECK-FIXES: void positiveUnnamedParam(const ExpensiveToCopyType&) {
+}
+
+void positiveAndNegative(const ExpensiveToCopyType ConstCopy, const ExpensiveToCopyType& ConstRef, ExpensiveToCopyType Copy);
+// CHECK-FIXES: void positiveAndNegative(const ExpensiveToCopyType& ConstCopy, const ExpensiveToCopyType& ConstRef, const ExpensiveToCopyType& Copy);
+void positiveAndNegative(const ExpensiveToCopyType ConstCopy, const ExpensiveToCopyType& ConstRef, ExpensiveToCopyType Copy) {
+ // CHECK-MESSAGES: [[@LINE-1]]:52: warning: the const qualified parameter 'ConstCopy'
+ // CHECK-MESSAGES: [[@LINE-2]]:120: warning: the parameter 'Copy'
+ // CHECK-FIXES: void positiveAndNegative(const ExpensiveToCopyType& ConstCopy, const ExpensiveToCopyType& ConstRef, const ExpensiveToCopyType& Copy) {
+}
+
+struct PositiveConstValueConstructor {
+ PositiveConstValueConstructor(const ExpensiveToCopyType ConstCopy) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:59: warning: the const qualified parameter 'ConstCopy'
+};
+
+template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType S, T V) {
+ // CHECK-MESSAGES: [[@LINE-1]]:90: warning: the const qualified parameter 'S'
+ // CHECK-FIXES: template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType& S, T V) {
+}
+
+void instantiated() {
+ templateWithNonTemplatizedParameter(ExpensiveToCopyType(), ExpensiveToCopyType());
+ templateWithNonTemplatizedParameter(ExpensiveToCopyType(), 5);
+}
+
+template <typename T> void negativeTemplateType(const T V) {
+}
+
+void negativeArray(const ExpensiveToCopyType[]) {
+}
+
+void negativePointer(ExpensiveToCopyType* Obj) {
+}
+
+void negativeConstPointer(const ExpensiveToCopyType* Obj) {
+}
+
+void negativeConstReference(const ExpensiveToCopyType& Obj) {
+}
+
+void negativeReference(ExpensiveToCopyType& Obj) {
+}
+
+void negativeUniversalReference(ExpensiveToCopyType&& Obj) {
+}
+
+void negativeSomewhatTrivialConstValue(const SomewhatTrivial Somewhat) {
+}
+
+void negativeSomewhatTrivialValue(SomewhatTrivial Somewhat) {
+}
+
+void negativeConstBuiltIn(const int I) {
+}
+
+void negativeValueBuiltIn(int I) {
+}
+
+void negativeValueIsMutatedByReference(ExpensiveToCopyType Obj) {
+ mutate(Obj);
+}
+
+void negativeValueIsMutatatedByPointer(ExpensiveToCopyType Obj) {
+ mutate(&Obj);
+}
+
+void negativeValueIsReassigned(ExpensiveToCopyType Obj) {
+ Obj = ExpensiveToCopyType();
+}
+
+void negativeValueNonConstMethodIsCalled(ExpensiveToCopyType Obj) {
+ Obj.nonConstMethod();
+}
+
+struct NegativeValueCopyConstructor {
+ NegativeValueCopyConstructor(ExpensiveToCopyType Copy) {}
+};
+
+template <typename T>
+struct Container {
+ typedef const T & const_reference;
+};
+
+void NegativeTypedefParam(const Container<ExpensiveToCopyType>::const_reference Param) {
+}
+
+#define UNNECESSARY_VALUE_PARAM_IN_MACRO_BODY() \
+ void inMacro(const ExpensiveToCopyType T) { \
+ } \
+// Ensure fix is not applied.
+// CHECK-FIXES: void inMacro(const ExpensiveToCopyType T) {
+
+UNNECESSARY_VALUE_PARAM_IN_MACRO_BODY()
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: the const qualified parameter 'T'
+
+#define UNNECESSARY_VALUE_PARAM_IN_MACRO_ARGUMENT(ARGUMENT) \
+ ARGUMENT
+
+UNNECESSARY_VALUE_PARAM_IN_MACRO_ARGUMENT(void inMacroArgument(const ExpensiveToCopyType InMacroArg) {})
+// CHECK-MESSAGES: [[@LINE-1]]:90: warning: the const qualified parameter 'InMacroArg'
+// CHECK-FIXES: void inMacroArgument(const ExpensiveToCopyType InMacroArg) {}
+
+struct VirtualMethod {
+ virtual ~VirtualMethod() {}
+ virtual void handle(ExpensiveToCopyType T) const = 0;
+};
+
+struct NegativeOverriddenMethod : public VirtualMethod {
+ void handle(ExpensiveToCopyType Overridden) const {
+ // CHECK-FIXES: handle(ExpensiveToCopyType Overridden) const {
+ }
+};
+
+struct NegativeDeletedMethod {
+ ~NegativeDeletedMethod() {}
+ NegativeDeletedMethod& operator=(NegativeDeletedMethod N) = delete;
+ // CHECK-FIXES: NegativeDeletedMethod& operator=(NegativeDeletedMethod N) = delete;
+};