[ViderStar]

Description

Fixes #5637.

Passing a NumPy array to a function that takes std::optional<std::reference_wrapper<Eigen::MatrixXd>> segfaults. The same happens for any T converted by value (Eigen types, strings, STL containers, custom casters), and not only inside std::optional: std::variant, std::vector/deque/list, std::array, std::set and std::map casters all load each element with a local caster and discard it, so a std::reference_wrapper<T> element refers to a destroyed T.

Change

• A caster can declare static constexpr bool value_refers_to_caster: the loaded value may refer to data owned by the caster. type_caster<std::reference_wrapper<T>> sets it when T's caster is not type_caster_generic-based (i.e. converts by value); the STL casters set it when an element type does. It is read through detail::loaded_value_refers_to_caster<T>.
• detail::subcaster_store<T> loads an element. When the trait is false it uses a local caster exactly as before (no allocation, casters stay copyable). When it is true it keeps the casters in a std::forward_list (constructed in place, so they need not be movable; addresses stable when the outer caster moves; the store is move-only). Used by optional_caster, variant_caster, list_caster, array_caster, set_caster, map_caster.
• cast_is_temporary_value_reference also covers such types, so py::cast<std::optional<std::reference_wrapper<std::string>>>(obj) (and py::cast<std::reference_wrapper<std::string>>, std::vector<std::reference_wrapper<std::string>>, ...), whose result would refer into a destroyed temporary caster, now hits the existing static_assert, and PYBIND11_OVERRIDE returning such a type uses its static caster like it does for references.
• optional_caster::load() empties the value and drops held casters before loading (also for None). The inner caster is still only constructed for non-None values.

Testing

• test_stl: optional, nested std::vector<std::optional<std::reference_wrapper<...>>>, variant, vector, array, set and map with a std::reference_wrapper to a by-value type. The test type tracks live instances by address and generation id, so a reused address is not taken for the original object. With the holding disabled they report dangling; with the fix they pass. Also a reload test (value then None), the non-movable/lazy inner caster test from the previous round, and static_asserts for the trait and caster copyability.
• Full test suite with C++11, 14, 17 and 20 (clang, macOS).
• Reproducers (Eigen in optional and variant, std::string in vector, array and map) under AddressSanitizer: clean with the fix; heap-use-after-free without it.
• py::cast of these types was checked to fail to compile (no compile-fail tests in the suite, so it is covered by static_asserts on the trait).

Suggested changelog entry:

• Fixed use-after-free when an argument holds std::reference_wrapper<T> to a type converted by value (e.g. Eigen matrices, std::string) inside std::optional, std::variant or an STL container. py::cast to such types (including plain std::reference_wrapper<T>), which returned a dangling reference, is now a compile-time error.

[rwgk]

codex GPT-6.1-Sol ultra:

Found two regressions at stl.h:565:

1. [P2] Preserve lazy construction for None. The member constructs the inner caster before checking for None. For std::optional<py::array_t<double>>, this imports NumPy and allocates an array. Without NumPy installed, f(None) now raises ModuleNotFoundError; the baseline accepts it. Reproducer: repro.cpp below.

2. [P2] Preserve support for nonmovable custom casters. If the inner caster contains nonmovable state, such as std::mutex, the optional caster now becomes nonmovable too. Previously valid obj.cast<std::optional<Item>>() fails compiling at load_type()’s return conv, even when Item is movable. Confirmed against baseline and PR headers. Reproducer: nonmovable_full.cpp below.

Lazy initialization through movable ownership could address both while preserving the lifetime fix.

repro.cpp

nonmovable_full.cpp

[ViderStar]

Thanks @rwgk, both regressions reproduced with the attached files (f(None) raising ModuleNotFoundError without NumPy, and cast<std::optional<Item>>() failing to compile with a nonmovable inner caster).

