std::vector::push_back a non-copyable object gives compiler error

Question

I get compilation errors on g++ (GCC) 4.7.2 but not on MSVC-2012 when trying to std::vector::push_back a non-copyable (private copy constructor) but moveable object. To me my example looks identical to many other examples on SO and elsewhere. The error message makes it looks like a problem with the struct not being 'direct constructible' - I don't know what this means so am doubly unsure about why an object needs to be 'direct constructible' to be pushed back.

#include <vector>
#include <memory>

struct MyStruct
{

    MyStruct(std::unique_ptr<int> p);
    MyStruct(MyStruct&& other);
    MyStruct&  operator=(MyStruct&& other);

    std::unique_ptr<int> mP;

private:
            // Non-copyable
    MyStruct(const MyStruct&);
    MyStruct& operator=(const MyStruct& other);
};

int main()
{

    MyStruct s(std::unique_ptr<int>(new int(5)));
    std::vector<MyStruct> v;

    auto other = std::move(s);       // Test it is moveable
    v.push_back(std::move(other));   // Fails to compile

    return 0;
}

Gives errors

/usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../include/c++/4.7.2/type_traits: In instantiation of ‘struct std::__is_direct_constructible_impl<MyStruct, const MyStruct&>’:
... snip ...
/usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../include/c++/4.7.2/bits/stl_vector.h:900:9:   required from ‘void std::vector<_Tp, _Alloc>::push_back(std::vector<_Tp, _Alloc>::value_type&&) [with _Tp = MyStruct; _Alloc = std::allocator<MyStruct>; std::vector<_Tp, _Alloc>::value_type = MyStruct]’
main.cpp:27:33:   required from here
main.cpp:16:5: error: ‘MyStruct::MyStruct(const MyStruct&)’ is private

Simple workaround from various answers:

• Use MyStruct(const MyStruct&) = delete; instead of private ctor hack
• Inherit boost::noncopyable (or another class with private ctor)

Answer 1

The failure is due to a limitation of G++ 4.7, which doesn't implement DR 1170, which was changed very late in the C++11 standardisation process to say that access checking should be done as part of template argument deduction.

The underlying cause is that libstdc++'s vector will move elements if the move operation is guaranteed not to throw (i.e. it's declared noexcept or throw()), otherwise if the type is copyable the elements will be copied, otherwise if the type is not copyable but does have a possibly-throwing move operation then it will be moved (and if an exception is thrown the results are undefined unspecified.) This is implemented with checks to the is_nothrow_move_constructible and is_copy_constructible type traits. In your case, the type is not nothrow move constructible, so the is_copy_constructible trait is checked. Your type has a copy constructor but it's not accessible, so the is_copy_constructible trait produces a compiler error with G++ 4.7 because access checking is not done during template argument deduction.

If you make your move constructor and move assignment operator noexcept then the type will be moved and doesn't need to be copyable, so the is_copy_constructible trait that fails is not used, and the code compiles OK.

Alternatively, (as also stated in the comments) if you make the copy constructor deleted then the is_copy_constructible trait gets the right result.

Another alternative is to use something like boost::noncopyable which implicitly makes the copy constructor deleted so the is_copy_constructible trait works properly (and also works with older compilers like MSVC that don't support deleted functions properly). I don't know what you mean about making it impossible to find the error, does MSVC not show you the full context of a compiler error?

Conclusion: use unique_ptr where appropriate but don't make classes explicitly movable

I disagree with this conclusion, it is too extreme. Instead make your classes nothrow movable whenever possible. Also, when possible, use deleted functions to make a type non-copyable instead of private+unimplemented functions, maybe using a macro for portability to older compilers e.g.

#if __cplusplus >= 201103L
#define NONCOPYABLE(TYPE) \
  TYPE(const TYPE&) = delete; TYPE& operator=(const TYPE&) = delete
#else
// must be used in private access region
#define NONCOPYABLE(TYPE) \
  TYPE(const TYPE&); TYPE& operator=(const TYPE&)
#endif

struct MyStruct
{
...
private:
    NONCOPYABLE(MyStruct);
};