From fc59f4e6e508f28c0e2896937660b9eae0f5ea51 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 12 Aug 2024 16:51:48 -0400 Subject: [PATCH 1/7] fix(cmake): add required emscripten flags (#5298) * fix(cmake): add required emscripten flags Signed-off-by: Henry Schreiner * Update emscripten.yaml * fix(cmake): add required emscripten flags to headers target Signed-off-by: Henry Schreiner * fix(cmake): incorrect detection of Emscripten Signed-off-by: Henry Schreiner * fix(cmake): allow pybind11::headers to be modified Signed-off-by: Henry Schreiner * fix(cmake): hide a warning when building the tests standalone Signed-off-by: Henry Schreiner * fix(cmake): use explicit variable for is config Signed-off-by: Henry Schreiner * fix(cmake): go back to ALIAS target Signed-off-by: Henry Schreiner * chore: reduce overall diff Signed-off-by: Henry Schreiner * chore: reduce overall diff Signed-off-by: Henry Schreiner * chore: shorten code a bit Signed-off-by: Henry Schreiner --------- Signed-off-by: Henry Schreiner --- .github/workflows/emscripten.yaml | 4 ++-- tests/pyproject.toml | 4 ++++ tools/pybind11Common.cmake | 30 ++++++++++++++++++++++++++++-- tools/pybind11Config.cmake.in | 2 +- 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/.github/workflows/emscripten.yaml b/.github/workflows/emscripten.yaml index cbd7f5d5..14b2b9dc 100644 --- a/.github/workflows/emscripten.yaml +++ b/.github/workflows/emscripten.yaml @@ -5,6 +5,8 @@ on: pull_request: branches: - master + - stable + - v* concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -23,8 +25,6 @@ jobs: - uses: pypa/cibuildwheel@v2.20 env: PYODIDE_BUILD_EXPORTS: whole_archive - CFLAGS: -fexceptions - LDFLAGS: -fexceptions with: package-dir: tests only: cp312-pyodide_wasm32 diff --git a/tests/pyproject.toml b/tests/pyproject.toml index 469c145d..044bf15c 100644 --- a/tests/pyproject.toml +++ b/tests/pyproject.toml @@ -10,6 +10,10 @@ name = "pybind11_tests" version = "0.0.1" dependencies = ["pytest", "pytest-timeout", "numpy", "scipy"] +[tool.scikit-build] +# Hide a warning while we also support CMake < 3.15 +cmake.version = ">=3.15" + [tool.scikit-build.cmake.define] PYBIND11_FINDPYTHON = true diff --git a/tools/pybind11Common.cmake b/tools/pybind11Common.cmake index 8467b45d..585ed3d2 100644 --- a/tools/pybind11Common.cmake +++ b/tools/pybind11Common.cmake @@ -2,7 +2,7 @@ Adds the following targets:: - pybind11::pybind11 - link to headers and pybind11 + pybind11::pybind11 - link to Python headers and pybind11::headers pybind11::module - Adds module links pybind11::embed - Adds embed links pybind11::lto - Link time optimizations (only if CMAKE_INTERPROCEDURAL_OPTIMIZATION is not set) @@ -75,6 +75,32 @@ set_property( APPEND PROPERTY INTERFACE_LINK_LIBRARIES pybind11::pybind11) +# -------------- emscripten requires exceptions enabled ------------- +# _pybind11_no_exceptions is a private mechanism to disable this addition. +# Please open an issue if you need to use it; it will be removed if no one +# needs it. +if(CMAKE_SYSTEM_NAME MATCHES Emscripten AND NOT _pybind11_no_exceptions) + if(CMAKE_VERSION VERSION_LESS 3.13) + message(WARNING "CMake 3.13+ is required to build for Emscripten. Some flags will be missing") + else() + if(_is_config) + set(_tmp_config_target pybind11::pybind11_headers) + else() + set(_tmp_config_target pybind11_headers) + endif() + + set_property( + TARGET ${_tmp_config_target} + APPEND + PROPERTY INTERFACE_LINK_OPTIONS -fexceptions) + set_property( + TARGET ${_tmp_config_target} + APPEND + PROPERTY INTERFACE_COMPILE_OPTIONS -fexceptions) + unset(_tmp_config_target) + endif() +endif() + # --------------------------- link helper --------------------------- add_library(pybind11::python_link_helper IMPORTED INTERFACE ${optional_global}) @@ -329,7 +355,7 @@ function(_pybind11_generate_lto target prefer_thin_lto) if(CMAKE_SYSTEM_PROCESSOR MATCHES "ppc64le" OR CMAKE_SYSTEM_PROCESSOR MATCHES "mips64") # Do nothing - elseif(CMAKE_SYSTEM_PROCESSOR MATCHES emscripten) + elseif(CMAKE_SYSTEM_NAME MATCHES Emscripten) # This compile is very costly when cross-compiling, so set this without checking set(PYBIND11_LTO_CXX_FLAGS "-flto${thin}${cxx_append}") set(PYBIND11_LTO_LINKER_FLAGS "-flto${thin}${linker_append}") diff --git a/tools/pybind11Config.cmake.in b/tools/pybind11Config.cmake.in index 304f1d90..2d9fa94f 100644 --- a/tools/pybind11Config.cmake.in +++ b/tools/pybind11Config.cmake.in @@ -84,7 +84,7 @@ you can either use the basic targets, or use the FindPython tools: # Python method: Python_add_library(MyModule2 src2.cpp) - target_link_libraries(MyModule2 pybind11::headers) + target_link_libraries(MyModule2 PUBLIC pybind11::headers) set_target_properties(MyModule2 PROPERTIES INTERPROCEDURAL_OPTIMIZATION ON CXX_VISIBILITY_PRESET ON From 8d90b83b19de61009779edaa363679cfb9babbb5 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 12 Aug 2024 19:49:56 -0400 Subject: [PATCH 2/7] chore(deps): bump actions/attest-build-provenance in the actions group (#5297) Bumps the actions group with 1 update: [actions/attest-build-provenance](https://github.com/actions/attest-build-provenance). Updates `actions/attest-build-provenance` from 1.4.0 to 1.4.1 - [Release notes](https://github.com/actions/attest-build-provenance/releases) - [Changelog](https://github.com/actions/attest-build-provenance/blob/main/RELEASE.md) - [Commits](https://github.com/actions/attest-build-provenance/compare/210c1913531870065f03ce1f9440dd87bc0938cd...310b0a4a3b0b78ef57ecda988ee04b132db73ef8) --- updated-dependencies: - dependency-name: actions/attest-build-provenance dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/pip.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pip.yml b/.github/workflows/pip.yml index 75074ff7..3edfa612 100644 --- a/.github/workflows/pip.yml +++ b/.github/workflows/pip.yml @@ -102,7 +102,7 @@ jobs: - uses: actions/download-artifact@v4 - name: Generate artifact attestation for sdist and wheel - uses: actions/attest-build-provenance@210c1913531870065f03ce1f9440dd87bc0938cd # v1.4.0 + uses: actions/attest-build-provenance@310b0a4a3b0b78ef57ecda988ee04b132db73ef8 # v1.4.1 with: subject-path: "*/pybind11*" From 40f2c7863b68f7d7568910c316291f4f0c51222b Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 13 Aug 2024 09:04:03 -0400 Subject: [PATCH 3/7] docs: prepare for 2.13.2 (#5299) * docs: prepare for 2.13.2 Signed-off-by: Henry Schreiner * Update changelog.rst * Update changelog.rst --------- Signed-off-by: Henry Schreiner --- docs/changelog.rst | 58 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ab6c713c..22b12e40 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,6 +15,64 @@ IN DEVELOPMENT Changes will be summarized here periodically. +New Features: + +* Support for Python 3.7 was removed. (Official end-of-life: 2023-06-27). + `#5191 `_ + +Support for CMake older than 3.15 and some older compilers will also be removed. + +Version 2.13.2 (August 13, 2024) +-------------------------------- + +New Features: + +* A ``pybind11::detail::type_caster_std_function_specializations`` feature was added, to support specializations for + ``std::function``'s with return types that require custom to-Python conversion behavior (to primary use case is to catch and + convert exceptions). + `#4597 `_ + + +Changes: + + +* Use ``PyMutex`` instead of ``std::mutex`` for internal locking in the free-threaded build. + `#5219 `_ + +* Add a special type annotation for C++ empty tuple. + `#5214 `_ + +* When compiling for WebAssembly, add the required exception flags (CMake 3.13+). + `#5298 `_ + +Bug fixes: + +* Make ``gil_safe_call_once_and_store`` thread-safe in free-threaded CPython. + `#5246 `_ + +* A missing ``#include `` in pybind11/typing.h was added to fix build errors (in case user code does not already depend + on that include). + `#5208 `_ + +* Fix regression introduced in #5201 for GCC<10.3 in C++20 mode. + `#5205 `_ + + +.. fix(cmake) + +* Remove extra = when assigning flto value in the case for Clang in CMake. + `#5207 `_ + + +Tests: + +* Adding WASM testing to our CI (Pyodide / Emscripten via scikit-build-core). + `#4745 `_ + +* clang-tidy (in GitHub Actions) was updated from clang 15 to clang 18. + `#5272 `_ + + Version 2.13.1 (June 26, 2024) ------------------------------ From 1fe92c7b35f9b80c7e952bb1a91841c4f362a3b1 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 13 Aug 2024 12:32:32 -0400 Subject: [PATCH 4/7] fix: emscripten cmake issue (#5301) Signed-off-by: Henry Schreiner --- tools/pybind11Common.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/pybind11Common.cmake b/tools/pybind11Common.cmake index 585ed3d2..7d8d94b1 100644 --- a/tools/pybind11Common.cmake +++ b/tools/pybind11Common.cmake @@ -83,7 +83,7 @@ if(CMAKE_SYSTEM_NAME MATCHES Emscripten AND NOT _pybind11_no_exceptions) if(CMAKE_VERSION VERSION_LESS 3.13) message(WARNING "CMake 3.13+ is required to build for Emscripten. Some flags will be missing") else() - if(_is_config) + if(is_config) set(_tmp_config_target pybind11::pybind11_headers) else() set(_tmp_config_target pybind11_headers) From 8d9f4d50cc7baa461365e977a0645c6c0d7b1b12 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 13 Aug 2024 13:02:15 -0400 Subject: [PATCH 5/7] fix: quote paths from pybind11-config (#5302) Signed-off-by: Henry Schreiner --- pybind11/__main__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pybind11/__main__.py b/pybind11/__main__.py index b656ce6f..dadf24f3 100644 --- a/pybind11/__main__.py +++ b/pybind11/__main__.py @@ -2,6 +2,7 @@ from __future__ import annotations import argparse +import shlex import sys import sysconfig @@ -22,7 +23,7 @@ def print_includes() -> None: if d and d not in unique_dirs: unique_dirs.append(d) - print(" ".join("-I" + d for d in unique_dirs)) + print(" ".join(shlex.quote(f"-I{d}") for d in unique_dirs)) def main() -> None: @@ -54,9 +55,9 @@ def main() -> None: if args.includes: print_includes() if args.cmakedir: - print(get_cmake_dir()) + print(shlex.quote(get_cmake_dir())) if args.pkgconfigdir: - print(get_pkgconfig_dir()) + print(shlex.quote(get_pkgconfig_dir())) if __name__ == "__main__": From 4a06eca59173cdc8c9bcb28a186e2c512cfc9f97 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 13 Aug 2024 13:25:23 -0400 Subject: [PATCH 6/7] docs: prepare for 2.13.3 Signed-off-by: Henry Schreiner --- docs/changelog.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 22b12e40..d19a9012 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -22,6 +22,19 @@ New Features: Support for CMake older than 3.15 and some older compilers will also be removed. +Version 2.13.3 (August 13, 2024) +-------------------------------- + +Bug fixes: + +* Quote paths from pybind11-config + `#5302 `_ + + +* Fix typo in Emscripten support when in config mode (CMake) + `#5301 `_ + + Version 2.13.2 (August 13, 2024) -------------------------------- From 0d44d720cb1b911de2d96849860d0c5beb368f95 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 14 Aug 2024 01:42:51 +0700 Subject: [PATCH 7/7] Make stl.h `list|set|map_caster` more user friendly. (#4686) * Add `test_pass_std_vector_int()`, `test_pass_std_set_int()` in test_stl * Change `list_caster` to also accept generator objects (`PyGen_Check(src.ptr()`). Note for completeness: This is a more conservative change than https://github.com/google/pywrapcc/pull/30042 * Drop in (currently unpublished) PyCLIF code, use in `list_caster`, adjust tests. * Use `PyObjectTypeIsConvertibleToStdSet()` in `set_caster`, adjust tests. * Use `PyObjectTypeIsConvertibleToStdMap()` in `map_caster`, add tests. * Simplify `list_caster` `load()` implementation, push str/bytes check into `PyObjectTypeIsConvertibleToStdVector()`. * clang-tidy cleanup with a few extra `(... != 0)` to be more consistent. * Also use `PyObjectTypeIsConvertibleToStdVector()` in `array_caster`. * Update comment pointing to clif/python/runtime.cc (code is unchanged). * Comprehensive test coverage, enhanced set_caster load implementation. * Resolve clang-tidy eror. * Add a long C++ comment explaining what led to the `PyObjectTypeIsConvertibleTo*()` implementations. * Minor function name change in test. * strcmp -> std::strcmp (thanks @Skylion007 for catching this) * Add `PyCallable_Check(items)` in `PyObjectTypeIsConvertibleToStdMap()` * Resolve clang-tidy error * Use `PyMapping_Items()` instead of `src.attr("items")()`, to be internally consistent with `PyMapping_Check()` * Update link to PyCLIF sources. * Fix typo (thanks @wangxf123456 for catching this) * Add `test_pass_std_vector_int()`, `test_pass_std_set_int()` in test_stl * Change `list_caster` to also accept generator objects (`PyGen_Check(src.ptr()`). Note for completeness: This is a more conservative change than https://github.com/google/pywrapcc/pull/30042 * Drop in (currently unpublished) PyCLIF code, use in `list_caster`, adjust tests. * Use `PyObjectTypeIsConvertibleToStdSet()` in `set_caster`, adjust tests. * Use `PyObjectTypeIsConvertibleToStdMap()` in `map_caster`, add tests. * Simplify `list_caster` `load()` implementation, push str/bytes check into `PyObjectTypeIsConvertibleToStdVector()`. * clang-tidy cleanup with a few extra `(... != 0)` to be more consistent. * Also use `PyObjectTypeIsConvertibleToStdVector()` in `array_caster`. * Update comment pointing to clif/python/runtime.cc (code is unchanged). * Comprehensive test coverage, enhanced set_caster load implementation. * Resolve clang-tidy eror. * Add a long C++ comment explaining what led to the `PyObjectTypeIsConvertibleTo*()` implementations. * Minor function name change in test. * strcmp -> std::strcmp (thanks @Skylion007 for catching this) * Add `PyCallable_Check(items)` in `PyObjectTypeIsConvertibleToStdMap()` * Resolve clang-tidy error * Use `PyMapping_Items()` instead of `src.attr("items")()`, to be internally consistent with `PyMapping_Check()` * Update link to PyCLIF sources. * Fix typo (thanks @wangxf123456 for catching this) * Fix typo discovered by new version of codespell. --- include/pybind11/stl.h | 210 ++++++++++++++++++++++++++++++++++------- tests/test_stl.cpp | 34 +++++++ tests/test_stl.py | 126 +++++++++++++++++++++++++ 3 files changed, 337 insertions(+), 33 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 71bc5902..096301bc 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -13,6 +13,7 @@ #include "detail/common.h" #include +#include #include #include #include @@ -35,6 +36,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 +150,10 @@ struct set_caster { } 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 +161,29 @@ struct set_caster { 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 +215,10 @@ struct map_caster { } 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 +229,25 @@ struct map_caster { 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 +280,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; + } + if (isinstance(src)) { + return convert_elements(src, convert); + } + if (!convert) { return false; } - auto s = reinterpret_borrow(src); + // 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 +318,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) { @@ -237,12 +366,8 @@ struct array_caster { return size == Size; } -public: - bool load(handle src, bool convert) { - if (!isinstance(src)) { - return false; - } - auto l = reinterpret_borrow(src); + bool convert_elements(handle seq, bool convert) { + auto l = reinterpret_borrow(seq); if (!require_size(l.size())) { return false; } @@ -257,6 +382,25 @@ struct array_caster { 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()); diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 48c907ff..c35f0d4c 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -167,6 +167,14 @@ struct type_caster> } // namespace detail } // namespace PYBIND11_NAMESPACE +int pass_std_vector_int(const std::vector &v) { + int zum = 100; + for (const int i : v) { + zum += 2 * i; + } + return zum; +} + TEST_SUBMODULE(stl, m) { // test_vector m.def("cast_vector", []() { return std::vector{1}; }); @@ -546,4 +554,30 @@ TEST_SUBMODULE(stl, m) { []() { return new std::vector(4513); }, // Without explicitly specifying `take_ownership`, this function leaks. py::return_value_policy::take_ownership); + + m.def("pass_std_vector_int", pass_std_vector_int); + m.def("pass_std_vector_pair_int", [](const std::vector> &v) { + int zum = 0; + for (const auto &ij : v) { + zum += ij.first * 100 + ij.second; + } + return zum; + }); + m.def("pass_std_array_int_2", [](const std::array &a) { + return pass_std_vector_int(std::vector(a.begin(), a.end())) + 1; + }); + m.def("pass_std_set_int", [](const std::set &s) { + int zum = 200; + for (const int i : s) { + zum += 3 * i; + } + return zum; + }); + m.def("pass_std_map_int", [](const std::map &m) { + int zum = 500; + for (const auto &p : m) { + zum += p.first * 1000 + p.second; + } + return zum; + }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 65fda54c..1896c322 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -381,3 +381,129 @@ def test_return_vector_bool_raw_ptr(): v = m.return_vector_bool_raw_ptr() assert isinstance(v, list) assert len(v) == 4513 + + +@pytest.mark.parametrize( + ("fn", "offset"), [(m.pass_std_vector_int, 0), (m.pass_std_array_int_2, 1)] +) +def test_pass_std_vector_int(fn, offset): + assert fn([7, 13]) == 140 + offset + assert fn({6, 2}) == 116 + offset + assert fn({"x": 8, "y": 11}.values()) == 138 + offset + assert fn({3: None, 9: None}.keys()) == 124 + offset + assert fn(i for i in [4, 17]) == 142 + offset + assert fn(map(lambda i: i * 3, [8, 7])) == 190 + offset # noqa: C417 + with pytest.raises(TypeError): + fn({"x": 0, "y": 1}) + with pytest.raises(TypeError): + fn({}) + + +def test_pass_std_vector_pair_int(): + fn = m.pass_std_vector_pair_int + assert fn({1: 2, 3: 4}.items()) == 406 + assert fn(zip([5, 17], [13, 9])) == 2222 + + +def test_list_caster_fully_consumes_generator_object(): + def gen_invalid(): + yield from [1, 2.0, 3] + + gen_obj = gen_invalid() + with pytest.raises(TypeError): + m.pass_std_vector_int(gen_obj) + assert not tuple(gen_obj) + + +def test_pass_std_set_int(): + fn = m.pass_std_set_int + assert fn({3, 15}) == 254 + assert fn({5: None, 12: None}.keys()) == 251 + with pytest.raises(TypeError): + fn([]) + with pytest.raises(TypeError): + fn({}) + with pytest.raises(TypeError): + fn({}.values()) + with pytest.raises(TypeError): + fn(i for i in []) + + +def test_set_caster_dict_keys_failure(): + dict_keys = {1: None, 2.0: None, 3: None}.keys() + # The asserts does not really exercise anything in pybind11, but if one of + # them fails in some future version of Python, the set_caster load + # implementation may need to be revisited. + assert tuple(dict_keys) == (1, 2.0, 3) + assert tuple(dict_keys) == (1, 2.0, 3) + with pytest.raises(TypeError): + m.pass_std_set_int(dict_keys) + assert tuple(dict_keys) == (1, 2.0, 3) + + +class FakePyMappingMissingItems: + def __getitem__(self, _): + raise RuntimeError("Not expected to be called.") + + +class FakePyMappingWithItems(FakePyMappingMissingItems): + def items(self): + return ((1, 3), (2, 4)) + + +class FakePyMappingBadItems(FakePyMappingMissingItems): + def items(self): + return ((1, 2), (3, "x")) + + +class FakePyMappingItemsNotCallable(FakePyMappingMissingItems): + @property + def items(self): + return ((1, 2), (3, 4)) + + +class FakePyMappingItemsWithArg(FakePyMappingMissingItems): + def items(self, _): + return ((1, 2), (3, 4)) + + +class FakePyMappingGenObj(FakePyMappingMissingItems): + def __init__(self, gen_obj): + super().__init__() + self.gen_obj = gen_obj + + def items(self): + yield from self.gen_obj + + +def test_pass_std_map_int(): + fn = m.pass_std_map_int + assert fn({1: 2, 3: 4}) == 4506 + with pytest.raises(TypeError): + fn([]) + assert fn(FakePyMappingWithItems()) == 3507 + with pytest.raises(TypeError): + fn(FakePyMappingMissingItems()) + with pytest.raises(TypeError): + fn(FakePyMappingBadItems()) + with pytest.raises(TypeError): + fn(FakePyMappingItemsNotCallable()) + with pytest.raises(TypeError): + fn(FakePyMappingItemsWithArg()) + + +@pytest.mark.parametrize( + ("items", "expected_exception"), + [ + (((1, 2), (3, "x"), (4, 5)), TypeError), + (((1, 2), (3, 4, 5), (6, 7)), ValueError), + ], +) +def test_map_caster_fully_consumes_generator_object(items, expected_exception): + def gen_invalid(): + yield from items + + gen_obj = gen_invalid() + with pytest.raises(expected_exception): + m.pass_std_map_int(FakePyMappingGenObj(gen_obj)) + assert not tuple(gen_obj)