Move Assignment incompatible with Standard Copy and Swap

Question

Testing out the new Move Semantics.

I just asked about an issues I was having with the Move Constructor. But as it turns out in the comments the problem is really that the "Move Assignment" operator and "Standard Assignment" operator clash when you use the standard "Copy and Swap" idiom.

This is the class I am using:

#include <string.h>
#include <utility>

class String
{
    int         len;
    char*       data;

    public:
        // Default constructor
        // In Terms of C-String constructor
        String()
            : String("")
        {}

        // Normal constructor that takes a C-String
        String(char const* cString)
            : len(strlen(cString))
            , data(new char[len+1]()) // Allocate and zero memory
        {
            memcpy(data, cString, len);
        }

        // Standard Rule of three
        String(String const& cpy)
            : len(cpy.len)
            , data(new char[len+1]())
        {
            memcpy(data, cpy.data, len);
        }
        String& operator=(String rhs)
        {
            rhs.swap(*this);
            return *this;
        }
        ~String()
        {
            delete [] data;
        }
        // Standard Swap to facilitate rule of three
        void swap(String& other) throw ()
        {
            std::swap(len,  other.len);
            std::swap(data, other.data);
        }

        // New Stuff
        // Move Operators
        String(String&& rhs) throw()
            : len(0)
            , data(null)
        {
            rhs.swap(*this);
        }
        String& operator=(String&& rhs) throw()
        {
            rhs.swap(*this);
            return *this;
        }
};

Pretty bog standard I think.

Then I tested my code like this:

int main()
{
    String  a("Hi");
    a   = String("Test Move Assignment");
}

Here the assignment to a should use the "Move Assignment" operator. But there is a clash with the "Standard Assignment" operator (which is written as your standard copy and swap).

> g++ --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk/usr/include/c++/4.2.1
Apple LLVM version 5.0 (clang-500.2.79) (based on LLVM 3.3svn)
Target: x86_64-apple-darwin13.0.0
Thread model: posix

> g++ -std=c++11 String.cpp
String.cpp:64:9: error: use of overloaded operator '=' is ambiguous (with operand types 'String' and 'String')
    a   = String("Test Move Assignment");
    ~   ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
String.cpp:32:17: note: candidate function
        String& operator=(String rhs)
                ^
String.cpp:54:17: note: candidate function
        String& operator=(String&& rhs)
                ^

Now I can fix this by modifying the "Standard Assignment" operator to:

    String& operator=(String const& rhs)
    {
        String copy(rhs);
        copy.swap(*this);
        return *this;
    }

But this is not good as it messes with the compiler's ability to optimize the copy and swap. See What is the copy-and-swap idiom? here and here

Am I missing something not so obvious?

Answer 1

Other answers suggest to have just one overload operator =(String rhs) taking the argument by value but this is not the most efficient implementation.

It's true that in this example by David Rodríguez - dribeas

String f();
String a;
a = f();   // with String& operator=(String)

no copy is made. However, assume just operator =(String rhs) is provided and consider this example:

String a("Hello"), b("World");
a = b;

What happens is

1. b is copied to rhs (memory allocation + memcpy);
2. a and rhs are swapped;
3. rhs is destroyed.

If we implement operator =(const String& rhs) and operator =(String&& rhs) then we can avoid the memory allocation in step 1 when the target has a length bigger than source's. For instance, this is a simple implementation (not perfect: could be better if String had a capacity member):

String& operator=(const String& rhs) {
    if (len < rhs.len) {
        String tmp(rhs);
        swap(tmp);
    else {
        len = rhs.len;
        memcpy(data, rhs.data, len);
        data[len] = 0;
    }
    return *this;
}

String& operator =(String&& rhs) {
    swap(rhs);
}

In addition to the performance point if swap is noexcept, then operator =(String&&) can be noexcept as well. (Which is not the case if memory allocation is "potentially" performed.)

See more details in this excellent explanation by Howard Hinnant.

Answer 2

All you need for copy and assignment is this:

    // As before
    String(const String& rhs);

    String(String&& rhs)
    :   len(0), data(0)
    {
        rhs.swap(*this);
    }

    String& operator = (String rhs)
    {
        rhs.swap(*this);
        return *this;
    }

   void swap(String& other) noexcept {
       // As before
   }

Answer 3

If you define the assignment operator to take a value, you should not (need not and cannot) define the assignment operator taking an rvalue-reference. There is no point to it.

In general, you only need to provide an overload taking an rvalue-reference when you need to differentiate an lvalue from an rvalue, but in this case your choice of implementation means that you don't need to make that distinction. Whether you have an lvalue or an rvalue you are going to create the argument and swap the contents.

String f();
String a;
a = f();   // with String& operator=(String)

In this case, the compiler will resolve the call to be a.operator=(f()); it will realize that the only reason for the return value is being the argument to operator= and will elide any copy --this is the point of making the function take a value in the first place!