always-cautious/ortools-clone
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cmake: Fix math_opt's elemental python binding

Browse Source
This commit is contained in:
Corentin Le Molgat
2025-04-15 14:34:13 +02:00
parent 9970130ec3
commit ec75b12963
4 changed files with 476 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
cmake/dependencies/CMakeLists.txt
View File

@@ -203,9 +203,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)

3
patches/BUILD.bazel
View File

@@ -15,8 +15,9 @@ exports_files([
"abseil-cpp-20250127.1.patch",
"highs-v1.10.patch",
"protobuf-v30.2.patch",
"pybind11_bazel.patch",
"pybind11_abseil.patch",
"pybind11_protobuf.patch",
"pybind11.patch",
"pybind11-v2.13.6.patch",
"scip-v922.patch",
])

464
patches/pybind11-v2.13.6.patch Normal file
View File

@@ -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 <deque>
+#include <initializer_list>
#include <list>
#include <map>
+#include <memory>
#include <ostream>
#include <set>
#include <unordered_map>
@@ -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<const char *> 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 <typename T, typename U>
@@ -66,17 +153,10 @@ private:
}
void reserve_maybe(const anyset &, void *) {}
-public:
- bool load(handle src, bool convert) {
- if (!isinstance<anyset>(src)) {
- return false;
- }
- auto s = reinterpret_borrow<anyset>(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<Key &&>(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<anyset>(src)) {
+ value.clear();
+ return convert_anyset(reinterpret_borrow<anyset>(src), convert);
+ }
+ if (!convert) {
+ return false;
+ }
+ assert(isinstance<iterable>(src));
+ value.clear();
+ return convert_iterable(reinterpret_borrow<iterable>(src), convert);
+ }
+
template <typename T>
static handle cast(T &&src, return_value_policy policy, handle parent) {
if (!std::is_lvalue_reference<T>::value) {
@@ -115,15 +218,10 @@ private:
}
void reserve_maybe(const dict &, void *) {}
-public:
- bool load(handle src, bool convert) {
- if (!isinstance<dict>(src)) {
- return false;
- }
- auto d = reinterpret_borrow<dict>(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<dict>(src)) {
+ return convert_elements(reinterpret_borrow<dict>(src), convert);
+ }
+ if (!convert) {
+ return false;
+ }
+ auto items = reinterpret_steal<object>(PyMapping_Items(src.ptr()));
+ if (!items) {
+ throw error_already_set();
+ }
+ assert(isinstance<iterable>(items));
+ return convert_elements(dict(reinterpret_borrow<iterable>(items)), convert);
+ }
+
template <typename T>
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<Value>;
bool load(handle src, bool convert) {
- if (!isinstance<sequence>(src) || isinstance<bytes>(src) || isinstance<str>(src)) {
+ if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) {
return false;
}
- auto s = reinterpret_borrow<sequence>(src);
+ if (isinstance<sequence>(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<iterable>(src));
+ return convert_elements(tuple(reinterpret_borrow<iterable>(src)), convert);
+ }
+
+private:
+ template <typename T = Type, enable_if_t<has_reserve_method<T>::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<sequence>(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 <typename T = Type, enable_if_t<has_reserve_method<T>::value, int> = 0>
- void reserve_maybe(const sequence &s, Type *) {
- value.reserve(s.size());
- }
- void reserve_maybe(const sequence &, void *) {}
-
public:
template <typename T>
static handle cast(T &&src, return_value_policy policy, handle parent) {
@@ -220,43 +352,87 @@ 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;
- }
-public:
- bool load(handle src, bool convert) {
- if (!isinstance<sequence>(src)) {
+ template <bool R = Resizable, enable_if_t<!R, int> = 0>
+ bool convert_elements(handle seq, bool convert) {
+ auto l = reinterpret_borrow<sequence>(seq);
+ if (l.size() != Size) {
return false;
}
- auto l = reinterpret_borrow<sequence>(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<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;
}
+public:
+ bool load(handle src, bool convert) {
+ if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) {
+ return false;
+ }
+ if (isinstance<sequence>(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<iterable>(src));
+ return convert_elements(tuple(reinterpret_borrow<iterable>(src)), convert);
+ }
+
template <typename T>
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<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>
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<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) {
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()

12
patches/pybind11.patch
View File

@@ -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()