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