Compilation error when returning an std::map of implicitly non-copyable structs on new versions of gcc

Question

I get this strange compilation error on new versions of gcc (4.9+).

Here is the code:

#include <iostream>
#include <vector>
#include <string>
#include <memory>
#include <map>

using namespace std;

struct ptrwrap
{
    unique_ptr<int> foo;
};

template <typename T>
struct holder
{
    holder() = default;

    holder(const holder& b)
        : t(b.t)
    {
    }

    holder(holder&& b)
        : t(std::move(b.t))
    {
    }

    holder& operator=(const holder& h)
    {
        t = h.t;
        return *this;
    }

    holder& operator=(holder&& h)
    {
        t = std::move(h.t);
        return *this;
    }

    T t;
};

struct y_u_no_elision
{
    holder<ptrwrap> elem;
};

typedef map<std::string, y_u_no_elision> mymap;

mymap foo();

int main()
{
    auto m = foo();
    m = foo();
    return 0;
}

Here it is on ideone too with the actual error. Basically it boils down to using the deleted copy constructor of ptrwrap. Which... shouldn't happen. The map is returned by value (ie moved), so no copies can exist.

Now the same code is compiled with no problems on older versions of gcc (I tried 4.2 and 4.3), all versions of clang I tried, and also Visual Studio 2015.

Curiously if I remove the explicit copy and move constructors of the holder template, it also compiles on gcc 4.9+. If I change the map to a vector or an unordered_map it also compiles fine (here is a link to a compiling version of the code with unordered_map)

So... is this a gcc 4.9 bug or are the other compilers being permissive of something I can't see? Is there anything I can do about this which doesn't involve changing the holder class?

Answer 1

Short answer: It's a bug in libstdc++. According to the allocator-aware container requirements table in [container.requirements.general] in the Standard (hasn't changed since C++11), container move assignment:

Requires: If allocator_traits<allocator_type>::propagate_on_container_move_assignment::value is false, T is MoveInsertable into X and MoveAssignable. [...]

(X is the container type and T is its value_type)

You're using the default allocator, which has using propagate_on_container_move_assignment = true_type;, so the above requirement doesn't apply; there should be no special requirements on the value_type.

Quick fix: If you cannot touch holder, one solution is to change y_u_no_elision, adding

y_u_no_elision(const y_u_no_elision&) = delete;
y_u_no_elision(y_u_no_elision&&) = default;

---

Long story: The bug is essentially caused by this line in stl_tree.h.

_Rb_tree is the underlying implementation of std::map and that line in its move assignment operator definition basically does the check specified by the Standard quote above. However, it does it using a simple if, which means that, even if the condition is satisfied, the other branch has to compile as well, even though it won't be executed at runtime. Lacking the shiny new C++17 if constexpr, this should be implemented using something like tag dispatching (for the first two conditions - the third one is a true runtime check), in order to avoid instantiating the code outside the taken branch.

The error is then caused by this line, which uses std::move_if_noexcept on value_type. And here comes the long story.

value_type is std::pair<const std::string, y_u_no_elision>.

In your initial code:

• holder has non-deleted, non-noexcept copy and move constructors.
• This means the corresponding implicitly-declared constructors of y_u_no_elision will also be non-deleted and non-noexcept.
• These characteristics propagate to the constructors of value_type.
• This results in std::move_if_noexcept returning const value_type& instead of value_type&& (it falls back to copy if it can - see these docs).
• That eventually causes the copy constructor of y_u_no_elision to be called, which will cause holder<ptrwrap>'s copy constructor definition to be instantiated, which tries to copy a std::unique_ptr.

Now, if you remove the user-declared copy and move constructors and assignment operators from holder:

• holder will get the implicitly-declared ones. The copy constructor will be deleted and the move constructor will be defaulted, not deleted and noexcept.
• This propagates up to value_type, with one exception unrelated to holder: value_type's move constructor will try to move from a const std::string; this will not call string's move constructor (which is noexcept in this case), but rather its copy constructor, as string&& cannot bind to an rvalue of type const string.
• string's copy constructor is not noexcept (it may have to allocate memory), so value_type's move constructor won't be either.
• So, why does the code compile? Because of the logic behind std::move_if_noexcept: it returns an rvalue reference even if the argument's move constructor isn't noexcept, as long as the argument is not copy-constructible (it falls back to non-noexcept move if it cannot fall back to copy); and value_type isn't, because of holder's deleted copy constructor.

This is the logic behind the quick fix above: you have to do something to make value_type have a valid move constructor and a deleted copy constructor, in order to get an rvalue reference from move_if_noexcept. This is because you won't be able to make value_type have a noexcept move constructor due to the const std::string, as explained above.