std::vector::push_back() doesn't compile on MSVC for an object with deleted move constructor

Question

I have a class with a deleted move constructor and when I try to call std::vector::push_back() in MSVC (v.15.8.7 Visual C++ 2017) I get an error saying that I am trying to access the deleted move constructor. If however I define the move constructor, the code compiles, but the move constructor is never called. Both versions compile and run as expected on gcc (v. 5.4).

Here's a simplified example:

#include <iostream>
#include <vector>

struct A
{
public:
    A() { std::cout << "ctor-dflt" << std::endl; }
    A(const A&) { std::cout << "ctor-copy" << std::endl; }
    A& operator=(const A&) { std::cout << "asgn-copy" << std::endl; return *this; }
    A(A&&) = delete;
    A& operator=(A&& other) = delete;
    ~A() { std::cout << "dtor" << std::endl; }
};


int main()
{
    std::vector<A> v{};
    A a;
    v.push_back(a);
}

which when compiled on Visual Studio gives the following error:

error C2280: 'A::A(A &&)': attempting to reference a deleted function  

If however I define the move constructor instead of deleting it

 A(A&&) { std::cout << "ctor-move" << std::endl; }

everything compiles and runs, with following output:

ctor-dflt
ctor-copy
dtor
dtor

as expected. No calls to the move constructor. (Live code: https://rextester.com/XWWA51341)

Moreover, both versions work perfectly well on gcc. (Live code: https://rextester.com/FMQERO10656)

So my question is, why doesn't a call to std::vector::push_back() on a non-movable object compile in MSVC, even though the move constructor is apparently never called?

Answer 1

It is undefined behavior, so both gcc and MSVC are correct.

I recently tweeted about a similar case using std::vector::emplace_back with a type that has a deleted move constructor and just like this one it is undefined behavior. So all the compilers are correct here, undefined behavior does not require a diagnostic although implementations are free to do so.

We can see the reasoning by starting out with [container.requirements.general] Table 88 which tells us that push_back requires T be CopyInsertable:

Requires: T shall be CopyInsertable into x

and we can see that CopyInsertable requires MoveInsertable [container.requirements#general]p15:

T is CopyInsertable into X means that, in addition to T being MoveInsertable into X...

In this case A is not MoveInsertable.

We can see this is undefined behavior by looling at [res.on.required]p1:

Violation of the preconditions specified in a function's Requires: paragraph results in undefined behavior unless the function's Throws: paragraph specifies throwing an exception when the precondition is violated.

[res.on.required] comes under Library-wide requirements.

In this case we don't have a throws paragraph so we have undefined behavior, which does not require a diagnostic as we can see from its definition:

behavior for which this International Standard imposes no requirements....

Note, this is very different from being ill-formed which requires a diagnostic, I explain all the detail in my answer here.

Answer 2

std::vector<T>::push_back() requires T to satisfy the MoveInsertable concept (which actually involves the allocator Alloc). This is because push_back on a vector may need to grow the vector, moving (or copying) all elements that are already in it.

If you declare the move c'tor of T as deleted, then, at least for the default allocator (std::allocator<T>), T is no longer MoveInsertable. Note that this is different from the case where the move constructor is not declared, e.g. because only a copy c'tor could be implicitly generated, or because only a copy c'tor has been declared, in which case the type is still MoveInsertable, but the copy c'tor is actually called (this is a bit counterintuitive TBH).

The reason why the move c'tor is never actually called is that you only insert one element, and no moving of existing elements is therefore needed at run time. Importantly, your argument to push_back itself is an lvalue and therefore is copied and not moved in any case.

UPDATE: I had to look at this more closely (thanks to the feedback in the comments). The versions of MSVC that reject the code are actually right in doing so (apparently, both 2015 and pre-2018 do that, but 2017 accepts the code). Since you are calling push_back(const T&), T is required to be CopyInsertable. However, CopyInsertable is defined as a strict subset of MoveInsertable. Since your type is not MoveInsertable, it is not CopyInsertable either, by definition (note that, as explained above, a type may satisfy both concepts, even if it is only copyable, as long as the move c'tor is not explicitly deleted).

This raises some more questions, of course: (A) Why do GCC, Clang and some versions of MSVC accept the code anyway, and (B) are they violating the standard by doing so?

As for (A), there is no way to know other than talking to standard library developers or looking at the source code… my quick guess would be that it is simply not necessary to implement this check if all you care about is making legal programs work. Relocating existing elements during a push_back (or a reserve etc.) can happen in any of three ways according to the standard:

• If std::is_nothrow_move_constructible_v<T>, then elements are nothrow-moved (and the operation is strongly exception safe).
• Otherwise, if T is CopyInsertable, then elements are copied (and the operation is strongly exception safe).
• Otherwise, elements are moved (and the effects of an exception raised in a move c'tor are unspecified).

Since your type is not nothrow move c'tible, but has a copy c'tor, the second option could be chosen. This is lenient in that no check is made for MoveInsertable. This could be an implementation oversight, or i could be deliberately ignored. (If the type is not MoveInsertable, then the whole call is ill-formed, therefore this missing check does not affect well-formed programs.)

As for (B), IMO the implementations that accept the code are violating the standard because they emit no diagnostic. This is because an implementation is required to emit a diagnostic for ill-formed programs (this includes programs that use language extensions provided by the implementation), unless otherwise noted in the standard, neither of which is the case here.