diff options
author | Franz Pöschel <franz.poeschel@gmail.com> | 2023-05-05 07:39:05 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-05-04 22:39:05 -0700 |
commit | f70165463328c218d118204efc13aac93783d17b (patch) | |
tree | f4263e103c4988b98e86de717060fc16e6316850 | |
parent | b3e88ecf894269b6b3fe822a7551c95247a3c47e (diff) | |
download | pybind11-f70165463328c218d118204efc13aac93783d17b.tar.gz |
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>
-rw-r--r-- | include/pybind11/detail/type_caster_base.h | 203 | ||||
-rw-r--r-- | include/pybind11/stl_bind.h | 8 | ||||
-rw-r--r-- | tests/test_copy_move.cpp | 238 | ||||
-rw-r--r-- | tests/test_stl_binders.cpp | 44 | ||||
-rw-r--r-- | 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<intrinsic_t<T>>::type, typename std::add_lvalue_reference<intrinsic_t<T>>::type>>; -// std::is_copy_constructible isn't quite enough: it lets std::vector<T> (and similar) through when -// T is non-copyable, but code containing such a copy constructor fails to actually compile. -template <typename T, typename SFINAE = void> -struct is_copy_constructible : std::is_copy_constructible<T> {}; +// Does the container have a mapped type and is it recursive? +// Implemented by specializations below. +template <typename Container, typename SFINAE = void> +struct container_mapped_type_traits { + static constexpr bool has_mapped_type = false; + static constexpr bool has_recursive_mapped_type = false; +}; + +template <typename Container> +struct container_mapped_type_traits< + Container, + typename std::enable_if< + std::is_same<typename Container::mapped_type, Container>::value>::type> { + static constexpr bool has_mapped_type = true; + static constexpr bool has_recursive_mapped_type = true; +}; + +template <typename Container> +struct container_mapped_type_traits< + Container, + typename std::enable_if< + negation<std::is_same<typename Container::mapped_type, Container>>::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 <typename Container, typename SFINAE = void> +struct container_value_type_traits : std::false_type { + static constexpr bool has_value_type = false; + static constexpr bool has_recursive_value_type = false; +}; + +template <typename Container> +struct container_value_type_traits< + Container, + typename std::enable_if< + std::is_same<typename Container::value_type, Container>::value>::type> { + static constexpr bool has_value_type = true; + static constexpr bool has_recursive_value_type = true; +}; + +template <typename Container> +struct container_value_type_traits< + Container, + typename std::enable_if< + negation<std::is_same<typename Container::value_type, Container>>::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 <typename T, bool is_this_a_map> +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 <typename A, typename B> +struct impl_type_to_check_recursively<std::pair<A, B>, /* is_this_a_map = */ true> { + using if_recursive = typename std::remove_const<A>::type; + using if_not_recursive = std::pair<typename std::remove_const<A>::type, B>; +}; -template <typename T, typename SFINAE = void> -struct is_move_constructible : std::is_move_constructible<T> {}; +/* + * Implementation of `recursive_container_traits` below. + */ +template <typename Container, typename SFINAE = void> +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 <typename Container> -struct is_copy_constructible< +struct impl_recursive_container_traits< Container, - enable_if_t< - all_of<std::is_copy_constructible<Container>, - std::is_same<typename Container::value_type &, typename Container::reference>, - // Avoid infinite recursion - negation<std::is_same<Container, typename Container::value_type>>>::value>> - : is_copy_constructible<typename Container::value_type> {}; + typename std::enable_if<container_value_type_traits<Container>::has_value_type>::type> { + static constexpr bool is_recursive + = container_mapped_type_traits<Container>::has_recursive_mapped_type + || container_value_type_traits<Container>::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<A const, B>`, 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<Container>::has_mapped_type>::if_recursive, + typename impl_type_to_check_recursively< + typename Container::value_type, + container_mapped_type_traits<Container>::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 <typename Container, typename SFINAE = void> +struct recursive_container_traits : impl_recursive_container_traits<Container> {}; + +template <typename T> +struct is_move_constructible + : all_of<std::is_move_constructible<T>, + is_move_constructible< + typename recursive_container_traits<T>::type_to_check_recursively>> {}; + +template <> +struct is_move_constructible<recursive_bottom> : 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 <typename T1, typename T2> +struct is_move_constructible<std::pair<T1, T2>> + : all_of<is_move_constructible<T1>, is_move_constructible<T2>> {}; + +// std::is_copy_constructible isn't quite enough: it lets std::vector<T> (and similar) through when +// T is non-copyable, but code containing such a copy constructor fails to actually compile. +template <typename T> +struct is_copy_constructible + : all_of<std::is_copy_constructible<T>, + is_copy_constructible< + typename recursive_container_traits<T>::type_to_check_recursively>> {}; + +template <> +struct is_copy_constructible<recursive_bottom> : 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<std::pair<T1, T2>> : all_of<is_copy_constructible<T1>, is_copy_constructible<T2>> {}; // The same problems arise with std::is_copy_assignable, so we use the same workaround. -template <typename T, typename SFINAE = void> -struct is_copy_assignable : std::is_copy_assignable<T> {}; -template <typename Container> -struct is_copy_assignable<Container, - enable_if_t<all_of<std::is_copy_assignable<Container>, - std::is_same<typename Container::value_type &, - typename Container::reference>>::value>> - : is_copy_assignable<typename Container::value_type> {}; +template <typename T> +struct is_copy_assignable + : all_of< + std::is_copy_assignable<T>, + is_copy_assignable<typename recursive_container_traits<T>::type_to_check_recursively>> { +}; + +template <> +struct is_copy_assignable<recursive_bottom> : std::true_type {}; + template <typename T1, typename T2> struct is_copy_assignable<std::pair<T1, T2>> : all_of<is_copy_assignable<T1>, is_copy_assignable<T2>> {}; 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 <typename T> -struct is_comparable<T, enable_if_t<container_traits<T>::is_vector>> { - static constexpr const bool value = is_comparable<typename T::value_type>::value; -}; +struct is_comparable<T, enable_if_t<container_traits<T>::is_vector>> + : is_comparable<typename recursive_container_traits<T>::type_to_check_recursively> {}; + +template <> +struct is_comparable<recursive_bottom> : std::true_type {}; /* For pairs, recursively check the two data types */ template <typename T> 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 <type_traits> + template <typename derived> 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<py::int_>(); }); } + +/* + * 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<NotMovable>::value, + "!std::is_move_constructible<NotMovable>::value"); +static_assert(std::is_copy_constructible<NotMovable>::value, + "std::is_copy_constructible<NotMovable>::value"); +static_assert(!pybind11::detail::is_move_constructible<NotMovable>::value, + "!pybind11::detail::is_move_constructible<NotMovable>::value"); +static_assert(pybind11::detail::is_copy_constructible<NotMovable>::value, + "pybind11::detail::is_copy_constructible<NotMovable>::value"); +static_assert(!std::is_move_assignable<NotMovable>::value, + "!std::is_move_assignable<NotMovable>::value"); +static_assert(std::is_copy_assignable<NotMovable>::value, + "std::is_copy_assignable<NotMovable>::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable<NotMovable>::value, +// "!pybind11::detail::is_move_assignable<NotMovable>::value"); +static_assert(pybind11::detail::is_copy_assignable<NotMovable>::value, + "pybind11::detail::is_copy_assignable<NotMovable>::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<NotCopyable>::value, + "std::is_move_constructible<NotCopyable>::value"); +static_assert(!std::is_copy_constructible<NotCopyable>::value, + "!std::is_copy_constructible<NotCopyable>::value"); +static_assert(pybind11::detail::is_move_constructible<NotCopyable>::value, + "pybind11::detail::is_move_constructible<NotCopyable>::value"); +static_assert(!pybind11::detail::is_copy_constructible<NotCopyable>::value, + "!pybind11::detail::is_copy_constructible<NotCopyable>::value"); +static_assert(std::is_move_assignable<NotCopyable>::value, + "std::is_move_assignable<NotCopyable>::value"); +static_assert(!std::is_copy_assignable<NotCopyable>::value, + "!std::is_copy_assignable<NotCopyable>::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable<NotCopyable>::value, +// "!pybind11::detail::is_move_assignable<NotCopyable>::value"); +static_assert(!pybind11::detail::is_copy_assignable<NotCopyable>::value, + "!pybind11::detail::is_copy_assignable<NotCopyable>::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<NotCopyableNotMovable>::value, + "!std::is_move_constructible<NotCopyableNotMovable>::value"); +static_assert(!std::is_copy_constructible<NotCopyableNotMovable>::value, + "!std::is_copy_constructible<NotCopyableNotMovable>::value"); +static_assert(!pybind11::detail::is_move_constructible<NotCopyableNotMovable>::value, + "!pybind11::detail::is_move_constructible<NotCopyableNotMovable>::value"); +static_assert(!pybind11::detail::is_copy_constructible<NotCopyableNotMovable>::value, + "!pybind11::detail::is_copy_constructible<NotCopyableNotMovable>::value"); +static_assert(!std::is_move_assignable<NotCopyableNotMovable>::value, + "!std::is_move_assignable<NotCopyableNotMovable>::value"); +static_assert(!std::is_copy_assignable<NotCopyableNotMovable>::value, + "!std::is_copy_assignable<NotCopyableNotMovable>::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable<NotCopyableNotMovable>::value, +// "!pybind11::detail::is_move_assignable<NotCopyableNotMovable>::value"); +static_assert(!pybind11::detail::is_copy_assignable<NotCopyableNotMovable>::value, + "!pybind11::detail::is_copy_assignable<NotCopyableNotMovable>::value"); + +struct NotMovableVector : std::vector<NotMovable> {}; +static_assert(std::is_move_constructible<NotMovableVector>::value, + "std::is_move_constructible<NotMovableVector>::value"); +static_assert(std::is_copy_constructible<NotMovableVector>::value, + "std::is_copy_constructible<NotMovableVector>::value"); +static_assert(!pybind11::detail::is_move_constructible<NotMovableVector>::value, + "!pybind11::detail::is_move_constructible<NotMovableVector>::value"); +static_assert(pybind11::detail::is_copy_constructible<NotMovableVector>::value, + "pybind11::detail::is_copy_constructible<NotMovableVector>::value"); +static_assert(std::is_move_assignable<NotMovableVector>::value, + "std::is_move_assignable<NotMovableVector>::value"); +static_assert(std::is_copy_assignable<NotMovableVector>::value, + "std::is_copy_assignable<NotMovableVector>::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable<NotMovableVector>::value, +// "!pybind11::detail::is_move_assignable<NotMovableVector>::value"); +static_assert(pybind11::detail::is_copy_assignable<NotMovableVector>::value, + "pybind11::detail::is_copy_assignable<NotMovableVector>::value"); + +struct NotCopyableVector : std::vector<NotCopyable> {}; +static_assert(std::is_move_constructible<NotCopyableVector>::value, + "std::is_move_constructible<NotCopyableVector>::value"); +static_assert(std::is_copy_constructible<NotCopyableVector>::value, + "std::is_copy_constructible<NotCopyableVector>::value"); +static_assert(pybind11::detail::is_move_constructible<NotCopyableVector>::value, + "pybind11::detail::is_move_constructible<NotCopyableVector>::value"); +static_assert(!pybind11::detail::is_copy_constructible<NotCopyableVector>::value, + "!pybind11::detail::is_copy_constructible<NotCopyableVector>::value"); +static_assert(std::is_move_assignable<NotCopyableVector>::value, + "std::is_move_assignable<NotCopyableVector>::value"); +static_assert(std::is_copy_assignable<NotCopyableVector>::value, + "std::is_copy_assignable<NotCopyableVector>::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable<NotCopyableVector>::value, +// "!pybind11::detail::is_move_assignable<NotCopyableVector>::value"); +static_assert(!pybind11::detail::is_copy_assignable<NotCopyableVector>::value, + "!pybind11::detail::is_copy_assignable<NotCopyableVector>::value"); + +struct NotCopyableNotMovableVector : std::vector<NotCopyableNotMovable> {}; +static_assert(std::is_move_constructible<NotCopyableNotMovableVector>::value, + "std::is_move_constructible<NotCopyableNotMovableVector>::value"); +static_assert(std::is_copy_constructible<NotCopyableNotMovableVector>::value, + "std::is_copy_constructible<NotCopyableNotMovableVector>::value"); +static_assert(!pybind11::detail::is_move_constructible<NotCopyableNotMovableVector>::value, + "!pybind11::detail::is_move_constructible<NotCopyableNotMovableVector>::value"); +static_assert(!pybind11::detail::is_copy_constructible<NotCopyableNotMovableVector>::value, + "!pybind11::detail::is_copy_constructible<NotCopyableNotMovableVector>::value"); +static_assert(std::is_move_assignable<NotCopyableNotMovableVector>::value, + "std::is_move_assignable<NotCopyableNotMovableVector>::value"); +static_assert(std::is_copy_assignable<NotCopyableNotMovableVector>::value, + "std::is_copy_assignable<NotCopyableNotMovableVector>::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable<NotCopyableNotMovableVector>::value, +// "!pybind11::detail::is_move_assignable<NotCopyableNotMovableVector>::value"); +static_assert(!pybind11::detail::is_copy_assignable<NotCopyableNotMovableVector>::value, + "!pybind11::detail::is_copy_assignable<NotCopyableNotMovableVector>::value"); + +struct NotMovableMap : std::map<int, NotMovable> {}; +static_assert(std::is_move_constructible<NotMovableMap>::value, + "std::is_move_constructible<NotMovableMap>::value"); +static_assert(std::is_copy_constructible<NotMovableMap>::value, + "std::is_copy_constructible<NotMovableMap>::value"); +static_assert(!pybind11::detail::is_move_constructible<NotMovableMap>::value, + "!pybind11::detail::is_move_constructible<NotMovableMap>::value"); +static_assert(pybind11::detail::is_copy_constructible<NotMovableMap>::value, + "pybind11::detail::is_copy_constructible<NotMovableMap>::value"); +static_assert(std::is_move_assignable<NotMovableMap>::value, + "std::is_move_assignable<NotMovableMap>::value"); +static_assert(std::is_copy_assignable<NotMovableMap>::value, + "std::is_copy_assignable<NotMovableMap>::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable<NotMovableMap>::value, +// "!pybind11::detail::is_move_assignable<NotMovableMap>::value"); +static_assert(pybind11::detail::is_copy_assignable<NotMovableMap>::value, + "pybind11::detail::is_copy_assignable<NotMovableMap>::value"); + +struct NotCopyableMap : std::map<int, NotCopyable> {}; +static_assert(std::is_move_constructible<NotCopyableMap>::value, + "std::is_move_constructible<NotCopyableMap>::value"); +static_assert(std::is_copy_constructible<NotCopyableMap>::value, + "std::is_copy_constructible<NotCopyableMap>::value"); +static_assert(pybind11::detail::is_move_constructible<NotCopyableMap>::value, + "pybind11::detail::is_move_constructible<NotCopyableMap>::value"); +static_assert(!pybind11::detail::is_copy_constructible<NotCopyableMap>::value, + "!pybind11::detail::is_copy_constructible<NotCopyableMap>::value"); +static_assert(std::is_move_assignable<NotCopyableMap>::value, + "std::is_move_assignable<NotCopyableMap>::value"); +static_assert(std::is_copy_assignable<NotCopyableMap>::value, + "std::is_copy_assignable<NotCopyableMap>::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable<NotCopyableMap>::value, +// "!pybind11::detail::is_move_assignable<NotCopyableMap>::value"); +static_assert(!pybind11::detail::is_copy_assignable<NotCopyableMap>::value, + "!pybind11::detail::is_copy_assignable<NotCopyableMap>::value"); + +struct NotCopyableNotMovableMap : std::map<int, NotCopyableNotMovable> {}; +static_assert(std::is_move_constructible<NotCopyableNotMovableMap>::value, + "std::is_move_constructible<NotCopyableNotMovableMap>::value"); +static_assert(std::is_copy_constructible<NotCopyableNotMovableMap>::value, + "std::is_copy_constructible<NotCopyableNotMovableMap>::value"); +static_assert(!pybind11::detail::is_move_constructible<NotCopyableNotMovableMap>::value, + "!pybind11::detail::is_move_constructible<NotCopyableNotMovableMap>::value"); +static_assert(!pybind11::detail::is_copy_constructible<NotCopyableNotMovableMap>::value, + "!pybind11::detail::is_copy_constructible<NotCopyableNotMovableMap>::value"); +static_assert(std::is_move_assignable<NotCopyableNotMovableMap>::value, + "std::is_move_assignable<NotCopyableNotMovableMap>::value"); +static_assert(std::is_copy_assignable<NotCopyableNotMovableMap>::value, + "std::is_copy_assignable<NotCopyableNotMovableMap>::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable<NotCopyableNotMovableMap>::value, +// "!pybind11::detail::is_move_assignable<NotCopyableNotMovableMap>::value"); +static_assert(!pybind11::detail::is_copy_assignable<NotCopyableNotMovableMap>::value, + "!pybind11::detail::is_copy_assignable<NotCopyableNotMovableMap>::value"); + +struct RecursiveVector : std::vector<RecursiveVector> {}; +static_assert(std::is_move_constructible<RecursiveVector>::value, + "std::is_move_constructible<RecursiveVector>::value"); +static_assert(std::is_copy_constructible<RecursiveVector>::value, + "std::is_copy_constructible<RecursiveVector>::value"); +static_assert(pybind11::detail::is_move_constructible<RecursiveVector>::value, + "pybind11::detail::is_move_constructible<RecursiveVector>::value"); +static_assert(pybind11::detail::is_copy_constructible<RecursiveVector>::value, + "pybind11::detail::is_copy_constructible<RecursiveVector>::value"); +static_assert(std::is_move_assignable<RecursiveVector>::value, + "std::is_move_assignable<RecursiveVector>::value"); +static_assert(std::is_copy_assignable<RecursiveVector>::value, + "std::is_copy_assignable<RecursiveVector>::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable<RecursiveVector>::value, +// "!pybind11::detail::is_move_assignable<RecursiveVector>::value"); +static_assert(pybind11::detail::is_copy_assignable<RecursiveVector>::value, + "pybind11::detail::is_copy_assignable<RecursiveVector>::value"); + +struct RecursiveMap : std::map<int, RecursiveMap> {}; +static_assert(std::is_move_constructible<RecursiveMap>::value, + "std::is_move_constructible<RecursiveMap>::value"); +static_assert(std::is_copy_constructible<RecursiveMap>::value, + "std::is_copy_constructible<RecursiveMap>::value"); +static_assert(pybind11::detail::is_move_constructible<RecursiveMap>::value, + "pybind11::detail::is_move_constructible<RecursiveMap>::value"); +static_assert(pybind11::detail::is_copy_constructible<RecursiveMap>::value, + "pybind11::detail::is_copy_constructible<RecursiveMap>::value"); +static_assert(std::is_move_assignable<RecursiveMap>::value, + "std::is_move_assignable<RecursiveMap>::value"); +static_assert(std::is_copy_assignable<RecursiveMap>::value, + "std::is_copy_assignable<RecursiveMap>::value"); +// pybind11 does not have this +// static_assert(!pybind11::detail::is_move_assignable<RecursiveMap>::value, +// "!pybind11::detail::is_move_assignable<RecursiveMap>::value"); +static_assert(pybind11::detail::is_copy_assignable<RecursiveMap>::value, + "pybind11::detail::is_copy_assignable<RecursiveMap>::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<RecursiveVector> { + using Parent = std::vector<RecursiveVector>; + using Parent::Parent; +}; + +struct RecursiveMap : std::map<int, RecursiveMap> { + using Parent = std::map<int, RecursiveMap>; + 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<int, MutuallyRecursiveContainerPairVM> {}; +struct MutuallyRecursiveContainerPairVM : std::vector<MutuallyRecursiveContainerPairMV> {}; + +namespace pybind11 { +namespace detail { +template <typename SFINAE> +struct recursive_container_traits<MutuallyRecursiveContainerPairMV, SFINAE> { + using type_to_check_recursively = recursive_bottom; +}; +template <typename SFINAE> +struct recursive_container_traits<MutuallyRecursiveContainerPairVM, SFINAE> { + using type_to_check_recursively = recursive_bottom; +}; +} // namespace detail +} // namespace pybind11 + TEST_SUBMODULE(stl_binders, m) { // test_vector_int py::bind_vector<std::vector<unsigned int>>(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<RecursiveVector>(m, "RecursiveVector"); + py::bind_map<RecursiveMap>(m, "RecursiveMap"); + py::bind_map<MutuallyRecursiveContainerPairMV>(m, "MutuallyRecursiveContainerPairMV"); + py::bind_vector<MutuallyRecursiveContainerPairVM>(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] |