Skip to content

Commit 015834b

Browse files
authored
Change array_caster in pybind11/stl.h to support value types that are not default-constructible. (#30034)
* Reproducer for `error: call to implicitly-deleted default constructor` triggered by stl.h `array_caster`: ``` clang++ -o pybind11/tests/test_stl_no_default_ctor.os -c -std=c++17 -fPIC -fvisibility=hidden -O0 -g -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated -Wundef -Wnon-virtual-dtor -Wunused-result -Werror -isystem /usr/include/python3.10 -isystem /usr/include/eigen3 -DPYBIND11_STRICT_ASSERTS_CLASS_HOLDER_VS_TYPE_CASTER_MIX -DPYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE -DPYBIND11_TEST_BOOST -Ipybind11/include -I/usr/local/google/home/rwgk/forked/pybind11/include -I/usr/local/google/home/rwgk/clone/pybind11/include /usr/local/google/home/rwgk/forked/pybind11/tests/test_stl_no_default_ctor.cpp In file included from /usr/local/google/home/rwgk/forked/pybind11/tests/test_stl_no_default_ctor.cpp:1: In file included from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/stl.h:12: In file included from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/pybind11.h:13: In file included from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/class.h:12: In file included from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/attr.h:14: In file included from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/cast.h:18: /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/type_caster_odr_guard.h:111:5: error: call to implicitly-deleted default constructor of 'pybind11::detail::type_caster<std::array<pybind11_tests::stl_no_default_ctor::Node, 2>>' type_caster_odr_guard() { ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/tuple:190:9: note: in instantiation of member function 'pybind11::detail::type_caster_odr_guard<std::array<pybind11_tests::stl_no_default_ctor::Node, 2>, pybind11::detail::type_caster<std::array<pybind11_tests::stl_no_default_ctor::Node, 2>>>::type_caster_odr_guard' requested here : _M_head_impl() { } ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/pybind11.h:104:9: note: in instantiation of function template specialization 'pybind11::cpp_function::initialize<(lambda at /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/pybind11.h:1583:29), void, pybind11_tests::stl_no_default_ctor::NodeArray &, const std::array<pybind11_tests::stl_no_default_ctor::Node, 2> &, pybind11::is_method>' requested here initialize( ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/pybind11.h:1583:16: note: in instantiation of function template specialization 'pybind11::cpp_function::cpp_function<(lambda at /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/pybind11.h:1583:29), pybind11::is_method, void>' requested here return cpp_function([pm](T &c, const D &value) { c.*pm = value; }, is_method(hdl)); ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/pybind11.h:1926:54: note: in instantiation of function template specialization 'pybind11::property_cpp_function<pybind11_tests::stl_no_default_ctor::NodeArray, std::array<pybind11_tests::stl_no_default_ctor::Node, 2>>::write<std::array<pybind11_tests::stl_no_default_ctor::Node, 2> pybind11_tests::stl_no_default_ctor::NodeArray::*, 0>' requested here property_cpp_function<type, D>::write(pm, *this), ^ /usr/local/google/home/rwgk/forked/pybind11/tests/test_stl_no_default_ctor.cpp:32:10: note: in instantiation of function template specialization 'pybind11::class_<pybind11_tests::stl_no_default_ctor::NodeArray>::def_readwrite<pybind11_tests::stl_no_default_ctor::NodeArray, std::array<pybind11_tests::stl_no_default_ctor::Node, 2>>' requested here .def_readwrite("arr", &NodeArray::arr); ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/stl.h:288:7: note: default constructor of 'type_caster<std::array<pybind11_tests::stl_no_default_ctor::Node, 2>>' is implicitly deleted because base class 'array_caster<std::array<Node, 2UL>, pybind11_tests::stl_no_default_ctor::Node, false, 2UL>' has a deleted default constructor : array_caster<std::array<Type, Size>, Type, false, Size> {}; ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/stl.h:278:5: note: default constructor of 'array_caster<std::array<pybind11_tests::stl_no_default_ctor::Node, 2>, pybind11_tests::stl_no_default_ctor::Node, false, 2>' is implicitly deleted because field 'value' has a deleted default constructor PYBIND11_TYPE_CASTER_RVPP(ArrayType, ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/cast.h:262:5: note: expanded from macro 'PYBIND11_TYPE_CASTER_RVPP' PYBIND11_TYPE_CASTER_IMPL(type, py_name, ::pybind11::return_value_policy_pack) ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/cast.h:232:10: note: expanded from macro 'PYBIND11_TYPE_CASTER_IMPL' type value; \ ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/array:115:56: note: default constructor of 'array<pybind11_tests::stl_no_default_ctor::Node, 2>' is implicitly deleted because field '_M_elems' has no default constructor typename _AT_Type::_Type _M_elems; ^ 1 error generated. ``` * Expand `PYBIND11_TYPE_CASTER_RVPP` macro. * Resolve clang-tidy and clang <= 5 errors. * Second try: Resolve clang-tidy and clang <= 5 errors. * First working version of `array_caster` that works for types without a default constructor. * Try `()` instead of `{}` to see if that resolves clang 3.6 and gcc 4.8.5 compilation errors. * Add minor comment. * Avoid `temp` storage if `Resizable` is `true`. * Merge test_stl_no_default_ctor into test_stl * Minor move of code blocks (no functional change). * Remove unused default value. * IncludeCleaner (IWYU) fixes * Add two comments: preserving existing behavior
1 parent 24a3b1c commit 015834b

File tree

3 files changed

+98
-19
lines changed

3 files changed

+98
-19
lines changed

include/pybind11/stl.h

Lines changed: 74 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,14 @@
1111

1212
#include "pybind11.h"
1313
#include "detail/common.h"
14+
#include "detail/descr.h"
15+
#include "detail/type_caster_base.h"
1416

1517
#include <deque>
1618
#include <initializer_list>
1719
#include <list>
1820
#include <map>
21+
#include <memory>
1922
#include <ostream>
2023
#include <set>
2124
#include <unordered_map>
@@ -352,36 +355,65 @@ struct type_caster<std::deque<Type, Alloc>> : list_caster<std::deque<Type, Alloc
352355
template <typename Type, typename Alloc>
353356
struct type_caster<std::list<Type, Alloc>> : list_caster<std::list<Type, Alloc>, Type> {};
354357

358+
template <typename ArrayType, typename V, size_t... I>
359+
ArrayType vector_to_array_impl(V &&v, index_sequence<I...>) {
360+
return {{std::move(v[I])...}};
361+
}
362+
363+
// Based on https://en.cppreference.com/w/cpp/container/array/to_array
364+
template <typename ArrayType, size_t N, typename V>
365+
ArrayType vector_to_array(V &&v) {
366+
return vector_to_array_impl<ArrayType, V>(std::forward<V>(v), make_index_sequence<N>{});
367+
}
368+
355369
template <typename ArrayType, typename Value, bool Resizable, size_t Size = 0>
356370
struct array_caster {
357371
using value_conv = make_caster<Value>;
358372

359373
private:
360-
template <bool R = Resizable>
361-
bool require_size(enable_if_t<R, size_t> size) {
362-
if (value.size() != size) {
363-
value.resize(size);
374+
std::unique_ptr<ArrayType> value;
375+
376+
template <bool R = Resizable, enable_if_t<R, int> = 0>
377+
bool convert_elements(handle seq, bool convert) {
378+
auto l = reinterpret_borrow<sequence>(seq);
379+
value.reset(new ArrayType{});
380+
// Using `resize` to preserve the behavior exactly as it was before google/pywrapcc#30034
381+
// For the `resize` to work, `Value` must be default constructible.
382+
// For `std::valarray`, this is a requirement:
383+
// https://en.cppreference.com/w/cpp/named_req/NumericType
384+
value->resize(l.size());
385+
size_t ctr = 0;
386+
for (const auto &it : l) {
387+
value_conv conv;
388+
if (!conv.load(it, convert)) {
389+
return false;
390+
}
391+
(*value)[ctr++] = cast_op<Value &&>(std::move(conv));
364392
}
365393
return true;
366394
}
367-
template <bool R = Resizable>
368-
bool require_size(enable_if_t<!R, size_t> size) {
369-
return size == Size;
370-
}
371395

396+
template <bool R = Resizable, enable_if_t<!R, int> = 0>
372397
bool convert_elements(handle seq, bool convert) {
373398
auto l = reinterpret_borrow<sequence>(seq);
374-
if (!require_size(l.size())) {
399+
if (l.size() != Size) {
375400
return false;
376401
}
377-
size_t ctr = 0;
378-
for (const auto &it : l) {
402+
// The `temp` storage is needed to support `Value` types that are not
403+
// default-constructible.
404+
// Deliberate choice: no template specializations, for simplicity, and
405+
// because the compile time overhead for the specializations is deemed
406+
// more significant than the runtime overhead for the `temp` storage.
407+
std::vector<Value> temp;
408+
temp.reserve(l.size());
409+
for (auto it : l) {
379410
value_conv conv;
380411
if (!conv.load(it, convert)) {
381412
return false;
382413
}
383-
value[ctr++] = cast_op<Value &&>(std::move(conv));
414+
temp.emplace_back(cast_op<Value &&>(std::move(conv)));
384415
}
416+
value.reset(new ArrayType(vector_to_array<ArrayType, Size>(std::move(temp))));
385417
return true;
386418
}
387419

@@ -420,13 +452,36 @@ struct array_caster {
420452
return l.release();
421453
}
422454

423-
PYBIND11_TYPE_CASTER_RVPP(ArrayType,
424-
const_name<Resizable>(const_name(""), const_name("Annotated["))
425-
+ const_name("list[") + value_conv::name + const_name("]")
426-
+ const_name<Resizable>(const_name(""),
427-
const_name(", FixedSize(")
428-
+ const_name<Size>()
429-
+ const_name(")]")));
455+
// Code copied from PYBIND11_TYPE_CASTER macro.
456+
// Intentionally preserving the behavior exactly as it was before google/pywrapcc#30034
457+
template <typename T_, enable_if_t<std::is_same<ArrayType, remove_cv_t<T_>>::value, int> = 0>
458+
static handle cast(T_ *src, const return_value_policy_pack &policy, handle parent) {
459+
if (!src) {
460+
return none().release();
461+
}
462+
if (policy == return_value_policy::take_ownership) {
463+
auto h = cast(std::move(*src), policy, parent);
464+
delete src; // WARNING: Assumes `src` was allocated with `new`.
465+
return h;
466+
}
467+
return cast(*src, policy, parent);
468+
}
469+
470+
// NOLINTNEXTLINE(google-explicit-constructor)
471+
operator ArrayType *() { return &(*value); }
472+
// NOLINTNEXTLINE(google-explicit-constructor)
473+
operator ArrayType &() { return *value; }
474+
// NOLINTNEXTLINE(google-explicit-constructor)
475+
operator ArrayType &&() && { return std::move(*value); }
476+
477+
template <typename T_>
478+
using cast_op_type = movable_cast_op_type<T_>;
479+
480+
static constexpr auto name
481+
= const_name<Resizable>(const_name(""), const_name("Annotated[")) + const_name("list[")
482+
+ value_conv::name + const_name("]")
483+
+ const_name<Resizable>(
484+
const_name(""), const_name(", FixedSize(") + const_name<Size>() + const_name(")]"));
430485
};
431486

432487
template <typename Type, size_t Size>

tests/test_stl.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,23 @@ TEST_SUBMODULE(stl, m) {
201201
m.def("cast_array", []() { return std::array<int, 2>{{1, 2}}; });
202202
m.def("load_array", [](const std::array<int, 2> &a) { return a[0] == 1 && a[1] == 2; });
203203

204+
struct NoDefaultCtor {
205+
explicit constexpr NoDefaultCtor(int val) : val{val} {}
206+
int val;
207+
};
208+
209+
struct NoDefaultCtorArray {
210+
explicit constexpr NoDefaultCtorArray(int i)
211+
: arr{{NoDefaultCtor(10 + i), NoDefaultCtor(20 + i)}} {}
212+
std::array<NoDefaultCtor, 2> arr;
213+
};
214+
215+
// test_array_no_default_ctor
216+
py::class_<NoDefaultCtor>(m, "NoDefaultCtor").def_readonly("val", &NoDefaultCtor::val);
217+
py::class_<NoDefaultCtorArray>(m, "NoDefaultCtorArray")
218+
.def(py::init<int>())
219+
.def_readwrite("arr", &NoDefaultCtorArray::arr);
220+
204221
// test_valarray
205222
m.def("cast_valarray", []() { return std::valarray<int>{1, 4, 9}; });
206223
m.def("load_valarray", [](const std::valarray<int> &v) {

tests/test_stl.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,13 @@ def test_array(doc):
4646
)
4747

4848

49+
def test_array_no_default_ctor():
50+
lst = m.NoDefaultCtorArray(3)
51+
assert [e.val for e in lst.arr] == [13, 23]
52+
lst.arr = m.NoDefaultCtorArray(4).arr
53+
assert [e.val for e in lst.arr] == [14, 24]
54+
55+
4956
def test_valarray(doc):
5057
"""std::valarray <-> list"""
5158
lst = m.cast_valarray()

0 commit comments

Comments
 (0)