Yarp: Move assignment operators and move constructor in YARP

Created on 15 Jun 2018  路  9Comments  路  Source: robotology/yarp

I think that it could be a nice thing to have for each class in YARP_OS that implements operator= and copy constructor, the respective move assignment operator and move constructors.

These should be implemented also in other libraries but let's start from OS.
I think that this will prevent from a lot of useless copies inside yarp.

Here some informations about:

YARP v3.2.0 FeaEnh Req Fixed Modernization

Most helpful comment

I would start from yarp::sig::Vector and yarp::sig::Matrix, I think the benefit could be concrete in several cases.

All 9 comments

Moreover in the signature they should be declared noexcept.
In this way standard containers' (e.g. std::vector) allocating and deallocating functions(e.g. swap, push_back) will invoke the move operators instead the copy ones.

See here : http://www.codingstandard.com/rule/12-5-4-declare-noexcept-the-move-constructor-and-move-assignment-operator/.

The policy of the c++11 containers is "move if you can, copy if you must".
In c++98 the push_back allocates n+1 elements, copy the memory and then delete the old one, if during the copy an exception is thrown the original memory is preserved.
In c++11 you have to ensure that during the move nothing can go wrong specifying noexcept in the signature of the function, otherwise it will do copies like c++98.

Yet Another good reason to implement move assignment operator and constructor.

I would start from yarp::sig::Vector and yarp::sig::Matrix, I think the benefit could be concrete in several cases.

One problem that I see for example in Vector there is a ManagedBytes member, so I think we should implement the move operator first in ManagedBytes and then in Vector.
For this reason I think we should start with the core classes for moving then to the ones that will take the real benefits like Image Matrix PointCloud.

I'd say let's start with yarp::os::Bytes and yarp::os::ManagedBytes...

A side note for general information.

If a class doesn't define the copy constructor/operator, move constructor/operator the compiler generates them to do copy/move member-wise.

But if you are explicitly defining the copy constructor/operator, the compiler understands that since the class need a special treatment for the copy, so do for the move, so the move constructor/operator is not generated -> when it should performs a move it performs a copy.

The same thing viceversa, defining explicitly the move operators will define delete the default copy operators, so we have do be careful to implement the move operators in the classes that already define the copy ones.

So move operations are generated for classes(when needed) when:

  • No copy operations are declared in the class
  • No move operations are declared in the class
  • No destructor is declared in the class

I'd say let's start with yarp::os::Bytes and yarp::os::ManagedBytes...

I'd rather check if we can get rid of them somehow... with C++11 do we really still need a Byte wrapper?

I'd rather check if we can get rid of them somehow... with C++11 do we really still need a Byte wrapper?

I think the closest equivalent to yarp::os::Bytes is std::span<std::byte>, but unfortunatly std::span is only available from C++20 . For yarp::os::ManagedBytes the fact that it inherits from yarp::os::Portable and it offers a constructor such as ManagedBytes(const Bytes& ext, bool owned = false) means that there is no direct equivalent in the STL .

For Matrix, Vector etc we should implement also operator like :

 yarp::sig::Vector operator+(const yarp::sig::Vector &&a, const yarp::sig::Vector &b);

For taking the benefits we were talking about.

Digging into the move/forward operator and the universal reference I discovered that the use of && in the constructor of template classes is very very dangerous, so I think we should avoid to use it in that context.

Fixed by #1957

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Giulero picture Giulero  路  3Comments

diegoferigo picture diegoferigo  路  3Comments

jeljaik picture jeljaik  路  3Comments

xEnVrE picture xEnVrE  路  3Comments

drdanz picture drdanz  路  3Comments