Add more readability tidy rules (#5924)

* Apply clang-tidy readibility fixes

Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>

* Add checks

Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>

* More fixes

Signed-off-by: cyy <cyyever@outlook.com>

---------

Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
Signed-off-by: cyy <cyyever@outlook.com>

.clang-tidy

@@ -48,7 +48,10 @@ Checks: |
   readability-misplaced-array-index,
   readability-non-const-parameter,
   readability-qualified-auto,
+  readability-redundant-casting,
   readability-redundant-function-ptr-dereference,
+  readability-redundant-inline-specifier,
+  readability-redundant-member-init,
   readability-redundant-smartptr-get,
   readability-redundant-string-cstr,
   readability-simplify-subscript-expr,

include/pybind11/cast.h

@@ -475,7 +475,7 @@ public:
 
 private:
     // Test if an object is a NumPy boolean (without fetching the type).
-    static inline bool is_numpy_bool(handle object) {
+    static bool is_numpy_bool(handle object) {
         const char *type_name = Py_TYPE(object.ptr())->tp_name;
         // Name changed to `numpy.bool` in NumPy 2, `numpy.bool_` is needed for 1.x support
         return std::strcmp("numpy.bool", type_name) == 0

include/pybind11/detail/common.h

@@ -590,14 +590,10 @@ enum class return_value_policy : uint8_t {
 
 PYBIND11_NAMESPACE_BEGIN(detail)
 
-inline static constexpr int log2(size_t n, int k = 0) {
+static constexpr int log2(size_t n, int k = 0) { return (n <= 1) ? k : log2(n >> 1, k + 1); }
-    return (n <= 1) ? k : log2(n >> 1, k + 1);
-}
 
 // Returns the size as a multiple of sizeof(void *), rounded up.
-inline static constexpr size_t size_in_ptrs(size_t s) {
+static constexpr size_t size_in_ptrs(size_t s) { return 1 + ((s - 1) >> log2(sizeof(void *))); }
-    return 1 + ((s - 1) >> log2(sizeof(void *)));
-}
 
 /**
  * The space to allocate for simple layout instance holders (see below) in multiple of the size of

include/pybind11/detail/internals.h

@@ -186,7 +186,7 @@ template <typename value_type>
 using type_map = std::unordered_map<std::type_index, value_type, type_hash, type_equal_to>;
 
 struct override_hash {
-    inline size_t operator()(const std::pair<const PyObject *, const char *> &v) const {
+    size_t operator()(const std::pair<const PyObject *, const char *> &v) const {
         size_t value = std::hash<const void *>()(v.first);
         value ^= std::hash<const void *>()(v.second) + 0x9e3779b9 + (value << 6) + (value >> 2);
         return value;
@@ -555,7 +555,7 @@ class internals_pp_manager {
 public:
     using on_fetch_function = void(InternalsType *);
 
-    inline static internals_pp_manager &get_instance(char const *id, on_fetch_function *on_fetch) {
+    static internals_pp_manager &get_instance(char const *id, on_fetch_function *on_fetch) {
         static internals_pp_manager instance(id, on_fetch);
         return instance;
     }

include/pybind11/gil_safe_call_once.h

@@ -87,7 +87,7 @@ public:
 
 private:
     alignas(T) char storage_[sizeof(T)] = {};
-    std::once_flag once_flag_ = {};
+    std::once_flag once_flag_;
 #ifdef Py_GIL_DISABLED
     std::atomic_bool
 #else

include/pybind11/numpy.h

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

include/pybind11/pybind11.h

@@ -811,9 +811,9 @@ protected:
             // so they cannot be freed. Once the function has been created, they can.
             // Check `make_function_record` for more details.
             if (free_strings) {
-                std::free((char *) rec->name);
+                std::free(rec->name);
-                std::free((char *) rec->doc);
+                std::free(rec->doc);
-                std::free((char *) rec->signature);
+                std::free(rec->signature);
                 for (auto &arg : rec->args) {
                     std::free(const_cast<char *>(arg.name));
                     std::free(const_cast<char *>(arg.descr));
@@ -2486,8 +2486,7 @@ private:
     static void init_holder_from_existing(const detail::value_and_holder &v_h,
                                           const holder_type *holder_ptr,
                                           std::true_type /*is_copy_constructible*/) {
-        new (std::addressof(v_h.holder<holder_type>()))
+        new (std::addressof(v_h.holder<holder_type>())) holder_type(*holder_ptr);
-            holder_type(*reinterpret_cast<const holder_type *>(holder_ptr));
     }
 
     static void init_holder_from_existing(const detail::value_and_holder &v_h,

include/pybind11/pytypes.h

@@ -861,7 +861,7 @@ bool isinstance(handle obj) {
 }
 
 template <>
-inline bool isinstance<handle>(handle) = delete;
+bool isinstance<handle>(handle) = delete;
 template <>
 inline bool isinstance<object>(handle obj) {
     return obj.ptr() != nullptr;
@@ -994,7 +994,7 @@ inline PyObject *dict_getitem(PyObject *v, PyObject *key) {
 
 inline PyObject *dict_getitemstringref(PyObject *v, const char *key) {
 #if PY_VERSION_HEX >= 0x030D0000
-    PyObject *rv;
+    PyObject *rv = nullptr;
     if (PyDict_GetItemStringRef(v, key, &rv) < 0) {
         throw error_already_set();
     }
@@ -1555,7 +1555,7 @@ private:
     }
 
 private:
-    object value = {};
+    object value;
 };
 
 class type : public object {
@@ -1914,7 +1914,7 @@ public:
         }
     }
     // NOLINTNEXTLINE(google-explicit-constructor)
-    float_(double value = .0) : object(PyFloat_FromDouble((double) value), stolen_t{}) {
+    float_(double value = .0) : object(PyFloat_FromDouble(value), stolen_t{}) {
         if (!m_ptr) {
             pybind11_fail("Could not allocate float object!");
         }
@@ -1922,7 +1922,7 @@ public:
     // NOLINTNEXTLINE(google-explicit-constructor)
     operator float() const { return (float) PyFloat_AsDouble(m_ptr); }
     // NOLINTNEXTLINE(google-explicit-constructor)
-    operator double() const { return (double) PyFloat_AsDouble(m_ptr); }
+    operator double() const { return PyFloat_AsDouble(m_ptr); }
 };
 
 class weakref : public object {

include/pybind11/stl_bind.h

@@ -244,7 +244,7 @@ void vector_modifiers(
             }
 
             auto *seq = new Vector();
-            seq->reserve((size_t) slicelength);
+            seq->reserve(slicelength);
 
             for (size_t i = 0; i < slicelength; ++i) {
                 seq->push_back(v[start]);

tests/test_builtin_casters.cpp

@@ -367,7 +367,7 @@ TEST_SUBMODULE(builtin_casters, m) {
     m.def("complex_noconvert", [](std::complex<float> x) { return x; }, py::arg{}.noconvert());
 
     // test int vs. long (Python 2)
-    m.def("int_cast", []() { return (int) 42; });
+    m.def("int_cast", []() { return 42; });
     m.def("long_cast", []() { return (long) 42; });
     m.def("longlong_cast", []() { return ULLONG_MAX; });
 

tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp

@@ -36,9 +36,9 @@ struct PySpBase : SpBase, py::trampoline_self_life_support {
 struct SpBaseTester {
     std::shared_ptr<SpBase> get_object() const { return m_obj; }
     void set_object(std::shared_ptr<SpBase> obj) { m_obj = std::move(obj); }
-    bool is_base_used() { return m_obj->is_base_used(); }
+    bool is_base_used() const { return m_obj->is_base_used(); }
-    bool has_instance() { return (bool) m_obj; }
+    bool has_instance() const { return (bool) m_obj; }
-    bool has_python_instance() { return m_obj && m_obj->has_python_instance(); }
+    bool has_python_instance() const { return m_obj && m_obj->has_python_instance(); }
     void set_nonpython_instance() { m_obj = std::make_shared<SpBase>(); }
     std::shared_ptr<SpBase> m_obj;
 };

tests/test_factory_constructors.cpp

@@ -73,7 +73,7 @@ public:
 // Inheritance test
 class TestFactory4 : public TestFactory3 {
 public:
-    TestFactory4() : TestFactory3() { print_default_created(this); }
+    TestFactory4() { print_default_created(this); }
     explicit TestFactory4(int v) : TestFactory3(v) { print_created(this, v); }
     ~TestFactory4() override { print_destroyed(this); }
 };

tests/test_numpy_array.cpp

@@ -17,8 +17,8 @@
 
 // Size / dtype checks.
 struct DtypeCheck {
-    py::dtype numpy{};
+    py::dtype numpy;
-    py::dtype pybind11{};
+    py::dtype pybind11;
 };
 
 template <typename T>
@@ -43,11 +43,11 @@ std::vector<DtypeCheck> get_concrete_dtype_checks() {
 }
 
 struct DtypeSizeCheck {
-    std::string name{};
+    std::string name;
     int size_cpp{};
     int size_numpy{};
     // For debugging.
-    py::dtype dtype{};
+    py::dtype dtype;
 };
 
 template <typename T>

tests/test_sequences_and_iterators.cpp

@@ -556,7 +556,7 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
     });
 
     m.def("count_nonzeros", [](const py::dict &d) {
-        return std::count_if(d.begin(), d.end(), [](std::pair<py::handle, py::handle> p) {
+        return std::count_if(d.begin(), d.end(), [](const std::pair<py::handle, py::handle> &p) {
             return p.second.cast<int>() != 0;
         });
     });

tests/test_smart_ptr.cpp

@@ -220,7 +220,7 @@ struct SharedPtrRef {
         ~A() { print_destroyed(this); }
     };
 
-    A value = {};
+    A value;
     std::shared_ptr<A> shared = std::make_shared<A>();
 };
 
@@ -228,13 +228,14 @@ struct SharedPtrRef {
 struct SharedFromThisRef {
     struct B : std::enable_shared_from_this<B> {
         B() { print_created(this); }
-        // NOLINTNEXTLINE(bugprone-copy-constructor-init)
+        // NOLINTNEXTLINE(bugprone-copy-constructor-init, readability-redundant-member-init)
         B(const B &) : std::enable_shared_from_this<B>() { print_copy_created(this); }
+        // NOLINTNEXTLINE(readability-redundant-member-init)
         B(B &&) noexcept : std::enable_shared_from_this<B>() { print_move_created(this); }
         ~B() { print_destroyed(this); }
     };
 
-    B value = {};
+    B value;
     std::shared_ptr<B> shared = std::make_shared<B>();
 };
 

tests/test_stl_binders.cpp

@@ -55,7 +55,7 @@ template <class Map>
 Map *times_ten(int n) {
     auto *m = new Map();
     for (int i = 1; i <= n; i++) {
-        m->emplace(int(i), E_nc(10 * i));
+        m->emplace(i, E_nc(10 * i));
     }
     return m;
 }
@@ -65,7 +65,7 @@ NestMap *times_hundred(int n) {
     auto *m = new NestMap();
     for (int i = 1; i <= n; i++) {
         for (int j = 1; j <= n; j++) {
-            (*m)[i].emplace(int(j * 10), E_nc(100 * j));
+            (*m)[i].emplace(j * 10, E_nc(100 * j));
         }
     }
     return m;