Narrows is_copy_constructible specialization for containers

[sdebionne]

This try to fix a corner use case that I came across where the type to bind looks like a container (e.g. it has value_type and reference with reference = value_type&) but it is more like a wrapper, something like:

template <typename T>
struct pixel
{
	typedef pixel value_type;
	typedef value_type& reference;
};

In this case, is_copy_constructible is called recursively with the same type and the compiler fails with invalid use of incomplete type 'struct is_copy_constructible<pixel<char> >'.

This PR narrows the availability of is_copy_constructible partial specialization for containers to types that really look like containers.

[bstaletic]

In this case, is_copy_constructible is called recursively with the same type and the compiler fails with invalid use of incomplete type 'struct is_copy_constructible<pixel >'.

This doesn't seem right, because the struct you posted is copy constructible, even with T = std::unique_ptr<int>.

https://godbolt.org/z/cDqoWW

[sdebionne]

This issue is about pybind11 is_copy_constructible. Here is a godbolt that illustrates the issue and the fix (just uncomment #define FIX):

https://godbolt.org/z/LMrfyh

GCC gives invalid use of incomplete type 'struct is_copy_constructible<pixel<char> > while MSVC gives a more sensible 'is_copy_constructible<pixel<char>,void>': a class cannot be its own base class.

[sdebionne]

@wjakob Any chance that this little patch could be considered for 2.3.1, please?

[wjakob]

Hi @sdebionne,

I understand the issue, but your fix is much too heavy-weight. Wouldn't the simplest way to defuse this be to add an extra check

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

?

[sdebionne]

@wjakob This is what I had in mind in the first place, but then I thought that, since the specialization if for containers, I should make it more specific (and heavy-weight). It might help to avoid ambiguities if other is_copy_constructible specializations are needed in the future. Now your suggestion is good enough for me, so I can revert to that if it gets better feedback, no problem.

[wjakob]

Yes, let's stick with the simpler variant.

[wjakob]

hrrm, or let me think a bit more about it :-)

[wjakob]

Yes, let's stick with the simpler variant here. The specific problem is really just that we don't want an infinite recursion, not that the type does not follow the full container API.

[sdebionne]

Done!

[wjakob]

You'll have to find a workaround for std::negation<>, see the failures on Travis.

[sdebionne]

My bad, I forgot it was ccp17. It looks like there is an implementation in pybind11/detail/common.h. Trying that now.

[wjakob]

Thanks!