Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

binding std::vector of non copy/move-constructible elements #487

Closed
albertziegenhagel opened this issue Nov 8, 2016 · 3 comments
Closed

Comments

@albertziegenhagel
Copy link

albertziegenhagel commented Nov 8, 2016

The following code fails to compile (tested with VS 2015 Update 3 and Apple LLVM version 8.0.0, which should be clang ~3.8):

class El2
{
public:
    El2(const El2&) = delete;
    El2& operator=(const El2&) = delete;
};

py::bind_vector<std::vector<El>>(m, "VectorEl");

This is because the tests in

#if !defined(_MSC_VER)
/* Only enabled when the types are {copy,move}-constructible *and* when the type
does not have a private operator new implementaton. */
template <typename T = type> static auto make_copy_constructor(const T *value) -> decltype(new T(*value), Constructor(nullptr)) {
return [](const void *arg) -> void * { return new T(*((const T *) arg)); }; }
template <typename T = type> static auto make_move_constructor(const T *value) -> decltype(new T(std::move(*((T *) value))), Constructor(nullptr)) {
return [](const void *arg) -> void * { return (void *) new T(std::move(*((T *) arg))); }; }
#else
/* Visual Studio 2015's SFINAE implementation doesn't yet handle the above robustly in all situations.
Use a workaround that only tests for constructibility for now. */
template <typename T = type, typename = enable_if_t<std::is_copy_constructible<T>::value>>
static Constructor make_copy_constructor(const T *value) {
return [](const void *arg) -> void * { return new T(*((const T *)arg)); }; }
template <typename T = type, typename = enable_if_t<std::is_move_constructible<T>::value>>
static Constructor make_move_constructor(const T *value) {
return [](const void *arg) -> void * { return (void *) new T(std::move(*((T *)arg))); }; }
#endif
check whether the vector is copy/move-constructible only, but not whether the element stored in the vector is copy/move-constructible as well. In the current C++ standard std::vector has a copy/move constructor even if it's value type is not copy/move constructible.

See http://stackoverflow.com/questions/18404108/issue-with-is-copy-constructible for some discussion, and a possible workaround, about that topic.

Please note that this may be of special interest because std::vector<std::unique_ptr<T>> is never copy-constructible but may be a common use case.

@jagerman
Copy link
Member

Is there any useful purpose to a std::vector<T> with T neither copyable nor movable, as in your example? As far as I can see, since T is neither MoveInsertable nor CopyInsertable, I can't see how such a container could ever be something other than empty, since every vector method that modifies the vector requires at least one of these.

Or is this bug really about a move-constructible, but not copy-constructible, vector value type?

@jagerman
Copy link
Member

See #490 for a fix for this (for movable, non-copyable types). Because there really is no concept of moving in Python, we really can't actually modify such a vector: when we get a value from Python, we can only copy and reference it, but we can't move it away, so there's basically no way to insert from Python.

That doesn't mean you can't insert at all: just that you have to handle any modification support yourself (i.e. by defining approprimate methods in the class_ object bind_vector returns), because any modification support is going to involve value_type-specific construction/emplacement.

@albertziegenhagel
Copy link
Author

Is there any useful purpose to a std::vector<T> with T neither copyable nor movable, as in your example?

As for me there is no use case for a neither copyable nor movable T. I was just mentioning it because std::is_move_constructible suffers from the same problems as std::is_copy_constructible when used on a std::vector and I thought it may be important to take this into account while trying to fix the issue.

And I totally understand that inserting into a vector with move-only type is not straight forward when done from python.

Anyway it is great to see this getting fixed. I appreciate your work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants