C++ Unified Assignment Operator move-semantics

Question

EDIT: solved see comments --don't know how to mark as solved with out an answer.

After watching a Channel 9 video on Perfect Forwarding / Move semantics in c++0x i was some what led into believing this was a good way to write the new assignment operators.

#include <string>
#include <vector>
#include <iostream>

struct my_type 
{
    my_type(std::string name_)
            :    name(name_)
            {}

    my_type(const my_type&)=default;

    my_type(my_type&& other)
    {
            this->swap(other);
    }

    my_type &operator=(my_type other)
    {
            swap(other);
            return *this;
    }

    void swap(my_type &other)
    {
            name.swap(other.name);
    }

private:
    std::string name;
    void operator=(const my_type&)=delete;  
    void operator=(my_type&&)=delete;
};


int main()
{
    my_type t("hello world");
    my_type t1("foo bar");
    t=t1;
    t=std::move(t1);
}

This should allow both r-values and const& s to assigned to it. By constructing a new object with the appropriate constructor and then swapping the contents with *this. This seems sound to me as no data is copied more than it need to be. And pointer arithmetic is cheap.

However my compiler disagrees. (g++ 4.6) And I get these error.

copyconsttest.cpp: In function ‘int main()’:
copyconsttest.cpp:40:4: error: ambiguous overload for ‘operator=’ in ‘t = t1’
copyconsttest.cpp:40:4: note: candidates are:
copyconsttest.cpp:18:11: note: my_type& my_type::operator=(my_type)
copyconsttest.cpp:30:11: note: my_type& my_type::operator=(const my_type&) <deleted>
copyconsttest.cpp:31:11: note: my_type& my_type::operator=(my_type&&) <near match>
copyconsttest.cpp:31:11: note:   no known conversion for argument 1 from ‘my_type’ to ‘my_type&&’
copyconsttest.cpp:41:16: error: ambiguous overload for ‘operator=’ in ‘t = std::move [with _Tp = my_type&, typename std::remove_reference< <template-parameter-1-1> >::type = my_type]((* & t1))’
copyconsttest.cpp:41:16: note: candidates are:
copyconsttest.cpp:18:11: note: my_type& my_type::operator=(my_type)
copyconsttest.cpp:30:11: note: my_type& my_type::operator=(const my_type&) <deleted>
copyconsttest.cpp:31:11: note: my_type& my_type::operator=(my_type&&) <deleted>

Am I doing something wrong? Is this bad practice (I don't think there is way of testing whether you are self assigning)? Is the compiler just not ready yet?

Thanks

Answer 1

Be very leery of the copy/swap assignment idiom. It can be sub-optimal, especially when applied without careful analysis. Even if you need strong exception safety for the assignment operator, that functionality can be otherwise obtained.

For your example I recommend:

struct my_type 
{
    my_type(std::string name_)
            :    name(std::move(name_))
            {}

    void swap(my_type &other)
    {
            name.swap(other.name);
    }

private:
    std::string name;
};

This will get you implicit copy and move semantics which forward to std::string's copy and move members. And the author of std::string knows best how to get those operations done.

If your compiler does not yet support implicit move generation, but does support defaulted special members, you can do this instead:

struct my_type 
{
    my_type(std::string name_)
            :    name(std::move(name_))
            {}

    my_type(const mytype&) = default;
    my_type& operator=(const mytype&) = default;
    my_type(mytype&&) = default;
    my_type& operator=(mytype&&) = default;

    void swap(my_type &other)
    {
            name.swap(other.name);
    }

private:
    std::string name;
};

You may also choose to do the above if you simply want to be explicit about your special members.

If you're dealing with a compiler that does not yet support defaulted special members (or implicit move members), then you can explicitly supply what the compiler should eventually default when it becomes fully C++11 conforming:

struct my_type 
{
    my_type(std::string name_)
            :    name(std::move(name_))
            {}

    my_type(const mytype& other)
        : name(other.name) {}
    my_type& operator=(const mytype& other)
    {
        name = other.name;
        return *this;
    }
    my_type(mytype&& other)
        : name(std::move(other.name)) {}
    my_type& operator=(mytype&& other)
    {
        name = std::move(other.name);
        return *this;
    }

    void swap(my_type &other)
    {
            name.swap(other.name);
    }

private:
    std::string name;
};

If you really need strong exception safety for assignment, design it once and be explicit about it (edit to include suggestion by Luc Danton):

template <class C>
typename std::enable_if
<
    std::is_nothrow_move_assignable<C>::value,
    C&
>::type
strong_assign(C& c, C other)
{
    c = std::move(other);
    return c;
}

template <class C>
typename std::enable_if
<
    !std::is_nothrow_move_assignable<C>::value,
    C&
>::type
strong_assign(C& c, C other)
{
    using std::swap;
    static_assert(std::is_nothrow_swappable_v<C>,  // C++17 only
                  "Not safe if you move other into this function");
    swap(c, other);
    return c;
}

Now your clients can choose between efficiency (my type::operator=), or strong exception safety using strong_assign.

Answer 2

Did you closely read the error message? It sees two errors, that you have multiple copy-assignment operators and multiple move-assignment operators. And it's exactly right!

Special members must be specified at most once, no matter if they're defaulted, deleted, conventionally defined, or implicitly handled by being left out. You have two copy-assignment operators (one taking my_type, the other taking my_type const &) and two move-assignment operators (one taking my_type, the other taking my_type &&). Note that the assignment operator that takes my_type can handle lvalue and rvalue references, so it acts as both copy- and move-assignment.

The function signature of most special members have multiple forms. You must pick one; you cannot use an unusual one and then delete the conventional one, because that'll be a double declaration. The compiler will automatically use an unusually-formed special member and won't synthesize a special member with the conventional signature.

(Notice that the errors mention three candidates. For each assignment type, it sees the appropriate deleted method, the method that takes my_type, and then the other deleted method as an emergency near-match.)

Answer 3

Are you supposed to be deleting those overloads of the assignment operator? Shouldn't your declaration of the assignment operator be a template or something? I don't really see how that is supposed to work.

Note that even if that worked, by implementing the move assignment operator that way, the resources held by the object that was just moved from will be released at the time its lifetime ends, and not at the point of the assignment. See here for more details:

http://cpp-next.com/archive/2009/09/your-next-assignment/