Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: change array_caster in pybind11/stl.h to support value types that are not default-constructible. #5305

Merged
merged 1 commit into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 74 additions & 18 deletions include/pybind11/stl.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@

#include "pybind11.h"
#include "detail/common.h"
#include "detail/descr.h"
#include "detail/type_caster_base.h"

#include <deque>
#include <initializer_list>
#include <list>
#include <map>
#include <memory>
#include <ostream>
#include <set>
#include <unordered_map>
Expand Down Expand Up @@ -349,36 +352,65 @@ struct type_caster<std::deque<Type, Alloc>> : list_caster<std::deque<Type, Alloc
template <typename Type, typename Alloc>
struct type_caster<std::list<Type, Alloc>> : list_caster<std::list<Type, Alloc>, Type> {};

template <typename ArrayType, typename V, size_t... I>
ArrayType vector_to_array_impl(V &&v, index_sequence<I...>) {
return {{std::move(v[I])...}};
}

// Based on https://en.cppreference.com/w/cpp/container/array/to_array
template <typename ArrayType, size_t N, typename V>
ArrayType vector_to_array(V &&v) {
return vector_to_array_impl<ArrayType, V>(std::forward<V>(v), make_index_sequence<N>{});
}

template <typename ArrayType, typename Value, bool Resizable, size_t Size = 0>
struct array_caster {
using value_conv = make_caster<Value>;

private:
template <bool R = Resizable>
bool require_size(enable_if_t<R, size_t> size) {
if (value.size() != size) {
value.resize(size);
std::unique_ptr<ArrayType> value;

template <bool R = Resizable, enable_if_t<R, int> = 0>
bool convert_elements(handle seq, bool convert) {
auto l = reinterpret_borrow<sequence>(seq);
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<Value &&>(std::move(conv));
}
return true;
}
template <bool R = Resizable>
bool require_size(enable_if_t<!R, size_t> size) {
return size == Size;
}

template <bool R = Resizable, enable_if_t<!R, int> = 0>
bool convert_elements(handle seq, bool convert) {
auto l = reinterpret_borrow<sequence>(seq);
if (!require_size(l.size())) {
if (l.size() != Size) {
return false;
}
size_t ctr = 0;
for (const auto &it : l) {
// 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<Value> temp;
temp.reserve(l.size());
for (auto it : l) {
value_conv conv;
if (!conv.load(it, convert)) {
return false;
}
value[ctr++] = cast_op<Value &&>(std::move(conv));
temp.emplace_back(cast_op<Value &&>(std::move(conv)));
}
value.reset(new ArrayType(vector_to_array<ArrayType, Size>(std::move(temp))));
return true;
}

Expand Down Expand Up @@ -416,12 +448,36 @@ struct array_caster {
return l.release();
}

PYBIND11_TYPE_CASTER(ArrayType,
const_name<Resizable>(const_name(""), const_name("Annotated["))
+ const_name("list[") + value_conv::name + const_name("]")
+ const_name<Resizable>(const_name(""),
const_name(", FixedSize(")
+ const_name<Size>() + const_name(")]")));
// Code copied from PYBIND11_TYPE_CASTER macro.
// Intentionally preserving the behavior exactly as it was before PR #5305
template <typename T_, enable_if_t<std::is_same<ArrayType, remove_cv_t<T_>>::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 <typename T_>
using cast_op_type = movable_cast_op_type<T_>;

static constexpr auto name
= const_name<Resizable>(const_name(""), const_name("Annotated[")) + const_name("list[")
+ value_conv::name + const_name("]")
+ const_name<Resizable>(
const_name(""), const_name(", FixedSize(") + const_name<Size>() + const_name(")]"));
};

template <typename Type, size_t Size>
Expand Down
17 changes: 17 additions & 0 deletions tests/test_stl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,23 @@ TEST_SUBMODULE(stl, m) {
m.def("cast_array", []() { return std::array<int, 2>{{1, 2}}; });
m.def("load_array", [](const std::array<int, 2> &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<NoDefaultCtor, 2> arr;
};

// test_array_no_default_ctor
py::class_<NoDefaultCtor>(m, "NoDefaultCtor").def_readonly("val", &NoDefaultCtor::val);
py::class_<NoDefaultCtorArray>(m, "NoDefaultCtorArray")
.def(py::init<int>())
.def_readwrite("arr", &NoDefaultCtorArray::arr);

// test_valarray
m.def("cast_valarray", []() { return std::valarray<int>{1, 4, 9}; });
m.def("load_valarray", [](const std::valarray<int> &v) {
Expand Down
7 changes: 7 additions & 0 deletions tests/test_stl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down