Introduce recursive_container_traits #4623

franzpoeschel · 2023-04-14T08:52:10Z

Description

This fix is necessary when trying to bind recursive map types such as:

struct RecursiveMap : std::map<int, RecursiveMap> {}

The old check (negation<std::is_same<Container, typename Container::value_type>>) is insufficient here, as Container::value_type is std::pair<int, RecursiveMap>. The check needs to additionally consider Container::mapped_type.

This PR introduces a new trait recursive_container_traits that defines a dependent type recursive_container_traits<Container>::type_to_check_recursively that guides the recursion scheme in recursively defined traits such as is_move_assignable and similar.

More complicated self-referential patterns (such as mutually referential lists and maps) are considered by this PR, but require manual intervention by the downstream user. The type_to_check_recursively needs to be manually specified. An example is given in tests/test_stl_binders.cpp.

Suggested changelog entry:

Introduce recursive_container_traits

Skylion007 · 2023-04-14T17:21:57Z

@franzpoeschel Can you add test to our test suite that compiles (and maybe uses) one of these recursive types?

rwgk · 2023-04-14T17:49:33Z

@franzpoeschel Can you add test to our test suite that compiles (and maybe uses) one of these recursive types?

I took a quick look and have questions about details, but without any tests I want to wait.

I'll look again when there are new test cases that fail without changes. (In the long run the tests are more important than the fix.)

franzpoeschel · 2023-04-14T19:14:49Z

Is there a test where adding this would logically make sense or should I just create a new one?

rwgk · 2023-04-14T19:26:10Z

Is there a test where adding this would logically make sense or should I just create a new one?

Adding is a little easier to get started.

First quick guess: test_stl.cpp,py

We can easily move the new test code elsewhere at the end, when we see the full picture.

FWIW

If I anticipate I may need to experiment a lot, I usually start a new test, to not have to deal with the overhead & distractions of existing tests in the same files.

franzpoeschel · 2023-04-14T19:40:02Z

Thanks for the guidance!
I don't expect much experimentation as this fix is for a very clearly-defined use case, so the STL test sounds like a good fit.
I will continue with this next week.

franzpoeschel · 2023-04-17T13:30:06Z

include/pybind11/stl_bind.h

+struct is_comparable<T,
+                     enable_if_t<container_traits<T>::is_vector &&
+                                 // Avoid this instantiation if the type is recursive
+                                 negation<std::is_same<T, typename T::value_type>>::value>> {


The comment above the specialization mentions map types, but the enable_if condition requires vector types.
Hence, I only added the simple check here.

franzpoeschel · 2023-04-18T08:29:41Z

I have added a test, and additionally an optional PR that shows the boundaries of the recursion check and an alternative way to bind more complex recursive map/vector types.
I think, someone needs to start the CI since I'm not approved to (last week, it started automatically after the precommit bot's push)

rwgk · 2023-04-18T14:10:41Z

someone needs to start the CI
Done

(will look at the code later today)

franzpoeschel · 2023-04-18T15:43:00Z

tests/test_stl_binders.cpp

+    py::bind_vector<RecursiveVector>(m, "RecursiveVector");
+    py::bind_map<RecursiveMap>(m, "RecursiveMap");
+    py::bind_map<MutuallyRecursiveMap>(m, "MutuallyRecursiveMap");
+    py::bind_vector<MutuallyRecursiveVector>(m, "MutuallyRecursiveVector");


It seems like these need to go before this block:

// The rest depends on numpy: try { py::module_::import("numpy"); } catch (...) { return; }

… to avoid the CI failures:

def test_recursive_containers(): > recursive_vector = m.RecursiveVector() E AttributeError: module 'pybind11_tests.stl_binders' has no attribute 'RecursiveVector'

It seems like these need to go before this block:

At first glance: that doesn't sound good. "Something else is probably wrong, somewhere."

I don't think that there is a large problem. I just previously put the bindings in a place that was only active if numpy is available, so any test without numpy failed as a consequence. Should be fixed now.

Yes, no problem at all: looking now at your latest commit, I misunderstood the before/after in your previous comment.

