From da91926295638b2b1c0c85a568a2d66ca3646565 Mon Sep 17 00:00:00 2001 From: biergaizi Date: Mon, 1 May 2023 14:14:52 +0000 Subject: [PATCH 1/4] fix: remove -stdlib=libc++ from setup helpers, not needed on modern Pythons (#4639) * Inject -stdlib=libc++ on macOS only when it's supported, close #4637. On macOS, by default, pybind11 currently unconditionally set the compiler flag "-stdlib=libc++" in Pybind11Extension.__init__(), regardless of which compiler is used. This flag is required for clang, but is invalid for GCC. If GCC is used, it causes compilation failures in all Python projects that use pybind11, with the error message: arm64-apple-darwin22-gcc: error: unrecognized command-line option -stdlib=libc++. This commit uses has_flag() to detect whether "-stdlib=libc++" on macOS, and injects this flag from build_ext.build_extensions(), rather than setting it unconditionally. Signed-off-by: Yifeng Li * revert: just remove flags --------- Signed-off-by: Yifeng Li Co-authored-by: Henry Schreiner --- pybind11/setup_helpers.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pybind11/setup_helpers.py b/pybind11/setup_helpers.py index 0fb4679e..cb279f27 100644 --- a/pybind11/setup_helpers.py +++ b/pybind11/setup_helpers.py @@ -144,7 +144,6 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: self.cxx_std = cxx_std cflags = [] - ldflags = [] if WIN: cflags += ["/EHsc", "/bigobj"] else: @@ -154,11 +153,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: c_cpp_flags = shlex.split(env_cflags) + shlex.split(env_cppflags) if not any(opt.startswith("-g") for opt in c_cpp_flags): cflags += ["-g0"] - if MACOS: - cflags += ["-stdlib=libc++"] - ldflags += ["-stdlib=libc++"] self._add_cflags(cflags) - self._add_ldflags(ldflags) @property def cxx_std(self) -> int: From dff75a62bd33ff35312aedf696c17f0300301b89 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 3 May 2023 00:34:51 -0400 Subject: [PATCH 2/4] chore(deps): bump pypa/gh-action-pypi-publish from 1.8.5 to 1.8.6 (#4650) Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.8.5 to 1.8.6. - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](https://github.com/pypa/gh-action-pypi-publish/compare/v1.8.5...v1.8.6) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/pip.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pip.yml b/.github/workflows/pip.yml index 7f686b13..6d9be3b1 100644 --- a/.github/workflows/pip.yml +++ b/.github/workflows/pip.yml @@ -98,13 +98,13 @@ jobs: - uses: actions/download-artifact@v3 - name: Publish standard package - uses: pypa/gh-action-pypi-publish@v1.8.5 + uses: pypa/gh-action-pypi-publish@v1.8.6 with: password: ${{ secrets.pypi_password }} packages-dir: standard/ - name: Publish global package - uses: pypa/gh-action-pypi-publish@v1.8.5 + uses: pypa/gh-action-pypi-publish@v1.8.6 with: password: ${{ secrets.pypi_password_global }} packages-dir: global/ From b3e88ecf894269b6b3fe822a7551c95247a3c47e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 3 May 2023 00:35:21 -0400 Subject: [PATCH 3/4] chore(deps): update pre-commit hooks (#4648) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/pre-commit/mirrors-mypy: v1.1.1 → v1.2.0](https://github.com/pre-commit/mirrors-mypy/compare/v1.1.1...v1.2.0) - [github.com/pre-commit/mirrors-clang-format: v16.0.0 → v16.0.2](https://github.com/pre-commit/mirrors-clang-format/compare/v16.0.0...v16.0.2) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 51b6f0a4..aca9b3ca 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -100,7 +100,7 @@ repos: # Check static types with mypy - repo: https://github.com/pre-commit/mirrors-mypy - rev: "v1.1.1" + rev: "v1.2.0" hooks: - id: mypy args: [] @@ -143,7 +143,7 @@ repos: # Clang format the codebase automatically - repo: https://github.com/pre-commit/mirrors-clang-format - rev: "v16.0.0" + rev: "v16.0.2" hooks: - id: clang-format types_or: [c++, c, cuda] From f70165463328c218d118204efc13aac93783d17b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 5 May 2023 07:39:05 +0200 Subject: [PATCH 4/4] Introduce recursive_container_traits (#4623) * Testing * Similar fix for std::vector * Fix infinite recursion check: 1) Apply to is_copy_assignable additionally 2) Check infinite recursion for map-like types * style: pre-commit fixes * Optional commit that demonstrates the limitations of this PR * Fix positioning of container bindings The bindings were previously in a block that was only activated if numpy was available. * Suggestions from code review: API side * Suggestions from code review: Test side * Suggestions from code review 1) Renaming: is_recursive_container and MutuallyRecursiveContainerPair(MV|VM) 2) Avoid ambiguous specializations of is_recursive_container * Some little fixes * Reordering of structs * Add recursive checks for is_move_constructible * Static testing for pybind11 type traits * More precise checking of recursive types Instead of a trait `is_recursive_container`, use a trait `recursive_container_traits` with dependent type `recursive_container_traits::type_to_check_recursively`. So, instead of just checking if a type is recursive and then trying to somehow deal with it, recursively-defined traits such as is_move_constructible can now directly ask this trait where the recursion should proceed. * Review suggestions 1. Use std::conditional 2. Fix typo * Remove leftover include from test --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- include/pybind11/detail/type_caster_base.h | 203 +++++++++++++++--- include/pybind11/stl_bind.h | 8 +- tests/test_copy_move.cpp | 238 +++++++++++++++++++++ tests/test_stl_binders.cpp | 44 ++++ tests/test_stl_binders.py | 18 ++ 5 files changed, 484 insertions(+), 27 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 8f3d0f37..16387506 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -822,26 +822,179 @@ using movable_cast_op_type typename std::add_rvalue_reference>::type, typename std::add_lvalue_reference>::type>>; -// std::is_copy_constructible isn't quite enough: it lets std::vector (and similar) through when -// T is non-copyable, but code containing such a copy constructor fails to actually compile. -template -struct is_copy_constructible : std::is_copy_constructible {}; +// Does the container have a mapped type and is it recursive? +// Implemented by specializations below. +template +struct container_mapped_type_traits { + static constexpr bool has_mapped_type = false; + static constexpr bool has_recursive_mapped_type = false; +}; + +template +struct container_mapped_type_traits< + Container, + typename std::enable_if< + std::is_same::value>::type> { + static constexpr bool has_mapped_type = true; + static constexpr bool has_recursive_mapped_type = true; +}; + +template +struct container_mapped_type_traits< + Container, + typename std::enable_if< + negation>::value>::type> { + static constexpr bool has_mapped_type = true; + static constexpr bool has_recursive_mapped_type = false; +}; + +// Does the container have a value type and is it recursive? +// Implemented by specializations below. +template +struct container_value_type_traits : std::false_type { + static constexpr bool has_value_type = false; + static constexpr bool has_recursive_value_type = false; +}; + +template +struct container_value_type_traits< + Container, + typename std::enable_if< + std::is_same::value>::type> { + static constexpr bool has_value_type = true; + static constexpr bool has_recursive_value_type = true; +}; + +template +struct container_value_type_traits< + Container, + typename std::enable_if< + negation>::value>::type> { + static constexpr bool has_value_type = true; + static constexpr bool has_recursive_value_type = false; +}; + +/* + * Tag to be used for representing the bottom of recursively defined types. + * Define this tag so we don't have to use void. + */ +struct recursive_bottom {}; + +/* + * Implementation detail of `recursive_container_traits` below. + * `T` is the `value_type` of the container, which might need to be modified to + * avoid recursive types and const types. + */ +template +struct impl_type_to_check_recursively { + /* + * If the container is recursive, then no further recursion should be done. + */ + using if_recursive = recursive_bottom; + /* + * Otherwise yield `T` unchanged. + */ + using if_not_recursive = T; +}; + +/* + * For pairs - only as value type of a map -, the first type should remove the `const`. + * Also, if the map is recursive, then the recursive checking should consider + * the first type only. + */ +template +struct impl_type_to_check_recursively, /* is_this_a_map = */ true> { + using if_recursive = typename std::remove_const::type; + using if_not_recursive = std::pair::type, B>; +}; -template -struct is_move_constructible : std::is_move_constructible {}; +/* + * Implementation of `recursive_container_traits` below. + */ +template +struct impl_recursive_container_traits { + using type_to_check_recursively = recursive_bottom; +}; -// Specialization for types that appear to be copy constructible but also look like stl containers -// (we specifically check for: has `value_type` and `reference` with `reference = value_type&`): if -// so, copy constructability depends on whether the value_type is copy constructible. template -struct is_copy_constructible< +struct impl_recursive_container_traits< Container, - enable_if_t< - all_of, - std::is_same, - // Avoid infinite recursion - negation>>::value>> - : is_copy_constructible {}; + typename std::enable_if::has_value_type>::type> { + static constexpr bool is_recursive + = container_mapped_type_traits::has_recursive_mapped_type + || container_value_type_traits::has_recursive_value_type; + /* + * This member dictates which type Pybind11 should check recursively in traits + * such as `is_move_constructible`, `is_copy_constructible`, `is_move_assignable`, ... + * Direct access to `value_type` should be avoided: + * 1. `value_type` might recursively contain the type again + * 2. `value_type` of STL map types is `std::pair`, the `const` + * should be removed. + * + */ + using type_to_check_recursively = typename std::conditional< + is_recursive, + typename impl_type_to_check_recursively< + typename Container::value_type, + container_mapped_type_traits::has_mapped_type>::if_recursive, + typename impl_type_to_check_recursively< + typename Container::value_type, + container_mapped_type_traits::has_mapped_type>::if_not_recursive>::type; +}; + +/* + * This trait defines the `type_to_check_recursively` which is needed to properly + * handle recursively defined traits such as `is_move_constructible` without going + * into an infinite recursion. + * Should be used instead of directly accessing the `value_type`. + * It cancels the recursion by returning the `recursive_bottom` tag. + * + * The default definition of `type_to_check_recursively` is as follows: + * + * 1. By default, it is `recursive_bottom`, so that the recursion is canceled. + * 2. If the type is non-recursive and defines a `value_type`, then the `value_type` is used. + * If the `value_type` is a pair and a `mapped_type` is defined, + * then the `const` is removed from the first type. + * 3. If the type is recursive and `value_type` is not a pair, then `recursive_bottom` is returned. + * 4. If the type is recursive and `value_type` is a pair and a `mapped_type` is defined, + * then `const` is removed from the first type and the first type is returned. + * + * This behavior can be extended by the user as seen in test_stl_binders.cpp. + * + * This struct is exactly the same as impl_recursive_container_traits. + * The duplication achieves that user-defined specializations don't compete + * with internal specializations, but take precedence. + */ +template +struct recursive_container_traits : impl_recursive_container_traits {}; + +template +struct is_move_constructible + : all_of, + is_move_constructible< + typename recursive_container_traits::type_to_check_recursively>> {}; + +template <> +struct is_move_constructible : std::true_type {}; + +// Likewise for std::pair +// (after C++17 it is mandatory that the move constructor not exist when the two types aren't +// themselves move constructible, but this can not be relied upon when T1 or T2 are themselves +// containers). +template +struct is_move_constructible> + : all_of, is_move_constructible> {}; + +// std::is_copy_constructible isn't quite enough: it lets std::vector (and similar) through when +// T is non-copyable, but code containing such a copy constructor fails to actually compile. +template +struct is_copy_constructible + : all_of, + is_copy_constructible< + typename recursive_container_traits::type_to_check_recursively>> {}; + +template <> +struct is_copy_constructible : std::true_type {}; // Likewise for std::pair // (after C++17 it is mandatory that the copy constructor not exist when the two types aren't @@ -852,14 +1005,16 @@ struct is_copy_constructible> : all_of, is_copy_constructible> {}; // The same problems arise with std::is_copy_assignable, so we use the same workaround. -template -struct is_copy_assignable : std::is_copy_assignable {}; -template -struct is_copy_assignable, - std::is_same>::value>> - : is_copy_assignable {}; +template +struct is_copy_assignable + : all_of< + std::is_copy_assignable, + is_copy_assignable::type_to_check_recursively>> { +}; + +template <> +struct is_copy_assignable : std::true_type {}; + template struct is_copy_assignable> : all_of, is_copy_assignable> {}; diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index 7cfd996d..49f1b778 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -61,9 +61,11 @@ struct is_comparable< /* For a vector/map data structure, recursively check the value type (which is std::pair for maps) */ template -struct is_comparable::is_vector>> { - static constexpr const bool value = is_comparable::value; -}; +struct is_comparable::is_vector>> + : is_comparable::type_to_check_recursively> {}; + +template <> +struct is_comparable : std::true_type {}; /* For pairs, recursively check the two data types */ template diff --git a/tests/test_copy_move.cpp b/tests/test_copy_move.cpp index 28c24456..f5473355 100644 --- a/tests/test_copy_move.cpp +++ b/tests/test_copy_move.cpp @@ -13,6 +13,8 @@ #include "constructor_stats.h" #include "pybind11_tests.h" +#include + template struct empty { static const derived &get_one() { return instance_; } @@ -293,3 +295,239 @@ TEST_SUBMODULE(copy_move_policies, m) { // Make sure that cast from pytype rvalue to other pytype works m.def("get_pytype_rvalue_castissue", [](double i) { return py::float_(i).cast(); }); } + +/* + * Rest of the file: + * static_assert based tests for pybind11 adaptations of + * std::is_move_constructible, std::is_copy_constructible and + * std::is_copy_assignable (no adaptation of std::is_move_assignable). + * Difference between pybind11 and std traits: pybind11 traits will also check + * the contained value_types. + */ + +struct NotMovable { + NotMovable() = default; + NotMovable(NotMovable const &) = default; + NotMovable(NotMovable &&) = delete; + NotMovable &operator=(NotMovable const &) = default; + NotMovable &operator=(NotMovable &&) = delete; +}; +static_assert(!std::is_move_constructible::value, + "!std::is_move_constructible::value"); +static_assert(std::is_copy_constructible::value, + "std::is_copy_constructible::value"); +static_assert(!pybind11::detail::is_move_constructible::value, + "!pybind11::detail::is_move_constructible::value"); +static_assert(pybind11::detail::is_copy_constructible::value, + "pybind11::detail::is_copy_constructible::value"); +static_assert(!std::is_move_assignable::value, + "!std::is_move_assignable::value"); +static_assert(std::is_copy_assignable::value, + "std::is_copy_assignable::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable::value, +// "!pybind11::detail::is_move_assignable::value"); +static_assert(pybind11::detail::is_copy_assignable::value, + "pybind11::detail::is_copy_assignable::value"); + +struct NotCopyable { + NotCopyable() = default; + NotCopyable(NotCopyable const &) = delete; + NotCopyable(NotCopyable &&) = default; + NotCopyable &operator=(NotCopyable const &) = delete; + NotCopyable &operator=(NotCopyable &&) = default; +}; +static_assert(std::is_move_constructible::value, + "std::is_move_constructible::value"); +static_assert(!std::is_copy_constructible::value, + "!std::is_copy_constructible::value"); +static_assert(pybind11::detail::is_move_constructible::value, + "pybind11::detail::is_move_constructible::value"); +static_assert(!pybind11::detail::is_copy_constructible::value, + "!pybind11::detail::is_copy_constructible::value"); +static_assert(std::is_move_assignable::value, + "std::is_move_assignable::value"); +static_assert(!std::is_copy_assignable::value, + "!std::is_copy_assignable::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable::value, +// "!pybind11::detail::is_move_assignable::value"); +static_assert(!pybind11::detail::is_copy_assignable::value, + "!pybind11::detail::is_copy_assignable::value"); + +struct NotCopyableNotMovable { + NotCopyableNotMovable() = default; + NotCopyableNotMovable(NotCopyableNotMovable const &) = delete; + NotCopyableNotMovable(NotCopyableNotMovable &&) = delete; + NotCopyableNotMovable &operator=(NotCopyableNotMovable const &) = delete; + NotCopyableNotMovable &operator=(NotCopyableNotMovable &&) = delete; +}; +static_assert(!std::is_move_constructible::value, + "!std::is_move_constructible::value"); +static_assert(!std::is_copy_constructible::value, + "!std::is_copy_constructible::value"); +static_assert(!pybind11::detail::is_move_constructible::value, + "!pybind11::detail::is_move_constructible::value"); +static_assert(!pybind11::detail::is_copy_constructible::value, + "!pybind11::detail::is_copy_constructible::value"); +static_assert(!std::is_move_assignable::value, + "!std::is_move_assignable::value"); +static_assert(!std::is_copy_assignable::value, + "!std::is_copy_assignable::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable::value, +// "!pybind11::detail::is_move_assignable::value"); +static_assert(!pybind11::detail::is_copy_assignable::value, + "!pybind11::detail::is_copy_assignable::value"); + +struct NotMovableVector : std::vector {}; +static_assert(std::is_move_constructible::value, + "std::is_move_constructible::value"); +static_assert(std::is_copy_constructible::value, + "std::is_copy_constructible::value"); +static_assert(!pybind11::detail::is_move_constructible::value, + "!pybind11::detail::is_move_constructible::value"); +static_assert(pybind11::detail::is_copy_constructible::value, + "pybind11::detail::is_copy_constructible::value"); +static_assert(std::is_move_assignable::value, + "std::is_move_assignable::value"); +static_assert(std::is_copy_assignable::value, + "std::is_copy_assignable::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable::value, +// "!pybind11::detail::is_move_assignable::value"); +static_assert(pybind11::detail::is_copy_assignable::value, + "pybind11::detail::is_copy_assignable::value"); + +struct NotCopyableVector : std::vector {}; +static_assert(std::is_move_constructible::value, + "std::is_move_constructible::value"); +static_assert(std::is_copy_constructible::value, + "std::is_copy_constructible::value"); +static_assert(pybind11::detail::is_move_constructible::value, + "pybind11::detail::is_move_constructible::value"); +static_assert(!pybind11::detail::is_copy_constructible::value, + "!pybind11::detail::is_copy_constructible::value"); +static_assert(std::is_move_assignable::value, + "std::is_move_assignable::value"); +static_assert(std::is_copy_assignable::value, + "std::is_copy_assignable::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable::value, +// "!pybind11::detail::is_move_assignable::value"); +static_assert(!pybind11::detail::is_copy_assignable::value, + "!pybind11::detail::is_copy_assignable::value"); + +struct NotCopyableNotMovableVector : std::vector {}; +static_assert(std::is_move_constructible::value, + "std::is_move_constructible::value"); +static_assert(std::is_copy_constructible::value, + "std::is_copy_constructible::value"); +static_assert(!pybind11::detail::is_move_constructible::value, + "!pybind11::detail::is_move_constructible::value"); +static_assert(!pybind11::detail::is_copy_constructible::value, + "!pybind11::detail::is_copy_constructible::value"); +static_assert(std::is_move_assignable::value, + "std::is_move_assignable::value"); +static_assert(std::is_copy_assignable::value, + "std::is_copy_assignable::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable::value, +// "!pybind11::detail::is_move_assignable::value"); +static_assert(!pybind11::detail::is_copy_assignable::value, + "!pybind11::detail::is_copy_assignable::value"); + +struct NotMovableMap : std::map {}; +static_assert(std::is_move_constructible::value, + "std::is_move_constructible::value"); +static_assert(std::is_copy_constructible::value, + "std::is_copy_constructible::value"); +static_assert(!pybind11::detail::is_move_constructible::value, + "!pybind11::detail::is_move_constructible::value"); +static_assert(pybind11::detail::is_copy_constructible::value, + "pybind11::detail::is_copy_constructible::value"); +static_assert(std::is_move_assignable::value, + "std::is_move_assignable::value"); +static_assert(std::is_copy_assignable::value, + "std::is_copy_assignable::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable::value, +// "!pybind11::detail::is_move_assignable::value"); +static_assert(pybind11::detail::is_copy_assignable::value, + "pybind11::detail::is_copy_assignable::value"); + +struct NotCopyableMap : std::map {}; +static_assert(std::is_move_constructible::value, + "std::is_move_constructible::value"); +static_assert(std::is_copy_constructible::value, + "std::is_copy_constructible::value"); +static_assert(pybind11::detail::is_move_constructible::value, + "pybind11::detail::is_move_constructible::value"); +static_assert(!pybind11::detail::is_copy_constructible::value, + "!pybind11::detail::is_copy_constructible::value"); +static_assert(std::is_move_assignable::value, + "std::is_move_assignable::value"); +static_assert(std::is_copy_assignable::value, + "std::is_copy_assignable::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable::value, +// "!pybind11::detail::is_move_assignable::value"); +static_assert(!pybind11::detail::is_copy_assignable::value, + "!pybind11::detail::is_copy_assignable::value"); + +struct NotCopyableNotMovableMap : std::map {}; +static_assert(std::is_move_constructible::value, + "std::is_move_constructible::value"); +static_assert(std::is_copy_constructible::value, + "std::is_copy_constructible::value"); +static_assert(!pybind11::detail::is_move_constructible::value, + "!pybind11::detail::is_move_constructible::value"); +static_assert(!pybind11::detail::is_copy_constructible::value, + "!pybind11::detail::is_copy_constructible::value"); +static_assert(std::is_move_assignable::value, + "std::is_move_assignable::value"); +static_assert(std::is_copy_assignable::value, + "std::is_copy_assignable::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable::value, +// "!pybind11::detail::is_move_assignable::value"); +static_assert(!pybind11::detail::is_copy_assignable::value, + "!pybind11::detail::is_copy_assignable::value"); + +struct RecursiveVector : std::vector {}; +static_assert(std::is_move_constructible::value, + "std::is_move_constructible::value"); +static_assert(std::is_copy_constructible::value, + "std::is_copy_constructible::value"); +static_assert(pybind11::detail::is_move_constructible::value, + "pybind11::detail::is_move_constructible::value"); +static_assert(pybind11::detail::is_copy_constructible::value, + "pybind11::detail::is_copy_constructible::value"); +static_assert(std::is_move_assignable::value, + "std::is_move_assignable::value"); +static_assert(std::is_copy_assignable::value, + "std::is_copy_assignable::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable::value, +// "!pybind11::detail::is_move_assignable::value"); +static_assert(pybind11::detail::is_copy_assignable::value, + "pybind11::detail::is_copy_assignable::value"); + +struct RecursiveMap : std::map {}; +static_assert(std::is_move_constructible::value, + "std::is_move_constructible::value"); +static_assert(std::is_copy_constructible::value, + "std::is_copy_constructible::value"); +static_assert(pybind11::detail::is_move_constructible::value, + "pybind11::detail::is_move_constructible::value"); +static_assert(pybind11::detail::is_copy_constructible::value, + "pybind11::detail::is_copy_constructible::value"); +static_assert(std::is_move_assignable::value, + "std::is_move_assignable::value"); +static_assert(std::is_copy_assignable::value, + "std::is_copy_assignable::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable::value, +// "!pybind11::detail::is_move_assignable::value"); +static_assert(pybind11::detail::is_copy_assignable::value, + "pybind11::detail::is_copy_assignable::value"); diff --git a/tests/test_stl_binders.cpp b/tests/test_stl_binders.cpp index ca9630bd..1681760a 100644 --- a/tests/test_stl_binders.cpp +++ b/tests/test_stl_binders.cpp @@ -70,6 +70,44 @@ NestMap *times_hundred(int n) { return m; } +/* + * Recursive data structures as test for issue #4623 + */ +struct RecursiveVector : std::vector { + using Parent = std::vector; + using Parent::Parent; +}; + +struct RecursiveMap : std::map { + using Parent = std::map; + using Parent::Parent; +}; + +/* + * Pybind11 does not catch more complicated recursion schemes, such as mutual + * recursion. + * In that case custom recursive_container_traits specializations need to be added, + * thus manually telling pybind11 about the recursion. + */ +struct MutuallyRecursiveContainerPairMV; +struct MutuallyRecursiveContainerPairVM; + +struct MutuallyRecursiveContainerPairMV : std::map {}; +struct MutuallyRecursiveContainerPairVM : std::vector {}; + +namespace pybind11 { +namespace detail { +template +struct recursive_container_traits { + using type_to_check_recursively = recursive_bottom; +}; +template +struct recursive_container_traits { + using type_to_check_recursively = recursive_bottom; +}; +} // namespace detail +} // namespace pybind11 + TEST_SUBMODULE(stl_binders, m) { // test_vector_int py::bind_vector>(m, "VectorInt", py::buffer_protocol()); @@ -129,6 +167,12 @@ TEST_SUBMODULE(stl_binders, m) { m, "VectorUndeclStruct", py::buffer_protocol()); }); + // Bind recursive container types + py::bind_vector(m, "RecursiveVector"); + py::bind_map(m, "RecursiveMap"); + py::bind_map(m, "MutuallyRecursiveContainerPairMV"); + py::bind_vector(m, "MutuallyRecursiveContainerPairVM"); + // The rest depends on numpy: try { py::module_::import("numpy"); diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index 7dca742a..e002f5b6 100644 --- a/tests/test_stl_binders.py +++ b/tests/test_stl_binders.py @@ -335,3 +335,21 @@ def test_map_view_types(): assert type(unordered_map_string_double.items()) is items_type assert type(map_string_double_const.items()) is items_type assert type(unordered_map_string_double_const.items()) is items_type + + +def test_recursive_vector(): + recursive_vector = m.RecursiveVector() + recursive_vector.append(m.RecursiveVector()) + recursive_vector[0].append(m.RecursiveVector()) + recursive_vector[0].append(m.RecursiveVector()) + # Can't use len() since test_stl_binders.cpp does not include stl.h, + # so the necessary conversion is missing + assert recursive_vector[0].count(m.RecursiveVector()) == 2 + + +def test_recursive_map(): + recursive_map = m.RecursiveMap() + recursive_map[100] = m.RecursiveMap() + recursive_map[100][101] = m.RecursiveMap() + recursive_map[100][102] = m.RecursiveMap() + assert list(recursive_map[100].keys()) == [101, 102]