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

Haystack

Why did you delete the two assignment operators? Simply ignoring them will be quite sufficient, unless I'm missing something. – Decrescendo

If you want to use any "+swap" idiom, you have to implement the swap function for both lvalue and rvalue references, I believe. And then it should be swap(std::move(other));. – Mole

@Kerrek: No, because internal to the class you always end up with l-values – Decrescendo

@Dennis Zickefoose my bad, this was a miss understanding of what the delete operator did. I assumed that deleting it would ensure that that I wouldn't get an implicit compiler generated operator. not stop any other matching overload form workng. Thanks again. – Haystack

@111111: To mark solved without an answer: either provide the correct answer yourself and accept that, or delete the question if it is no longer relevant. – Figured

By the way, I'd think this question could stick around if only for Howard Hinnant's excellent posting. it would be a shame to let that disappear – Figured

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.

Lyrate

Thanks for the great response. My problem is how do I know whether the compiler is actually choosing to do these properly. With move semantics I starting to return "on the stack" heavier and heavier objects with the knowledge that move semantics make it cheap. Of course if the the compiler is doing as I hope the then I am actively shooting myself in the foot by copying lots of strings and other heavy containers. Non the less thanks for your answer I will do as you advise. – Haystack

@111: if the object is "heavy", then I wonder if move semantics are important. Move semantics are useful for light controller objects that manage a heavy resource, where deep copying of the resource is undesirable or impossible. – Mole

@111111: You'll know what the compiler is doing because it is completely specified. Just like the defaulted copy is specified by copying each base and member, defaulted move is specified by moving each base and member. If that's the right thing for your class, then go with the default, else write it yourself or disable it (no different than dealing with copy). For types like std::string, you should be able to count on them having a fast move constructor and move assignment. If you're unsure, measure. There's a great new <chrono> header making it very simple to do accurate timings! :-) – Lyrate

Kerrek: By heavy object I mean a heavy object to copy for example an object containing a number of string (say 10) in my opinion is ok to move. as swapping a string is a light weight operation. however I would not want to be copying an object containing 10 string with any sort of frequency. – Haystack

Howard: I will check out chrono. I currently am just using 'time' to see if there is any discernible difference. I am not so concerned about what exactly a default move constructor would do but I am more concerned about whether or not the compiler would actually use it, because it is hard to see the difference (until the program reaches a certain size) – Haystack

Some questions on what you're doing: I was under the impression that writing valued constructors (and setters/valued assignment) using pass-by-value like you did was the new idiom, but that moving the parameter into the member was preferred; and why write a swap here? Is this meant to be illustrative only, for cases where that member does more than what's shown here? Or perhaps in case there are members that do not necessarily have nothrow move? I would have thought the default std::swap satisfactory here. – Outstare

@Luc: I've improved strong_assign based on your comment (thanks!). If C is no_throw_move_assignable, it will use move assignment, else it will use swap. An even better implementation would check for is_nothrow_swappable, but unfortunately that trait is not in <type_traits>. Though one could certainly build it. – Lyrate

Not exactly what I had in mind when I made my previous comment :) Can you explain why you're not doing name(std::move(name_)) in the constructor and why you're writing a swap member at all? – Outstare

Ah! Those parts are copy/paste from the original question. I left them in to give context with respect to the original question but otherwise ignored them. My comments are really directed at the assignment operator which appears to be the focus of this question. I agree with your observation that one should name(std::move(name_)). And yes, the general swap should be fine, unless you implement move assignment in terms of swap, in which case you get infinite recursion. And a specialized swap will be about twice as fast as using the general swap, should that be important for your class. – Lyrate

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.)

Korry

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/

Avail

No, the internal data or the R-value will be swapped into the new object (the parameter for the operator=) and the empty data in the new object is what is deleted when the r-value is release. – Haystack