cjdb created this revision.
cjdb added a reviewer: aaron.ballman.
Herald added subscribers: xazax.hun, mgorny.
cjdb requested review of this revision.
Herald added projects: LLVM, clang-tools-extra.
Herald added subscribers: cfe-commits, llvm-commits.
Checks ``const``-qualified parameters to determine if they should be
passed by value or by reference-to-``const`` for function definitions.
Ignores proper function declarations, parameters without ``const`` in
their type specifier, and dependent types.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D107873
Files:
clang-tools-extra/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/clang-tidy/performance/ConstParameterValueOrRef.cpp
clang-tools-extra/clang-tidy/performance/ConstParameterValueOrRef.h
clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
clang-tools-extra/docs/clang-tidy/checks/performance-const-parameter-value-or-ref.rst
clang-tools-extra/test/clang-tidy/checkers/performance-const-parameter-value-or-ref.cpp
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/performance/BUILD.gn
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/performance/BUILD.gn
===================================================================
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/performance/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/performance/BUILD.gn
@@ -12,6 +12,7 @@
"//llvm/lib/Support",
]
sources = [
+ "ConstParameterValueOrRef.cpp",
"FasterStringFindCheck.cpp",
"ForRangeCopyCheck.cpp",
"ImplicitConversionInLoopCheck.cpp",
Index: clang-tools-extra/test/clang-tidy/checkers/performance-const-parameter-value-or-ref.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-const-parameter-value-or-ref.cpp
@@ -0,0 +1,260 @@
+// RUN: %check_clang_tidy %s performance-const-parameter-value-or-ref %t
+
+template <class T, int N>
+struct array {
+ T base[N];
+};
+
+using MaxSmall = array<char, 16>;
+using MinLarge = array<char, 17>;
+
+class NotTriviallyCopyable {
+public:
+ NotTriviallyCopyable() = default;
+
+ NotTriviallyCopyable(const NotTriviallyCopyable &) {}
+};
+
+// No warnings for forward declarations: this is the domain of
+// readability-avoid-const-params-in-decls
+void f1(const __int128);
+void f1(const MaxSmall);
+void f1(const MinLarge);
+void f1(const NotTriviallyCopyable);
+
+struct S1 {
+ void f(const __int128);
+ void f(const MaxSmall);
+ void f(const MinLarge);
+ void f(const NotTriviallyCopyable);
+};
+
+void f2(const __int128 &);
+void f2(const MaxSmall &);
+void f2(const MinLarge &);
+void f2(const NotTriviallyCopyable &);
+void f2(const array<NotTriviallyCopyable, 1> &);
+
+struct S2 {
+ void f(const __int128 &);
+ void f(const MaxSmall &);
+ void f(const MinLarge &);
+ void f(const NotTriviallyCopyable &);
+ void f(const array<NotTriviallyCopyable, 1> &);
+};
+
+// No warnings for function definitions that have been forward declared (pass
+// by value only).
+void f1(const MinLarge) {}
+void f1(const NotTriviallyCopyable) {}
+
+// No warnings when passing by value + mutable parameter
+void f3(__int128) {}
+void f3(MaxSmall) {}
+void f3(MinLarge) {}
+void f3(NotTriviallyCopyable) {}
+void f3(array<NotTriviallyCopyable, 1>) {}
+
+struct S3 {
+ void f(__int128) {}
+ void f(MaxSmall) {}
+ void f(MinLarge) {}
+ void f(NotTriviallyCopyable) {}
+ void f(array<NotTriviallyCopyable, 1>) {}
+};
+
+// No warnings when passing by value + "small" parameter
+void f4(const __int128) {}
+void f4(const MaxSmall) {}
+
+struct S4 {
+ void f(const __int128) {}
+ void f(const MaxSmall) {}
+};
+
+// No warnings when passing by reference-to-const + "large"/non-trivial parameter
+void f5(const MinLarge &) {}
+void f5(const NotTriviallyCopyable &) {}
+void f5(const array<NotTriviallyCopyable, 1> &) {}
+
+struct S5 {
+ void f(const MinLarge &) {}
+ void f(const NotTriviallyCopyable &) {}
+ void f(const array<NotTriviallyCopyable, 1> &) {}
+};
+
+// No warnings for dependent types
+template <class T>
+T f6(const T t) { return t; }
+
+const auto R1 = f6(false);
+const auto R2 = f6(MaxSmall());
+const auto R3 = f6(MinLarge());
+const auto R4 = f6(NotTriviallyCopyable());
+
+struct S6 {
+ template <class T>
+ T f(const T t) { return t; }
+
+ S6() {
+ const auto R1 = f(false);
+ const auto R2 = f(MaxSmall());
+ const auto R3 = f(MinLarge());
+ const auto R4 = f(NotTriviallyCopyable());
+ }
+};
+
+template <class T>
+T f7(const T &t) { return t; }
+
+const auto R5 = f7(false);
+const auto R6 = f7(MaxSmall());
+const auto R7 = f7(MinLarge());
+const auto R8 = f7(NotTriviallyCopyable());
+
+struct S7 {
+ template <class T>
+ T f(const T &t) { return t; }
+
+ S7() {
+ const auto R5 = f(false);
+ const auto R6 = f(MaxSmall());
+ const auto R7 = f(MinLarge());
+ const auto R8 = f(NotTriviallyCopyable());
+ }
+};
+
+////////////////////////////////////////////////////////////////////////////////
+// Warn when passing a "small" parameter by reference-to-const.
+////////////////////////////////////////////////////////////////////////////////
+void f2(const __int128 &) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'const __int128' is a small, trivially-copyable type: pass by value instead
+// CHECK-FIXES: void f2(const __int128) {}
+
+void f8(const __int128 &) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'const __int128' is a small, trivially-copyable type: pass by value instead
+// CHECK-FIXES: void f8(const __int128) {}
+
+void f8(const MaxSmall &) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'const MaxSmall' (aka 'const array<char, 16>') is a small, trivially-copyable type: pass by value instead
+// CHECK-FIXES: void f8(const MaxSmall) {}
+
+template <class T>
+void f8(T, const __int128 &) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'const __int128' is a small, trivially-copyable type: pass by value instead
+// CHECK-FIXES: void f8(T, const __int128) {}
+
+template <class T>
+void f8(const MaxSmall &, T) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'const MaxSmall' (aka 'const array<char, 16>') is a small, trivially-copyable type: pass by value instead
+// CHECK-FIXES: void f8(const MaxSmall, T) {}
+
+struct S8 {
+ void f128(const __int128 &) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'const __int128' is a small, trivially-copyable type: pass by value instead
+ // CHECK-FIXES: void f128(const __int128) {}
+
+ void fMaxSmall(const MaxSmall &) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: 'const MaxSmall' (aka 'const array<char, 16>') is a small, trivially-copyable type: pass by value instead
+ // CHECK-FIXES: void fMaxSmall(const MaxSmall) {}
+
+ template <class T>
+ void template128(T, const __int128 &) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: 'const __int128' is a small, trivially-copyable type: pass by value instead
+ // CHECK-FIXES: void template128(T, const __int128) {}
+
+ template <class T>
+ void templateMaxSmall(const MaxSmall &, T) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: 'const MaxSmall' (aka 'const array<char, 16>') is a small, trivially-copyable type: pass by value instead
+ // CHECK-FIXES: void templateMaxSmall(const MaxSmall, T) {}
+};
+
+////////////////////////////////////////////////////////////////////////////////
+// Warn when passing a "large" or non-trivially-copyable parameter by value (and
+// it's const-qualified).
+////////////////////////////////////////////////////////////////////////////////
+void f9(const MinLarge x) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'const MinLarge' (aka 'const array<char, 17>') is a large type: pass by reference-to-const instead
+// CHECK-FIXES: void f9(const MinLarge &x) {}
+
+void f9(const NotTriviallyCopyable x) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: 'const NotTriviallyCopyable' is not trivially-copyable: pass by reference-to-const instead
+// CHECK-FIXES: void f9(const NotTriviallyCopyable &x) {}
+
+void f9(const array<NotTriviallyCopyable, 1> x) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: 'const array<NotTriviallyCopyable, 1>' is not trivially-copyable: pass by reference-to-const instead
+// CHECK-FIXES: void f9(const array<NotTriviallyCopyable, 1> &x) {}
+
+template <class T>
+void f9(T, const MinLarge x) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'const MinLarge' (aka 'const array<char, 17>') is a large type: pass by reference-to-const instead
+// CHECK-FIXES: void f9(T, const MinLarge &x) {}
+
+template <class T>
+void f9(const NotTriviallyCopyable x, T) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: 'const NotTriviallyCopyable' is not trivially-copyable: pass by reference-to-const instead
+// CHECK-FIXES: void f9(const NotTriviallyCopyable &x, T) {}
+
+template <class T>
+void f9(const array<NotTriviallyCopyable, 1> x, T) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: 'const array<NotTriviallyCopyable, 1>' is not trivially-copyable: pass by reference-to-const instead
+// CHECK-FIXES: void f9(const array<NotTriviallyCopyable, 1> &x, T) {}
+
+struct S9 {
+ void fMinLarge(const MinLarge x) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: 'const MinLarge' (aka 'const array<char, 17>') is a large type: pass by reference-to-const instead
+ // CHECK-FIXES: void fMinLarge(const MinLarge &x) {}
+
+ void fNTC(const NotTriviallyCopyable x) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: 'const NotTriviallyCopyable' is not trivially-copyable: pass by reference-to-const instead
+ // CHECK-FIXES: void fNTC(const NotTriviallyCopyable &x) {}
+
+ void fTNTC(const array<NotTriviallyCopyable, 1> x) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:51: warning: 'const array<NotTriviallyCopyable, 1>' is not trivially-copyable: pass by reference-to-const instead
+ // CHECK-FIXES: void fTNTC(const array<NotTriviallyCopyable, 1> &x) {}
+
+ template <class T>
+ void templateMinLarge(T, const MinLarge x) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: 'const MinLarge' (aka 'const array<char, 17>') is a large type: pass by reference-to-const instead
+ // CHECK-FIXES: void templateMinLarge(T, const MinLarge &x) {}
+
+ template <class T>
+ void templateNTC(const NotTriviallyCopyable x, T) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: 'const NotTriviallyCopyable' is not trivially-copyable: pass by reference-to-const instead
+ // CHECK-FIXES: void templateNTC(const NotTriviallyCopyable &x, T) {}
+
+ template <class T>
+ void templateTNTC(const array<NotTriviallyCopyable, 1> x, T) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:58: warning: 'const array<NotTriviallyCopyable, 1>' is not trivially-copyable: pass by reference-to-const instead
+ // CHECK-FIXES: void templateTNTC(const array<NotTriviallyCopyable, 1> &x, T) {}
+};
+
+////////////////////////////////////////////////////////////////////////////////
+// Default non-trivially copyable exemptions: no warnings expected
+////////////////////////////////////////////////////////////////////////////////
+
+namespace boost {
+template <class>
+struct shared_ptr {
+ shared_ptr(const shared_ptr &);
+};
+
+void can_pass_by_value(const shared_ptr<int>) {}
+void can_pass_by_value(const shared_ptr<shared_ptr<double>>) {}
+
+void can_pass_by_ref_to_const(const shared_ptr<int> &) {}
+void can_pass_by_ref_to_const(const shared_ptr<shared_ptr<double>> &) {}
+} // namespace boost
+
+namespace std {
+template <class>
+struct shared_ptr {
+ shared_ptr(const shared_ptr &);
+};
+
+void can_pass_by_value(const shared_ptr<int>) {}
+void can_pass_by_value(const shared_ptr<shared_ptr<double>>) {}
+
+void can_pass_by_ref_to_const(const shared_ptr<int> &) {}
+void can_pass_by_ref_to_const(const shared_ptr<shared_ptr<double>> &) {}
+} // namespace std
Index: clang-tools-extra/docs/clang-tidy/checks/performance-const-parameter-value-or-ref.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-const-parameter-value-or-ref.rst
@@ -0,0 +1,125 @@
+.. title:: clang-tidy - performance-const-parameter-value-or-ref
+
+performance-const-parameter-value-or-ref
+========================================
+
+Checks ``const``-qualified parameters to determine if they should be passed by
+value or by reference-to-``const`` for function definitions. Ignores proper
+function declarations, parameters without ``const`` in their type specifier, and
+dependent types.
+
+A type is recommended to be passed by value if it is both "small" and trivially
+copyable; otherwise the type recommendation flips to being pass by
+reference-to-``const``.
+
+This check works well with ``readability-avoid-const-params-in-decls``.
+
+
+Examples
+--------
+
+.. code-block:: c++
+
+ // No warnings for proper function declarations
+ void f1(const int &);
+ void f1(const std::string); // use readability-avoid-const-params-in-decls
+
+ // No warnings when value parameter is non-const
+ void f2(int i) {}
+ void f2(std::string s) {}
+
+ // No warnings when passing by value and parameter is "small" + trivially
+ // copyable
+ void f3(const char c) {}
+ void f3(const std::array<int, 8> a) {}
+
+ // No warning when passing by reference-to-const and parameter is not "small"
+ void f4(const std::array<int, 9> &a) {}
+
+ // No warning when passing by reference-to-const and type isn't trivially
+ // copyable
+ void f5(const std::string &) {}
+
+ void f6(int const& i) {}
+ // warning: 'const int' is a small, trivially copyable type: pass by value instead
+ // fix-it: void f6(const int i) {}
+
+ void f7(const std::array<int, 9> a) {}
+ // warning: 'const std::array<int, 9>' is a large type: pass by reference-to-const instead
+ // fix-it: void f7(const std::array<int, 9> &a) {}
+
+ void f8(const std::string s) {}
+ // warning: 'const std::string' is not trivially copyable: pass by reference-to-const instead
+
+ void f9(const std::shared_ptr<int>) {} // no warning by default: see below
+ void f10(const std::shared_ptr<int> &) {} // no warning by default: see below
+
+Options
+-------
+
+.. option:: SmallMaxSize
+
+Positive integer that indicates the largest size an object can be while being
+passed by ``const``-qualified value. The default value is 32 bytes.
+
+.. option:: EnableSharedPtrWarningFor
+
+**Accepted values**: ``'ConstValue'``, ``'RefToConst'``, ``Neither``
+
+Directs clang-tidy to warn about ``std::shared_ptr`` and ``boost::shared_ptr``
+on a particular condition.
+
+
+Design
+------
+
+Liberal use of ``const`` is good: it improves the integrity of a program and
+gives readers confidence that certain objects won't be modified throughout their
+lifetime. To mitigate performance bugs with respect to unnecessarily passing by
+value without compromising the aforementioned integrity, this check performs two
+tests: a ``const``-qualified value parameter is allowed if the programmer
+considers the type to be small, and if the type is trivially copyable.
+
+A type that's not trivially copyable is difficult to reason about at a glance.
+Well-designed types usually use copy constructors and copy-assignment operators
+to allocate new resources and copy the data of the other object in some
+non-constant time fashion. There are a few exceptions to this rule, such as
+``std::shared_ptr``, which often needs to be passed by value to increase the
+reference count (but can be marked as ``const`` to prevent releasing, etc.).
+Passing a ``shared_ptr`` by reference-to-``const`` is also acceptable in certain
+situaitons. As such, ``std::shared_ptr<T>`` and ``boost::shared_ptr<T>`` will
+not warn in either case by default.
+
+If a function has a visible forward declaration, then the check will ignore a
+const-qualified value parameter that's ``const``-qualified.
+
+.. code-block:: c++
+ // No warning because there's a forward declaration
+ void f(std::vector<int>);
+ void f(const std::vector<int>) {}
+
+The default value for ``SmallMaxSize`` was determined by through benchmarking
+when passing by value was no longer competetive with passing by reference for
+various `boards <https://git.io/JRKGv>`_. The benchmark can be found on
+`Compiler Explorer <https://godbolt.org/z/rfW1Wqajh>`_, with the used to inform
+the threshold.
+
+
+Limitations and future work
+---------------------------
+
+The forward declaration approach to asserting to the compiler that a parameter
+is intended to be passed by value is only applicable to free functions and
+member functions defined out-of-line. Inline member functions, hidden friends,
+and lambdas miss out on this in-source warning suppression.
+
+This check only supports ``std::shared_ptr`` and ``boost::shared_ptr`` as
+flexible types. Future work will establish a way for users to declare
+non-trivially copyable types that can be flexible.
+
+Template type parameters types aren't yet supported. It isn't clear how to
+support type parameters types in the general case (especially without proper
+static analysis), but there may be a way to set up mnemonics for clang-tidy to
+follow when using concepts. For example, a type parameter requiring
+``std::integral`` is always an integer, so it might be possible to use this
+knowledge to our advantage. Future work will look into this.
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
@@ -9,6 +9,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "ConstParameterValueOrRef.h"
#include "FasterStringFindCheck.h"
#include "ForRangeCopyCheck.h"
#include "ImplicitConversionInLoopCheck.h"
@@ -32,6 +33,8 @@
class PerformanceModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<ConstParameterValueOrRef>(
+ "performance-const-parameter-value-or-ref");
CheckFactories.registerCheck<FasterStringFindCheck>(
"performance-faster-string-find");
CheckFactories.registerCheck<ForRangeCopyCheck>(
Index: clang-tools-extra/clang-tidy/performance/ConstParameterValueOrRef.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/performance/ConstParameterValueOrRef.h
@@ -0,0 +1,50 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_CONSTPARAMETERVALUEORREF_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_CONSTPARAMETERVALUEORREF_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+// Checks ``const``-qualified parameters to determine if they should be passed
+// by value or by reference-to-``const`` for function definitions. Ignores
+// proper function declarations, parameters without ``const`` in their type
+// specifier, and dependent types.
+class ConstParameterValueOrRef : public ClangTidyCheck {
+public:
+ using ClangTidyCheck::ClangTidyCheck;
+
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ Optional<int> MaxSize;
+
+ void checkPassByValueIsAppropriate(const ParmVarDecl &Param,
+ const FunctionDecl &Func,
+ const ASTContext &Context);
+ void checkPassByRefIsAppropriate(const ParmVarDecl &Param,
+ const ASTContext &Context);
+};
+
+struct MaxSize {
+ int Size;
+};
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_CONSTPARAMETERVALUEORREF_H
Index: clang-tools-extra/clang-tidy/performance/ConstParameterValueOrRef.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/performance/ConstParameterValueOrRef.cpp
@@ -0,0 +1,135 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "ConstParameterValueOrRef.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "llvm/ADT/ArrayRef.h"
+#include <utility>
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+using ast_matchers::allOf;
+using ast_matchers::forEach;
+using ast_matchers::functionDecl;
+using ast_matchers::has;
+using ast_matchers::hasType;
+using ast_matchers::isConstQualified;
+using ast_matchers::isDefinition;
+using ast_matchers::isTemplateInstantiation;
+using ast_matchers::MatchFinder;
+using ast_matchers::parmVarDecl;
+using ast_matchers::references;
+using ast_matchers::typeLoc;
+using ast_matchers::unless;
+
+void ConstParameterValueOrRef::registerMatchers(MatchFinder *Finder) {
+ const auto ConstValueParamDecl =
+ parmVarDecl(hasType(isConstQualified())).bind("value");
+ Finder->addMatcher(
+ functionDecl(allOf(isDefinition(),
+ has(typeLoc(forEach(ConstValueParamDecl))),
+ unless(isTemplateInstantiation())))
+ .bind("func-value"),
+ this);
+
+ const auto RefToConstParamDecl =
+ parmVarDecl(hasType(references(isConstQualified()))).bind("ref");
+ Finder->addMatcher(
+ functionDecl(
+ allOf(isDefinition(), has(typeLoc(forEach(RefToConstParamDecl)))),
+ unless(isTemplateInstantiation()))
+ .bind("func-ref"),
+ this);
+}
+
+namespace {
+bool isSharedPtr(const QualType &T) {
+ if (auto R = T->getAsCXXRecordDecl())
+ return R->getQualifiedNameAsString() == "std::shared_ptr" ||
+ R->getQualifiedNameAsString() == "boost::shared_ptr";
+ return false;
+}
+} // namespace
+
+void ConstParameterValueOrRef::checkPassByValueIsAppropriate(
+ const ParmVarDecl &Param, const FunctionDecl &Func,
+ const ASTContext &Context) {
+ if (!Func.isFirstDecl())
+ return;
+
+ const QualType &T = Param.getType();
+
+ if (isSharedPtr(T))
+ return;
+
+ Optional<CharUnits> Size = Context.getTypeSizeInCharsIfKnown(T);
+ if (!Size.hasValue())
+ return;
+
+ SourceLocation Loc = Param.getLocation();
+ FixItHint Hint = FixItHint::CreateInsertion(Loc, "&");
+ if (Size->getQuantity() > *MaxSize) {
+ diag(Loc, "%0 is a large type: pass by reference-to-const instead")
+ << T << Hint;
+ return;
+ }
+
+ if (!T.isTriviallyCopyableType(Context)) {
+ diag(Loc,
+ "%0 is not trivially-copyable: pass by reference-to-const instead")
+ << T << Hint;
+ return;
+ }
+}
+
+void ConstParameterValueOrRef::checkPassByRefIsAppropriate(
+ const ParmVarDecl &Param, const ASTContext &Context) {
+ const QualType &T = Param.getType().getNonReferenceType();
+
+ Optional<CharUnits> Size = Context.getTypeSizeInCharsIfKnown(T);
+ if (!Size.hasValue())
+ return;
+
+ if (isSharedPtr(T))
+ return;
+
+ if (Size->getQuantity() <= *MaxSize && T.isTriviallyCopyableType(Context)) {
+ diag(Param.getTypeSpecEndLoc(),
+ "%0 is a small, trivially-copyable type: pass by value instead")
+ << T << FixItHint::CreateRemoval(Param.getTypeSpecEndLoc());
+ return;
+ }
+}
+
+void ConstParameterValueOrRef::check(const MatchFinder::MatchResult &Result) {
+ if (!MaxSize.hasValue()) {
+ // If the user hasn't provided a default, set the default to sixteen bytes,
+ // which is the largest power-of-two size before performance is impacted
+ // across multiple chipsets. See user-facing documentation for more info.
+ static constexpr int DefaultMaxSize = 16;
+ MaxSize = DefaultMaxSize;
+ }
+
+ if (const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("value")) {
+ const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func-value");
+ assert(Func != nullptr);
+ return checkPassByValueIsAppropriate(*Param, *Func, *Result.Context);
+ }
+
+ if (const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("ref"))
+ return checkPassByRefIsAppropriate(*Param, *Result.Context);
+}
+} // namespace performance
+} // namespace tidy
+} // namespace clang
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
@@ -4,6 +4,7 @@
)
add_clang_library(clangTidyPerformanceModule
+ ConstParameterValueOrRef.cpp
FasterStringFindCheck.cpp
ForRangeCopyCheck.cpp
ImplicitConversionInLoopCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
