Pass by Value Assignment Operator?

[TheToaster]

I have a question regarding assignment operators. Currently, let's say I have a class defined as follows:

class Spreadsheet
{
 public:
  Spreadsheet& operator=(const Spreadsheet& rhs);
  friend void swap(Spreadsheet& first, Spreadsheet& second) noexcept;
 //code omitted for brevity
};

void swap(Spreadsheet& first, Spreadsheet& second) noexcept
{
  using std::swap;
  swap(first.mWidth, second.mWidth);
  swap(first.mHeight, second.mHeight);
  swap(first.mCells, second.mCells);
}

Spreadsheet& Spreadsheet::operator=(const Spreadsheet& rhs)
{
// Check for self-assignment
  if (this == &rhs) return *this;

  Spreadsheet temp(rhs); // Do all the work in a temporary

  swap(*this, temp); // Commit the work with only non-throwing operations
  return *this;
}

My question is, what if I had written the assignment operator like this:

Spreadsheet& Spreadsheet::operator=(Spreadsheet rhs)
{
// Check for self-assignment
if (this == &rhs) return *this;

swap(*this, rhs); 
return *this;
}

As you can see, in this assignment operator, I am using pass by value mechanics to let the compiler make a copy of rhs. My reasoning behind this is, if the type Spreadsheet has a move ctor defined, then when I pass in a rvalue to this operator=, it will be moved in rather than copied.

But if I can use the same assignment operator in this way using the copy and swap idiom to implement both regular assignment and move assignment, what is the point of even using a move assignment operator at all, or defining one or letting the compiler generate one? Can't this operator= be used for both regular and move assignment?

[JLBorges]

> Can't this operator= be used for both regular and move assignment?

Yes; we can implement a "unifying assignment operator".
See: http://olympiad.cs.uct.ac.za/docs/cppreference-doc-20130729/w/cpp/language/as_operator.html#Copy_and_swap

Remove the const from the signature:

// Spreadsheet& Spreadsheet::operator=(const Spreadsheet rhs)
Spreadsheet& Spreadsheet::operator=(Spreadsheet rhs)

and ideally make the move constructor and swap non-throwing (noexcept).

[dhayden]

My reasoning behind this is, if the type Spreadsheet has a move ctor defined, then when I pass in a rvalue to this operator=, it will be moved in rather than copied.

If that were true, then this would happen:

{
    Spreadsheet a, b;  // two spreadsheets
    // Do stuff to fill spreadsheet b
    a = b;  // b gets moved to the rhs parameter of operator=
    // b has been modified (maybe) because it was moved!
}

We usually expect the right hand side of an assignment to remain unmodified. If it's moved somewhere else, that would be quite a surprise.

[keskiverto]

On the latter (this == &rhs) is never true. You will always create a copy and swap.

On the move assignment no copies should be created at all; just swap.

[TheToaster]

@dhayden Well in your case, a normal copy would be made. When you do Spreadsheet a, b; both a and b are named lvalues, not rvalues. Thus, when operator= is called, b will be unaffected and will simply be copied to rhs as long as you don't do std::move. But if you were to use std::move or pass in a Spreadsheet(), then the compiler would automatically call the move ctor

@JLBorges I see. In that case, if I use a unifying assignment operator, there is no need to define a move assignment operator? Like if I explicitly delete the move assignment operator generated by the compiler move assignment will still work with the unifying operator? Then why should one even define a move assignment operator if they can use this trick instead?

[TheToaster]

@keskiverto You're right, I just forgot to delete that part when I copy/pasted the code and didnt pay attention. But if I can simply use the regular operator= for both regular and move assignment, then why define a move assignment operator at all if I can just pass by value in the regular assignment? Can I explicitly delete Spreadsheet& operator=(Spreadsheet&&) = delete and be able to use the single operator= for regular assignment for move assignment as well?

[JLBorges]

> why should one even define a move assignment operator if they can use this trick instead?

There are many situations where assignment can/may benefit from reuse of resources.
A typical example would be std::vector, where the resource is dynamically allocated memory.

#include <iostream>
#include <vector>

