cjdb updated this revision to Diff 366457.
cjdb retitled this revision from "[WIP][clang-tidy] adds a const-qualified
parameter check" to "[clang-tidy] adds a const-qualified parameter check".
cjdb added a comment.
removes WIP tag from patch name
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107873/new/
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/clang-tidy/utils/FixItHintUtils.cpp
clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
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/custom-max-size.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-const-parameter-value-or-ref/defaults.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-const-parameter-value-or-ref/forward-declarations-suppress-warnings-false.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/forward-declarations-suppress-warnings-false.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-const-parameter-value-or-ref/forward-declarations-suppress-warnings-false.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s performance-const-parameter-value-or-ref %t -- \
+// RUN: -config="{CheckOptions: [ \
+// RUN: {key: performance-const-parameter-value-or-ref.ForwardDeclarationsSuppressWarnings,\
+// RUN: value: false}\
+// RUN: ]}"
+
+class NotTriviallyCopyable {
+public:
+ NotTriviallyCopyable() = default;
+
+ NotTriviallyCopyable(const NotTriviallyCopyable &);
+};
+
+void f1(NotTriviallyCopyable);
+void f1(const NotTriviallyCopyable) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: 'const NotTriviallyCopyable' is not trivially copyable: pass by reference-to-const instead
+// CHECK-FIXES: void f1(const NotTriviallyCopyable&) {}
Index: clang-tools-extra/test/clang-tidy/checkers/performance-const-parameter-value-or-ref/defaults.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-const-parameter-value-or-ref/defaults.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]]:24: 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]]:24: 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]]:24: 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]]:27: 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]]:24: 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]]:28: 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]]:33: 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]]:38: 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]]:40: 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/test/clang-tidy/checkers/performance-const-parameter-value-or-ref/custom-max-size.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-const-parameter-value-or-ref/custom-max-size.cpp
@@ -0,0 +1,236 @@
+// RUN: %check_clang_tidy %s performance-const-parameter-value-or-ref %t -- \
+// RUN: -config="{\
+// RUN: CheckOptions: [ \
+// RUN: {key: performance-const-parameter-value-or-ref.SmallMaxSize, value: 15}\
+// RUN: ]\
+// RUN: }"
+
+template <class T, int N>
+struct array {
+ T base[N];
+};
+
+using MaxSmall = array<char, 15>;
+using MinLarge = array<char, 16>;
+
+class NotTriviallyCopyable {
+public:
+ NotTriviallyCopyable() = default;
+
+ NotTriviallyCopyable(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 long long) {}
+void f4(const MaxSmall) {}
+
+struct S4 {
+ void f(const long long) {}
+ 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 long long &) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 'const long long' is a small, trivially copyable type: pass by value instead
+// CHECK-FIXES: void f2(const long long) {}
+
+void f8(const long long &) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 'const long long' is a small, trivially copyable type: pass by value instead
+// CHECK-FIXES: void f8(const long long) {}
+
+void f8(const MaxSmall &) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'const MaxSmall' (aka 'const array<char, 15>') is a small, trivially copyable type: pass by value instead
+// CHECK-FIXES: void f8(const MaxSmall) {}
+
+template <class T>
+void f8(T, const long long &) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: 'const long long' is a small, trivially copyable type: pass by value instead
+// CHECK-FIXES: void f8(T, const long long) {}
+
+template <class T>
+void f8(const MaxSmall &, T) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'const MaxSmall' (aka 'const array<char, 15>') is a small, trivially copyable type: pass by value instead
+// CHECK-FIXES: void f8(const MaxSmall, T) {}
+
+struct S8 {
+ void f128(const long long &) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: 'const long long' is a small, trivially copyable type: pass by value instead
+ // CHECK-FIXES: void f128(const long long) {}
+
+ void fMaxSmall(const MaxSmall &) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: 'const MaxSmall' (aka 'const array<char, 15>') is a small, trivially copyable type: pass by value instead
+ // CHECK-FIXES: void fMaxSmall(const MaxSmall) {}
+
+ template <class T>
+ void template128(T, const long long &) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: 'const long long' is a small, trivially copyable type: pass by value instead
+ // CHECK-FIXES: void template128(T, const long long) {}
+
+ template <class T>
+ void templateMaxSmall(const MaxSmall &, T) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: 'const MaxSmall' (aka 'const array<char, 15>') 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 __int128) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'const __int128' is a large type: pass by reference-to-const instead
+// CHECK-FIXES: void f9(const __int128&) {}
+
+void f9(const MinLarge x) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'const MinLarge' (aka 'const array<char, 16>') 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, 16>') 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, 16>') 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, 16>') 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,123 @@
+.. 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 16 bytes.
+
+.. option:: ForwardDeclarationSuppressesWarnings
+
+**Accepted values**: `true`, `false`
+
+Directs clang-tidy to warn when a non-trivially copyable type is passed as a
+``const``-value parameter if there is a visible forward declaration. Defaults to
+``true``.
+
+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
+situations. As such, ``std::shared_ptr<T>`` and ``boost::shared_ptr<T>`` will
+not warn.
+
+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 benchmarking to
+determine when passing by value was no longer competetive with passing by
+reference for various `boards <https://git.io/JRKGv>`_. The benchmark source can
+be found on `Compiler Explorer <https://godbolt.org/z/rfW1Wqajh>`_.
+
+
+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/utils/FixItHintUtils.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
@@ -19,7 +19,13 @@
namespace fixit {
/// Creates fix to make ``VarDecl`` a reference by adding ``&``.
-FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context);
+FixItHint changeVarDeclToReference(const VarDecl &Var,
+ const ASTContext &Context);
+
+/// Creates fix to make ``VarDecl`` a value by removing ``&`` and all
+/// surrounding whitespace.
+FixItHint changeReferenceDeclToValue(const VarDecl &Var,
+ const ASTContext &Context);
/// This enum defines where the qualifier shall be preferably added.
enum class QualifierPolicy {
Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
@@ -10,13 +10,15 @@
#include "LexerUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Type.h"
+#include "llvm/ADT/StringExtras.h"
namespace clang {
namespace tidy {
namespace utils {
namespace fixit {
-FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context) {
+FixItHint changeVarDeclToReference(const VarDecl &Var,
+ const ASTContext &Context) {
SourceLocation AmpLocation = Var.getLocation();
auto Token = utils::lexer::getPreviousToken(
AmpLocation, Context.getSourceManager(), Context.getLangOpts());
@@ -223,6 +225,40 @@
return None;
}
+
+template <class Pred>
+// requires std::predicate<Pred, char>
+static SourceLocation FindLast(SourceLocation First, SourceLocation Last,
+ const SourceManager &SM, Pred pred) {
+ assert(First.isValid());
+ assert(Last.isValid());
+ int Step = First < Last ? 1 : -1;
+ for (SourceLocation Next = First; Next.isValid() && Next != Last;
+ Next = Next.getLocWithOffset(Step)) {
+ if (!pred(*SM.getCharacterData(Next))) {
+ break;
+ }
+ First = Next;
+ }
+ return First;
+}
+
+FixItHint changeReferenceDeclToValue(const VarDecl &Var,
+ const ASTContext &Context) {
+ const SourceManager &SM = Context.getSourceManager();
+ SourceLocation AmpLoc = Var.getTypeSpecEndLoc();
+ assert(*SM.getCharacterData(AmpLoc) == '&');
+ SourceLocation First = FindLast(AmpLoc.getLocWithOffset(-1),
+ Var.getBeginLoc(), SM, isWhitespace);
+ SourceLocation Last =
+ FindLast(AmpLoc.getLocWithOffset(1), Var.getEndLoc(), SM, isWhitespace);
+ SourceRange R(
+ First.isValid() && isWhitespace(*SM.getCharacterData(First)) ? First
+ : AmpLoc,
+ Last.isValid() && isWhitespace(*SM.getCharacterData(Last)) ? Last
+ : AmpLoc);
+ return FixItHint::CreateRemoval(R);
+}
} // namespace fixit
} // namespace utils
} // namespace tidy
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,56 @@
+//===--- ConstParameterValueOrRef.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_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;
+ void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+
+private:
+ // 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 DefaultSmallMaxSize = 16;
+ int SmallMaxSize = Options.get("SmallMaxSize", DefaultSmallMaxSize);
+
+ bool ForwardDeclarationsSuppressWarnings =
+ Options.get("ForwardDeclarationsSuppressWarnings", true);
+
+ void checkPassByValueIsAppropriate(const ParmVarDecl &Param,
+ const FunctionDecl &Func,
+ const ASTContext &Context);
+ void checkPassByRefIsAppropriate(const ParmVarDecl &Param,
+ const ASTContext &Context);
+};
+
+} // 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,136 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "../utils/FixItHintUtils.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 "llvm/ADT/SmallVector.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 (CXXRecordDecl *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() && ForwardDeclarationsSuppressWarnings)
+ 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 = utils::fixit::changeVarDeclToReference(Param, Context);
+ if (Size->getQuantity() > SmallMaxSize) {
+ 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() <= SmallMaxSize &&
+ T.isTriviallyCopyableType(Context)) {
+ diag(Param.getTypeSpecEndLoc(),
+ "%0 is a small, trivially copyable type: pass by value instead")
+ << T << utils::fixit::changeReferenceDeclToValue(Param, Context);
+ return;
+ }
+}
+
+void ConstParameterValueOrRef::check(const MatchFinder::MatchResult &Result) {
+ 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);
+}
+
+void ConstParameterValueOrRef::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "MaxSize", SmallMaxSize);
+ Options.store(Opts, "ForwardDeclarationsSuppressWarnings",
+ ForwardDeclarationsSuppressWarnings);
+}
+} // 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
