diff --git a/cmake/dependencies/CMakeLists.txt b/cmake/dependencies/CMakeLists.txt index a44bd2870d..642ced627a 100644 --- a/cmake/dependencies/CMakeLists.txt +++ b/cmake/dependencies/CMakeLists.txt @@ -181,9 +181,16 @@ if(BUILD_PYTHON) FetchContent_Declare( pybind11 GIT_REPOSITORY "https://github.com/pybind/pybind11.git" - GIT_TAG "v2.13.6" - GIT_SHALLOW TRUE - PATCH_COMMAND git apply --ignore-whitespace "${CMAKE_CURRENT_LIST_DIR}/../../patches/pybind11.patch" + #GIT_TAG "master" # COMPILE FAIL + #GIT_TAG "f365314" # 2025/03/24 COMPILE FAIL + #GIT_TAG "48eb5ad" # 2025/03/23 TEST PASS + # ... + #GIT_TAG "a1d00916" # 2024/08/15 TEST PASS + #GIT_TAG "bd5951b6" # 2024/08/14 TEST FAIL + GIT_TAG "v2.13.6" # TEST PASS with a1d00916 patch apply + #GIT_SHALLOW TRUE + PATCH_COMMAND git apply --ignore-whitespace + "${CMAKE_CURRENT_LIST_DIR}/../../patches/pybind11-v2.13.6.patch" ) FetchContent_MakeAvailable(pybind11) list(POP_BACK CMAKE_MESSAGE_INDENT) diff --git a/patches/BUILD.bazel b/patches/BUILD.bazel index 807d5cb550..089e93af72 100644 --- a/patches/BUILD.bazel +++ b/patches/BUILD.bazel @@ -15,8 +15,9 @@ exports_files([ "abseil-cpp-20250127.1.patch", "highs-v1.9.0.patch", "protobuf-v30.2.patch", + "pybind11_bazel.patch", "pybind11_abseil.patch", "pybind11_protobuf.patch", - "pybind11.patch", + "pybind11-v2.13.6.patch", "scip-v922.patch", ]) diff --git a/patches/pybind11-v2.13.6.patch b/patches/pybind11-v2.13.6.patch new file mode 100644 index 0000000000..3177753348 --- /dev/null +++ b/patches/pybind11-v2.13.6.patch @@ -0,0 +1,464 @@ +diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h +index 71bc5902..6a148e74 100644 +--- a/include/pybind11/stl.h ++++ b/include/pybind11/stl.h +@@ -11,10 +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 +@@ -35,6 +39,89 @@ + PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) + PYBIND11_NAMESPACE_BEGIN(detail) + ++// ++// Begin: Equivalent of ++// https://github.com/google/clif/blob/ae4eee1de07cdf115c0c9bf9fec9ff28efce6f6c/clif/python/runtime.cc#L388-L438 ++/* ++The three `PyObjectTypeIsConvertibleTo*()` functions below are ++the result of converging the behaviors of pybind11 and PyCLIF ++(http://github.com/google/clif). ++ ++Originally PyCLIF was extremely far on the permissive side of the spectrum, ++while pybind11 was very far on the strict side. Originally PyCLIF accepted any ++Python iterable as input for a C++ `vector`/`set`/`map` argument, as long as ++the elements were convertible. The obvious (in hindsight) problem was that ++any empty Python iterable could be passed to any of these C++ types, e.g. `{}` ++was accepted for C++ `vector`/`set` arguments, or `[]` for C++ `map` arguments. ++ ++The functions below strike a practical permissive-vs-strict compromise, ++informed by tens of thousands of use cases in the wild. A main objective is ++to prevent accidents and improve readability: ++ ++- Python literals must match the C++ types. ++ ++- For C++ `set`: The potentially reducing conversion from a Python sequence ++ (e.g. Python `list` or `tuple`) to a C++ `set` must be explicit, by going ++ through a Python `set`. ++ ++- However, a Python `set` can still be passed to a C++ `vector`. The rationale ++ is that this conversion is not reducing. Implicit conversions of this kind ++ are also fairly commonly used, therefore enforcing explicit conversions ++ would have an unfavorable cost : benefit ratio; more sloppily speaking, ++ such an enforcement would be more annoying than helpful. ++*/ ++ ++inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, ++ std::initializer_list tp_names) { ++ if (PyType_Check(obj)) { ++ return false; ++ } ++ const char *obj_tp_name = Py_TYPE(obj)->tp_name; ++ for (const auto *tp_name : tp_names) { ++ if (std::strcmp(obj_tp_name, tp_name) == 0) { ++ return true; ++ } ++ } ++ return false; ++} ++ ++inline bool PyObjectTypeIsConvertibleToStdVector(PyObject *obj) { ++ if (PySequence_Check(obj) != 0) { ++ return !PyUnicode_Check(obj) && !PyBytes_Check(obj); ++ } ++ return (PyGen_Check(obj) != 0) || (PyAnySet_Check(obj) != 0) ++ || PyObjectIsInstanceWithOneOfTpNames( ++ obj, {"dict_keys", "dict_values", "dict_items", "map", "zip"}); ++} ++ ++inline bool PyObjectTypeIsConvertibleToStdSet(PyObject *obj) { ++ return (PyAnySet_Check(obj) != 0) || PyObjectIsInstanceWithOneOfTpNames(obj, {"dict_keys"}); ++} ++ ++inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { ++ if (PyDict_Check(obj)) { ++ return true; ++ } ++ // Implicit requirement in the conditions below: ++ // A type with `.__getitem__()` & `.items()` methods must implement these ++ // to be compatible with https://docs.python.org/3/c-api/mapping.html ++ if (PyMapping_Check(obj) == 0) { ++ return false; ++ } ++ PyObject *items = PyObject_GetAttrString(obj, "items"); ++ if (items == nullptr) { ++ PyErr_Clear(); ++ return false; ++ } ++ bool is_convertible = (PyCallable_Check(items) != 0); ++ Py_DECREF(items); ++ return is_convertible; ++} ++ ++// ++// End: Equivalent of clif/python/runtime.cc ++// ++ + /// Extracts an const lvalue reference or rvalue reference for U based on the type of T (e.g. for + /// forwarding a container element). Typically used indirect via forwarded_type(), below. + template +@@ -66,17 +153,10 @@ private: + } + void reserve_maybe(const anyset &, void *) {} + +-public: +- bool load(handle src, bool convert) { +- if (!isinstance(src)) { +- return false; +- } +- auto s = reinterpret_borrow(src); +- value.clear(); +- reserve_maybe(s, &value); +- for (auto entry : s) { ++ bool convert_iterable(const iterable &itbl, bool convert) { ++ for (const auto &it : itbl) { + key_conv conv; +- if (!conv.load(entry, convert)) { ++ if (!conv.load(it, convert)) { + return false; + } + value.insert(cast_op(std::move(conv))); +@@ -84,6 +164,29 @@ public: + return true; + } + ++ bool convert_anyset(anyset s, bool convert) { ++ value.clear(); ++ reserve_maybe(s, &value); ++ return convert_iterable(s, convert); ++ } ++ ++public: ++ bool load(handle src, bool convert) { ++ if (!PyObjectTypeIsConvertibleToStdSet(src.ptr())) { ++ return false; ++ } ++ if (isinstance(src)) { ++ value.clear(); ++ return convert_anyset(reinterpret_borrow(src), convert); ++ } ++ if (!convert) { ++ return false; ++ } ++ assert(isinstance(src)); ++ value.clear(); ++ return convert_iterable(reinterpret_borrow(src), convert); ++ } ++ + template + static handle cast(T &&src, return_value_policy policy, handle parent) { + if (!std::is_lvalue_reference::value) { +@@ -115,15 +218,10 @@ private: + } + void reserve_maybe(const dict &, void *) {} + +-public: +- bool load(handle src, bool convert) { +- if (!isinstance(src)) { +- return false; +- } +- auto d = reinterpret_borrow(src); ++ bool convert_elements(const dict &d, bool convert) { + value.clear(); + reserve_maybe(d, &value); +- for (auto it : d) { ++ for (const auto &it : d) { + key_conv kconv; + value_conv vconv; + if (!kconv.load(it.first.ptr(), convert) || !vconv.load(it.second.ptr(), convert)) { +@@ -134,6 +232,25 @@ public: + return true; + } + ++public: ++ bool load(handle src, bool convert) { ++ if (!PyObjectTypeIsConvertibleToStdMap(src.ptr())) { ++ return false; ++ } ++ if (isinstance(src)) { ++ return convert_elements(reinterpret_borrow(src), convert); ++ } ++ if (!convert) { ++ return false; ++ } ++ auto items = reinterpret_steal(PyMapping_Items(src.ptr())); ++ if (!items) { ++ throw error_already_set(); ++ } ++ assert(isinstance(items)); ++ return convert_elements(dict(reinterpret_borrow(items)), convert); ++ } ++ + template + static handle cast(T &&src, return_value_policy policy, handle parent) { + dict d; +@@ -166,13 +283,35 @@ struct list_caster { + using value_conv = make_caster; + + bool load(handle src, bool convert) { +- if (!isinstance(src) || isinstance(src) || isinstance(src)) { ++ if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) { + return false; + } +- auto s = reinterpret_borrow(src); ++ if (isinstance(src)) { ++ return convert_elements(src, convert); ++ } ++ if (!convert) { ++ return false; ++ } ++ // Designed to be behavior-equivalent to passing tuple(src) from Python: ++ // The conversion to a tuple will first exhaust the generator object, to ensure that ++ // the generator is not left in an unpredictable (to the caller) partially-consumed ++ // state. ++ assert(isinstance(src)); ++ return convert_elements(tuple(reinterpret_borrow(src)), convert); ++ } ++ ++private: ++ template ::value, int> = 0> ++ void reserve_maybe(const sequence &s, Type *) { ++ value.reserve(s.size()); ++ } ++ void reserve_maybe(const sequence &, void *) {} ++ ++ bool convert_elements(handle seq, bool convert) { ++ auto s = reinterpret_borrow(seq); + value.clear(); + reserve_maybe(s, &value); +- for (const auto &it : s) { ++ for (const auto &it : seq) { + value_conv conv; + if (!conv.load(it, convert)) { + return false; +@@ -182,13 +321,6 @@ struct list_caster { + return true; + } + +-private: +- template ::value, int> = 0> +- void reserve_maybe(const sequence &s, Type *) { +- value.reserve(s.size()); +- } +- void reserve_maybe(const sequence &, void *) {} +- + public: + template + static handle cast(T &&src, return_value_policy policy, handle parent) { +@@ -220,43 +352,87 @@ 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); ++ std::unique_ptr value; ++ ++ template = 0> ++ bool convert_elements(handle seq, bool convert) { ++ auto l = reinterpret_borrow(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(std::move(conv)); + } + return true; + } +- template +- bool require_size(enable_if_t size) { +- return size == Size; +- } + +-public: +- bool load(handle src, bool convert) { +- if (!isinstance(src)) { ++ template = 0> ++ bool convert_elements(handle seq, bool convert) { ++ auto l = reinterpret_borrow(seq); ++ if (l.size() != Size) { + return false; + } +- auto l = reinterpret_borrow(src); +- if (!require_size(l.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 temp; ++ temp.reserve(l.size()); ++ for (auto it : l) { + value_conv conv; + if (!conv.load(it, convert)) { + return false; + } +- value[ctr++] = cast_op(std::move(conv)); ++ 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())) { ++ return false; ++ } ++ if (isinstance(src)) { ++ return convert_elements(src, convert); ++ } ++ if (!convert) { ++ return false; ++ } ++ // Designed to be behavior-equivalent to passing tuple(src) from Python: ++ // The conversion to a tuple will first exhaust the generator object, to ensure that ++ // the generator is not left in an unpredictable (to the caller) partially-consumed ++ // state. ++ assert(isinstance(src)); ++ return convert_elements(tuple(reinterpret_borrow(src)), convert); ++ } ++ + template + static handle cast(T &&src, return_value_policy policy, handle parent) { + list l(src.size()); +@@ -272,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 e7db8aaa..84bd4755 100644 +--- a/tests/test_stl.cpp ++++ b/tests/test_stl.cpp +@@ -193,6 +193,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 65fda54c..340cdc35 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() +diff --git a/tools/pybind11NewTools.cmake b/tools/pybind11NewTools.cmake +index a8b0800b..1bd6e9e1 100644 +--- a/tools/pybind11NewTools.cmake ++++ b/tools/pybind11NewTools.cmake +@@ -23,6 +23,7 @@ else() + endif() + + if(NOT Python_FOUND AND NOT Python3_FOUND) ++ message(FATAL_ERROR "Should not pass here") + if(NOT DEFINED Python_FIND_IMPLEMENTATIONS) + set(Python_FIND_IMPLEMENTATIONS CPython PyPy) + endif() diff --git a/patches/pybind11.patch b/patches/pybind11.patch deleted file mode 100644 index 579896e0c7..0000000000 --- a/patches/pybind11.patch +++ /dev/null @@ -1,12 +0,0 @@ -diff --git a/tools/pybind11NewTools.cmake b/tools/pybind11NewTools.cmake -index 7d7424a7..c007fb01 100644 ---- a/tools/pybind11NewTools.cmake -+++ b/tools/pybind11NewTools.cmake -@@ -23,6 +23,7 @@ else() - endif() - - if(NOT Python_FOUND AND NOT Python3_FOUND) -+ message(FATAL_ERROR "Should not pass here") - if(NOT DEFINED Python_FIND_IMPLEMENTATIONS) - set(Python_FIND_IMPLEMENTATIONS CPython PyPy) - endif()