Why do QString and vector> appear incompatible here?

Question

I'm trying to compile some code, which reduces to this:

#include <memory>
#include <vector>
#include <QString>

class Category
{
    std::vector<std::unique_ptr<int>> data;
    QString name;
};

int main()
{
    std::vector<Category> categories;
    categories.emplace_back();
};

Compiled as is, it results in the following error from g++ and similar for clang++:

In file included from /opt/gcc-4.8/include/c++/4.8.2/memory:64:0,
                 from test.cpp:1:
/opt/gcc-4.8/include/c++/4.8.2/bits/stl_construct.h: In instantiation of ‘void std::_Construct(_T1*, _Args&& ...) [with _T1 = std::unique_ptr<int>; _Args = {const std::unique_ptr<int, std::default_delete<int> >&}]’:
/opt/gcc-4.8/include/c++/4.8.2/bits/stl_uninitialized.h:75:53:   required from ‘static _ForwardIterator std::__uninitialized_copy<_TrivialValueTypes>::__uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = __gnu_cxx::__normal_iterator<const std::unique_ptr<int>*, std::vector<std::unique_ptr<int> > >; _ForwardIterator = std::unique_ptr<int>*; bool _TrivialValueTypes = false]’
/opt/gcc-4.8/include/c++/4.8.2/bits/stl_uninitialized.h:117:41:   required from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = __gnu_cxx::__normal_iterator<const std::unique_ptr<int>*, std::vector<std::unique_ptr<int> > >; _ForwardIterator = std::unique_ptr<int>*]’
/opt/gcc-4.8/include/c++/4.8.2/bits/stl_uninitialized.h:258:63:   required from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, std::allocator<_Tp>&) [with _InputIterator = __gnu_cxx::__normal_iterator<const std::unique_ptr<int>*, std::vector<std::unique_ptr<int> > >; _ForwardIterator = std::unique_ptr<int>*; _Tp = std::unique_ptr<int>]’
/opt/gcc-4.8/include/c++/4.8.2/bits/stl_vector.h:316:32:   required from ‘std::vector<_Tp, _Alloc>::vector(const std::vector<_Tp, _Alloc>&) [with _Tp = std::unique_ptr<int>; _Alloc = std::allocator<std::unique_ptr<int> >]’
test.cpp:5:7:   [ skipping 2 instantiation contexts, use -ftemplate-backtrace-limit=0 to disable ]
/opt/gcc-4.8/include/c++/4.8.2/bits/stl_uninitialized.h:117:41:   required from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = Category*; _ForwardIterator = Category*]’
/opt/gcc-4.8/include/c++/4.8.2/bits/stl_uninitialized.h:258:63:   required from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, std::allocator<_Tp>&) [with _InputIterator = Category*; _ForwardIterator = Category*; _Tp = Category]’
/opt/gcc-4.8/include/c++/4.8.2/bits/stl_uninitialized.h:281:69:   required from ‘_ForwardIterator std::__uninitialized_move_if_noexcept_a(_InputIterator, _InputIterator, _ForwardIterator, _Allocator&) [with _InputIterator = Category*; _ForwardIterator = Category*; _Allocator = std::allocator<Category>]’
/opt/gcc-4.8/include/c++/4.8.2/bits/vector.tcc:415:43:   required from ‘void std::vector<_Tp, _Alloc>::_M_emplace_back_aux(_Args&& ...) [with _Args = {}; _Tp = Category; _Alloc = std::allocator<Category>]’
/opt/gcc-4.8/include/c++/4.8.2/bits/vector.tcc:101:54:   required from ‘void std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {}; _Tp = Category; _Alloc = std::allocator<Category>]’
test.cpp:14:29:   required from here
/opt/gcc-4.8/include/c++/4.8.2/bits/stl_construct.h:75:7: error: use of deleted function ‘std::unique_ptr<_Tp, _Dp>::unique_ptr(const std::unique_ptr<_Tp, _Dp>&) [with _Tp = int; _Dp = std::default_delete<int>]’
     { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }
       ^
In file included from /opt/gcc-4.8/include/c++/4.8.2/memory:81:0,
                 from test.cpp:1:
/opt/gcc-4.8/include/c++/4.8.2/bits/unique_ptr.h:273:7: error: declared here
       unique_ptr(const unique_ptr&) = delete;
       ^

• If I remove name member from Category, it compiles fine.
• If I make data just a single unique_ptr<int> instead of a vector of pointers, it compiles fine.
• If I create a single Category in main() instead of a creating a vector and doing emplace_back(), it compiles fine.
• If I replace QString with std::string, it compiles fine.

What's going on? What makes this code ill-formed? Is it result of bugs in g++ & clang++?

Answer 1

The key issue here is that std::vector tries to offer the strong exception safety guarantee for as many operations as possible, but, in order to do that, it needs support from the element type. For push_back, emplace_back and friends, the main problem is what happens if a reallocation is necessary, as the existing elements need to be copied / moved to the new storage.

The relevant standard wording is in [23.3.6.5p1]:

Remarks: Causes reallocation if the new size is greater than the old capacity. If no reallocation happens, all the iterators and references before the insertion point remain valid. If an exception is thrown other than by the copy constructor, move constructor, assignment operator, or move assignment operator of T or by any InputIterator operation there are no effects. If an exception is thrown while inserting a single element at the end and T is CopyInsertable or is_nothrow_move_constructible<T>::value is true, there are no effects. Otherwise, if an exception is thrown by the move constructor of a non-CopyInsertable T, the effects are unspecified.

