Does Visual Studio 2017 need an explicit move constructor declaration?

Question

The below code can be compiled successfully using Visual Studio 2015, but it failed using Visual Studio 2017. Visual Studio 2017 reports:

error C2280: “std::pair::pair(const std::pair &)”: attempting to reference a deleted function

Code

#include <unordered_map>
#include <memory>

struct Node
{
  std::unordered_map<int, std::unique_ptr<int>> map_;
  // Uncommenting the following two lines will pass Visual Studio 2017 compilation
  //Node(Node&& o) = default;
  //Node() = default;
};

int main()
{
  std::vector<Node> vec;
  Node node;
  vec.push_back(std::move(node));
  return 0;
}

It looks like Visual Studio 2017 explicit needs a move constructor declaration. What is the reason?

Answer 1

Minimal example:

#include <memory>
#include <unordered_map>
#include <vector>

int main() {
  std::vector<std::unordered_map<int, std::unique_ptr<int>>> vec;
  vec.reserve(1);
}

Live demo on GodBolt: https://godbolt.org/z/VApPkH.

---

Another example:

std::unordered_map<int, std::unique_ptr<int>> m;
auto m2 = std::move(m);              // ok
auto m3 = std::move_if_noexcept(m);  // error C2280

---

UPDATE

I believe the compilation error is legal. Vector's reallocation function can transfer (contents of) elements by using std::move_if_noexcept, therefore preferring copy constructors to throwing move constructors.

In libstdc++ (GCC) / libc++ (clang), move constructor of std::unordered_map is (seemingly) noexcept. Consequently, move constructor of Node is noexcept as well, and its copy constructor is not at all involved.

On the other hand, implementation from MSVC 2017 seemingly does not specify move constructor of std::unordered_map as noexcept. Therefore, move constructor of Node is not noexcept as well, and vector's reallocation function via std::move_if_noexcept tries to invoke copy constructor of Node.

Copy constructor of Node is implicitly defined such that is invokes copy constructor of std::unordered_map. However, the latter may not be invoked here, since the value type of map (std::pair<const int, std::unique_ptr<int>> in this case) is not copyable.

Finally, if you user-define move constructor of Node, its implicitly declared copy constructor is defined as deleted. And, IIRC, deleted implicitly declared copy constructor does not participate in overload resolution. But, the deleted copy constructor is not considered by std::move_if_noexcept, therefore it will use throwing move constructor of Node.

Answer 2

When you declare a move constructor, the implicitly declared copy constructor is defined as deleted. On the other hand, when you don't declare a move constructor, the compiler implicitly defines the copy constructor when it need it. And this implicit definition is ill-formed.

unique_ptr is not CopyInsertable in a container that uses a standard allocator because it is not copy constructible so the copy constructor of map_ is ill-formed (it could have been declared as deleted, but this is not required by the standard).

As your example code show us, with newer version of MSVC, this ill-formed definition is generated with this example code. I do not think there is something in the standard that forbids it (even if this is realy surprising).

So you should indeed ensure that the copy constructor of Node is declared or implicitly defined as deleted.

Answer 3

Let's look at the std::vector source code (I replaced pointer and _Ty with actual types):

void _Umove_if_noexcept1(Node* First, Node* Last, Node* Dest, true_type)
    {   // move [First, Last) to raw Dest, using allocator
    _Uninitialized_move(First, Last, Dest, this->_Getal());
    }

void _Umove_if_noexcept1(Node* First, Node* Last, Node* Dest, false_type)
{   // copy [First, Last) to raw Dest, using allocator
    _Uninitialized_copy(First, Last, Dest, this->_Getal());
}

void _Umove_if_noexcept(Node* First, Node* Last, Node* Dest)
{   // move_if_noexcept [First, Last) to raw Dest, using allocator
    _Umove_if_noexcept1(First, Last, Dest,
        bool_constant<disjunction_v<is_nothrow_move_constructible<Node>, negation<is_copy_constructible<Node>>>>{});
}

If Node is no-throw move-constructible or is not copy-constructible, _Uninitialized_move is called, otherwise, _Uninitialized_copy is called.

The problem is that the type trait std::is_copy_constructible_v is true for Node if you do not declare a move constructor explicitly. This declaration makes copy-constructor deleted.

libstdc++ implements std::vector in a similar way, but there std::is_nothrow_move_constructible_v<Node> is true in contrast to MSVC, where it is false. So, move semantics is used and the compiler does not try to generate the copy-constructor.

But if we force is_nothrow_move_constructible_v to become false

struct Base {
    Base() = default;
    Base(const Base&) = default;
    Base(Base&&) noexcept(false) { }
};

struct Node : Base {
    std::unordered_map<int, std::unique_ptr<int>> map;
};

int main() {
    std::vector<Node> vec;
    vec.reserve(1);
}

the same error occurs:

/usr/include/c++/7/ext/new_allocator.h:136:4: error: use of deleted function ‘std::pair<_T1, _T2>::pair(const std::pair<_T1, _T2>&) [with _T1 = const int; _T2 = std::unique_ptr<int>]’
  { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); }
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Answer 4

Visual Studio 2017:

As @Evg indicated, the Visual Studio 2017's vector sourcecode finally call into _Uninitialized_copy because the implicitly-declared move constructor of Node is considered as not-nothrow (is_nothrow_move_constructible<Node> is false) and is_copy_constructible<Node> is true in Visual Studio 2017.

1) About is_nothrow_move_constructible<Node>:

https://en.cppreference.com/w/cpp/language/move_constructor says:

The implicitly-declared (or defaulted on its first declaration) move constructor has an exception specification as described in dynamic exception specification (until C++17)exception specification (since C++17)

Maybe it's reasonable to consider is_nothrow_move_constructible<Node> as false beacause Node's data member std::unordered_map's move constructor is not marked as noexcept.

2) About is_copy_constructible<Node>:

As @Oliv says, it's seemingly not logical to compute is_copy_constructible<Node> as true, specially considering the fact that Node is not copy_constructible has been detected and reported as compile error by Visual Studio 2017 compiler. Node is not copy_constructible beacause std::unique_ptr is not copy_constructible.

Visual Studio 2015:

Visual Studio 2015's vector has a different implemetation. vec.push_back->_Reserve->_Reallocate->_Umove->_Uninitialized_move_al_unchecked->_Uninitialized_move_al_unchecked1->std::move(node). is_nothrow_move_constructible<Node> and is_copy_constructible<Node> are not involved. It just call std::move(node) instead of copy constructor. So the sample code can be compiled successfully using Visual Studio 2015.