rwgk · 2023-04-19T16:06:55Z

(Waiting for CI to finish. The 1 failure is almost certainly a flake. I'll rerun.)

rwgk · 2023-04-20T11:29:30Z

include/pybind11/detail/type_caster_base.h

@@ -827,6 +827,18 @@ using movable_cast_op_type
 template <typename T, typename SFINAE = void>
 struct is_copy_constructible : std::is_copy_constructible<T> {};

+// True if Container has a dependent type mapped_type that is equivalent


Could we generalize this and at the same time make this more uniform?

But a silly thing first: the naming here completely throws me off. It kind-of makes sense with my mind in the context (rare occasion), but imaging myself skimming through this while debugging or working on something completely different (the usual situation), my snap-judgement of what this is doing would go in a completely wrong direction.

I'll use is_container_with_self_referential_mapped_type below.

Currently we have two special cases:

std::is_same<Container, Container::value_type>

is_container_with_self_referential_mapped_type<Container>

Could we turn this into

is_self_referential_container_type<Container>

that

invokes the other two?

can be specialized by users?

struct is_self_referential_container_type<MySelfReferentialContainerType> : std::true_type {};

I have done this and I like the way that it turned out, especially since adding custom user specializations now involves overriding a single struct, which just tells Pybind that this type is recursive:

namespace pybind11 { namespace detail { template <> struct is_container_with_self_referential_mapped_type<MutuallyRecursiveContainerPairB> : std::true_type {}; template <> struct is_container_with_self_referential_mapped_type<MutuallyRecursiveContainerPairA> : std::true_type {}; } // namespace detail } // namespace pybind11

rwgk · 2023-04-20T11:31:57Z

include/pybind11/detail/type_caster_base.h

+// to Container itself
+template <typename Container, typename MappedType = Container>
+struct map_self_referential {
+    constexpr static bool value = false;


Could we inherit from std::false_type here (and std::true_type below)? I think it would be better/more readable to do that consistently, at least in all new or changed code.

Sure, I only found out that you typically use these after writing the above code. Changed it.

rwgk · 2023-04-20T11:35:20Z

tests/test_stl_binders.cpp

+/*
+ * Pybind11 does not catch more complicated recursion schemes, such as mutual
+ * recursion.
+ * In that case, an alternative is to add a custom overload.


Did you mean "custom template specialization"? (The term "overload" is usually only used for function overloads.)

Yes, that's what I meant, it's updated now.

rwgk · 2023-04-20T11:40:27Z

tests/test_stl_binders.cpp

+namespace pybind11 {
+namespace detail {
+template <>
+struct is_comparable<MutuallyRecursiveVector, void> : std::true_type {};


It's unusual to specify the conventional SFINAE void in specializations, could that be omitted here (and everywhere below)?

I've separated the SFINAE type now from the publically specializable type:

template <typename Container, typename MappedType = Container> struct sfinae_is_container_with_self_referential_mapped_type : std::false_type {}; // ... specializations for mapped_type and value_type ... // wrapper type without SFINAE parameter template <typename Container> struct is_container_with_self_referential_mapped_type : sfinae_is_container_with_self_referential_mapped_type<Container> {};

This gives the struct a cleaner public API for specialization purposes, and the SFINAE parameter can be publically ignored.

rwgk · 2023-04-20T11:52:05Z

tests/test_stl_binders.cpp

+ * In that case, an alternative is to add a custom overload.
+ */
+struct MutuallyRecursiveMap;
+struct MutuallyRecursiveVector;


It's only test code, but anyway, careful naming in tests is important, too.

What do you think about

struct MutuallyRecursiveContainerPairA; struct MutuallyRecursiveContainerPairB;

?

Two motivations:

Bake "pair" into the name. (Only a pair can have something mutual.)

Bake in a hint that Map and Vector are more-or-less arbitrary choices for the purpose of testing something more general.

These are long names, but they are also very clear at first sight, which I think is best for something that only ever appears in a few places.

I've used these suggestions now. (Personally, I would prefer keeping the reference to "vector" and "map", since that information is very relevant in using the types later on. But since you're the maintainer, this is your decision more than it is mine.)

How about making this

struct MutuallyRecursiveContainerPairMV; struct MutuallyRecursiveContainerPairVM;

?
I.e. a compromise. The Pair is there, and M referencing V, V referencing M (I think M and V are clear enough, we don't need to spell out MutuallyRecursiveContainerPairMapVector, MutuallyRecursiveContainerPairVectorMap).

Personally, I find this far less confusing, I'm using that now.

tests/test_stl_binders.py

rwgk · 2023-04-20T12:05:00Z

tests/test_stl_binders.py

+
+    recursive_map = m.RecursiveMap()
+    recursive_map[1] = m.RecursiveMap()
+    recursive_map[1][1] = m.RecursiveMap()


Test failures later (new compiler with bugs, refactoring with bugs) will be easier to debug if you only go as deep as you need to. I think that's just one level here. The extra level is is also useful as an additional stress test, but I'd definitely not skip the more minimal easier to debug test. I think it's only a couple lines extra (assert list(recursive_map[1].keys()) == [1, 2]).

In a similar vein, although slightly on the paranoid side, using values less special than 0, 1, 2 can help a lot in troubleshooting later, e.g. 101, 202, anything that's more unique, easier to pin-point in a pile of debug output, and less likely to appear by accident.

I've reduced the recursion depth for each test and used more Ctrl+F-friendly numbers for the RecursiveMap-test now

rwgk · 2023-04-21T19:37:59Z

include/pybind11/detail/type_caster_base.h

+
+// Use a helper struct in order to give this a nicer public API without helper template parameter.
+// This makes this struct nicer to specialize by users.
+template <typename Container>


template <typename Container, typename SFINAE = void>

We don't want SFINAE to stop here, users may still need enable_if for their own types.

Ok, but I've now done it in such a way that the SFINAE parameter of the publically specializable struct defaults to void as is usual for SFINAE.

rwgk · 2023-04-21T19:42:25Z

include/pybind11/detail/type_caster_base.h

+// Use a helper struct in order to give this a nicer public API without helper template parameter.
+// This makes this struct nicer to specialize by users.
+template <typename Container>
+struct is_container_with_self_referential_mapped_type


Not sure, but tossing this around in my mind, I feel

is_recursive_container

would be great. It takes a small mental leap back to "self-referential", vs a small leap forward from "self-referential" to "recursive". "recursive" is shorter, a little more intuitive at least in my mind, and we don't have the trouble with mapped_type vs value_type, which is is more-or-less a case-specific detail.

rwgk · 2023-04-21T19:46:43Z

include/pybind11/detail/type_caster_base.h

+// The specializations are only valid if both conditions are fulfilled:
+// 1) The mapped_type (respectively value_type) exists
+// 2) And it is equivalent to Container
+// So, in each case, only one specialization will activate.


That's a really neat trick.

What do you think about is_recursive_container_common as a name (see other comment for the is_recursive_container part).

As you noticed below, there is a catch to the trick, so I've split this in two now for structs like

struct Self { using mapped_type = Self; using value_type = Self; };

I like is_recursive_container, using that now

rwgk · 2023-04-21T19:57:04Z

include/pybind11/detail/type_caster_base.h

+
+template <typename Container>
+struct sfinae_is_container_with_self_referential_mapped_type<Container,
+                                                             typename Container::value_type>


Mainly just disclosing that I'm not sure about something: could there ever be a non-non-sensical container that has both mapped_type and value_type? And what would happen then?

Maybe that's a non-sense thought? And in the worst case we could fix when we actually get into trouble.

But also thinking: do we really need this helper type? Totally untested:

struct is_recursive_container<Container, enable_if_t<std::is_same<Container, Container::value_type>::value>> : std::true_type; struct is_recursive_container<Container, enable_if_t<std::is_same<Container, Container::mapped_type>::value>> : std::true_type;

or

struct is_recursive_container<Container, enable_if_t<!std::is_same<Container, Container::value_type>::value && std::is_same<Container, Container::mapped_type>::value>> : std::true_type;

in case that could ever happen?

Yes, you are right. And it's not so nonsensical either, I can imagine that some vector-like structs exist somewhere, that additionally define mapped_type for some reason.

The following struct fails selecting a correct instantiation:

struct Self { using mapped_type = Self; using value_type = Self; };

The reason why we need helper types is because vector types don't define a mapped_type, so SFINAE is necessary to check if that even exists. value_type always exists for STL containers, but at that point I'd say it's a matter of consistency.

I'm using two different helper structs for mapped_type and value_type now and combining them via any_of, so there is no need at all for tie breaking:

// True if Container has a dependent type mapped_type that is equivalent // to Container itself // Actual implementation in the SFINAE specialization below template <typename Container, typename MappedType = Container> struct is_container_with_recursive_mapped_type : std::false_type {}; // This specialization is only valid if both conditions are fulfilled: // 1) The mapped_type exists // 2) And it is equivalent to Container template <typename Container> struct is_container_with_recursive_mapped_type<Container, typename Container::mapped_type> : std::true_type {}; // True if Container has a dependent type value_type that is equivalent // to Container itself // Actual implementation in the SFINAE specialization below template <typename Container, typename MappedType = Container> struct is_container_with_recursive_value_type : std::false_type {}; // This specialization is only valid if both conditions are fulfilled: // 1) The value_type exists // 2) And it is equivalent to Container template <typename Container> struct is_container_with_recursive_value_type<Container, typename Container::value_type> : std::true_type {}; // True constant if the type contains itself recursively. // By default, this will check the mapped_type and value_type dependent types. // In more complex recursion patterns, users can specialize this struct. // The second template parameter SFINAE=void is for use of std::enable_if in specializations. // An example is found in tests/test_stl_binders.cpp. template <typename Container, typename SFINAE = void> struct is_recursive_container_type : any_of<is_container_with_recursive_value_type<Container>, is_container_with_recursive_mapped_type<Container>> {};

1) Apply to is_copy_assignable additionally 2) Check infinite recursion for map-like types

The bindings were previously in a block that was only activated if numpy was available.

1) Renaming: is_recursive_container and MutuallyRecursiveContainerPair(MV|VM) 2) Avoid ambiguous specializations of is_recursive_container