int main()
{
    std::vector a { 1, 2, 3, 4, 5 } ;
    std::cout << "vector storage starts at " << a.data() << ", contains [ " ;
    for( int v : a ) std::cout << v << ' ' ;
    std::cout << "]\n" ;

    const std::vector b { 5, 6, 7, 8 } ;
    a = b ; // vector tries to reuse its existing resource
    std::cout << "after assignment,\nvector storage starts at " << a.data() << ", contains [ " ;
    for( int v : a ) std::cout << v << ' ' ;
    std::cout << "]\n" ;
}

http://coliru.stacked-crooked.com/a/4823e1c01de68094

In those situations where copy assignment cannot benefit from resource reuse (it does not manage a heap-allocated array and does not have a (possibly transitive) member that does, such as a member std::vector or std::string), there is a popular convenient shorthand: the copy-and-swap assignment operator, which takes its parameter by value (thus working as both copy- and move-assignment depending on the value category of the argument), swaps with the parameter, and lets the destructor clean it up.

https://en.cppreference.com/w/cpp/language/operators#Assignment_operator

[TheToaster]

I understand that, but the example you gave me doesnt really answer my question regarding move assignment operators. My question is, if you can do this:
Spreadsheet& Spreadsheet::operator=(Spreadsheet rhs);
Then what is the point of doing this?

Spreadsheet& Spreadsheet::operator=(const Spreadsheet &rhs);
Spreadsheet& Spreadsheet::operator=(Spreadsheet&&); 

Wouldnt it be unnecessary to create a move operator= if the unifying operator does both?

[JLBorges]

> Wouldnt it be unnecessary to create a move operator= if the unifying operator does both?

A unifying assignment operator may be less efficient that a separately hand crafted copy assignment operator.

For an illustration, run this trivialised example, first with a unifying assignment operator:

#include <iostream>
#include <cstring>
#include <cstdlib>
#include <iomanip>

#define UNIFYING_ASSIGNMENT_OPERATOR

struct toy_str
{
    toy_str( const char* cstr ) : sz( std::strlen(cstr) ), contents( new char[sz+1] )
    {
        std::cout << this << " constructor: allocated " << sz+1 << " bytes\n" ;
        std::strcpy( contents, cstr ) ;
        std::cout << this << " constructor: copied " << sz+1 << " characters "
                  << *this << '\n' ;
    }

    toy_str( const toy_str& that ) : sz( that.sz ), contents( new char[sz+1] )
    {
        std::cout << this << " copy constructor: allocated " << sz+1 << " bytes\n" ;
        std::strcpy( contents, that.contents ) ;
        std::cout << this << " copy constructor: copied " << sz+1 << " characters "
                  << *this << '\n' ;
    }

    toy_str( toy_str&& that ) { swap(that) ; }

    #ifdef UNIFYING_ASSIGNMENT_OPERATOR

        toy_str& operator= ( toy_str that ) noexcept { return swap(that) ; }

    #else

        toy_str& operator= ( const toy_str& that )
        {
            if( this != std::addressof(that ) )
            {
                std::cout << "currently allocated: " << sz+1 << " bytes, "
                          << "copy requires: " << that.sz+1 << " bytes\n\n" ;
                // if size of buffer is too small, we need to resize it
                if( sz < that.sz )
                {
                    delete[] contents ;
                    std::cout << this << " copy assignment : deallocated " << sz+1 << " bytes\n" ;
                    contents = new char[that.sz+1] ;
                    std::cout << this << " copy assignment : allocated " << that.sz+1 << " bytes\n" ;
                }


                sz = that.sz ;
                std::strcpy( contents, that.contents ) ;
                std::cout << this << " copy assignment: copied " << sz+1 << " characters "
                          << that << '\n' ;
            }
            return *this ;
        }

        toy_str& operator= ( toy_str&& that ) noexcept { return swap(that) ; }

    #endif // UNIFYING_ASSIGNMENT_OPERATOR

    ~toy_str()
    {
        delete[] contents ;
        std::cout << this << " destructor: deallocated " << sz+1 << " bytes\n" ;
    }

    toy_str& swap( toy_str& that ) noexcept
    {
        std::cout << "swap " << this << ' ' << *this << " with "
                  << std::addressof(that) << ' ' << that << '\n' ;
        using std::swap ;
        swap( sz, that.sz ) ;
        swap( contents, that.contents ) ;
        return *this ;
    }

    private:
        std::size_t sz ; // TO DO: add member variable to keep track of reserved space
        char* contents ;

