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?

Goldfinch

Well, you could take a look at the std::vector header on both VS2017 and GCC and compare these. I imagine the VS2017 version requires a move constructor. – Draft

Try updating visual studio. On godbolt the latest version of msvc compiles the code wihtout any problems, but the pre 2018 version indeed spits out the errors mentioned by you. – Militiaman

gcc compiles that only in permissive mode, for pedantic mode it's an error. Starting with 4.8.1 version – Whittling

@Swift-FridayPie I think the only reason it fails on gcc 4.8.1 is b/c it does not default to C++11 otherwise it does not provide a diagnostic. As I answered below this is UB, so everyone is correct here. – Cannabis

The accepted answer is not totally correct, since this is undefined behavior, both gcc and MSVC are correct. – Cannabis

@ShafikYaghmour No, I implied that flag for standard selection was active. Without it it would fail for different reason - neither rvalue references nor deletion would be valid code. – Whittling

Undefined behavior and ill-formed program are quite different barrels of fish – Whittling

Answer 1

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.

Alleviation

Clang and gcc compile the given code though. – Stave

@VTT The latest version of msvc also compiles the code without errors – Militiaman

@Lorand 15.8.7 /permissive- /std:c++17 does not compile it. – Stave

@Lorand Godbolt has outdated versions of vc++ – Stave

@ArneVogel It is actually the newer version of MSVC that rejects the code. – Stave

@ArneVogel It is written as "The older version of MSVC is actually right to reject the code! " – Stave

@VTT Both 2015 and pre-2018 reject the code, but 2017 accepts it (according to quick Godbolt testing). I changed the answer to no longer refer to "older" or "newer" versions of MSVC… – Alleviation

@ArneVogel i have visual studio 2017 (up to date 15.8.7) on my local machine and it rejects the code – Manager

@VTT it is undefined behavior, everyone is correct, I explain it in my answer. – Cannabis

@VTT which is not the same as ill-formed which does indeed require a diagnostic. – Cannabis

@Lorand it is undefined behavior to both a diagnostic and no diagnostic is acceptable behavior. – Cannabis

Answer 2

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.

Cannabis