Add more readability tidy rules (#5924)

cyyever · web-flow · commit 3ebbecb8af05 · 2025-12-08T09:36:51.000-08:00
* Apply clang-tidy readibility fixes

Signed-off-by: Yuanyuan Chen &lt;cyyever@outlook.com&gt;

* Add checks

Signed-off-by: Yuanyuan Chen &lt;cyyever@outlook.com&gt;

* More fixes

Signed-off-by: cyy &lt;cyyever@outlook.com&gt;

---------

Signed-off-by: Yuanyuan Chen &lt;cyyever@outlook.com&gt;
Signed-off-by: cyy &lt;cyyever@outlook.com&gt;
diff --git a/.clang-tidy b/.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,
diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
@@ -475,7 +475,7 @@ class type_caster<bool> {
 
 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
diff --git a/include/pybind11/detail/common.h b/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) {
-    return (n <= 1) ? k : log2(n >> 1, k + 1);
-}
+static constexpr int log2(size_t n, int k = 0) { 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) {
-    return 1 + ((s - 1) >> log2(sizeof(void *)));
-}
+static constexpr size_t size_in_ptrs(size_t s) { return 1 + ((s - 1) >> log2(sizeof(void *))); }
 
 /**
  * The space to allocate for simple layout instance holders (see below) in multiple of the size of
diff --git a/include/pybind11/detail/internals.h b/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;
     }
diff --git a/include/pybind11/gil_safe_call_once.h b/include/pybind11/gil_safe_call_once.h
@@ -87,7 +87,7 @@ class gil_safe_call_once_and_store {
 
 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
diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h
@@ -1860,7 +1860,7 @@ class common_iterator {
     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()) {
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
@@ -811,9 +811,9 @@ class cpp_function : public function {
             // 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((char *) rec->doc);
-                std::free((char *) rec->signature);
+                std::free(rec->name);
+                std::free(rec->doc);
+                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 @@ class class_ : public detail::generic_type {
     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>()))
-            holder_type(*reinterpret_cast<const holder_type *>(holder_ptr));
+        new (std::addressof(v_h.holder<holder_type>())) holder_type(*holder_ptr);
     }
 
     static void init_holder_from_existing(const detail::value_and_holder &v_h,
diff --git a/include/pybind11/pytypes.h b/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 @@ class iterator : public object {
     }
 
 private:
-    object value = {};
+    object value;
 };
 
 class type : public object {
@@ -1914,15 +1914,15 @@ class float_ : public object {
         }
     }
     // 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!");
         }
     }
     // 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 {
diff --git a/include/pybind11/stl_bind.h b/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]);
diff --git a/tests/test_builtin_casters.cpp b/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; });
 
diff --git a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp b/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 has_instance() { return (bool) m_obj; }
-    bool has_python_instance() { return m_obj && m_obj->has_python_instance(); }
+    bool is_base_used() const { return m_obj->is_base_used(); }
+    bool has_instance() const { return (bool) m_obj; }
+    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;
 };
diff --git a/tests/test_factory_constructors.cpp b/tests/test_factory_constructors.cpp
@@ -73,7 +73,7 @@ class TestFactory3 {
 // 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); }
 };
diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp
@@ -17,8 +17,8 @@
 
 // Size / dtype checks.
 struct DtypeCheck {
-    py::dtype numpy{};
-    py::dtype pybind11{};
+    py::dtype numpy;
+    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>
diff --git a/tests/test_sequences_and_iterators.cpp b/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;
         });
     });
diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp
@@ -220,21 +220,22 @@ struct SharedPtrRef {
         ~A() { print_destroyed(this); }
     };
 
-    A value = {};
+    A value;
     std::shared_ptr<A> shared = std::make_shared<A>();
 };
 
 // test_shared_ptr_from_this_and_references
 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>();
 };
 
diff --git a/tests/test_stl_binders.cpp 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;

Original file line number	Diff line number	Diff line change
`@@ -861,7 +861,7 @@ bool isinstance(handle obj) {`
`861`	`861`	`}`
`862`	`862`
`863`	`863`	`template <>`
`864`		`-inline bool isinstance<handle>(handle) = delete;`
	`864`	`+bool isinstance<handle>(handle) = delete;`
`865`	`865`	`template <>`
`866`	`866`	`inline bool isinstance<object>(handle obj) {`
`867`	`867`	`return obj.ptr() != nullptr;`
`@@ -994,7 +994,7 @@ inline PyObject dict_getitem(PyObject v, PyObject *key) {`
`994`	`994`
`995`	`995`	`inline PyObject dict_getitemstringref(PyObject v, const char *key) {`
`996`	`996`	`#if PY_VERSION_HEX >= 0x030D0000`
`997`		`- PyObject *rv;`
	`997`	`+ PyObject *rv = nullptr;`
`998`	`998`	`if (PyDict_GetItemStringRef(v, key, &rv) < 0) {`
`999`	`999`	`throw error_already_set();`
`1000`	`1000`	`}`
`@@ -1555,7 +1555,7 @@ class iterator : public object {`
`1555`	`1555`	`}`
`1556`	`1556`
`1557`	`1557`	`private:`
`1558`		`- object value = {};`
	`1558`	`+ object value;`
`1559`	`1559`	`};`
`1560`	`1560`
`1561`	`1561`	`class type : public object {`
`@@ -1914,15 +1914,15 @@ class float_ : public object {`
`1914`	`1914`	`}`
`1915`	`1915`	`}`
`1916`	`1916`	`// NOLINTNEXTLINE(google-explicit-constructor)`
`1917`		`- float_(double value = .0) : object(PyFloat_FromDouble((double) value), stolen_t{}) {`
	`1917`	`+ float_(double value = .0) : object(PyFloat_FromDouble(value), stolen_t{}) {`
`1918`	`1918`	`if (!m_ptr) {`
`1919`	`1919`	`pybind11_fail("Could not allocate float object!");`
`1920`	`1920`	`}`
`1921`	`1921`	`}`
`1922`	`1922`	`// NOLINTNEXTLINE(google-explicit-constructor)`
`1923`	`1923`	`operator float() const { return (float) PyFloat_AsDouble(m_ptr); }`
`1924`	`1924`	`// NOLINTNEXTLINE(google-explicit-constructor)`
`1925`		`- operator double() const { return (double) PyFloat_AsDouble(m_ptr); }`
	`1925`	`+ operator double() const { return PyFloat_AsDouble(m_ptr); }`
`1926`	`1926`	`};`
`1927`	`1927`
`1928`	`1928`	`class weakref : public object {`
Original file line number	Diff line number	Diff line change
`@@ -244,7 +244,7 @@ void vector_modifiers(`
`244`	`244`	`}`
`245`	`245`
`246`	`246`	`auto *seq = new Vector();`
`247`		`- seq->reserve((size_t) slicelength);`
	`247`	`+ seq->reserve(slicelength);`
`248`	`248`
`249`	`249`	`for (size_t i = 0; i < slicelength; ++i) {`
`250`	`250`	`seq->push_back(v[start]);`