    friend std::ostream& operator<< ( std::ostream& stm, const toy_str& str )
    { return stm << std::quoted(str.contents) ; }
};

int main()
{
    toy_str a = "abcdefghijklmnop" ;
    const toy_str b = "01234567890123" ;

    std::cout << "\n\n------- copy assignment ------------\n\n" ;

    #ifdef UNIFYING_ASSIGNMENT_OPERATOR
        std::cout << "with UNIFYING_ASSIGNMENT_OPERATOR\n\n" ;
    #endif // UNIFYING_ASSIGNMENT_OPERATOR

    a = b ; // note that a.sz > b.sz

    std::cout << "\n----- end copy assignment ----------\n\n" ;
}

http://coliru.stacked-crooked.com/a/03df6b87794d8363
https://rextester.com/AUSV75929

Then, with separate copy assignment and move assignment operators (comment out line# 6)
http://coliru.stacked-crooked.com/a/7c88b29c3a84cded
https://rextester.com/CPDUH15485

[TheToaster]

I think I get it now. The seperate copy assignment reuses memory while the unifying one doesn't
However, looking at your code I have another question. Given the following statements:

toy_str a = "abcdefghijklmnop" ;
const toy_str b = "01234567890123" ;

This means "a" takes up 17 bytes (with trailing \0 character) and b takes 15. However, after strcpy doesnt that mean the remaining character from the a string will still be at the end of the resulting string?

[TheToaster]

Also, another question. Since this class does not have a default ctor, what happens in the move ctor? I usually see that people will delegate a call to the default ctor in the move ctor that uses swap. How is the "this" object constructed to use in line 26? Namely if you have this
toy_str my_toy = toy_str("hello");

[keskiverto]

Yes. The array was not resized and still spans 17 bytes.

Note the comment:

// TO DO: add member variable to keep track of reserved space

The intent seems that the class should keep track of both currently stored characters 'sz' and actual capacity of the array.

That is what std::string and std::vector do. See http://www.cplusplus.com/reference/string/string/capacity/

[TheToaster]

I see. And how come in this code I cannot call the move ctor? It always calls the copy ctor, even if I do this
toy_str my_toy(toy_str("hello"));

[JLBorges]

> However, after strcpy doesnt that mean the remaining character from the a string
> will still be at the end of the resulting string?

Yes. We ignore those characters because strcpy null terminates (, and we keep track of the size).


> How is the "this" object constructed to use in line 26?

It is uninitialized before the swap; this is a (disastrous) bug. One way to fix it is to add default member initializers.
For instance, as in this this crude implementation which does not want to check for nullptr:
http://coliru.stacked-crooked.com/a/144a3868ef061dc9 (lines 82, 83)

[JLBorges]

> And how come in this code I cannot call the move ctor? It always calls the copy ctor, even if I do this
> toy_str my_toy(toy_str("hello"));

There is no copy; copy elision would apply, and toy_str would be initialised with the c-style string "hello"
(directly call the constructor which takes const char*)

copy elision: https://en.cppreference.com/w/cpp/language/copy_elision
snippet: http://coliru.stacked-crooked.com/a/3957395a51ff1d06

[TheToaster]

Alright. But how do I get it to call the move ctor? I tried running it in rextester with -fno-elide-constructors but it didn't work. Also when I tried toy_str my_toy(std::move(toy_str("hello"))); it exited with an error code

[JLBorges]

> But how do I get it to call the move ctor?

int main()
{
    toy_str a = "abcdefghijklmnop" ;
    const toy_str b = std::move(a) ; // move construct
}

[TheToaster]

Still getting an error. Try it in rextester. It says: "Error(s): Process exit code is not 0: -1073741819"

It happens whenever I use std::move().

[Albatross]

JLBorges's move constructor unfortunately leaves the moved-from object in an invalid state, due to it swapping garbage from the moved-to object.

This problem can be resolved by initializing the moved-to object to a valid state in the move constructor before swap gets called. That, or setting the moved-from object's members to valid values in the move constructor after the swap.

EDIT:
toy_str( toy_str&& that ): sz(0), contents(new char[1]{'\0'}) { swap(that) ; }
The explicit zero-init is probably redundant, but eh.

-Albatross

[TheToaster]

Thanks Albatross. That's fixed it. So was the error an uncaught exception? Is that was why the program existed with a strange exit code? If so, was it std::swap that threw it?