c++move-semanticsraii

Why adding a defaulted move assignment operator breaks compilation of standard swap function?


In the following code, if move assignment is uncommented the swap function stops compilation of the program. I observed this behavior on all 3 major compilers (GCC, Clang, MSVC).

#include <utility>
#include <memory>

struct test
{
    test() = default;

    test(test&& other) noexcept = default;
    //test& operator=(test&& other) noexcept = default;

    test(const test& other)
    : ptr(std::make_unique<int>(*other.ptr))
    {}

    test& operator=(test other) noexcept
    {
        std::swap(*this, other);
        return *this;
    }

    std::unique_ptr<int> ptr;
};

Godbolt test: https://godbolt.org/z/v1hGzzEaz

Looking into standard library implementations, they use SFINAE or concepts to enable/disable std::swap overloads and when the special function is uncommented, for some reason, some traits are failing (is_move_constructible and/or is_move_assignable on libstdc++).

My question is: why adding a defaulted special member function prevents the standard library from viewing the type as moveable?

Edit 1: turns out the problem is that xvalues have no preference in overload resolution between T and T&& (which causes ambiguous overload error) so standard library traits fail to recognize the type as moveable.

Edit 2: important: note that the example code IS NOT a proper implementation of the copy and swap idiom. It should be done with custom swap function or use all 5 special member functions. The current implementation creates infinite recursion of move/copy assignment and swap calls.


Solution

  • The main implementation of std::swap uses move assignment internally and looks something like:

    template <typename T>
      requires std::is_swappable_v<T> // exposition-only, constraint might be different
    void swap(T& a, T& b) {
        T c = std::move(a);
        a = std::move(b);
        b = std::move(c);
    }
    

    This means that move assignment needs to be valid for your type, but it isn't. If you were to call the move assignment operator, you would get an error:

    <source>:18:15: error: use of overloaded operator '=' is ambiguous [...]
    [...] |
    <source>:9:11: note: candidate function
        9 |     test& operator=(test&& other) noexcept = default;
          |           ^
    <source>:15:11: note: candidate function
       15 |     test& operator=(test other);
          |           ^
    

    std::swap is constrained so that only MoveAssignable types can be swapped, and yours can't be. Both = operators can be called with an xvalue, and neither is a better match. This is due to the fact that only lvalue-to-rvalue conversion lenghtens the conversion sequence, and initializing an object from an xvalue is considered "free" (as in, this isn't a conversion and not any worse than binding a reference).

    Even if you could call std::swap, you cannot simultaneously

    This would be infinite recursion as the definition of = and std::swap is circular.

    Solutions

    You can define a custom swap for your type so that you no longer rely on std::swap. Only keep operator=(test), which would look like:

    friend void swap(test& a, test& b) noexcept {
        using std::swap;    // Use swap() functions found through ADL, fall back
        swap(a.ptr, b.ptr); // onto std::swap if there are none.
    }                       // (not necessary if you only have a std::unique_ptr member,
                            // you could just use std::swap(a.ptr, b.ptr)).
    
    test& operator=(test other) noexcept {
        swap(*this, other); // custom function, not std::swap
        return *this;
    }
    

    You can also define separate operator=(const test&) and operator=(test&&) manually, so that std::swap would use the move assignment operator and there would be no ambiguity in overload resolution.