franzpoeschel · 2023-04-24T13:36:26Z

The master branch has newly introduced is_move_constructible, which does not do any kind of recursive checks, but caused a conflict with this branch.
I have rebased the branch now, leaving is_move_constructible untouched.

rwgk · 2023-04-24T14:54:43Z

The master branch has newly introduced is_move_constructible, which does not do any kind of recursive checks, but caused a conflict with this branch. I have rebased the branch now, leaving is_move_constructible untouched.

Oh, gee, sorry, I was totally sleeping on the wheel there. (Interesting coincidence!)

Could you please add the recursive checks to is_move_constructible as well?

rwgk

A couple small things I noticed glancing through.

I only have a tiny (laptop) screen right now and can look carefully only later today.

Overall, this looks really great already.

rwgk · 2023-04-24T15:02:43Z

tests/test_stl_binders.cpp

+/*
+ * Pybind11 does not catch more complicated recursion schemes, such as mutual
+ * recursion.
+ * In that case, an alternative is to add a custom specialization to


In that case custom is_recursive_container specializations need to be added.

rwgk · 2023-04-24T15:04:25Z

include/pybind11/stl_bind.h

+                     enable_if_t<container_traits<T>::is_vector &&
+                                 // Special case: The vector type is recursive
+                                 std::is_same<T, typename T::value_type>::value>> {
+    static constexpr const bool value = true;


true_type?

Here it was me sleeping on the wheel:

This is now:

template <typename T> struct is_comparable<T, enable_if_t<container_traits<T>::is_vector && // Special case: The vector type is recursive is_recursive_container<T>::value>> { static constexpr const bool value = container_traits<T>::is_comparable; };

Skylion007 · 2023-04-26T13:47:18Z

tests/test_stl_binders.cpp

@@ -129,6 +163,12 @@ TEST_SUBMODULE(stl_binders, m) {
            m, "VectorUndeclStruct", py::buffer_protocol());
    });

+    // Bind recursive container types
+    py::bind_vector<RecursiveVector>(m, "RecursiveVector");


Do these tests cover the detail:is_move_constructable templates too?

I have added static testing for the pybind11 type traits now (and coincidentally, uncovered a slight bug in pybind11::detail::is_copy_assignable)

franzpoeschel · 2023-04-28T09:26:38Z

Very nice, thanks a lot!

Thank you too for your help in shaping this PR!

For context: We have been using pybind11 for a long time now in our openPMD-api project, and the larger share of our users likely use it via the Pybind11-shaped Python bindings.
I am currently experimenting with an extension that will allow arbitrary-depth user-defined hierarchical structures, where we will need support for recursive data structures in Pybind11.

rwgk

Awesome! The extra tests look like they were a lot of leg work, thanks for being so thorough!

rwgk · 2023-04-28T14:34:47Z

tests/test_copy_move.cpp

+
+/*
+ * Rest of the file:
+ * static_assert based tests for pybind11 adaptations of of


rwgk · 2023-04-28T14:53:50Z

include/pybind11/detail/type_caster_base.h

+     * Need to remove the const qualifier from T1 here since the value_type in
+     * STL map types is std::pair<const Key, T>, and const types are never assignable
+     */
+    : all_of<is_copy_assignable<typename std::remove_const<T1>::type>, is_copy_assignable<T2>> {};


