[clang-tools-extra] r264694 - [clang-tidy] Add performance check to flag function parameters of expensive to copy types that can be safely converted to const references.
Felix Berger via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 28 19:42:38 PDT 2016
Author: flx
Date: Mon Mar 28 21:42:38 2016
New Revision: 264694
URL: http://llvm.org/viewvc/llvm-project?rev=264694&view=rev
Log:
[clang-tidy] Add performance check to flag function parameters of expensive to copy types that can be safely converted to const references.
Reviewers: alexfh
Subscribers: fowles, cfe-commits
Differential Revision: http://reviews.llvm.org/D17491
Added:
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt?rev=264694&r1=264693&r2=264694&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt Mon Mar 28 21:42:38 2016
@@ -6,6 +6,7 @@ add_clang_library(clangTidyPerformanceMo
ImplicitCastInLoopCheck.cpp
PerformanceTidyModule.cpp
UnnecessaryCopyInitialization.cpp
+ UnnecessaryValueParamCheck.cpp
LINK_LIBS
clangAST
Modified: clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp?rev=264694&r1=264693&r2=264694&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp Mon Mar 28 21:42:38 2016
@@ -15,6 +15,7 @@
#include "ForRangeCopyCheck.h"
#include "ImplicitCastInLoopCheck.h"
#include "UnnecessaryCopyInitialization.h"
+#include "UnnecessaryValueParamCheck.h"
namespace clang {
namespace tidy {
@@ -31,6 +32,8 @@ public:
"performance-implicit-cast-in-loop");
CheckFactories.registerCheck<UnnecessaryCopyInitialization>(
"performance-unnecessary-copy-initialization");
+ CheckFactories.registerCheck<UnnecessaryValueParamCheck>(
+ "performance-unnecessary-value-param");
}
};
Added: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp?rev=264694&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp Mon Mar 28 21:42:38 2016
@@ -0,0 +1,87 @@
+//===--- 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
Added: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h?rev=264694&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h Mon Mar 28 21:42:38 2016
@@ -0,0 +1,36 @@
+//===--- 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
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=264694&r1=264693&r2=264694&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Mon Mar 28 21:42:38 2016
@@ -89,6 +89,7 @@ Clang-Tidy Checks
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
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst?rev=264694&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst Mon Mar 28 21:42:38 2016
@@ -0,0 +1,33 @@
+.. 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);
+ }
Added: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param.cpp?rev=264694&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param.cpp Mon Mar 28 21:42:38 2016
@@ -0,0 +1,171 @@
+// 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;
+};
More information about the cfe-commits
mailing list