From 313a41e2f18bd5431f2a9d00aafb23979e4156f3 Mon Sep 17 00:00:00 2001 From: Corentin Le Molgat Date: Tue, 22 Apr 2025 14:43:34 +0200 Subject: [PATCH] bazel: Fix math_opt's elemental python binding * use a patched pybind11_bazel archive applying the pybind11-v2.13.6.patch --- MODULE.bazel | 8 + patches/pybind11_bazel.patch | 498 +++++++++++++++++++++++++++++++++++ 2 files changed, 506 insertions(+) create mode 100644 patches/pybind11_bazel.patch diff --git a/MODULE.bazel b/MODULE.bazel index 9d8f50d7b5..1ea264ede4 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -42,6 +42,14 @@ bazel_dep(name = "scip", version = "9.2.2") bazel_dep(name = "swig", version = "4.3.0") bazel_dep(name = "zlib", version = "1.3.1.bcr.5") +git_override( + module_name = "pybind11_bazel", + commit = "2b6082a4d9d163a52299718113fa41e4b7978db5", + patch_strip = 1, + patches = ["//patches:pybind11_bazel.patch"], + remote = "https://github.com/pybind/pybind11_bazel.git", +) + git_override( module_name = "pybind11_abseil", commit = "70f8b693b3b70573ca785ef62d9f48054f45d786", diff --git a/patches/pybind11_bazel.patch b/patches/pybind11_bazel.patch new file mode 100644 index 0000000000..01bd534acd --- /dev/null +++ b/patches/pybind11_bazel.patch @@ -0,0 +1,498 @@ +diff --git a/MODULE.bazel b/MODULE.bazel +index cfb0974..a95bf40 100644 +--- a/MODULE.bazel ++++ b/MODULE.bazel +@@ -6,8 +6,8 @@ module( + + bazel_dep(name = "bazel_skylib", version = "1.7.1") + bazel_dep(name = "platforms", version = "0.0.10") +-bazel_dep(name = "rules_cc", version = "0.0.9") +-bazel_dep(name = "rules_python", version = "0.34.0") ++bazel_dep(name = "rules_cc", version = "0.1.1") ++bazel_dep(name = "rules_python", version = "0.40.0") + + internal_configure = use_extension("//:internal_configure.bzl", "internal_configure_extension") + use_repo(internal_configure, "pybind11") +diff --git a/internal_configure.bzl b/internal_configure.bzl +index 7b6c0e9..2616117 100644 +--- a/internal_configure.bzl ++++ b/internal_configure.bzl +@@ -24,6 +24,8 @@ def _internal_configure_extension_impl(module_ctx): + strip_prefix = "pybind11-%s" % version, + url = "https://github.com/pybind/pybind11/archive/refs/tags/v%s.tar.gz" % version, + integrity = _INTEGRITIES.get(version), ++ patches = ["pybind11-v%s.patch" % version], ++ patch_args = ["-p1"], + ) + + internal_configure_extension = module_extension(implementation = _internal_configure_extension_impl) +diff --git a/pybind11-v2.13.6.patch b/pybind11-v2.13.6.patch +new file mode 100644 +index 0000000..3177753 +--- /dev/null ++++ b/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()