The asymmetry here seems odd, did you already think this through?

Concretely, would it make sense to apply std::remove_const also to T2?

Because for std::pair by itself (not considering how it's used elsewhere), the order of T1 and T2 doesn't have a function AFAIK.

Which leads me to think: It has to be either none or both.

Or we actually have a trait with subtly different semantics compared to the "pure" is_copy_assignable.

Agreed on this point, the asymmetry is a bit odd.

~~PLEASE DO NOT REVIEW YET, I need to go over this on my own again first.~~

Fixing this required changing the logic of this PR quite a bit.
The trouble of is_recursive_container<T> is that it only tells you that the type is recursive at all and then tells you to deal with it somehow. What I did now instead was replacing that with a recursive_container_traits that has a dependent type recursive_container_traits<Container>::type_to_check_recursively:

/* * 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 {}; template <typename Container, typename SFINAE = void> struct impl_recursive_container_traits { using type_to_check_recursively = recursive_bottom; }; // and its main logic in the specialization

The type_to_check_recursively is defined such:

By default, it is recursive_bottom, so that the recursion is canceled.

If the type is non-recursive and defines a value_type, then the value_type is used. If the value_type is a pair, the const` is removed from the first type.

If the type is recursive and value_type is not a pair, then recursive_bottom is returned.

If the type is recursive and value_type is a pair, then const is removed from the first type and the first type is returned.

Having this trait makes the definitions of custom traits such as is_move_constructible very straightforward and no longer requires SFINAE constructs there:

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>> {};

Fixing this required changing the logic of this PR quite a bit.

Oh, wow. Thanks a lot for going the extra mile!

I've fixed some comments now, should be ready for review :)

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.

rwgk

This looks really awesome now. I could only spend 15 minutes right now, I need to find a block of time to go through more carefully (asap).

rwgk · 2023-05-02T16:46:05Z

include/pybind11/detail/type_caster_base.h

+template <bool Condition, typename Then, typename Else>
+struct if_then_else {};

-template <typename T, typename SFINAE = void>
-struct is_move_constructible : std::is_move_constructible<T> {};
+template <typename Then, typename Else>
+struct if_then_else<true, Then, Else> {
+    using type = Then;
+};
+
+template <typename Then, typename Else>
+struct if_then_else<false, Then, Else> {
+    using type = Else;
+};


Looks like you reinvented std::conditional.

Also, we have conditional_t in pybind11/detail/common.h.

I like if_then_else as a name a lot better than conditional btw, but standard is standard :-)

1. Use std::conditional 2. Fix typo

rwgk

I'm running this through Google's global testing, to guard against surprises. That will take 6-8 hours to complete.

If you get a chance to update the PR title & description that would be great. Maybe the title could be "Introduce recursive_container_traits".

For the record: I wish recursive_container_traits lived in it's own include file, but looking through the changes, I got to think that's beyond of what can be achieved in one PR, given the current state of the code organization.

rwgk · 2023-05-04T14:07:10Z

tests/test_stl_binders.cpp

@@ -10,6 +10,7 @@
 #include <pybind11/numpy.h>
 #include <pybind11/stl_bind.h>

+#include "pybind11/detail/type_caster_base.h"


Could you please remove this include? To not encourage others to include a detail header. — The pybind11 code organization has me wishing a lot. Improving the organization will be harder if we implicitly suggest that the detail headers are part of the public interface.

Sure, I accidentally left it there before committing.

franzpoeschel · 2023-05-04T14:23:14Z

I'm running this through Google's global testing, to guard against surprises. That will take 6-8 hours to complete.

If you get a chance to update the PR title & description that would be great. Maybe the title could be "Introduce recursive_container_traits".

Done

For the record: I wish recursive_container_traits lived in it's own include file, but looking through the changes, I got to think that's beyond of what can be achieved in one PR, given the current state of the code organization.

As I did not really know the code structure of Pybind, I introduced new structs/traits where I needed to use them, which is probably not necessarily the best placement. If things should be moved within the scope of this PR, we can do it.
(Also, I think you should be able to push to the branch if you prefer doing this yourself)

rwgk · 2023-05-04T14:35:06Z

If things should be moved within the scope of this PR,

If you want to give it a try, I'l re-review.

When going through the motions just in my mind I got to think it's to difficult to untangle the new code from the existing traits (is_copy_constructible etc.). Maybe I'm too pessimistic. But then again, I think the current state is a great milestone to maybe improve from in follow-on work. Also, there are far bigger organizational problems (e.g. cast.h and type_caster_base.h far too monolithic), is a focus on recursive_container_traits actually the best starting point for improving the code organization? I doubt it.

franzpoeschel · 2023-05-04T15:35:35Z

In that case, I'd leave things in the current state.

rwgk · 2023-05-05T05:38:11Z

Global testing found no issues (internal test ID is (OCL:529394047:BASE:529572152:1683249617215:288a60c7).

The one GHA failure here is a notorious test_iostream flake.

Thanks for the great work!

rwgk · 2023-05-05T05:43:14Z

Are these two sentences in the PR description still accurate?

Since mapped_type does not exist for all container-like types, this uses SFINAE in the helper struct map_self_referential.

More complicated self-referential patterns (such as mutually referential lists and maps) are not considered by this PR.

Maybe it's better to point to the now-merged test code?

franzpoeschel · 2023-05-05T14:37:55Z

I have updated the PR description to the final state of the PR.
Thanks again for the help!

rwgk · 2023-05-05T14:39:46Z

I have updated the PR description to the final state of the PR. Thanks again for the help!

Awesome, thanks!

Skylion007 requested review from rwgk and henryiii April 14, 2023 14:00

franzpoeschel force-pushed the fix-infinite-recursion-check branch from d7df924 to 4e9e079 Compare April 17, 2023 13:27

franzpoeschel commented Apr 17, 2023

View reviewed changes

franzpoeschel force-pushed the fix-infinite-recursion-check branch from a2ef8ec to 737accc Compare April 17, 2023 13:55

franzpoeschel commented Apr 18, 2023

View reviewed changes

rwgk reviewed Apr 20, 2023

View reviewed changes

rwgk reviewed Apr 21, 2023

View reviewed changes

franzpoeschel and others added 9 commits April 24, 2023 15:30

Testing

541c360

Similar fix for std::vector

2c6002c

Fix infinite recursion check:

4c59860

1) Apply to is_copy_assignable additionally 2) Check infinite recursion for map-like types

style: pre-commit fixes

2c48ca9

Optional commit that demonstrates the limitations of this PR

8d35d80

Fix positioning of container bindings

79782b5

The bindings were previously in a block that was only activated if numpy was available.

Suggestions from code review: API side

ba0aadf

Suggestions from code review: Test side

0ecf5f0

Suggestions from code review

9ba38bf

1) Renaming: is_recursive_container and MutuallyRecursiveContainerPair(MV|VM) 2) Avoid ambiguous specializations of is_recursive_container

franzpoeschel force-pushed the fix-infinite-recursion-check branch from d087c32 to 9ba38bf Compare April 24, 2023 13:35

rwgk reviewed Apr 24, 2023

View reviewed changes

Some little fixes

531ceb4

rwgk requested a review from Skylion007 April 25, 2023 22:46

Skylion007 reviewed Apr 26, 2023

View reviewed changes

Skylion007 approved these changes Apr 26, 2023

View reviewed changes

Static testing for pybind11 type traits

eba6239

rwgk approved these changes Apr 28, 2023

View reviewed changes

franzpoeschel force-pushed the fix-infinite-recursion-check branch 2 times, most recently from 46cfdbb to 517b48e Compare May 2, 2023 14:55

franzpoeschel force-pushed the fix-infinite-recursion-check branch from 517b48e to 77e2c56 Compare May 2, 2023 15:18

rwgk reviewed May 2, 2023

View reviewed changes

franzpoeschel mentioned this pull request May 3, 2023

Custom Hierarchies openPMD/openPMD-api#1432

Open

12 tasks

Review suggestions

3ed36b8

1. Use std::conditional 2. Fix typo

rwgk approved these changes May 4, 2023

View reviewed changes

franzpoeschel changed the title ~~type_caster_base.h: extend infinite recursion checks for is_copy_constructible and is_copy_assignable~~ Introduce recursive_container_traits May 4, 2023

Remove leftover include from test

71521c4

rwgk merged commit f701654 into pybind:master May 5, 2023

github-actions bot added the needs changelog Possibly needs a changelog entry label May 5, 2023

rwgk mentioned this pull request May 5, 2023

[smart_holder] git merge master #4653

Merged

rwgk removed the needs changelog Possibly needs a changelog entry label May 22, 2023

franzpoeschel mentioned this pull request Aug 1, 2023

Update Pybind to 2.11.1 openPMD/openPMD-api#1489

Merged

1 task

This was referenced Dec 14, 2023

Stabilizing the build and enabling tests NVIDIA/cuda-quantum#1001

Merged

Upgrade pybind11 to the latest version NVIDIA/cuda-quantum#1046

Closed

Introduce recursive_container_traits #4623

Introduce recursive_container_traits #4623

Conversation

franzpoeschel commented Apr 14, 2023 • edited Loading

Description

Suggested changelog entry:

Skylion007 commented Apr 14, 2023

rwgk commented Apr 14, 2023

franzpoeschel commented Apr 14, 2023

rwgk commented Apr 14, 2023

franzpoeschel commented Apr 14, 2023

Choose a reason for hiding this comment

franzpoeschel commented Apr 18, 2023

rwgk commented Apr 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwgk commented Apr 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwgk Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwgk Apr 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

franzpoeschel commented Apr 24, 2023

rwgk commented Apr 24, 2023

rwgk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

franzpoeschel commented Apr 28, 2023

rwgk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

franzpoeschel May 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwgk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwgk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

franzpoeschel commented May 4, 2023 • edited Loading

rwgk commented May 4, 2023

franzpoeschel commented May 4, 2023

rwgk commented May 5, 2023

rwgk commented May 5, 2023

franzpoeschel commented May 5, 2023

rwgk commented May 5, 2023

franzpoeschel commented Apr 14, 2023 •

edited

Loading

rwgk Apr 20, 2023 •

edited

Loading

rwgk Apr 21, 2023 •

edited

Loading

franzpoeschel May 2, 2023 •

edited

Loading

franzpoeschel commented May 4, 2023 •

edited

Loading