fix(clang-tidy): clang-tidy readability and misc fixes, like adding const (#3052)

* Enable and apply clang-tidy readability and misc fixes.

* Revert deprecated tester

* add space to tests/test_constants_and_functions.cpp

.clang-tidy

@@ -2,17 +2,30 @@ FormatStyle: file
 
 Checks: '
 llvm-namespace-comment,
+misc-misplaced-const,
+misc-static-assert,
+misc-uniqueptr-reset-release,
 modernize-avoid-bind,
 modernize-replace-auto-ptr,
 modernize-replace-disallow-copy-and-assign-macro,
 modernize-shrink-to-fit,
 modernize-use-auto,
+modernize-use-bool-literals,
 modernize-use-equals-default,
 modernize-use-equals-delete,
+modernize-use-default-member-init,
+modernize-use-noexcept,
 modernize-use-emplace,
 modernize-use-override,
 modernize-use-using,
 readability-container-size-empty,
+readability-make-member-function-const,
+readability-redundant-function-ptr-dereference,
+readability-redundant-smartptr-get,
+readability-redundant-string-cstr,
+readability-simplify-subscript-expr,
+readability-string-compare,
+readability-uniqueptr-delete-release,
 '
 
 CheckOptions:

include/pybind11/detail/common.h

@@ -480,7 +480,7 @@ struct instance {
     void allocate_layout();
 
     /// Destroys/deallocates all of the above
-    void deallocate_layout();
+    void deallocate_layout() const;
 
     /// Returns the value_and_holder wrapper for the given type (or the first, if `find_type`
     /// omitted).  Returns a default-constructed (with `.inst = nullptr`) object on failure if

include/pybind11/detail/descr.h

@@ -23,9 +23,9 @@ PYBIND11_NAMESPACE_BEGIN(detail)
 /* Concatenate type signatures at compile time */
 template <size_t N, typename... Ts>
 struct descr {
-    char text[N + 1];
+    char text[N + 1]{'\0'};
 
-    constexpr descr() : text{'\0'} { }
+    constexpr descr() = default;
     constexpr descr(char const (&s)[N+1]) : descr(s, make_index_sequence<N>()) { }
 
     template <size_t... Is>

include/pybind11/detail/type_caster_base.h

@@ -241,7 +241,7 @@ struct value_and_holder {
             ? inst->simple_holder_constructed
             : inst->nonsimple.status[index] & instance::status_holder_constructed;
     }
-    void set_holder_constructed(bool v = true) {
+    void set_holder_constructed(bool v = true) const {
         if (inst->simple_layout)
             inst->simple_holder_constructed = v;
         else if (v)
@@ -254,7 +254,7 @@ struct value_and_holder {
             ? inst->simple_instance_registered
             : inst->nonsimple.status[index] & instance::status_instance_registered;
     }
-    void set_instance_registered(bool v = true) {
+    void set_instance_registered(bool v = true) const {
         if (inst->simple_layout)
             inst->simple_instance_registered = v;
         else if (v)
@@ -397,7 +397,7 @@ PYBIND11_NOINLINE inline void instance::allocate_layout() {
     owned = true;
 }
 
-PYBIND11_NOINLINE inline void instance::deallocate_layout() {
+PYBIND11_NOINLINE inline void instance::deallocate_layout() const {
     if (!simple_layout)
         PyMem_Free(nonsimple.values_and_holders);
 }

include/pybind11/numpy.h

@@ -1291,7 +1291,7 @@ public:
     using value_type = container_type::value_type;
     using size_type = container_type::size_type;
 
-    common_iterator() : p_ptr(0), m_strides() {}
+    common_iterator() : m_strides() {}
 
     common_iterator(void* ptr, const container_type& strides, const container_type& shape)
         : p_ptr(reinterpret_cast<char*>(ptr)), m_strides(strides.size()) {
@@ -1312,7 +1312,7 @@ public:
     }
 
 private:
-    char* p_ptr;
+    char *p_ptr{0};
     container_type m_strides;
 };
 

include/pybind11/pybind11.h

@@ -395,7 +395,8 @@ protected:
             rec->def = new PyMethodDef();
             std::memset(rec->def, 0, sizeof(PyMethodDef));
             rec->def->ml_name = rec->name;
-            rec->def->ml_meth = reinterpret_cast<PyCFunction>(reinterpret_cast<void (*) (void)>(*dispatcher));
+            rec->def->ml_meth
+                = reinterpret_cast<PyCFunction>(reinterpret_cast<void (*)(void)>(dispatcher));
             rec->def->ml_flags = METH_VARARGS | METH_KEYWORDS;
 
             capsule rec_capsule(unique_rec.release(), [](void *ptr) {

tests/local_bindings.h

@@ -56,7 +56,7 @@ class Pet {
 public:
     Pet(std::string name) : name_(name) {}
     std::string name_;
-    const std::string &name() { return name_; }
+    const std::string &name() const { return name_; }
 };
 } // namespace pets
 

tests/test_constants_and_functions.cpp

@@ -1,5 +1,6 @@
 /*
-    tests/test_constants_and_functions.cpp -- global constants and functions, enumerations, raw byte strings
+    tests/test_constants_and_functions.cpp -- global constants and functions, enumerations, raw
+    byte strings
 
     Copyright (c) 2016 Wenzel Jakob <wenzel.jakob@epfl.ch>
 
@@ -60,6 +61,7 @@ int f3(int x) noexcept(false) { return x+3; }
 #  pragma GCC diagnostic push
 #  pragma GCC diagnostic ignored "-Wdeprecated"
 #endif
+// NOLINTNEXTLINE(modernize-use-noexcept)
 int f4(int x) throw() { return x+4; } // Deprecated equivalent to noexcept(true)
 #if defined(__GNUG__) && !defined(__INTEL_COMPILER)
 #  pragma GCC diagnostic pop
@@ -75,8 +77,10 @@ struct C {
 #  pragma GCC diagnostic push
 #  pragma GCC diagnostic ignored "-Wdeprecated"
 #endif
-    int m7(int x) throw() { return x-7; }
+    // NOLINTNEXTLINE(modernize-use-noexcept)
-    int m8(int x) const throw() { return x-8; }
+    int m7(int x) throw() { return x - 7; }
+    // NOLINTNEXTLINE(modernize-use-noexcept)
+    int m8(int x) const throw() { return x - 8; }
 #if defined(__GNUG__) && !defined(__INTEL_COMPILER)
 #  pragma GCC diagnostic pop
 #endif

tests/test_factory_constructors.cpp

@@ -77,7 +77,7 @@ public:
     TestFactory6(const TestFactory6 &f) { print_copy_created(this); value = f.value; alias = f.alias; }
     virtual ~TestFactory6() { print_destroyed(this); }
     virtual int get() { return value; }
-    bool has_alias() { return alias; }
+    bool has_alias() const { return alias; }
 };
 class PyTF6 : public TestFactory6 {
 public:
@@ -102,7 +102,7 @@ public:
     TestFactory7(const TestFactory7 &f) { print_copy_created(this); value = f.value; alias = f.alias; }
     virtual ~TestFactory7() { print_destroyed(this); }
     virtual int get() { return value; }
-    bool has_alias() { return alias; }
+    bool has_alias() const { return alias; }
 };
 class PyTF7 : public TestFactory7 {
 public:

tests/test_iostream.cpp

@@ -34,7 +34,7 @@ void noisy_funct_dual(std::string msg, std::string emsg) {
 // simply repeatedly write to std::cerr until stopped
 // redirect is called at some point to test the safety of scoped_estream_redirect
 struct TestThread {
-    TestThread() : t_{nullptr}, stop_{false} {
+    TestThread() : stop_{false} {
         auto thread_f = [this] {
             while (!stop_) {
                 std::cout << "x" << std::flush;
@@ -49,7 +49,7 @@ struct TestThread {
 
     void stop() { stop_ = true; }
 
-    void join() {
+    void join() const {
         py::gil_scoped_release gil_lock;
         t_->join();
     }
@@ -59,7 +59,7 @@ struct TestThread {
         std::this_thread::sleep_for(std::chrono::milliseconds(50));
     }
 
-    std::thread * t_;
+    std::thread *t_{nullptr};
     std::atomic<bool> stop_;
 };
 

tests/test_methods_and_attributes.cpp

@@ -25,9 +25,7 @@ public:
     ExampleMandA(ExampleMandA &&e) : value(e.value) { print_move_created(this); }
     ~ExampleMandA() { print_destroyed(this); }
 
-    std::string toString() {
+    std::string toString() const { return "ExampleMandA[value=" + std::to_string(value) + "]"; }
-        return "ExampleMandA[value=" + std::to_string(value) + "]";
-    }
 
     void operator=(const ExampleMandA &e) { print_copy_assigned(this); value = e.value; }
     void operator=(ExampleMandA &&e) { print_move_assigned(this); value = e.value; }
@@ -48,13 +46,13 @@ public:
 
     ExampleMandA self1() { return *this; }                          // return by value
     ExampleMandA &self2() { return *this; }                         // return by reference
-    const ExampleMandA &self3() { return *this; }                   // return by const reference
+    const ExampleMandA &self3() const { return *this; }             // return by const reference
     ExampleMandA *self4() { return this; }                          // return by pointer
-    const ExampleMandA *self5() { return this; }                    // return by const pointer
+    const ExampleMandA *self5() const { return this; }              // return by const pointer
 
-    int internal1() { return value; }                               // return by value
+    int internal1() const { return value; }                         // return by value
     int &internal2() { return value; }                              // return by reference
-    const int &internal3() { return value; }                        // return by const reference
+    const int &internal3() const { return value; }                  // return by const reference
     int *internal4() { return &value; }                             // return by pointer
     const int *internal5() { return &value; }                       // return by const pointer
 

tests/test_modules.cpp

@@ -24,7 +24,8 @@ TEST_SUBMODULE(modules, m) {
         ~A() { print_destroyed(this); }
         A(const A&) { print_copy_created(this); }
         A& operator=(const A &copy) { print_copy_assigned(this); v = copy.v; return *this; }
-        std::string toString() { return "A[" + std::to_string(v) + "]"; }
+        std::string toString() const { return "A[" + std::to_string(v) + "]"; }
+
     private:
         int v;
     };

tests/test_multiple_inheritance.cpp

@@ -48,12 +48,12 @@ int VanillaStaticMix2::static_value = 12;
 // test_multiple_inheritance_virtbase
 struct Base1a {
     Base1a(int i) : i(i) { }
-    int foo() { return i; }
+    int foo() const { return i; }
     int i;
 };
 struct Base2a {
     Base2a(int i) : i(i) { }
-    int bar() { return i; }
+    int bar() const { return i; }
     int i;
 };
 struct Base12a : Base1a, Base2a {
@@ -78,7 +78,7 @@ TEST_SUBMODULE(multiple_inheritance, m) {
     // test_multiple_inheritance_mix2
     struct Base1 {
         Base1(int i) : i(i) { }
-        int foo() { return i; }
+        int foo() const { return i; }
         int i;
     };
     py::class_<Base1> b1(m, "Base1");
@@ -87,7 +87,7 @@ TEST_SUBMODULE(multiple_inheritance, m) {
 
     struct Base2 {
         Base2(int i) : i(i) { }
-        int bar() { return i; }
+        int bar() const { return i; }
         int i;
     };
     py::class_<Base2> b2(m, "Base2");

tests/test_numpy_vectorize.cpp

@@ -62,7 +62,7 @@ TEST_SUBMODULE(numpy_vectorize, m) {
     // test_method_vectorization
     struct VectorizeTestClass {
         VectorizeTestClass(int v) : value{v} {};
-        float method(int x, float y) { return y + (float) (x + value); }
+        float method(int x, float y) const { return y + (float) (x + value); }
         int value = 0;
     };
     py::class_<VectorizeTestClass> vtc(m, "VectorizeTestClass");

tests/test_smart_ptr.cpp

@@ -240,7 +240,7 @@ struct ElementBase {
 
 struct ElementA : ElementBase {
     ElementA(int v) : v(v) { }
-    int value() { return v; }
+    int value() const { return v; }
     int v;
 };
 

tests/test_stl_binders.cpp

@@ -125,5 +125,7 @@ TEST_SUBMODULE(stl_binders, m) {
     PYBIND11_NUMPY_DTYPE(VStruct, w, x, y, z);
     py::class_<VStruct>(m, "VStruct").def_readwrite("x", &VStruct::x);
     py::bind_vector<std::vector<VStruct>>(m, "VectorStruct", py::buffer_protocol());
-    m.def("get_vectorstruct", [] {return std::vector<VStruct> {{0, 5, 3.0, 1}, {1, 30, -1e4, 0}};});
+    m.def("get_vectorstruct", [] {
+        return std::vector<VStruct>{{false, 5, 3.0, true}, {true, 30, -1e4, false}};
+    });
 }