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.