Fixed in f116885: the inner caster is now held by std::unique_ptr and constructed only when the value is not None, so the optional caster stays movable and the referenced value still lives as long as the caster (#5637 regression test and the ASan reproducer still pass). Added a test with a nonmovable, construction-counting custom caster that covers both the None case and cast<std::optional<T>>().

[henryiii]

Here's a Fable 5.1 review too:

🤖 AI text below 🤖

1. Fix is optional-only. The same local-inner-caster-then-discard pattern exists in variant_caster::load_alternative, list_caster, array_caster, map_caster and set_caster. A std::vector<std::reference_wrapper<std::string>> or a std::variant<std::reference_wrapper<Eigen::MatrixXd>, int> argument still dangles. Either apply the held-subcaster treatment to all container casters, or static_assert against reference_wrapper over non-type_caster_base casters inside containers.

2. Heap allocation on every optional load. inner_caster.reset(new value_conv()) adds malloc/free per call for every non-None std::optional<T>, including std::optional<int>. Suggested: construct the caster in place when it is movable, or only hold the subcaster when Value is a std::reference_wrapper.

3. optional_caster is no longer copyable. The std::unique_ptr member makes type_caster<std::optional<T>> and derived user casters, such as the documented boost::optional one, non-copy-constructible. The changelog entry does not mention this.

4. The py::cast path still dangles. py::cast<std::optional<std::reference_wrapper<T>>>(obj) returns the caster as a temporary, so the result now reads freed heap memory. cast_is_temporary_value_reference does not cover reference_wrapper, so no static_assert fires. The new tests only cover the argument-loader path.

5. Second load with None keeps old state. Loading an int then None on the same caster reports success but leaves value engaged and inner_caster allocated. Pre-existing for value, but the hunk now also retains the stale inner caster. Reset both on the None path.

6. Test has an ABA hole. LiveTracked::is_alive() checks address membership in a static set, so a later object at the same address masks a dangling reference. A generation id would close it. The static set is also unsynchronized.

Items 1, 2 and 4 are the ones that affect the design of the fix.

[ViderStar]

Thanks for the review. Reworked in bf831fe:

1. Other casters. Fixed in all of them rather than adding a static_assert. A caster can now declare static constexpr bool value_refers_to_caster (read via detail::loaded_value_refers_to_caster<T>): type_caster<std::reference_wrapper<T>> sets it when T's caster is not type_caster_generic-based, and the STL casters set it when an element type does, so nesting works too. A small helper, detail::subcaster_store<T>, does the element load in optional_caster, variant_caster, list_caster, array_caster, set_caster and map_caster; it keeps the casters only when that trait is true. I chose this over a static_assert because it makes these signatures work (which is what [BUG]: function with argument type std::optional<std::reference_wrapper<Eigen::...>> gives segmentation fault #5637 asks for for std::optional), and the same small helper covers every caster. The trait limits the extra cost to the case that was broken. Tests cover variant, vector, array, set, map and std::vector<std::optional<std::reference_wrapper<...>>>; they report dangling with holding disabled. An ASan reproducer with std::string in vector, array and map and Eigen in optional and variant is clean.
2. Heap allocation. Gone for everything except the reference_wrapper case: when the trait is false, subcaster_store uses a local caster exactly as before. When it is true, the casters go into a std::forward_list (one node per held caster, constructed in place).
3. Copyability. optional_caster<std::optional<int>> and derived casters such as the boost::optional one are copyable again (static_assert in the tests). Only casters whose value refers into held subcasters are move-only, because a copy would refer into the original's subcasters.
4. py::cast path. cast_is_temporary_value_reference now also covers these types, so py::cast<std::optional<std::reference_wrapper<std::string>>>(obj) hits the existing static_assert. The same goes for plain std::reference_wrapper<std::string>, which dangled the same way on master. PYBIND11_OVERRIDE with such a return type now uses its static caster, as it does for references. There are no compile-fail tests in the suite, so the trait is checked with static_asserts and the py::cast failure was checked locally. The changelog entry in the description mentions the new compile error.
5. Reload with None. optional_caster::load() now empties value and drops held casters before loading, also for None. It doesn't assign, because Value may not be assignable and std::experimental::optional has no reset(). There is a test for int then None. The container and variant casters also clear their held casters at the start of a load.
6. Test. Live instances are tracked as address to generation id, and a reference must point to a distinct live instance loaded in this call, so a reused address fails the check. The tracking state is thread_local, since instances are created and destroyed on the calling thread.

The changes to variant_caster::load_alternative keep its signature. Full suite passes locally with C++11/14/17/20; I updated the description and changelog entry.