(The original wording in C++11 has been clarified by the resolution of LWG 2252.)

Note that is_nothrow_move_constructible<T>::value == true doesn't necessarily mean that T has a noexcept move constructor; a noexcept copy constructor taking const T& will do as well.

What this means in practice is that, conceptually, a vector implementation typically tries to generate code for one of the following solutions for copying / moving existing elements to the new storage, in decreasing order of preference (T is the element type, and we're interested in class types here):

• If T has a usable (present, not deleted, not ambiguous, accessible, etc.) noexcept move constructor, use it; exceptions cannot be thrown while constructing the elements in the new storage, so there's no need to revert to the previous state.
• Otherwise, if T has a usable copy constructor, noexcept or not, that takes a const T&, use that; even if copying throws an exception, we can revert to the previous state, as the originals are still there, unmodified.
• Otherwise, if T has a usable move constructor that may throw exceptions, use that; however, the strong exception safety guarantee cannot be offered anymore.
• Otherwise, the code doesn't compile.

The above can be achieved by using std::move_if_noexcept or something similar.

---

Let's see what Category offers in terms of constructors. None is declared explicitly, so a default constructor, a copy constructor and a move constructor are implicitly declared.

The copy constructor uses the respective copy constructors of the members:

• data is a std::vector, and vector's copy constructor cannot be noexcept (it generally needs to allocate new memory), so Category's copy constructor cannot be noexcept regardless of what QString has.
• The definition of std::vector<std::unique_ptr<int>>'s copy constructor calls std::unique_ptr<int>'s copy constructor, which is explicitly deleted, but this only affects the definition, which is only instantiated if needed. Overload resolution only needs the declarations, so Category has an implicitly declared copy constructor that will cause a compile error if called.

The move constructor:

• std::vector has a noexcept move constructor (see the note below), so data is not a problem.
• Old versions of QString (before Qt 5.2):
  • A move constructor is not explicitly declared (see Praetorian's comment above), so, because there is an explicitly declared copy constructor, a move constructor will not be implicitly declared at all.
  • The definition of the implicitly declared move constructor of Category will use QString's copy constructor that takes a const QString&, which can bind to rvalues (the constructors for subobjects are chosen using overload resolution).
  • In these old versions, QString's copy constructor is not specified as noexcept, so Category's move constructor can't be noexcept either.
• Since Qt 5.2, QString has an explicitly declared move constructor, which will be used by Category's move constructor. However, before Qt 5.5, QString's move constructor was not noexcept, so Category's move constructor can't be noexcept either.
• Since Qt 5.5, QString's move constructor is specified as noexcept, so Category's move constructor is noexcept as well.

Note that Category does have a move constructor in all cases, but it may not move name, and it may not be noexcept.

---

Given all of the above, we can see that categories.emplace_back() won't generate code that uses Category's move constructor when Qt 4 is used (OP's case), because it's not noexcept. (Of course, there are no existing elements to move in this case, but that's a runtime decision; emplace_back has to include a code path that handles the general case, and that code path has to compile.) So, the generated code calls Category's copy constructor, which causes the compile error.

A solution is to provide a move constructor for Category and mark it noexcept (otherwise it won't help). QString uses copy-on-write anyway, so it's unlikely to throw while copying.

Something like this should work:

class Category
{
   std::vector<std::unique_ptr<int>> data;
   QString name;
public:
   Category() = default;
   Category(const Category&) = default;
   Category(Category&& c) noexcept : data(std::move(c.data)), name(std::move(c.name)) { }
   // assignment operators
};

This will pick up QString's move constructor if declared, and use the copy constructor otherwise (just like the implicitly declared move constructor would). Now that the constructors are user-declared, the assignment operators have to be taken into account as well.

The explanations for bullets 1, 3 and 4 in the question should now be pretty clear. Bullet 2 (make data just a single unique_ptr<int>) is more interesting:

• unique_ptr has a deleted copy constructor; this causes Category's implicitly declared copy constructor to be defined as deleted as well.
• Category's move constructor is still declared as above (not noexcept in the OP's case).
• This means that the code generated for emplace_back cannot use Category's copy constructor, so it has to use the move constructor, even though it can throw (see the first section above). The code compiles, but it no longer offers the strong exception safety guarantee.

---

Note: vector's move constructor has only recently been specified as noexcept in the Standard, after C++14, as a result of the adoption of N4258 into the working draft. In practice, however, both libstdc++ and libc++ have provided a noexcept move constructor for vector since the times of C++0x; an implementation is allowed to strengthen an exception specification compared to the Standard's specification, so that's OK.

libc++ actually uses noexcept(is_nothrow_move_constructible<allocator_type>::value) for C++14 and below, but allocators are required to be nothrow move and copy constructible since C++11 (table 28 in [17.6.3.5]), so that's redundant for Standard-conforming allocators.

---

Note (updated): The discussion about the strong exception safety guarantee doesn't apply to the standard library implementation that comes with MSVC before version 2017: up to and including Visual Studio 2015 Update 3, it always tries to move, regardless of the noexcept specification.

According to this blog post by Stephan T. Lavavej, the implementation in MSVC 2017 has been overhauled and now behaves correctly as described above.

---

Standard references are to working draft N4567 unless otherwise noted.