Allow move construction of SerialStream

[raymoo]

Is there anything preventing SerialStream from being move constructible? It has prevented us from factoring our stream creation into a function that returns a SerialStream without pointer indirection.

[crayzeewulf]

SerialStream only contains a std::unique_ptr<SerialStreamBuf> as a data member. So, in principle, a move constructor can be written by simply moving this data member (or swapping it). Will it be possible for you to try it out in your setup? Something like the following should be sufficient:

SerialStream::SerialStream(const SerialStream&& otherSerialStream) : 
    SerialStream()
{
    mIOBuffer = std::move(otherSerialStream.mIOBuffer) ;
}

It might also be useful to enable move assignment if move construction is allowed.

[raymoo]

Couldn't I just do SerialStream::SerialStream(SerialStream&& otherSerialStream) = default? (move constructor takes a not-const rvalue reference because the move can modify the object).

I don't have the opportunity to touch the code immediately, but I can try it when I do.

[crayzeewulf]

@raymoo you are right on both counts: move constructor argument must be non-const rvalue reference and default is the right way to do this. I blame sleep deprivation. :D

In any case, if you do try this out, please let us know so we can add it to the library code.

[mcsauder]

Hi @crayzeewulf and @raymoo . I thought I'd give this a try, but something is eluding me.. My addition to the unit tests for checking copy/move assignment is not compiling. Let me know if you can spot what I don't understand! https://github.com/crayzeewulf/libserial/tree/copy_and_move_constructors

Here is a compare: https://github.com/crayzeewulf/libserial/compare/copy_and_move_constructors

UPDATE: I'm learning: https://stackoverflow.com/questions/5966698/error-use-of-deleted-function

UPDATE 2: The use of std::unique_ptr<Implementation> mImpl; is what is causing issues with using the default copy/move constructors and the compile error I am running into. Unfortunately, and I'm not finding much of a way around it.

[crayzeewulf]

Mark:

I have not tried this but the default move constructor for SerialStream should work without any error when placed in the header file SerialStream.h. However, it will (probably) not work for SerialStreamBuf and SerialPort because the definitions of their corresponding Implementation classes are (intentionally) hidden in the .cpp files.

The workaround for this is to declare the move constructor in the header and define it in the .cpp file. For example, in SerialPort.h:

class SerialPort
{
public:
    // ...
    SerialPort(SerialPort&& otherSerialPort) ;
    // ...
} ;

and then put the definition in SerialPort.cpp:

SerialPort::SerialPort(SerialPort&& otherSerialPort) = default ;

Please give this a try and let me know what happens. If you still get an error, could you please also post the error message (I am not going to be at a computer where I can try this out till Friday).

Thanks as always!

[crayzeewulf]

P.S. Also, the other three (copy constructor, copy assignment, and move assignment) should be left deleted. Eventually, it might be useful to enable move assignment as well. But till then we want to leave it deleted (following rule of five). We probably do not want to support copy constructor and copy assignment as the underlying file descriptor may not be be usable from two different instance of these classes.

[crayzeewulf]

PPS. (as the car-talk guys would say): More about this at C++ Core Guidelines.