From 0318287fd1491ca5780b866c7186406894df4ffb Mon Sep 17 00:00:00 2001 From: Rosdf Date: Sat, 7 Jun 2025 11:43:08 +0400 Subject: [PATCH 01/13] add support for const pointers in smart pointers --- include/pybind11/detail/init.h | 6 ++-- include/pybind11/detail/value_and_holder.h | 6 +++- tests/CMakeLists.txt | 1 + tests/test_const_smart_ptr.cpp | 40 ++++++++++++++++++++++ tests/test_const_smart_ptr.py | 11 ++++++ 5 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 tests/test_const_smart_ptr.cpp create mode 100644 tests/test_const_smart_ptr.py diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index aa993f495d..c6b9980df4 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -37,7 +37,7 @@ class type_caster { PYBIND11_NAMESPACE_BEGIN(initimpl) -inline void no_nullptr(void *ptr) { +inline void no_nullptr(const void *ptr) { if (!ptr) { throw type_error("pybind11::init(): factory function returned nullptr"); } @@ -61,7 +61,7 @@ bool is_alias(Cpp *ptr) { } // Failing fallback version of the above for a no-alias class (always returns false) template -constexpr bool is_alias(void *) { +constexpr bool is_alias(const void *) { return false; } @@ -167,7 +167,7 @@ void construct(value_and_holder &v_h, Holder holder, bool need_alias) { "is not an alias instance"); } - v_h.value_ptr() = ptr; + v_h.value_ptr>() = ptr; v_h.type->init_instance(v_h.inst, &holder); } diff --git a/include/pybind11/detail/value_and_holder.h b/include/pybind11/detail/value_and_holder.h index 87c92f8e49..41cdec49bd 100644 --- a/include/pybind11/detail/value_and_holder.h +++ b/include/pybind11/detail/value_and_holder.h @@ -33,7 +33,11 @@ struct value_and_holder { template V *&value_ptr() const { - return reinterpret_cast(vh[0]); + if constexpr (std::is_const_v) { + return reinterpret_cast(const_cast(vh[0])); + } else { + return reinterpret_cast(vh[0]); + } } // True if this `value_and_holder` has a non-null value pointer explicit operator bool() const { return value_ptr() != nullptr; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2cf18c3547..ee1fb7ebaa 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -135,6 +135,7 @@ set(PYBIND11_TEST_FILES test_class_sh_unique_ptr_member test_class_sh_virtual_py_cpp_mix test_const_name + test_const_smart_ptr test_constants_and_functions test_copy_move test_cpp_conduit diff --git a/tests/test_const_smart_ptr.cpp b/tests/test_const_smart_ptr.cpp new file mode 100644 index 0000000000..897979c688 --- /dev/null +++ b/tests/test_const_smart_ptr.cpp @@ -0,0 +1,40 @@ +#include "pybind11/cast.h" +#include "pybind11_tests.h" + +#include + +template +class StaticPtr { +public: + explicit StaticPtr(const T *ptr) : ptr_(ptr) {} + + const T *get() const { return ptr_; } + + const T &operator*() const { return *ptr_; } + const T *operator->() const { return ptr_; } + +private: + const T *ptr_ = nullptr; +}; + +PYBIND11_DECLARE_HOLDER_TYPE(T, StaticPtr, true) + +class MyData { +public: + static StaticPtr create(std::string name) { + return StaticPtr(new MyData(std::move(name))); + } + + const std::string &getName() const { return name_; } + +private: + explicit MyData(std::string &&name) : name_(std::move(name)) {} + + std::string name_; +}; + +TEST_SUBMODULE(const_module, m) { + py::class_>(m, "Data") + .def(py::init([](const std::string &name) { return MyData::create(name); })) + .def_property_readonly("name", &MyData::getName); +} diff --git a/tests/test_const_smart_ptr.py b/tests/test_const_smart_ptr.py new file mode 100644 index 0000000000..8422899e7b --- /dev/null +++ b/tests/test_const_smart_ptr.py @@ -0,0 +1,11 @@ +from __future__ import annotations + +import pytest + +asyncio = pytest.importorskip("asyncio") +m = pytest.importorskip("pybind11_tests.const_module") + + +def test_const_smart_ptr(): + cons = m.Data("my_name") + assert cons.name == "my_name" From 8fe9412dfbe46a49c4f5beaf5c13f8eabc0fe832 Mon Sep 17 00:00:00 2001 From: Rosdf Date: Mon, 9 Jun 2025 18:27:09 +0400 Subject: [PATCH 02/13] use c++11 compatible code --- include/pybind11/detail/init.h | 2 +- include/pybind11/detail/value_and_holder.h | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index c6b9980df4..c0e8f45c9b 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -167,7 +167,7 @@ void construct(value_and_holder &v_h, Holder holder, bool need_alias) { "is not an alias instance"); } - v_h.value_ptr>() = ptr; + v_h.value_ptr::type>() = ptr; v_h.type->init_instance(v_h.inst, &holder); } diff --git a/include/pybind11/detail/value_and_holder.h b/include/pybind11/detail/value_and_holder.h index 41cdec49bd..0c7a5b8445 100644 --- a/include/pybind11/detail/value_and_holder.h +++ b/include/pybind11/detail/value_and_holder.h @@ -33,11 +33,7 @@ struct value_and_holder { template V *&value_ptr() const { - if constexpr (std::is_const_v) { - return reinterpret_cast(const_cast(vh[0])); - } else { - return reinterpret_cast(vh[0]); - } + return cast_void(vh[0]); } // True if this `value_and_holder` has a non-null value pointer explicit operator bool() const { return value_ptr() != nullptr; } @@ -76,6 +72,17 @@ struct value_and_holder { inst->nonsimple.status[index] &= (std::uint8_t) ~instance::status_instance_registered; } } + +private: + template ::value, bool> = true> + static V *&cast_void(void *&ptr) { + return reinterpret_cast(ptr); + } + + template ::value, bool> = true> + static V *&cast_void(void *&ptr) { + return reinterpret_cast(const_cast(ptr)); + } }; // This is a semi-public API to check if the corresponding instance has been constructed with a From 51a1942c662a871696808271e3aaff3c8e50f743 Mon Sep 17 00:00:00 2001 From: Rosdf Date: Mon, 9 Jun 2025 19:23:21 +0400 Subject: [PATCH 03/13] add template parameter in test --- tests/test_const_smart_ptr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_const_smart_ptr.cpp b/tests/test_const_smart_ptr.cpp index 897979c688..340b36505f 100644 --- a/tests/test_const_smart_ptr.cpp +++ b/tests/test_const_smart_ptr.cpp @@ -22,7 +22,7 @@ PYBIND11_DECLARE_HOLDER_TYPE(T, StaticPtr, true) class MyData { public: static StaticPtr create(std::string name) { - return StaticPtr(new MyData(std::move(name))); + return StaticPtr(new MyData(std::move(name))); } const std::string &getName() const { return name_; } From fc8ca60fc980463256bb9beeaec9e45fdb402f55 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 9 Jun 2025 09:54:22 -0700 Subject: [PATCH 04/13] Make the const-removal clearly visible. This simplifies the production code changes significantly. For background see: https://claude.ai/share/4085d9ab-a859-44cc-bb56-450e472f817a --- include/pybind11/detail/init.h | 7 ++++++- include/pybind11/detail/value_and_holder.h | 13 +------------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index c0e8f45c9b..913515c17b 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -167,7 +167,12 @@ void construct(value_and_holder &v_h, Holder holder, bool need_alias) { "is not an alias instance"); } - v_h.value_ptr::type>() = ptr; + // Cast away constness to store in void* storage. + // The value_and_holder storage is fundamentally untyped (void**), so we lose + // const-correctness here by design. The const qualifier will be restored + // when the pointer is later retrieved and cast back to the original type. + // This explicit const_cast makes the const-removal clearly visible. + v_h.value_ptr() = const_cast(static_cast(ptr)); v_h.type->init_instance(v_h.inst, &holder); } diff --git a/include/pybind11/detail/value_and_holder.h b/include/pybind11/detail/value_and_holder.h index 0c7a5b8445..87c92f8e49 100644 --- a/include/pybind11/detail/value_and_holder.h +++ b/include/pybind11/detail/value_and_holder.h @@ -33,7 +33,7 @@ struct value_and_holder { template V *&value_ptr() const { - return cast_void(vh[0]); + return reinterpret_cast(vh[0]); } // True if this `value_and_holder` has a non-null value pointer explicit operator bool() const { return value_ptr() != nullptr; } @@ -72,17 +72,6 @@ struct value_and_holder { inst->nonsimple.status[index] &= (std::uint8_t) ~instance::status_instance_registered; } } - -private: - template ::value, bool> = true> - static V *&cast_void(void *&ptr) { - return reinterpret_cast(ptr); - } - - template ::value, bool> = true> - static V *&cast_void(void *&ptr) { - return reinterpret_cast(const_cast(ptr)); - } }; // This is a semi-public API to check if the corresponding instance has been constructed with a From 9102c5c473d844478ced844118b465eddeb8669c Mon Sep 17 00:00:00 2001 From: Rosdf Date: Mon, 9 Jun 2025 21:27:25 +0400 Subject: [PATCH 05/13] test without leaks --- tests/test_const_smart_ptr.cpp | 44 +++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/tests/test_const_smart_ptr.cpp b/tests/test_const_smart_ptr.cpp index 340b36505f..5f2b212aa8 100644 --- a/tests/test_const_smart_ptr.cpp +++ b/tests/test_const_smart_ptr.cpp @@ -1,12 +1,43 @@ #include "pybind11/cast.h" #include "pybind11_tests.h" +#include #include +#include template -class StaticPtr { +class SingleThreadedSharedPtr { public: - explicit StaticPtr(const T *ptr) : ptr_(ptr) {} + explicit SingleThreadedSharedPtr(const T *ptr) { + try { + counter_ = new uint64_t(1); + } catch (...) { + delete ptr; + } + + ptr_ = ptr; + } + + SingleThreadedSharedPtr(const SingleThreadedSharedPtr &other) + : ptr_(other.ptr_), counter_(other.counter_) { + ++*counter_; + } + + SingleThreadedSharedPtr(SingleThreadedSharedPtr &&other) noexcept + : ptr_(std::exchange(other.ptr_, nullptr)), + counter_(std::exchange(other.counter_, nullptr)) {} + + ~SingleThreadedSharedPtr() { + if (!counter_) { + return; + } + + --*counter_; + + if (*counter_ == 0) { + delete ptr_; + } + } const T *get() const { return ptr_; } @@ -15,14 +46,15 @@ class StaticPtr { private: const T *ptr_ = nullptr; + uint64_t *counter_ = nullptr; }; -PYBIND11_DECLARE_HOLDER_TYPE(T, StaticPtr, true) +PYBIND11_DECLARE_HOLDER_TYPE(T, SingleThreadedSharedPtr, true) class MyData { public: - static StaticPtr create(std::string name) { - return StaticPtr(new MyData(std::move(name))); + static SingleThreadedSharedPtr create(std::string name) { + return SingleThreadedSharedPtr(new MyData(std::move(name))); } const std::string &getName() const { return name_; } @@ -34,7 +66,7 @@ class MyData { }; TEST_SUBMODULE(const_module, m) { - py::class_>(m, "Data") + py::class_>(m, "Data") .def(py::init([](const std::string &name) { return MyData::create(name); })) .def_property_readonly("name", &MyData::getName); } From ab77b171d2c40cc5b9f505a428e1e41a5bc11a98 Mon Sep 17 00:00:00 2001 From: Rosdf Date: Mon, 9 Jun 2025 21:41:13 +0400 Subject: [PATCH 06/13] add namespace for test --- tests/test_const_smart_ptr.cpp | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/test_const_smart_ptr.cpp b/tests/test_const_smart_ptr.cpp index 5f2b212aa8..bb63600b4f 100644 --- a/tests/test_const_smart_ptr.cpp +++ b/tests/test_const_smart_ptr.cpp @@ -5,10 +5,12 @@ #include #include +namespace const_smart_ptr { + template -class SingleThreadedSharedPtr { +class non_sync_const_shared_ptr { public: - explicit SingleThreadedSharedPtr(const T *ptr) { + explicit non_sync_const_shared_ptr(const T *ptr) { try { counter_ = new uint64_t(1); } catch (...) { @@ -18,16 +20,16 @@ class SingleThreadedSharedPtr { ptr_ = ptr; } - SingleThreadedSharedPtr(const SingleThreadedSharedPtr &other) + non_sync_const_shared_ptr(const non_sync_const_shared_ptr &other) : ptr_(other.ptr_), counter_(other.counter_) { ++*counter_; } - SingleThreadedSharedPtr(SingleThreadedSharedPtr &&other) noexcept + non_sync_const_shared_ptr(non_sync_const_shared_ptr &&other) noexcept : ptr_(std::exchange(other.ptr_, nullptr)), counter_(std::exchange(other.counter_, nullptr)) {} - ~SingleThreadedSharedPtr() { + ~non_sync_const_shared_ptr() { if (!counter_) { return; } @@ -49,12 +51,10 @@ class SingleThreadedSharedPtr { uint64_t *counter_ = nullptr; }; -PYBIND11_DECLARE_HOLDER_TYPE(T, SingleThreadedSharedPtr, true) - class MyData { public: - static SingleThreadedSharedPtr create(std::string name) { - return SingleThreadedSharedPtr(new MyData(std::move(name))); + static non_sync_const_shared_ptr create(std::string name) { + return non_sync_const_shared_ptr(new MyData(std::move(name))); } const std::string &getName() const { return name_; } @@ -64,9 +64,14 @@ class MyData { std::string name_; }; +} // namespace const_smart_ptr + +PYBIND11_DECLARE_HOLDER_TYPE(T, const_smart_ptr::non_sync_const_shared_ptr, true) TEST_SUBMODULE(const_module, m) { - py::class_>(m, "Data") - .def(py::init([](const std::string &name) { return MyData::create(name); })) - .def_property_readonly("name", &MyData::getName); + py::class_>(m, "Data") + .def(py::init( + [](const std::string &name) { return const_smart_ptr::MyData::create(name); })) + .def_property_readonly("name", &const_smart_ptr::MyData::getName); } From fd5624c3d1a36945ee9953149d0d28e6ecad8b45 Mon Sep 17 00:00:00 2001 From: Rosdf Date: Mon, 9 Jun 2025 21:51:18 +0400 Subject: [PATCH 07/13] rename test --- tests/CMakeLists.txt | 2 +- ...mart_ptr.cpp => test_const_only_smart_ptr.cpp} | 15 ++++++++------- ..._smart_ptr.py => test_const_only_smart_ptr.py} | 0 3 files changed, 9 insertions(+), 8 deletions(-) rename tests/{test_const_smart_ptr.cpp => test_const_only_smart_ptr.cpp} (75%) rename tests/{test_const_smart_ptr.py => test_const_only_smart_ptr.py} (100%) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ee1fb7ebaa..f91639ff88 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -135,7 +135,7 @@ set(PYBIND11_TEST_FILES test_class_sh_unique_ptr_member test_class_sh_virtual_py_cpp_mix test_const_name - test_const_smart_ptr + test_const_only_smart_ptr test_constants_and_functions test_copy_move test_cpp_conduit diff --git a/tests/test_const_smart_ptr.cpp b/tests/test_const_only_smart_ptr.cpp similarity index 75% rename from tests/test_const_smart_ptr.cpp rename to tests/test_const_only_smart_ptr.cpp index bb63600b4f..27326c9136 100644 --- a/tests/test_const_smart_ptr.cpp +++ b/tests/test_const_only_smart_ptr.cpp @@ -5,7 +5,7 @@ #include #include -namespace const_smart_ptr { +namespace const_only_smart_ptr { template class non_sync_const_shared_ptr { @@ -64,14 +64,15 @@ class MyData { std::string name_; }; -} // namespace const_smart_ptr +} // namespace const_only_smart_ptr -PYBIND11_DECLARE_HOLDER_TYPE(T, const_smart_ptr::non_sync_const_shared_ptr, true) +PYBIND11_DECLARE_HOLDER_TYPE(T, const_only_smart_ptr::non_sync_const_shared_ptr, true) TEST_SUBMODULE(const_module, m) { - py::class_>(m, "Data") + py::class_>( + m, "Data") .def(py::init( - [](const std::string &name) { return const_smart_ptr::MyData::create(name); })) - .def_property_readonly("name", &const_smart_ptr::MyData::getName); + [](const std::string &name) { return const_only_smart_ptr::MyData::create(name); })) + .def_property_readonly("name", &const_only_smart_ptr::MyData::getName); } diff --git a/tests/test_const_smart_ptr.py b/tests/test_const_only_smart_ptr.py similarity index 100% rename from tests/test_const_smart_ptr.py rename to tests/test_const_only_smart_ptr.py From 4384d08ed8b3f64cfc5798eeaaf6bf2734df83c8 Mon Sep 17 00:00:00 2001 From: Rosdf Date: Mon, 9 Jun 2025 22:43:41 +0400 Subject: [PATCH 08/13] fix test compilation --- tests/test_const_only_smart_ptr.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_const_only_smart_ptr.cpp b/tests/test_const_only_smart_ptr.cpp index 27326c9136..8b96019d2c 100644 --- a/tests/test_const_only_smart_ptr.cpp +++ b/tests/test_const_only_smart_ptr.cpp @@ -3,7 +3,6 @@ #include #include -#include namespace const_only_smart_ptr { @@ -26,8 +25,10 @@ class non_sync_const_shared_ptr { } non_sync_const_shared_ptr(non_sync_const_shared_ptr &&other) noexcept - : ptr_(std::exchange(other.ptr_, nullptr)), - counter_(std::exchange(other.counter_, nullptr)) {} + : ptr_(other.ptr_), counter_(other.counter_) { + other.ptr_ = nullptr; + other.counter_ = nullptr; + } ~non_sync_const_shared_ptr() { if (!counter_) { From 769964c0380bba945dd853607953e5bdb62479e5 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 9 Jun 2025 15:55:37 -0700 Subject: [PATCH 09/13] using namespace test_const_only_smart_ptr; --- tests/test_const_only_smart_ptr.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/test_const_only_smart_ptr.cpp b/tests/test_const_only_smart_ptr.cpp index 8b96019d2c..950f7c5cd2 100644 --- a/tests/test_const_only_smart_ptr.cpp +++ b/tests/test_const_only_smart_ptr.cpp @@ -4,7 +4,7 @@ #include #include -namespace const_only_smart_ptr { +namespace test_const_only_smart_ptr { template class non_sync_const_shared_ptr { @@ -65,15 +65,15 @@ class MyData { std::string name_; }; -} // namespace const_only_smart_ptr -PYBIND11_DECLARE_HOLDER_TYPE(T, const_only_smart_ptr::non_sync_const_shared_ptr, true) +} // namespace test_const_only_smart_ptr + +using namespace test_const_only_smart_ptr; + +PYBIND11_DECLARE_HOLDER_TYPE(T, non_sync_const_shared_ptr, true) TEST_SUBMODULE(const_module, m) { - py::class_>( - m, "Data") - .def(py::init( - [](const std::string &name) { return const_only_smart_ptr::MyData::create(name); })) - .def_property_readonly("name", &const_only_smart_ptr::MyData::getName); + py::class_>(m, "Data") + .def(py::init([](const std::string &name) { return MyData::create(name); })) + .def_property_readonly("name", &MyData::getName); } From 39bca7ad90f70409c690f4b0bf08bfa47f8e05a9 Mon Sep 17 00:00:00 2001 From: Rosdf Date: Tue, 10 Jun 2025 08:01:03 +0400 Subject: [PATCH 10/13] fix smartptr in test --- tests/test_const_only_smart_ptr.cpp | 79 +++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 16 deletions(-) diff --git a/tests/test_const_only_smart_ptr.cpp b/tests/test_const_only_smart_ptr.cpp index 950f7c5cd2..6664887790 100644 --- a/tests/test_const_only_smart_ptr.cpp +++ b/tests/test_const_only_smart_ptr.cpp @@ -4,16 +4,21 @@ #include #include -namespace test_const_only_smart_ptr { +namespace const_only_smart_ptr { template class non_sync_const_shared_ptr { public: + non_sync_const_shared_ptr() = default; + explicit non_sync_const_shared_ptr(const T *ptr) { - try { - counter_ = new uint64_t(1); - } catch (...) { - delete ptr; + if (ptr) { + try { + counter_ = new size_t(1); + } catch (...) { + delete ptr; + throw; + } } ptr_ = ptr; @@ -21,7 +26,9 @@ class non_sync_const_shared_ptr { non_sync_const_shared_ptr(const non_sync_const_shared_ptr &other) : ptr_(other.ptr_), counter_(other.counter_) { - ++*counter_; + if (counter_) { + ++*counter_; + } } non_sync_const_shared_ptr(non_sync_const_shared_ptr &&other) noexcept @@ -30,26 +37,67 @@ class non_sync_const_shared_ptr { other.counter_ = nullptr; } - ~non_sync_const_shared_ptr() { - if (!counter_) { - return; + non_sync_const_shared_ptr &operator=(const non_sync_const_shared_ptr &other) { + if (this == &other) { + return *this; } - --*counter_; + release(); - if (*counter_ == 0) { - delete ptr_; + ptr_ = other.ptr_; + counter_ = other.counter_; + + if (counter_) { + ++*counter_; } + + return *this; } + non_sync_const_shared_ptr &operator=(non_sync_const_shared_ptr &&other) noexcept { + if (this == &other) { + return *this; + } + + release(); + + ptr_ = other.ptr_; + counter_ = other.counter_; + + other.ptr_ = nullptr; + other.counter_ = nullptr; + + return *this; + } + + ~non_sync_const_shared_ptr() { release(); } + const T *get() const { return ptr_; } const T &operator*() const { return *ptr_; } const T *operator->() const { return ptr_; } + explicit operator bool() const { return ptr_ != nullptr; } + private: + void release() noexcept { + if (!counter_) { + return; + } + + --*counter_; + + if (*counter_ == 0) { + delete ptr_; + delete counter_; + } + + ptr_ = nullptr; + counter_ = nullptr; + } + const T *ptr_ = nullptr; - uint64_t *counter_ = nullptr; + size_t *counter_ = nullptr; }; class MyData { @@ -65,10 +113,9 @@ class MyData { std::string name_; }; +} // namespace const_only_smart_ptr -} // namespace test_const_only_smart_ptr - -using namespace test_const_only_smart_ptr; +using namespace const_only_smart_ptr; PYBIND11_DECLARE_HOLDER_TYPE(T, non_sync_const_shared_ptr, true) From 05e756b521352a0881d459adc63bae32173ac82c Mon Sep 17 00:00:00 2001 From: Rosdf Date: Tue, 10 Jun 2025 21:50:52 +0400 Subject: [PATCH 11/13] smaller test body --- tests/test_const_only_smart_ptr.cpp | 101 +++++----------------------- 1 file changed, 16 insertions(+), 85 deletions(-) diff --git a/tests/test_const_only_smart_ptr.cpp b/tests/test_const_only_smart_ptr.cpp index 6664887790..85465bbd10 100644 --- a/tests/test_const_only_smart_ptr.cpp +++ b/tests/test_const_only_smart_ptr.cpp @@ -7,103 +7,34 @@ namespace const_only_smart_ptr { template -class non_sync_const_shared_ptr { +class const_only_shared_ptr { public: - non_sync_const_shared_ptr() = default; - - explicit non_sync_const_shared_ptr(const T *ptr) { - if (ptr) { - try { - counter_ = new size_t(1); - } catch (...) { - delete ptr; - throw; - } - } - - ptr_ = ptr; - } - - non_sync_const_shared_ptr(const non_sync_const_shared_ptr &other) - : ptr_(other.ptr_), counter_(other.counter_) { - if (counter_) { - ++*counter_; - } - } - - non_sync_const_shared_ptr(non_sync_const_shared_ptr &&other) noexcept - : ptr_(other.ptr_), counter_(other.counter_) { - other.ptr_ = nullptr; - other.counter_ = nullptr; - } - - non_sync_const_shared_ptr &operator=(const non_sync_const_shared_ptr &other) { - if (this == &other) { - return *this; - } - - release(); - - ptr_ = other.ptr_; - counter_ = other.counter_; - - if (counter_) { - ++*counter_; - } - - return *this; - } + const_only_shared_ptr() = default; - non_sync_const_shared_ptr &operator=(non_sync_const_shared_ptr &&other) noexcept { - if (this == &other) { - return *this; - } + explicit const_only_shared_ptr(const T *ptr) : ptr_(ptr) {} - release(); + const_only_shared_ptr(const const_only_shared_ptr &) = default; + const_only_shared_ptr(const_only_shared_ptr &&) = default; - ptr_ = other.ptr_; - counter_ = other.counter_; + const_only_shared_ptr &operator=(const const_only_shared_ptr &) = default; + const_only_shared_ptr &operator=(const_only_shared_ptr &&) = default; - other.ptr_ = nullptr; - other.counter_ = nullptr; - - return *this; - } - - ~non_sync_const_shared_ptr() { release(); } - - const T *get() const { return ptr_; } + ~const_only_shared_ptr() = default; + const T *get() const { return ptr_.get(); } const T &operator*() const { return *ptr_; } - const T *operator->() const { return ptr_; } - + const T *operator->() const { return ptr_.get(); } explicit operator bool() const { return ptr_ != nullptr; } private: - void release() noexcept { - if (!counter_) { - return; - } - - --*counter_; - - if (*counter_ == 0) { - delete ptr_; - delete counter_; - } - - ptr_ = nullptr; - counter_ = nullptr; - } - - const T *ptr_ = nullptr; - size_t *counter_ = nullptr; + // for demonstration purpose only, this imitates smart pointer with a const-only pointer + std::shared_ptr ptr_; }; class MyData { public: - static non_sync_const_shared_ptr create(std::string name) { - return non_sync_const_shared_ptr(new MyData(std::move(name))); + static const_only_shared_ptr create(std::string name) { + return const_only_shared_ptr(new MyData(std::move(name))); } const std::string &getName() const { return name_; } @@ -117,10 +48,10 @@ class MyData { using namespace const_only_smart_ptr; -PYBIND11_DECLARE_HOLDER_TYPE(T, non_sync_const_shared_ptr, true) +PYBIND11_DECLARE_HOLDER_TYPE(T, const_only_shared_ptr, true) TEST_SUBMODULE(const_module, m) { - py::class_>(m, "Data") + py::class_>(m, "Data") .def(py::init([](const std::string &name) { return MyData::create(name); })) .def_property_readonly("name", &MyData::getName); } From 7e6af28e065334193e209225567f08a40b55c8ff Mon Sep 17 00:00:00 2001 From: Rosdf Date: Wed, 11 Jun 2025 09:12:58 +0400 Subject: [PATCH 12/13] move test --- tests/CMakeLists.txt | 1 - tests/test_const_only_smart_ptr.cpp | 57 ----------------------------- tests/test_const_only_smart_ptr.py | 11 ------ tests/test_smart_ptr.cpp | 20 ++++++++++ tests/test_smart_ptr.py | 6 +++ 5 files changed, 26 insertions(+), 69 deletions(-) delete mode 100644 tests/test_const_only_smart_ptr.cpp delete mode 100644 tests/test_const_only_smart_ptr.py diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f91639ff88..2cf18c3547 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -135,7 +135,6 @@ set(PYBIND11_TEST_FILES test_class_sh_unique_ptr_member test_class_sh_virtual_py_cpp_mix test_const_name - test_const_only_smart_ptr test_constants_and_functions test_copy_move test_cpp_conduit diff --git a/tests/test_const_only_smart_ptr.cpp b/tests/test_const_only_smart_ptr.cpp deleted file mode 100644 index 85465bbd10..0000000000 --- a/tests/test_const_only_smart_ptr.cpp +++ /dev/null @@ -1,57 +0,0 @@ -#include "pybind11/cast.h" -#include "pybind11_tests.h" - -#include -#include - -namespace const_only_smart_ptr { - -template -class const_only_shared_ptr { -public: - const_only_shared_ptr() = default; - - explicit const_only_shared_ptr(const T *ptr) : ptr_(ptr) {} - - const_only_shared_ptr(const const_only_shared_ptr &) = default; - const_only_shared_ptr(const_only_shared_ptr &&) = default; - - const_only_shared_ptr &operator=(const const_only_shared_ptr &) = default; - const_only_shared_ptr &operator=(const_only_shared_ptr &&) = default; - - ~const_only_shared_ptr() = default; - - const T *get() const { return ptr_.get(); } - const T &operator*() const { return *ptr_; } - const T *operator->() const { return ptr_.get(); } - explicit operator bool() const { return ptr_ != nullptr; } - -private: - // for demonstration purpose only, this imitates smart pointer with a const-only pointer - std::shared_ptr ptr_; -}; - -class MyData { -public: - static const_only_shared_ptr create(std::string name) { - return const_only_shared_ptr(new MyData(std::move(name))); - } - - const std::string &getName() const { return name_; } - -private: - explicit MyData(std::string &&name) : name_(std::move(name)) {} - - std::string name_; -}; -} // namespace const_only_smart_ptr - -using namespace const_only_smart_ptr; - -PYBIND11_DECLARE_HOLDER_TYPE(T, const_only_shared_ptr, true) - -TEST_SUBMODULE(const_module, m) { - py::class_>(m, "Data") - .def(py::init([](const std::string &name) { return MyData::create(name); })) - .def_property_readonly("name", &MyData::getName); -} diff --git a/tests/test_const_only_smart_ptr.py b/tests/test_const_only_smart_ptr.py deleted file mode 100644 index 8422899e7b..0000000000 --- a/tests/test_const_only_smart_ptr.py +++ /dev/null @@ -1,11 +0,0 @@ -from __future__ import annotations - -import pytest - -asyncio = pytest.importorskip("asyncio") -m = pytest.importorskip("pybind11_tests.const_module") - - -def test_const_smart_ptr(): - cons = m.Data("my_name") - assert cons.name == "my_name" diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 10a65f535f..7d009c9a84 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -69,6 +69,20 @@ class unique_ptr_with_addressof_operator { T **operator&() { throw std::logic_error("Call of overloaded operator& is not expected"); } }; +// Simple custom holder that imitates smart pointer, that always stores cpointer to const +template +class const_only_shared_ptr { + std::shared_ptr ptr_; + +public: + const_only_shared_ptr() = default; + explicit const_only_shared_ptr(const T *ptr) : ptr_(ptr) {} + const T *get() const { return ptr_.get(); } + +private: + // for demonstration purpose only, this imitates smart pointer with a const-only pointer +}; + // Custom object with builtin reference counting (see 'object.h' for the implementation) class MyObject1 : public Object { public: @@ -283,6 +297,7 @@ struct holder_helper> { // Make pybind aware of the ref-counted wrapper type (s): PYBIND11_DECLARE_HOLDER_TYPE(T, ref, true) +PYBIND11_DECLARE_HOLDER_TYPE(T, const_only_shared_ptr, true) // The following is not required anymore for std::shared_ptr, but it should compile without error: PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr) PYBIND11_DECLARE_HOLDER_TYPE(T, huge_unique_ptr) @@ -397,6 +412,11 @@ TEST_SUBMODULE(smart_ptr, m) { m.def("print_myobject2_4", [](const std::shared_ptr *obj) { py::print((*obj)->toString()); }); + m.def("make_myobject2_3", + [](int val) { return const_only_shared_ptr(new MyObject2(val)); }); + m.def("print_myobject2_5", + [](const const_only_shared_ptr &obj) { py::print(obj.get()->toString()); }); + py::class_>(m, "MyObject3").def(py::init()); m.def("make_myobject3_1", []() { return new MyObject3(8); }); m.def("make_myobject3_2", []() { return std::make_shared(9); }); diff --git a/tests/test_smart_ptr.py b/tests/test_smart_ptr.py index 3c20458755..4f9ed20101 100644 --- a/tests/test_smart_ptr.py +++ b/tests/test_smart_ptr.py @@ -350,3 +350,9 @@ def test_move_only_holder_caster_shared_ptr_with_smart_holder_support_enabled(): assert ( m.return_std_unique_ptr_example_drvd() == "move_only_holder_caster_traits_test" ) + +def test_const_only_holder(capture): + o = m.make_myobject2_3(4) + with capture: + m.print_myobject2_5(o) + assert capture == f"MyObject2[4]\n" From a8fa43ed58f58ebfc7a5de9f43391215ef46e565 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 11 Jun 2025 05:14:23 +0000 Subject: [PATCH 13/13] style: pre-commit fixes --- tests/test_smart_ptr.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_smart_ptr.py b/tests/test_smart_ptr.py index 4f9ed20101..e1d51ca06c 100644 --- a/tests/test_smart_ptr.py +++ b/tests/test_smart_ptr.py @@ -351,8 +351,9 @@ def test_move_only_holder_caster_shared_ptr_with_smart_holder_support_enabled(): m.return_std_unique_ptr_example_drvd() == "move_only_holder_caster_traits_test" ) + def test_const_only_holder(capture): o = m.make_myobject2_3(4) with capture: m.print_myobject2_5(o) - assert capture == f"MyObject2[4]\n" + assert capture == "MyObject2[4]\n"