From 4e393950d9cb10479af38f8b6e7440abd67e9aab Mon Sep 17 00:00:00 2001 From: Artsem Lebiadzevich Date: Sun, 25 Oct 2026 23:44:11 +0300 Subject: [PATCH 1/4] fix: avoid dangling reference in std::optional> arguments optional_caster::load() used a local inner caster, so for T with a by-value caster (Eigen matrices, STL containers, ...) the loaded reference_wrapper pointed into a destroyed caster. Keep the inner caster as a member so it lives as long as the optional caster. Fixes #5637 --- include/pybind11/stl.h | 7 ++++++- tests/test_stl.cpp | 47 ++++++++++++++++++++++++++++++++++++++++++ tests/test_stl.py | 7 +++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 01be0b47c6..ab1c88e363 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -548,7 +548,6 @@ struct optional_caster { if (src.is_none()) { return true; // default-constructed value is already empty } - value_conv inner_caster; if (!inner_caster.load(src, convert)) { return false; } @@ -558,6 +557,12 @@ struct optional_caster { } PYBIND11_TYPE_CASTER(Type, value_conv::name | make_caster::name); + +private: + // A member rather than a local in load(): the loaded value may refer to data owned by the + // inner caster (e.g. std::optional> with a by-value caster for T), + // so the inner caster must live as long as this caster. + value_conv inner_caster; }; #if defined(PYBIND11_HAS_OPTIONAL) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index b5d1979d04..b1cd9eabb7 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -18,6 +18,7 @@ #include +#include #include #include @@ -35,6 +36,40 @@ struct type_caster : void_caster {}; } // namespace PYBIND11_NAMESPACE #endif +// Converted by value (not a registered class) and tracks which instances are alive, to check +// that a loaded std::optional> does not refer to a destroyed object. +struct LiveTracked { + static std::set &live() { + static std::set instances; + return instances; + } + int value = 0; + LiveTracked() { live().insert(this); } + LiveTracked(const LiveTracked &other) : value(other.value) { live().insert(this); } + LiveTracked &operator=(const LiveTracked &) = default; + ~LiveTracked() { live().erase(this); } + bool is_alive() const { return live().count(this) != 0; } +}; + +namespace PYBIND11_NAMESPACE { +namespace detail { +template <> +struct type_caster { + PYBIND11_TYPE_CASTER(LiveTracked, const_name("int")); + bool load(handle src, bool) { + if (!isinstance(src)) { + return false; + } + value.value = src.cast(); + return true; + } + static handle cast(const LiveTracked &src, return_value_policy, handle) { + return int_(src.value).release(); + } +}; +} // namespace detail +} // namespace PYBIND11_NAMESPACE + // Test with `std::variant` in C++17 mode, or with `boost::variant` in C++11/14 #if defined(PYBIND11_HAS_VARIANT) using std::variant; @@ -357,6 +392,18 @@ TEST_SUBMODULE(stl, m) { .def(pybind11::init<>()) .def_property_readonly("access_by_ref", &opt_props::access_by_ref) .def_property_readonly("access_by_copy", &opt_props::access_by_copy); + + // #5637: the referenced value must outlive the inner caster used during load + m.def("optional_refwrap_value", + [](std::optional> x) -> py::object { + if (!x) { + return py::none(); + } + if (!x->get().is_alive()) { + return py::str("dangling"); + } + return py::int_(x->get().value); + }); #endif #ifdef PYBIND11_HAS_EXP_OPTIONAL diff --git a/tests/test_stl.py b/tests/test_stl.py index f3f4ccc736..40e660aeaf 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -293,6 +293,13 @@ def test_optional(): assert int(props.access_by_copy) == 42 +@pytest.mark.skipif(not hasattr(m, "has_optional"), reason="no ") +def test_optional_reference_wrapper_to_converted_value(): + # #5637: used to refer to a value owned by an already destroyed caster + assert m.optional_refwrap_value(None) is None + assert m.optional_refwrap_value(7) == 7 + + @pytest.mark.skipif( not hasattr(m, "has_exp_optional"), reason="no " ) From f116885eeef32ce8fdcc8251404fdce23ef8de6b Mon Sep 17 00:00:00 2001 From: Artsem Lebiadzevich Date: Mon, 26 Oct 2026 10:18:59 +0300 Subject: [PATCH 2/4] fix: construct the optional's inner caster lazily and keep the caster movable Hold the inner caster of optional_caster through std::unique_ptr, created only for non-None values. This keeps the #5637 lifetime fix while restoring lazy construction for None (e.g. std::optional> no longer needs NumPy for None) and movability of the optional caster when the inner caster is not movable (needed by cast>()). --- include/pybind11/stl.h | 16 +++++++++------ tests/test_stl.cpp | 45 ++++++++++++++++++++++++++++++++++++++++++ tests/test_stl.py | 12 +++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index ab1c88e363..7b68aad56f 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -548,21 +548,25 @@ struct optional_caster { if (src.is_none()) { return true; // default-constructed value is already empty } - if (!inner_caster.load(src, convert)) { + // Constructed only for non-None values: some casters have costly or failing + // constructors (e.g. py::array_t imports NumPy). + inner_caster.reset(new value_conv()); + if (!inner_caster->load(src, convert)) { return false; } - value.emplace(cast_op(std::move(inner_caster))); + value.emplace(cast_op(std::move(*inner_caster))); return true; } PYBIND11_TYPE_CASTER(Type, value_conv::name | make_caster::name); private: - // A member rather than a local in load(): the loaded value may refer to data owned by the - // inner caster (e.g. std::optional> with a by-value caster for T), - // so the inner caster must live as long as this caster. - value_conv inner_caster; + // Kept alive with this caster rather than as a local in load(): the loaded value may refer + // to data owned by the inner caster (e.g. std::optional> with a + // by-value caster for T). Held by pointer so this caster stays movable (and the reference + // stays valid after a move) even if the inner caster is not movable. + std::unique_ptr inner_caster; }; #if defined(PYBIND11_HAS_OPTIONAL) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index b1cd9eabb7..c8f32a3ccb 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -70,6 +70,41 @@ struct type_caster { } // namespace detail } // namespace PYBIND11_NAMESPACE +// Converted by value with a caster that counts its constructions and is not movable, to check +// that std::optional constructs the inner caster only for non-None values and stays movable. +struct NonMovableCasterItem { + int value = 0; +}; + +namespace PYBIND11_NAMESPACE { +namespace detail { +template <> +struct type_caster { + PYBIND11_TYPE_CASTER(NonMovableCasterItem, const_name("int")); + static int &constructions() { + static int count = 0; + return count; + } + type_caster() { ++constructions(); } + type_caster(const type_caster &) = delete; + type_caster(type_caster &&) = delete; + type_caster &operator=(const type_caster &) = delete; + type_caster &operator=(type_caster &&) = delete; + ~type_caster() = default; + bool load(handle src, bool) { + if (!isinstance(src)) { + return false; + } + value.value = src.cast(); + return true; + } + static handle cast(const NonMovableCasterItem &src, return_value_policy, handle) { + return int_(src.value).release(); + } +}; +} // namespace detail +} // namespace PYBIND11_NAMESPACE + // Test with `std::variant` in C++17 mode, or with `boost::variant` in C++11/14 #if defined(PYBIND11_HAS_VARIANT) using std::variant; @@ -404,6 +439,16 @@ TEST_SUBMODULE(stl, m) { } return py::int_(x->get().value); }); + + // The inner caster is constructed lazily (not for None) and held so that the optional caster + // stays movable, as required by cast>(), even if the inner caster is not. + m.def("optional_nonmovable_caster_constructions", + []() { return py::detail::make_caster::constructions(); }); + m.def("optional_nonmovable_caster_arg", [](std::optional x) { + return x ? py::object(py::int_(x->value)) : py::object(py::none()); + }); + m.def("optional_nonmovable_caster_cast", + [](const py::object &o) { return o.cast>(); }); #endif #ifdef PYBIND11_HAS_EXP_OPTIONAL diff --git a/tests/test_stl.py b/tests/test_stl.py index 40e660aeaf..55897e7859 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -300,6 +300,18 @@ def test_optional_reference_wrapper_to_converted_value(): assert m.optional_refwrap_value(7) == 7 +@pytest.mark.skipif(not hasattr(m, "has_optional"), reason="no ") +def test_optional_inner_caster_lazy_and_nonmovable(): + count = m.optional_nonmovable_caster_constructions + before = count() + assert m.optional_nonmovable_caster_arg(None) is None + assert m.optional_nonmovable_caster_cast(None) is None + assert count() == before # no inner caster for None + assert m.optional_nonmovable_caster_arg(3) == 3 + assert m.optional_nonmovable_caster_cast(4) == 4 + assert count() == before + 2 + + @pytest.mark.skipif( not hasattr(m, "has_exp_optional"), reason="no " ) From bf831feb209b1e96bd289d5c1b0153b77e85aead Mon Sep 17 00:00:00 2001 From: Artsem Lebiadzevich Date: Tue, 27 Oct 2026 12:02:09 +0300 Subject: [PATCH 3/4] fix: keep subcasters alive only for reference_wrapper elements, in all STL casters Values loaded as std::reference_wrapper, where T is converted by value, refer into the caster that loaded them. Instead of always holding the optional's inner caster on the heap, a caster now declares this with `static constexpr bool value_refers_to_caster` (std::reference_wrapper over a non-type_caster_generic caster, and the STL casters with such elements), exposed as detail::loaded_value_refers_to_caster. detail::subcaster_store loads elements through a local caster as before when that trait is false (no allocation, casters stay copyable), and keeps the casters in a std::forward_list otherwise (constructed in place, stable across moves, move-only). It is used by optional_caster, variant_caster, list_caster, array_caster, set_caster and map_caster, so e.g. std::vector> and std::variant> no longer dangle. cast_is_temporary_value_reference now also covers such types, so py::cast>>(obj) (whose result would refer into a destroyed temporary caster) fails to compile with the existing static_assert, and PYBIND11_OVERRIDE uses its static caster. optional_caster::load() now empties the value and drops held casters before loading, including for None. The tests track live LiveTracked instances by address and generation id (thread_local), so a reused address is detected. --- include/pybind11/cast.h | 23 +++++- include/pybind11/stl.h | 167 +++++++++++++++++++++++++++++++--------- tests/test_stl.cpp | 152 ++++++++++++++++++++++++++++++++---- tests/test_stl.py | 22 +++++- 4 files changed, 308 insertions(+), 56 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 891f1c9d18..6bc87a1854 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -188,6 +188,9 @@ class type_caster> { template using cast_op_type = std::reference_wrapper; explicit operator std::reference_wrapper() { return cast_op(subcaster); } + // The loaded reference points into `subcaster` unless `type` is a registered class. + static constexpr bool value_refers_to_caster + = !std::is_base_of::value; }; #define PYBIND11_TYPE_CASTER(type, py_name) \ @@ -1619,15 +1622,27 @@ struct move_if_unreferenced< template using move_never = none_of, move_if_unreferenced>; +// Whether a value loaded by make_caster can refer to data owned by that caster, so that the +// caster must outlive the value. Casters opt in with `static constexpr bool +// value_refers_to_caster = true;`: type_caster> when U is converted by +// value, and the STL casters (optional, variant, containers) holding such elements. +template +struct loaded_value_refers_to_caster : std::false_type {}; +template +struct loaded_value_refers_to_caster::value_refers_to_caster>> + : std::true_type {}; + // Detect whether returning a `type` from a cast on type's type_caster is going to result in a // reference or pointer to a local variable of the type_caster. Basically, only // non-reference/pointer `type`s and reference/pointers from a type_caster_generic are safe; -// everything else returns a reference/pointer to a local variable. +// everything else returns a reference/pointer to a local variable. The same holds for values +// that refer into their caster, e.g. `std::optional>`. template using cast_is_temporary_value_reference - = bool_constant<(std::is_reference::value || std::is_pointer::value) - && !std::is_base_of>::value - && !std::is_same, void>::value>; + = bool_constant<((std::is_reference::value || std::is_pointer::value) + && !std::is_base_of>::value + && !std::is_same, void>::value) + || loaded_value_refers_to_caster::value>; // When a value returned from a C++ function is being cast back to Python, we almost always want to // force `policy = move`, regardless of the return value policy the function/method was declared diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 7b68aad56f..851a22543e 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -15,6 +15,7 @@ #include "detail/type_caster_base.h" #include +#include #include #include #include @@ -155,6 +156,55 @@ constexpr forwarded_type forward_like(U &&u) { template using has_reserve_method = std::is_same().reserve(0)), void>; +/// Loads values of type T with make_caster. Usually each caster is a local, discarded after its +/// value has been moved out. If the loaded value can refer into its caster +/// (loaded_value_refers_to_caster, e.g. std::reference_wrapper), the casters are +/// kept here instead, so they live as long as the caster that owns this store. +template ::value> +class subcaster_store { +public: + /// Loads `src` and, on success, calls `use(caster)` to take the value out of the caster. + template + bool load(handle src, bool convert, Use &&use) { + make_caster caster; + if (!caster.load(src, convert)) { + return false; + } + use(caster); + return true; + } + void clear() {} +}; + +template +class subcaster_store { +public: + template + bool load(handle src, bool convert, Use &&use) { + casters.emplace_front(); + if (!casters.front().load(src, convert)) { + casters.pop_front(); + return false; + } + use(casters.front()); + return true; + } + void clear() { casters.clear(); } + + subcaster_store() = default; + subcaster_store(subcaster_store &&) = default; + subcaster_store &operator=(subcaster_store &&) = default; + // A copy would hold values that still refer into the casters of the original. + subcaster_store(const subcaster_store &) = delete; + subcaster_store &operator=(const subcaster_store &) = delete; + ~subcaster_store() = default; + +private: + // Node-based: casters are constructed in place (they need not be movable), and their + // addresses do not change when the store is moved. + std::forward_list> casters; +}; + template struct set_caster { using type = Type; @@ -169,35 +219,42 @@ struct set_caster { bool convert_iterable(const iterable &itbl, bool convert) { for (const auto &it : itbl) { - key_conv conv; - if (!conv.load(it, convert)) { + if (!key_casters.load(it, convert, [this](key_conv &conv) { + value.insert(cast_op(std::move(conv))); + })) { return false; } - value.insert(cast_op(std::move(conv))); } return true; } bool convert_anyset(const anyset &s, bool convert) { - value.clear(); + clear(); reserve_maybe(s, &value); return convert_iterable(s, convert); } + void clear() { + value.clear(); + key_casters.clear(); + } + + subcaster_store key_casters; + public: bool load(handle src, bool convert) { if (!object_is_convertible_to_std_set(src, convert)) { return false; } if (isinstance(src)) { - value.clear(); + clear(); return convert_anyset(reinterpret_borrow(src), convert); } if (!convert) { return false; } assert(isinstance(src)); - value.clear(); + clear(); return convert_iterable(reinterpret_borrow(src), convert); } @@ -220,6 +277,7 @@ struct set_caster { PYBIND11_TYPE_CASTER(type, io_name("collections.abc.Set", "set") + const_name("[") + key_conv::name + const_name("]")); + static constexpr bool value_refers_to_caster = loaded_value_refers_to_caster::value; }; template @@ -236,18 +294,30 @@ struct map_caster { bool convert_elements(const dict &d, bool convert) { value.clear(); + key_casters.clear(); + value_casters.clear(); reserve_maybe(d, &value); for (const auto &it : d) { - key_conv kconv; - value_conv vconv; - if (!kconv.load(it.first.ptr(), convert) || !vconv.load(it.second.ptr(), convert)) { + bool value_loaded = false; + if (!key_casters.load(it.first.ptr(), + convert, + [&](key_conv &kconv) { + value_loaded = value_casters.load( + it.second.ptr(), convert, [&](value_conv &vconv) { + value.emplace(cast_op(std::move(kconv)), + cast_op(std::move(vconv))); + }); + }) + || !value_loaded) { return false; } - value.emplace(cast_op(std::move(kconv)), cast_op(std::move(vconv))); } return true; } + subcaster_store key_casters; + subcaster_store value_casters; + public: bool load(handle src, bool convert) { if (!object_is_convertible_to_std_map(src, convert)) { @@ -293,6 +363,8 @@ struct map_caster { io_name("collections.abc.Mapping", "dict") + const_name("[") + key_conv::name + const_name(", ") + value_conv::name + const_name("]")); + static constexpr bool value_refers_to_caster + = loaded_value_refers_to_caster::value || loaded_value_refers_to_caster::value; }; template @@ -327,17 +399,20 @@ struct list_caster { bool convert_elements(handle seq, bool convert) { auto s = reinterpret_borrow(seq); value.clear(); + value_casters.clear(); reserve_maybe(s, &value); for (const auto &it : seq) { - value_conv conv; - if (!conv.load(it, convert)) { + if (!value_casters.load(it, convert, [this](value_conv &conv) { + value.push_back(cast_op(std::move(conv))); + })) { return false; } - value.push_back(cast_op(std::move(conv))); } return true; } + subcaster_store value_casters; + public: template static handle cast(T &&src, return_value_policy policy, handle parent) { @@ -360,6 +435,7 @@ struct list_caster { PYBIND11_TYPE_CASTER(Type, io_name("collections.abc.Sequence", "list") + const_name("[") + value_conv::name + const_name("]")); + static constexpr bool value_refers_to_caster = loaded_value_refers_to_caster::value; }; template @@ -388,11 +464,13 @@ struct array_caster { private: std::unique_ptr value; + subcaster_store value_casters; template = 0> bool convert_elements(handle seq, bool convert) { auto l = reinterpret_borrow(seq); value.reset(new ArrayType{}); + value_casters.clear(); // Using `resize` to preserve the behavior exactly as it was before PR #5305 // For the `resize` to work, `Value` must be default constructible. // For `std::valarray`, this is a requirement: @@ -400,11 +478,11 @@ struct array_caster { value->resize(l.size()); size_t ctr = 0; for (const auto &it : l) { - value_conv conv; - if (!conv.load(it, convert)) { + if (!value_casters.load(it, convert, [&](value_conv &conv) { + (*value)[ctr++] = cast_op(std::move(conv)); + })) { return false; } - (*value)[ctr++] = cast_op(std::move(conv)); } return true; } @@ -420,14 +498,16 @@ struct array_caster { // Deliberate choice: no template specializations, for simplicity, and // because the compile time overhead for the specializations is deemed // more significant than the runtime overhead for the `temp` storage. + value.reset(); + value_casters.clear(); std::vector temp; temp.reserve(l.size()); for (auto it : l) { - value_conv conv; - if (!conv.load(it, convert)) { + if (!value_casters.load(it, convert, [&](value_conv &conv) { + temp.emplace_back(cast_op(std::move(conv))); + })) { return false; } - temp.emplace_back(cast_op(std::move(conv))); } value.reset(new ArrayType(vector_to_array(std::move(temp)))); return true; @@ -499,6 +579,7 @@ struct array_caster { + const_name(const_name(""), const_name(", \"FixedSize(") + const_name() + const_name(")\"]")); + static constexpr bool value_refers_to_caster = loaded_value_refers_to_caster::value; }; template @@ -545,28 +626,28 @@ struct optional_caster { if (!src) { return false; } - if (src.is_none()) { - return true; // default-constructed value is already empty + if (value) { + // Empty it without assigning: Value need not be assignable, and + // std::experimental::optional has no reset(). + value.~Type(); + new (&value) Type(); } - // Constructed only for non-None values: some casters have costly or failing - // constructors (e.g. py::array_t imports NumPy). - inner_caster.reset(new value_conv()); - if (!inner_caster->load(src, convert)) { - return false; + value_casters.clear(); + if (src.is_none()) { + return true; } - - value.emplace(cast_op(std::move(*inner_caster))); - return true; + // The inner caster is only constructed for non-None values: some casters have costly + // or failing constructors (e.g. py::array_t imports NumPy). + return value_casters.load(src, convert, [this](value_conv &conv) { + value.emplace(cast_op(std::move(conv))); + }); } PYBIND11_TYPE_CASTER(Type, value_conv::name | make_caster::name); + static constexpr bool value_refers_to_caster = loaded_value_refers_to_caster::value; private: - // Kept alive with this caster rather than as a local in load(): the loaded value may refer - // to data owned by the inner caster (e.g. std::optional> with a - // by-value caster for T). Held by pointer so this caster stays movable (and the reference - // stays valid after a move) even if the inner caster is not movable. - std::unique_ptr inner_caster; + subcaster_store value_casters; }; #if defined(PYBIND11_HAS_OPTIONAL) @@ -622,9 +703,10 @@ struct variant_caster> { template bool load_alternative(handle src, bool convert, type_list) { - auto caster = make_caster(); - if (caster.load(src, convert)) { - value = cast_op(std::move(caster)); + auto &casters = std::get(alternative_casters); + if (casters.load(src, convert, [this](make_caster &caster) { + value = cast_op(std::move(caster)); + })) { return true; } return load_alternative(src, convert, type_list{}); @@ -633,6 +715,7 @@ struct variant_caster> { bool load_alternative(handle, bool, type_list<>) { return false; } bool load(handle src, bool convert) { + clear_alternative_casters(make_index_sequence{}); // Do a first pass without conversions to improve constructor resolution. // E.g. `py::int_(1).cast>()` needs to fill the `int` // slot of the variant. Without two-pass loading `double` would be filled @@ -651,6 +734,16 @@ struct variant_caster> { using Type = V; PYBIND11_TYPE_CASTER(Type, ::pybind11::detail::union_concat(make_caster::name...)); + static constexpr bool value_refers_to_caster + = any_of...>::value; + +private: + template + void clear_alternative_casters(index_sequence) { + PYBIND11_EXPAND_SIDE_EFFECTS(std::get(alternative_casters).clear()); + } + + std::tuple...> alternative_casters; }; #if defined(PYBIND11_HAS_VARIANT) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index c8f32a3ccb..3fe09e3172 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -18,6 +18,9 @@ #include +#include +#include +#include #include #include #include @@ -36,21 +39,67 @@ struct type_caster : void_caster {}; } // namespace PYBIND11_NAMESPACE #endif -// Converted by value (not a registered class) and tracks which instances are alive, to check -// that a loaded std::optional> does not refer to a destroyed object. +// Converted by value (not a registered class), to check that a loaded +// std::reference_wrapper (in an optional, variant or container) does not refer to a +// destroyed object. Each instance gets a new id; the live instances are tracked by address and id, +// so a reference is checked without reading the object, and an address reused by a later +// instance is not mistaken for the original. Instances are created and destroyed on the thread +// of the call, hence thread_local rather than shared state. struct LiveTracked { - static std::set &live() { - static std::set instances; + static std::map &live() { + static thread_local std::map instances; return instances; } + // Ids of the instances filled by type_caster::load(), in load order. + static std::vector &loaded() { + static thread_local std::vector ids; + return ids; + } + static unsigned next_id() { + static thread_local unsigned id = 0; + return ++id; + } int value = 0; - LiveTracked() { live().insert(this); } - LiveTracked(const LiveTracked &other) : value(other.value) { live().insert(this); } - LiveTracked &operator=(const LiveTracked &) = default; + LiveTracked() { live()[this] = next_id(); } + LiveTracked(const LiveTracked &other) : value(other.value) { live()[this] = next_id(); } + LiveTracked &operator=(const LiveTracked &other) { + value = other.value; + return *this; + } ~LiveTracked() { live().erase(this); } - bool is_alive() const { return live().count(this) != 0; } }; +static bool operator<(const LiveTracked &a, const LiveTracked &b) { return a.value < b.value; } + +// The values of `refs`, or "dangling" unless each one refers to a distinct live instance that was +// loaded since the previous check. +static py::object live_values(const std::vector &refs) { + std::vector loaded; + loaded.swap(LiveTracked::loaded()); + std::set seen; + py::list values; + for (const auto *ref : refs) { + auto it = LiveTracked::live().find(ref); + if (it == LiveTracked::live().end() + || std::find(loaded.begin(), loaded.end(), it->second) == loaded.end() + || !seen.insert(it->second).second) { + return py::str("dangling"); + } + values.append(ref->value); + } + return std::move(values); +} + +// live_values() of a container of std::reference_wrapper +template +py::object live_values_of(const Container &refs) { + std::vector ptrs; + for (const auto &ref : refs) { + ptrs.push_back(&ref.get()); + } + return live_values(ptrs); +} + namespace PYBIND11_NAMESPACE { namespace detail { template <> @@ -61,6 +110,7 @@ struct type_caster { return false; } value.value = src.cast(); + LiveTracked::loaded().push_back(LiveTracked::live().at(&value)); return true; } static handle cast(const LiveTracked &src, return_value_policy, handle) { @@ -82,7 +132,7 @@ template <> struct type_caster { PYBIND11_TYPE_CASTER(NonMovableCasterItem, const_name("int")); static int &constructions() { - static int count = 0; + static thread_local int count = 0; return count; } type_caster() { ++constructions(); } @@ -434,14 +484,45 @@ TEST_SUBMODULE(stl, m) { if (!x) { return py::none(); } - if (!x->get().is_alive()) { - return py::str("dangling"); + return live_values({&x->get()}); + }); + m.def("vector_optional_refwrap_values", + [](const std::vector>> &v) { + std::vector refs; + for (const auto &x : v) { + if (x) { + refs.push_back(&x->get()); + } } - return py::int_(x->get().value); + return live_values(refs); }); - // The inner caster is constructed lazily (not for None) and held so that the optional caster - // stays movable, as required by cast>(), even if the inner caster is not. + // Loading None into a caster that holds a value releases the value and its inner caster + m.def("optional_refwrap_reload", []() { + auto live_before = LiveTracked::live().size(); + py::detail::make_caster>> caster; + auto &value + = py::detail::cast_op> &>(caster); + bool loaded_int = caster.load(py::int_(5), true) && value && value->get().value == 5; + bool loaded_none = caster.load(py::none(), true) && !value; + LiveTracked::loaded().clear(); + return py::make_tuple(loaded_int, loaded_none, LiveTracked::live().size() == live_before); + }); + + // Without a reference_wrapper element, the optional caster holds no inner caster and stays + // copyable (e.g. for user casters derived from it). + static_assert(std::is_copy_constructible>>::value, + ""); + static_assert( + !py::detail::cast_is_temporary_value_reference>::value, ""); + // py::cast() would return a value referring into a destroyed caster, so it is rejected at + // compile time (by the same static_assert as for references to by-value types). + static_assert(py::detail::cast_is_temporary_value_reference< + std::optional>>::value, + ""); + + // The inner caster is constructed lazily (not for None) and the optional caster stays + // movable, as required by cast>(), even if the inner caster is not. m.def("optional_nonmovable_caster_constructions", []() { return py::detail::make_caster::constructions(); }); m.def("optional_nonmovable_caster_arg", [](std::optional x) { @@ -629,6 +710,18 @@ TEST_SUBMODULE(stl, m) { }); # if defined(PYBIND11_HAS_VARIANT) + // #5637: same for a std::reference_wrapper alternative + m.def("variant_refwrap_value", + [](const std::variant> &v) { + if (v.index() == 0) { + return py::object(py::str(std::get<0>(v))); + } + return live_values({&std::get<1>(v).get()}); + }); + static_assert(py::detail::cast_is_temporary_value_reference< + std::variant>>::value, + ""); + // std::monostate tests. m.def("load_monostate_variant", [](const variant &v) -> const char * { @@ -664,6 +757,37 @@ TEST_SUBMODULE(stl, m) { std::ref(p1), std::ref(p2), std::ref(p3), p4}; }); + // #5637: containers of std::reference_wrapper to by-value types keep the element casters + m.def("vector_refwrap_values", [](const std::vector> &v) { + return live_values_of(v); + }); + m.def("array_refwrap_values", [](const std::array, 3> &a) { + return live_values_of(a); + }); + m.def("set_refwrap_values", [](const std::set> &s) { + return live_values_of(s); + }); + m.def("map_refwrap_values", + [](const std::map> &d) { + std::vector refs; + for (const auto &kv : d) { + refs.push_back(&kv.second.get()); + } + return live_values(refs); + }); + static_assert(py::detail::cast_is_temporary_value_reference< + std::vector>>::value, + ""); + static_assert(py::detail::cast_is_temporary_value_reference< + std::map>>::value, + ""); + // References to registered types point to the Python object, not into the caster + static_assert(!py::detail::cast_is_temporary_value_reference< + std::vector>>::value, + ""); + static_assert(std::is_copy_constructible>>::value, + ""); + // test_stl_pass_by_pointer m.def("stl_pass_by_pointer", [](std::vector *v) { return *v; }, "v"_a = nullptr); diff --git a/tests/test_stl.py b/tests/test_stl.py index 55897e7859..774f1c177e 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -297,7 +297,10 @@ def test_optional(): def test_optional_reference_wrapper_to_converted_value(): # #5637: used to refer to a value owned by an already destroyed caster assert m.optional_refwrap_value(None) is None - assert m.optional_refwrap_value(7) == 7 + assert m.optional_refwrap_value(7) == [7] + assert m.vector_optional_refwrap_values([1, None, 2, 3]) == [1, 2, 3] + # loading None after a value empties the optional and drops the inner caster + assert m.optional_refwrap_reload() == (True, True, True) @pytest.mark.skipif(not hasattr(m, "has_optional"), reason="no ") @@ -519,6 +522,23 @@ def test_variant_monostate(doc): ) +def test_containers_of_reference_wrapper_to_converted_value(): + # #5637: same as for std::optional, for each element + assert m.vector_refwrap_values([1, 2, 3]) == [1, 2, 3] + assert m.vector_refwrap_values([]) == [] + assert m.array_refwrap_values([4, 5, 6]) == [4, 5, 6] + assert m.set_refwrap_values({9, 7, 8}) == [7, 8, 9] + assert m.map_refwrap_values({"a": 1, "b": 2}) == [1, 2] + with pytest.raises(TypeError): + m.vector_refwrap_values([1, "x"]) + + +@pytest.mark.skipif(not hasattr(m, "variant_refwrap_value"), reason="no ") +def test_variant_reference_wrapper_to_converted_value(): + assert m.variant_refwrap_value(5) == [5] + assert m.variant_refwrap_value("s") == "s" + + def test_vec_of_reference_wrapper(): """#171: Can't return reference wrappers (or STL structures containing them)""" assert ( From 70ff990dc9ec1620ace370544ab72d8969d4dff5 Mon Sep 17 00:00:00 2001 From: Artsem Lebiadzevich Date: Wed, 28 Oct 2026 20:28:54 +0300 Subject: [PATCH 4/4] tests: fix -Werror and clang-tidy warnings in the reference_wrapper tests Return the list as py::object directly (GCC 14 -Wredundant-move), default LiveTracked's copy assignment, and reserve the pointer vectors before filling them (clang-tidy). --- tests/test_stl.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 3fe09e3172..73d09b0f88 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -62,10 +62,7 @@ struct LiveTracked { int value = 0; LiveTracked() { live()[this] = next_id(); } LiveTracked(const LiveTracked &other) : value(other.value) { live()[this] = next_id(); } - LiveTracked &operator=(const LiveTracked &other) { - value = other.value; - return *this; - } + LiveTracked &operator=(const LiveTracked &) = default; ~LiveTracked() { live().erase(this); } }; @@ -87,13 +84,14 @@ static py::object live_values(const std::vector &refs) { } values.append(ref->value); } - return std::move(values); + return py::object(std::move(values)); } // live_values() of a container of std::reference_wrapper template py::object live_values_of(const Container &refs) { std::vector ptrs; + ptrs.reserve(refs.size()); for (const auto &ref : refs) { ptrs.push_back(&ref.get()); } @@ -770,6 +768,7 @@ TEST_SUBMODULE(stl, m) { m.def("map_refwrap_values", [](const std::map> &d) { std::vector refs; + refs.reserve(d.size()); for (const auto &kv : d) { refs.push_back(&kv.second.get()); }