From c9e0945fcc70c639df906dfe86788e353a2f4864 Mon Sep 17 00:00:00 2001 From: Yuanyuan Chen Date: Mon, 8 Dec 2025 10:39:29 +0800 Subject: [PATCH 1/3] Apply clang-tidy readibility fixes Signed-off-by: Yuanyuan Chen --- include/pybind11/cast.h | 2 +- include/pybind11/detail/internals.h | 4 ++-- include/pybind11/gil_safe_call_once.h | 2 +- include/pybind11/numpy.h | 2 +- include/pybind11/pybind11.h | 9 ++++----- include/pybind11/pytypes.h | 8 ++++---- include/pybind11/stl_bind.h | 2 +- 7 files changed, 14 insertions(+), 15 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 5ecded36f7..3acb560b04 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -475,7 +475,7 @@ class type_caster { 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/internals.h b/include/pybind11/detail/internals.h index 2600d43567..4d6c147db3 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -186,7 +186,7 @@ template using type_map = std::unordered_map; struct override_hash { - inline size_t operator()(const std::pair &v) const { + size_t operator()(const std::pair &v) const { size_t value = std::hash()(v.first); value ^= std::hash()(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 index 44e68f0294..2abd8fc326 100644 --- 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 index 7f62157f5c..50d1284ff3 100644 --- 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() {} common_iterator(void *ptr, const container_type &strides, const container_type &shape) : p_ptr(reinterpret_cast(ptr)), m_strides(strides.size()) { diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 60db0a0871..a2043b2647 100644 --- 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(arg.name)); std::free(const_cast(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(*reinterpret_cast(holder_ptr)); + new (std::addressof(v_h.holder())) 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 index cee4ab5623..a28568145c 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -861,7 +861,7 @@ bool isinstance(handle obj) { } template <> -inline bool isinstance(handle) = delete; +bool isinstance(handle) = delete; template <> inline bool isinstance(handle obj) { return obj.ptr() != nullptr; @@ -1555,7 +1555,7 @@ class iterator : public object { } private: - object value = {}; + object value; }; class type : public object { @@ -1914,7 +1914,7 @@ 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!"); } @@ -1922,7 +1922,7 @@ class float_ : public 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 index 3eb1e53f45..8202300c70 100644 --- 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]); From f7d0d2427b8fc7a3764f9371aadb794f91f45a43 Mon Sep 17 00:00:00 2001 From: Yuanyuan Chen Date: Mon, 8 Dec 2025 10:52:22 +0800 Subject: [PATCH 2/3] Add checks Signed-off-by: Yuanyuan Chen --- .clang-tidy | 3 +++ include/pybind11/detail/common.h | 8 ++------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 3a1995c326..a375f76141 100644 --- 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/detail/common.h b/include/pybind11/detail/common.h index 05d6755896..ce9014a7e9 100644 --- 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 From d846515688c16cb282914e55db6a0b4ac7f5a624 Mon Sep 17 00:00:00 2001 From: Yuanyuan Chen Date: Mon, 8 Dec 2025 11:52:35 +0800 Subject: [PATCH 3/3] More fixes Signed-off-by: cyy --- include/pybind11/numpy.h | 2 +- include/pybind11/pytypes.h | 2 +- tests/test_builtin_casters.cpp | 2 +- tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp | 6 +++--- tests/test_factory_constructors.cpp | 2 +- tests/test_numpy_array.cpp | 8 ++++---- tests/test_sequences_and_iterators.cpp | 2 +- tests/test_smart_ptr.cpp | 7 ++++--- tests/test_stl_binders.cpp | 4 ++-- 9 files changed, 18 insertions(+), 17 deletions(-) diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 50d1284ff3..236d90bd32 100644 --- 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() {} + common_iterator() = default; common_iterator(void *ptr, const container_type &strides, const container_type &shape) : p_ptr(reinterpret_cast(ptr)), m_strides(strides.size()) { diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index a28568145c..b28692fd74 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -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(); } diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp index 1aa9f89b42..6cde727ad8 100644 --- 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 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 index 5580848c6e..6936379c2c 100644 --- 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 get_object() const { return m_obj; } void set_object(std::shared_ptr 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(); } std::shared_ptr m_obj; }; diff --git a/tests/test_factory_constructors.cpp b/tests/test_factory_constructors.cpp index e50494b33a..c96a3a31fb 100644 --- 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 index 28359c46d4..ac6b1cfe35 100644 --- 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 @@ -43,11 +43,11 @@ std::vector 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 diff --git a/tests/test_sequences_and_iterators.cpp b/tests/test_sequences_and_iterators.cpp index ccb0d14990..3f03daf38a 100644 --- 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 p) { + return std::count_if(d.begin(), d.end(), [](const std::pair &p) { return p.second.cast() != 0; }); }); diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 2e98d469f2..0ac1a41bd9 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -220,7 +220,7 @@ struct SharedPtrRef { ~A() { print_destroyed(this); } }; - A value = {}; + A value; std::shared_ptr shared = std::make_shared(); }; @@ -228,13 +228,14 @@ struct SharedPtrRef { struct SharedFromThisRef { struct B : std::enable_shared_from_this { 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() { print_copy_created(this); } + // NOLINTNEXTLINE(readability-redundant-member-init) B(B &&) noexcept : std::enable_shared_from_this() { print_move_created(this); } ~B() { print_destroyed(this); } }; - B value = {}; + B value; std::shared_ptr shared = std::make_shared(); }; diff --git a/tests/test_stl_binders.cpp b/tests/test_stl_binders.cpp index f846ae8482..f399ec0e43 100644 --- a/tests/test_stl_binders.cpp +++ b/tests/test_stl_binders.cpp @@ -55,7 +55,7 @@ template 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;