From a1d00916b26b187e583f3bce39cd59c3b0652c32 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 15 Aug 2024 12:30:29 +0700 Subject: [PATCH] Backport of https://github.com/google/pywrapcc/pull/30034 (#5305) --- include/pybind11/stl.h | 98 +++++++++++++++++++++++++++++++++--------- tests/test_stl.cpp | 17 ++++++++ tests/test_stl.py | 7 +++ 3 files changed, 101 insertions(+), 21 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 096301bc7..6a148e740 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -11,11 +11,14 @@ #include "pybind11.h" #include "detail/common.h" +#include "detail/descr.h" +#include "detail/type_caster_base.h" #include #include #include #include +#include #include #include #include @@ -349,39 +352,68 @@ struct type_caster> : list_caster struct type_caster> : list_caster, Type> {}; +template +ArrayType vector_to_array_impl(V &&v, index_sequence) { + return {{std::move(v[I])...}}; +} + +// Based on https://en.cppreference.com/w/cpp/container/array/to_array +template +ArrayType vector_to_array(V &&v) { + return vector_to_array_impl(std::forward(v), make_index_sequence{}); +} + template struct array_caster { using value_conv = make_caster; private: - template - bool require_size(enable_if_t size) { - if (value.size() != size) { - value.resize(size); - } - return true; - } - template - bool require_size(enable_if_t size) { - return size == Size; - } + std::unique_ptr value; + template = 0> bool convert_elements(handle seq, bool convert) { auto l = reinterpret_borrow(seq); - if (!require_size(l.size())) { - return false; - } + value.reset(new ArrayType{}); + // 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: + // https://en.cppreference.com/w/cpp/named_req/NumericType + value->resize(l.size()); size_t ctr = 0; for (const auto &it : l) { value_conv conv; if (!conv.load(it, convert)) { return false; } - value[ctr++] = cast_op(std::move(conv)); + (*value)[ctr++] = cast_op(std::move(conv)); } return true; } + template = 0> + bool convert_elements(handle seq, bool convert) { + auto l = reinterpret_borrow(seq); + if (l.size() != Size) { + return false; + } + // The `temp` storage is needed to support `Value` types that are not + // default-constructible. + // 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. + std::vector temp; + temp.reserve(l.size()); + for (auto it : l) { + value_conv conv; + if (!conv.load(it, convert)) { + return false; + } + temp.emplace_back(cast_op(std::move(conv))); + } + value.reset(new ArrayType(vector_to_array(std::move(temp)))); + return true; + } + public: bool load(handle src, bool convert) { if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) { @@ -416,12 +448,36 @@ public: return l.release(); } - PYBIND11_TYPE_CASTER(ArrayType, - const_name(const_name(""), const_name("Annotated[")) - + const_name("list[") + value_conv::name + const_name("]") - + const_name(const_name(""), - const_name(", FixedSize(") - + const_name() + const_name(")]"))); + // Code copied from PYBIND11_TYPE_CASTER macro. + // Intentionally preserving the behavior exactly as it was before PR #5305 + template >::value, int> = 0> + static handle cast(T_ *src, return_value_policy policy, handle parent) { + if (!src) { + return none().release(); + } + if (policy == return_value_policy::take_ownership) { + auto h = cast(std::move(*src), policy, parent); + delete src; // WARNING: Assumes `src` was allocated with `new`. + return h; + } + return cast(*src, policy, parent); + } + + // NOLINTNEXTLINE(google-explicit-constructor) + operator ArrayType *() { return &(*value); } + // NOLINTNEXTLINE(google-explicit-constructor) + operator ArrayType &() { return *value; } + // NOLINTNEXTLINE(google-explicit-constructor) + operator ArrayType &&() && { return std::move(*value); } + + template + using cast_op_type = movable_cast_op_type; + + static constexpr auto name + = const_name(const_name(""), const_name("Annotated[")) + const_name("list[") + + value_conv::name + const_name("]") + + const_name( + const_name(""), const_name(", FixedSize(") + const_name() + const_name(")]")); }; template diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index c35f0d4ca..6240524a0 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -201,6 +201,23 @@ TEST_SUBMODULE(stl, m) { m.def("cast_array", []() { return std::array{{1, 2}}; }); m.def("load_array", [](const std::array &a) { return a[0] == 1 && a[1] == 2; }); + struct NoDefaultCtor { + explicit constexpr NoDefaultCtor(int val) : val{val} {} + int val; + }; + + struct NoDefaultCtorArray { + explicit constexpr NoDefaultCtorArray(int i) + : arr{{NoDefaultCtor(10 + i), NoDefaultCtor(20 + i)}} {} + std::array arr; + }; + + // test_array_no_default_ctor + py::class_(m, "NoDefaultCtor").def_readonly("val", &NoDefaultCtor::val); + py::class_(m, "NoDefaultCtorArray") + .def(py::init()) + .def_readwrite("arr", &NoDefaultCtorArray::arr); + // test_valarray m.def("cast_valarray", []() { return std::valarray{1, 4, 9}; }); m.def("load_valarray", [](const std::valarray &v) { diff --git a/tests/test_stl.py b/tests/test_stl.py index 1896c3221..6f548789e 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -48,6 +48,13 @@ def test_array(doc): ) +def test_array_no_default_ctor(): + lst = m.NoDefaultCtorArray(3) + assert [e.val for e in lst.arr] == [13, 23] + lst.arr = m.NoDefaultCtorArray(4).arr + assert [e.val for e in lst.arr] == [14, 24] + + def test_valarray(doc): """std::valarray <-> list""" lst = m.cast_valarray()