nullptr.cpp updated this revision to Diff 332145.
nullptr.cpp edited the summary of this revision.
nullptr.cpp added a comment.
- Warn on both declarations and definitions
- Add check for parameter passing
- Add options `CheckParamPassing` and `ExpensiveToCopySize`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98692/new/
https://reviews.llvm.org/D98692
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.h
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-comparison-operator.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator-check-param-passing.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator-expensive-to-copy-size.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator.cpp
@@ -0,0 +1,194 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-comparison-operator %t -- --fix-notes
+
+/// member function
+namespace test_member_function {
+struct A {
+ int Var;
+
+ bool operator==(const A &a) const;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator==' should not be member function [cppcoreguidelines-comparison-operator]
+
+ bool operator!=(const A &a) const;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator!=' should not be member function [cppcoreguidelines-comparison-operator]
+
+ bool operator<(const A &a) const;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator<' should not be member function [cppcoreguidelines-comparison-operator]
+
+ bool operator<=(const A &a) const;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator<=' should not be member function [cppcoreguidelines-comparison-operator]
+
+ bool operator>(const A &a) const;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator>' should not be member function [cppcoreguidelines-comparison-operator]
+
+ bool operator>=(const A &a) const;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator>=' should not be member function [cppcoreguidelines-comparison-operator]
+};
+} // namespace test_member_function
+
+/// parameters types differ
+namespace test_params_type_differ {
+struct B;
+bool operator==(const B &lh, int rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+
+bool operator!=(const B &lh, int rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator!=' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+
+bool operator<(const B &lh, int rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator<' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+
+bool operator<=(const B &lh, int rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator<=' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+
+bool operator>(const B &lh, int rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator>' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+
+bool operator>=(const B &lh, int rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator>=' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+} // namespace test_params_type_differ
+
+/// can throw
+namespace test_can_throw {
+struct C;
+bool operator==(const C &lh, const C &rh);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should be marked noexcept [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept'
+// CHECK-FIXES: bool operator==(const C &lh, const C &rh) noexcept
+
+bool operator!=(const C &lh, const C &rh);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator!=' should be marked noexcept [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept'
+// CHECK-FIXES: bool operator!=(const C &lh, const C &rh) noexcept;
+
+bool operator<(const C &lh, const C &rh);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator<' should be marked noexcept [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept'
+// CHECK-FIXES: bool operator<(const C &lh, const C &rh) noexcept;
+
+bool operator<=(const C &lh, const C &rh);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator<=' should be marked noexcept [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept'
+// CHECK-FIXES: bool operator<=(const C &lh, const C &rh) noexcept;
+
+bool operator>(const C &lh, const C &rh);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator>' should be marked noexcept [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept'
+// CHECK-FIXES: bool operator>(const C &lh, const C &rh) noexcept;
+
+bool operator>=(const C &lh, const C &rh);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator>=' should be marked noexcept [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept'
+// CHECK-FIXES: bool operator>=(const C &lh, const C &rh) noexcept;
+} // namespace test_can_throw
+
+/// ok
+namespace test_ok {
+struct D;
+bool operator==(const D &lh, const D &rh) noexcept;
+bool operator!=(const D &lh, const D &rh) noexcept;
+bool operator<(const D &lh, const D &rh) noexcept;
+bool operator<=(const D &lh, const D &rh) noexcept;
+bool operator>(const D &lh, const D &rh) noexcept;
+bool operator>=(const D &lh, const D &rh) noexcept;
+} // namespace test_ok
+
+/// should be 'in' parameters
+namespace test_parameter_passing {
+/// cheap to copy
+struct E1 {
+ int Var;
+};
+
+bool operator==(E1 lh, E1 rh) noexcept; // ok
+
+bool operator==(E1 lh, const E1 &rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: 'rh' of type 'test_parameter_passing::E1' is cheap to copy, should be passed by value [cppcoreguidelines-comparison-operator]
+
+bool operator==(const E1 &lh, E1 rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: 'lh' of type 'test_parameter_passing::E1' is cheap to copy, should be passed by value [cppcoreguidelines-comparison-operator]
+
+bool operator==(const E1 &lh, const E1 &rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'lh' of type 'test_parameter_passing::E1' is cheap to copy, should be passed by value [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:41: warning: 'rh' of type 'test_parameter_passing::E1' is cheap to copy, should be passed by value [cppcoreguidelines-comparison-operator]
+
+/// expensive to copy
+struct E2 {
+ int Array[1024];
+};
+
+bool operator==(const E2 &lh, const E2 &rh) noexcept; // ok
+
+bool operator==(E2 lh, const E2 &rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: 'lh' of type 'test_parameter_passing::E2' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+
+bool operator==(const E2 &lh, E2 rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: 'rh' of type 'test_parameter_passing::E2' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+
+bool operator==(E2 lh, E2 rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'lh' of type 'test_parameter_passing::E2' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: 'rh' of type 'test_parameter_passing::E2' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+
+/// incomplete type
+struct E3;
+bool operator==(const E3 &lh, const E3 &rh) noexcept; // ok
+bool operator==(E3 lh, E3 rh) noexcept; // ok
+} // namespace test_parameter_passing
+
+/// warn on both declaration and definition
+namespace test_warn_declaration_definition {
+/// member function
+struct A {
+ int Var;
+ bool operator==(const A &a) const;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator==' should not be member function [cppcoreguidelines-comparison-operator]
+};
+
+bool A::operator==(const A &) const {
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: 'operator==' should not be member function [cppcoreguidelines-comparison-operator]
+ return true;
+}
+
+/// parameters types differ
+struct B;
+bool operator==(const B &lh, int rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+
+bool operator==(const B &lh, int rh) noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+ return true;
+}
+
+/// can throw
+struct C;
+bool operator==(const C &lh, const C &rh);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should be marked noexcept [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept'
+// CHECK-FIXES: bool operator==(const C &lh, const C &rh) noexcept
+
+bool operator==(const C &lh, const C &rh) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should be marked noexcept [cppcoreguidelines-comparison-operator]
+ // CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept'
+ // CHECK-FIXES: bool operator==(const C &lh, const C &rh) noexcept {
+ return true;
+}
+
+/// should be 'in' parameters
+struct E { // expensive to copy
+ int Array[1024];
+};
+
+bool operator==(E lh, E rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 'lh' of type 'test_warn_declaration_definition::E' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:25: warning: 'rh' of type 'test_warn_declaration_definition::E' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+
+bool operator==(E lh, E rh) noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 'lh' of type 'test_warn_declaration_definition::E' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+ // CHECK-MESSAGES: :[[@LINE-2]]:25: warning: 'rh' of type 'test_warn_declaration_definition::E' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+
+ return true;
+}
+} // namespace test_warn_declaration_definition
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator-expensive-to-copy-size.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator-expensive-to-copy-size.cpp
@@ -0,0 +1,61 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-comparison-operator %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: cppcoreguidelines-comparison-operator.ExpensiveToCopySize, \
+// RUN: value: 64}]}"
+
+/// cheap to copy
+struct E1 {
+ char Array[64];
+};
+
+bool operator==(E1 lh, E1 rh) noexcept; // ok
+bool operator==(E1 lh, const E1 &rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: 'rh' of type 'E1' is cheap to copy, should be passed by value [cppcoreguidelines-comparison-operator]
+
+bool operator==(const E1 &lh, E1 rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: 'lh' of type 'E1' is cheap to copy, should be passed by value [cppcoreguidelines-comparison-operator]
+
+bool operator==(const E1 &lh, const E1 &rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'lh' of type 'E1' is cheap to copy, should be passed by value [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:41: warning: 'rh' of type 'E1' is cheap to copy, should be passed by value [cppcoreguidelines-comparison-operator]
+
+/// expensive to copy
+struct E2 {
+ int Array[65];
+};
+
+bool operator==(const E2 &lh, const E2 &rh) noexcept; // ok
+bool operator==(E2 lh, const E2 &rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: 'lh' of type 'E2' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+
+bool operator==(const E2 &lh, E2 rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: 'rh' of type 'E2' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+
+bool operator==(E2 lh, E2 rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'lh' of type 'E2' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: 'rh' of type 'E2' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+
+/// incomplete type
+struct E3;
+bool operator==(const E3 &lh, const E3 &rh) noexcept; // ok
+bool operator==(E3 lh, E3 rh) noexcept; // ok
+
+/// definition
+struct E4 { // expensive to copy
+ int Array[65];
+};
+
+bool operator==(E4 lh, E4 rh) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'lh' of type 'E4' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: 'rh' of type 'E4' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+
+bool operator==(E4 lh, E4 rh) noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'lh' of type 'E4' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+ // CHECK-MESSAGES: :[[@LINE-2]]:27: warning: 'rh' of type 'E4' is expensive to copy, should be passed by reference to const [cppcoreguidelines-comparison-operator]
+
+ return true;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator-check-param-passing.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator-check-param-passing.cpp
@@ -0,0 +1,47 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-comparison-operator %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: cppcoreguidelines-comparison-operator.CheckParamPassing, \
+// RUN: value: false}]}"
+
+/// cheap to copy
+struct E1 {
+ int Var;
+};
+
+bool operator==(E1 lh, E1 rh) noexcept; // ok
+bool operator==(E1 lh, const E1 &rh) noexcept; // not warn on parameter passing
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+
+bool operator==(const E1 &lh, E1 rh) noexcept; // not warn on parameter passing
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+
+bool operator==(const E1 &lh, const E1 &rh) noexcept; // not warn on parameter passing
+
+/// expensive to copy
+struct E2 {
+ int Array[1024];
+};
+
+bool operator==(const E2 &lh, const E2 &rh) noexcept; // ok
+bool operator==(E2 lh, const E2 &rh) noexcept; // not warn on parameter passing
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+
+bool operator==(const E2 &lh, E2 rh) noexcept; // not warn on parameter passing
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator]
+
+bool operator==(E2 lh, E2 rh) noexcept; // not warn on parameter passing
+
+/// incomplete type
+struct E3;
+bool operator==(const E3 &lh, const E3 &rh) noexcept; // ok
+bool operator==(E3 lh, E3 rh) noexcept; // ok
+
+/// definition
+struct E4 { // expensive to copy
+ int Array[1024];
+};
+
+bool operator==(E4 lh, E4 rh) noexcept; // not warn on parameter passing
+bool operator==(E4 lh, E4 rh) noexcept { // not warn on parameter passing
+ return true;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -142,6 +142,7 @@
`concurrency-thread-canceltype-asynchronous <concurrency-thread-canceltype-asynchronous.html>`_,
`cppcoreguidelines-avoid-goto <cppcoreguidelines-avoid-goto.html>`_,
`cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines-avoid-non-const-global-variables.html>`_,
+ `cppcoreguidelines-comparison-operator <cppcoreguidelines-comparison-operator.html>`_, "Yes"
`cppcoreguidelines-init-variables <cppcoreguidelines-init-variables.html>`_, "Yes"
`cppcoreguidelines-interfaces-global-init <cppcoreguidelines-interfaces-global-init.html>`_,
`cppcoreguidelines-macro-usage <cppcoreguidelines-macro-usage.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-comparison-operator.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-comparison-operator.rst
@@ -0,0 +1,74 @@
+.. title:: clang-tidy - cppcoreguidelines-comparison-operator
+
+cppcoreguidelines-comparison-operator
+=====================================
+
+Finds comparison operators(``==``, ``!=``, ``<``, ``<=``, ``>``, ``>=``)
+which are not ``noexcept``, are defined as member functions, have parameters of
+different type or pass parameters not by value for cheap to copy types and not
+by reference to const for expensive to copy types.
+
+Examples:
+
+.. code-block:: c++
+
+ struct A {
+ int Var;
+ bool operator==(const A &a) const; // 'operator==' should not be member function
+ };
+
+.. code-block:: c++
+
+ struct C;
+ bool operator==(const C &lh, const C &rh); // 'operator==' should be marked noexcept
+
+.. code-block:: c++
+
+ struct B;
+ bool operator==(const B &lh, int rh) noexcept; // 'operator==' should have 2 parameters of the same type
+
+.. code-block:: c++
+
+ struct C { // cheap to copy
+ int Var;
+ };
+
+ bool operator==(const C &lh, const C &rh) noexcept;
+ // 'lh' of type 'C' is cheap to copy, should be passed by value
+ // 'rh' of type 'C' is cheap to copy, should be passed by value
+
+.. code-block:: c++
+
+ struct D { // expensive to copy
+ int Array[1024];
+ };
+
+ bool operator==(D lh, D rh) noexcept;
+ // 'lh' of type 'D' is expensive to copy, should be passed by reference to const
+ // 'rh' of type 'D' is expensive to copy, should be passed by reference to const
+
+Options
+-------
+.. option:: CheckParamPassing
+
+ Boolean flag to warn when operators pass parameters not by value for cheap
+ to copy types and not by reference to const for expensive to copy types.
+ Default value is `true`.
+
+.. option:: ExpensiveToCopySize
+
+ When parameters have a size greater than `ExpensiveToCopySize` `bytes`,
+ treat them as expensive to copy types.
+ Default value is `0`, which means to use `2 * sizeof(void*)` as the
+ threshold.
+
+The relevant rules in the C++ Core Guidelines are:
+
+- `C.86: Make == symmetric with respect to operand types and noexcept
+ <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c86-make--symmetric-with-respect-to-operand-types-and-noexcept>`_
+
+- `C.161: Use non-member functions for symmetric operators
+ <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c161-use-non-member-functions-for-symmetric-operators>`_
+
+- `F.16: For "in" parameters, pass cheaply-copied types by value and others by reference to const
+ <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#f16-for-in-parameters-pass-cheaply-copied-types-by-value-and-others-by-reference-to-const>`_
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -83,6 +83,14 @@
Finds ``pthread_setcanceltype`` function calls where a thread's cancellation
type is set to asynchronous.
+- New :doc:`cppcoreguidelines-comparison-operator
+ <clang-tidy/checks/cppcoreguidelines-comparison-operator>` check.
+
+ Finds comparison operators(``==``, ``!=``, ``<``, ``<=``, ``>``, ``>=``)
+ which are not ``noexcept``, are defined as member functions, have parameters
+ of different type or pass parameters not by value for cheap to copy types and
+ not by reference to const for expensive to copy types.
+
- New :doc:`cppcoreguidelines-prefer-member-initializer
<clang-tidy/checks/cppcoreguidelines-prefer-member-initializer>` check.
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -16,6 +16,7 @@
#include "../readability/MagicNumbersCheck.h"
#include "AvoidGotoCheck.h"
#include "AvoidNonConstGlobalVariablesCheck.h"
+#include "ComparisonOperatorCheck.h"
#include "InitVariablesCheck.h"
#include "InterfacesGlobalInitCheck.h"
#include "MacroUsageCheck.h"
@@ -52,6 +53,8 @@
"cppcoreguidelines-avoid-magic-numbers");
CheckFactories.registerCheck<AvoidNonConstGlobalVariablesCheck>(
"cppcoreguidelines-avoid-non-const-global-variables");
+ CheckFactories.registerCheck<ComparisonOperatorCheck>(
+ "cppcoreguidelines-comparison-operator");
CheckFactories.registerCheck<modernize::UseOverrideCheck>(
"cppcoreguidelines-explicit-virtual-functions");
CheckFactories.registerCheck<InitVariablesCheck>(
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.h
@@ -0,0 +1,47 @@
+//===--- ComparisonOperatorCheck.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_CPPCOREGUIDELINES_COMPARISONOPERATORCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_COMPARISONOPERATORCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Finds comparison operators which are not noexcept, are defined as member
+/// functions, have parameters of different type or pass parameters not by value
+/// for cheap to copy types and not by reference to const for expensive to copy
+/// types.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-comparison-operator.html
+class ComparisonOperatorCheck : public ClangTidyCheck {
+public:
+ ComparisonOperatorCheck(StringRef Name, ClangTidyContext *Context);
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ void checkParamPassing(const FunctionDecl *FD,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+ void diagCanThrow(const FunctionDecl *FD);
+ const bool CheckParamPassing;
+ const size_t ExpensiveToCopySize;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_COMPARISONOPERATORCHECK_H
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp
@@ -0,0 +1,176 @@
+//===--- ComparisonOperatorCheck.cpp - clang-tidy -------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ComparisonOperatorCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+constexpr size_t DefaultExpensiveToCopySize = 0;
+
+ComparisonOperatorCheck::ComparisonOperatorCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ CheckParamPassing(Options.get("CheckParamPassing", true)),
+ ExpensiveToCopySize(
+ Options.get("ExpensiveToCopySize", DefaultExpensiveToCopySize)) {}
+
+void ComparisonOperatorCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "CheckParamPassing", CheckParamPassing);
+ Options.store(Opts, "ExpensiveToCopySize", ExpensiveToCopySize);
+}
+
+void ComparisonOperatorCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(functionDecl(hasAnyOverloadedOperatorName("==", "!=", "<",
+ "<=", ">", ">="))
+ .bind("operator"),
+ this);
+}
+
+static bool canThrow(const FunctionDecl *FD) {
+ const auto *Proto = FD->getType()->getAs<FunctionProtoType>();
+ if (isUnresolvedExceptionSpec(Proto->getExceptionSpecType()))
+ return false;
+
+ return Proto->canThrow() == CT_Can;
+}
+
+static bool hasSameParam(const FunctionDecl *FD) {
+ constexpr unsigned int ExpectedNumParams = 2;
+ if (FD->getNumParams() != ExpectedNumParams)
+ return false;
+
+ QualType LH = FD->getParamDecl(0)->getOriginalType();
+ QualType RH = FD->getParamDecl(1)->getOriginalType();
+
+ return LH == RH;
+}
+
+void ComparisonOperatorCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Operator = Result.Nodes.getNodeAs<FunctionDecl>("operator");
+
+ if (isa<CXXMethodDecl>(Operator)) {
+ diag(Operator->getLocation(), "%0 should not be member function")
+ << Operator;
+ return;
+ }
+
+ if (!hasSameParam(Operator))
+ diag(Operator->getLocation(),
+ "%0 should have 2 parameters of the same type")
+ << Operator;
+
+ if (CheckParamPassing)
+ checkParamPassing(Operator, Result);
+
+ if (getLangOpts().CPlusPlus11 && canThrow(Operator))
+ diagCanThrow(Operator);
+}
+
+/// An 'in' parameter of type 'T' should be:
+/// - const T& --- if `T` is expensive to copy.
+/// - T --- if `T` is cheap to copy.
+enum InParamPassingKind {
+ IPP_Eligible, // 'in' parameter obeys the above rule
+ IPP_ExpensiveToCopy, // expensive to copy but not 'const T&'
+ IPP_CheapToCopy, // cheap to copy but not 'T'
+ IPP_Unknown
+};
+
+static llvm::Optional<bool> isExpensiveToCopy(QualType Type,
+ const ASTContext &Context,
+ size_t ExpensiveToCopySize) {
+ llvm::Optional<CharUnits> Size =
+ Context.getTypeSizeInCharsIfKnown(Type.getNonReferenceType());
+ if (!Size.hasValue() || Size.getValue().isNegative())
+ return llvm::None;
+
+ if (ExpensiveToCopySize != DefaultExpensiveToCopySize)
+ return static_cast<size_t>(Size.getValue().getQuantity()) >
+ ExpensiveToCopySize;
+
+ llvm::Optional<CharUnits> VoidPtrSize =
+ Context.getTypeSizeInCharsIfKnown(Context.VoidPtrTy);
+ if (!VoidPtrSize.hasValue() || VoidPtrSize.getValue().isNegative())
+ return llvm::None;
+
+ // Default to treat types which have a size greater than sizeof(void*)*2 as
+ // expensive to copy types
+ return Size.getValue() > VoidPtrSize.getValue() * 2;
+}
+
+static InParamPassingKind getInParamPassingKind(const ParmVarDecl *PVD,
+ const ASTContext &Context,
+ size_t ExpensiveToCopySize) {
+ QualType Type = PVD->getOriginalType();
+ QualType NonRefType = Type.getNonReferenceType();
+ llvm::Optional<bool> IsExpensive =
+ isExpensiveToCopy(NonRefType, Context, ExpensiveToCopySize);
+ if (!IsExpensive.hasValue())
+ return IPP_Unknown;
+
+ // expensive to copy, not 'const T&'
+ if (IsExpensive.getValue() &&
+ !(NonRefType.isConstQualified() && Type->isLValueReferenceType()))
+ return IPP_ExpensiveToCopy;
+
+ // cheap to copy, not 'T'
+ if (!IsExpensive.getValue() &&
+ !(!NonRefType.isConstQualified() && !Type->isReferenceType()))
+ return IPP_CheapToCopy;
+
+ return IPP_Eligible;
+}
+
+void ComparisonOperatorCheck::checkParamPassing(
+ const FunctionDecl *FD, const MatchFinder::MatchResult &Result) {
+ for (const ParmVarDecl *PVD : FD->parameters()) {
+ InParamPassingKind IPPK =
+ getInParamPassingKind(PVD, *Result.Context, ExpensiveToCopySize);
+
+ if (IPPK == IPP_ExpensiveToCopy || IPPK == IPP_CheapToCopy) {
+ QualType Type =
+ PVD->getOriginalType().getNonReferenceType().getUnqualifiedType();
+ diag(PVD->getLocation(), "%0 of type %1 is %select{cheap|expensive}2 to "
+ "copy, should be passed by "
+ "%select{value|reference to const}2")
+ << PVD << Type << (IPPK == IPP_ExpensiveToCopy);
+ }
+ }
+}
+
+static SourceLocation getInsertionLoc(const FunctionDecl *FD) {
+ FunctionTypeLoc FTL = FD->getFunctionTypeLoc();
+ if (!FTL)
+ return SourceLocation();
+
+ SourceLocation RParenLoc = FTL.getRParenLoc();
+ return RParenLoc.getLocWithOffset(1);
+}
+
+void ComparisonOperatorCheck::diagCanThrow(const FunctionDecl *FD) {
+ diag(FD->getLocation(), "%0 should be marked noexcept") << FD;
+
+ DiagnosticBuilder FixDiag =
+ diag(FD->getLocation(), "mark 'noexcept'", DiagnosticIDs::Note);
+ SourceRange ExceptionSpecRange = FD->getExceptionSpecSourceRange();
+ SourceLocation InsertionLoc = getInsertionLoc(FD);
+
+ if (ExceptionSpecRange.isValid())
+ FixDiag << FixItHint::CreateReplacement(ExceptionSpecRange, "noexcept");
+ else if (InsertionLoc.isValid())
+ FixDiag << FixItHint::CreateInsertion(InsertionLoc, " noexcept");
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -6,6 +6,7 @@
add_clang_library(clangTidyCppCoreGuidelinesModule
AvoidGotoCheck.cpp
AvoidNonConstGlobalVariablesCheck.cpp
+ ComparisonOperatorCheck.cpp
CppCoreGuidelinesTidyModule.cpp
InitVariablesCheck.cpp
InterfacesGlobalInitCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
