Problems with Move Constructor and Move Overloaded Assignment Operator

problems with Move constructor and Move overloaded assignment operator?

Your class A has several issues:

• Your assignment operator don't handle self assignment and leak:

  A& A::operator=(const A& a)
  {
      std::cout<<"overload operator=\n";
      if (this != &a)
      {
          p = a.p;
          delete[] s;
          s = new char[strlen(a.s) + 1];
          strcpy(s, a.s);
      }
      return *this;
  }
  

• Your move doesn't move but copy:

A::A(A&& a) : p(a.p), s(a.s)
{
    a.s = nullptr;
    std::cout << "Move constructor\n";
}

A& A::operator=(A&& a)
{
    std::cout << "Move overload operator=\n";

    if (this != &a) {
        p = a.p;
        delete [] s;
        s = a.s;
        a.s = nullptr;
    }
    return *this;
}

Now, about

A make_A()
{
    A a(2,"bonapart"); // Constructor
    return a;
}

There are several scenario because of potential copy elision (NRVO)
(gcc has flag as -fno-elide-constructors to control that)

if NRVO apply, then a is construct "in-place" so no extra destruction/move happens;

else there is a move constructor and the destruction of a.

A make_A()
{
    A a(2,"bonapart"); // #2 ctor(int const char*)
    return a; // #3 move (elided with NRVO)
} // #4 destruction of a, (elided with NRVO)

int main()
{
    A a1; // #1: default ctor
    a1 = // #5: Move assignment (done after make_A)
      make_A(); // #6: destructor of temporary create by make_A

    
    a1.display();
} // #8: destructor of a1

With NRVO

default ctor
ctor(int const char*)
move assignment
destructor
display
destructor

without NRVO (-fno-elide-constructors)

default ctor
ctor(int const char*)
move ctor
destructor
move assignment
destructor
display
destructor

Demo

For

A a1,a2;
a2 = a1 = make_A();

a1 = make_A(); use move assignment.
a2 = (a1 = make_A()) use copy assignment as move assignment returns (correctly) A&

4
In Move constructor and Move overloaded = operator I used a.s=nullptr; This statement is always used in Move semantics fredoverflow(user) explained something like "now the source no longer owns the object it" but I am not getting it. Because if I did not write this statement still no problem everything works fine. please explain this point

Your issue is that you do copy instead of move.

If you do s = a.s; instead of the copy

s = new char[strlen(a.s) + 1];
strcpy(s, a.s);

then both this->s and a.s would point of same data, and both this and a would free the (same) memory in their destructor -> double free error.

a.s = nullptr; would fix that issue.

Do move constructors and move assignment operators make sense in classes that don't hold pointers as member variables and don't manage resources?

An explicit move constructor or a move operator can be specified for any class, whether it has pointer members or not. It is true that move constructors and operators are usually used with classes that have pointers of some kind. But that's only because in these cases there's a way to effect a move that avoids the more expensive overhead of copying.

A default move operation for a trivial type is equivalent to a copy operation.

A default move operation for a class is equivalent to a move operation for each member of the class.

Therefore, in your example, the move operation is equivalent to a copy operation, and has no observable differences.

However it is certainly possible to need a move operator for a class that has no pointer members, for whatever reason. A classical example is std::thread, which has a move constructor and an assignment operator but not the equivalent copy constructor and copy assignment operator. The contents of std::thread are implementation defined, but typically consist of an opaque handle for an operating system-specific thread identifier. This handle cannot be "copied" in any meaningful way, but its ownership can be meaningful transferred to another std::thread, and that's why you have move operators there.

Your own classes may have similar semantical requirements, too, even if they don't have any pointers of any kind.

C++ - What is wrong with my move/copy constructors and move/copy assign operators?

Your copy assignment operator copies the m_vec pointer, instead of copying its contents (like the copy constructor does).

Understanding the reasoning between copy/move constructors and operators

Congratulations, you found a core issue of C++!

There are still a lot of discussions around the behavior you see with your example code.

There are suggestions like:

A&& helper_alt(A a) {
    std::cout << ".." << std::endl;
    return std::move(a);
}

This will do what you want, simply use the move assignment but emits a warning from g++ "warning: reference to local variable 'a' returned", even if the variable goes immediately out of scope.

Already other people found that problem and this is already made a c++ standard language core issue

Interestingly the issue was already found in 2010 but not solved until now...

To give you an answer to your question "In the last case, why is the move constructor being called and then the move assignment operator, instead of just the move assignment operator?" is, that also C++ committee does not have an answer until now. To be precise, there is a proposed solution and this one is accepted but until now not part of the language.

From: Comment Status

Amend paragraph 34 to explicitly exclude function parameters from copy elision. Amend paragraph 35 to include function parameters as eligible for move-construction.

Are my move constructor and move assignment operator written well?

Your code is correct.

But there is also a pattern to implement move assignments using std::swap:

Data& operator=(Data&& _d)
{
    std::swap(data, _d.data);
    std::swap(s, _d.size);
    return *this;
}

Here the old data will be released when _d destructor is invoked.

This is not a "more correct way" but it is easier to write and leaves less space to forget to properly dispose the resources.

Move constructor can reuse move assignment:

Data(Data&& _d)
    : this() 
{
    *this = std::move(_d);
}

Again, this is not necessary a better way but reduces amount of duplicated code.

c++: operator = is ambiguous when implementing move assignment

You should have:

TUndirectedG& operator=(const TUndirectedG&);
TUndirectedG& operator=(TUndirectedG&&);

or

TUndirectedG& operator=(TUndirectedG);

Having both

TUndirectedG& operator=(TUndirectedG);   // Lead to ambiguous call
TUndirectedG& operator=(TUndirectedG&&); // Lead to ambiguous call

would lead to ambiguous call with rvalue.

Implicit move constructor and assignment operator

If you look further down you linked page, you will see that your classes compiler generated move constructor (and move assignment operator) will actually be Trivial:

Trivial move constructor

The move constructor for class T is trivial if all of the following is true:

• it is not user-provided (meaning, it is implicitly-defined or defaulted);
• T has no virtual member functions;
• T has no virtual base classes
• the move constructor selected for every direct base of T is trivial;
• the move constructor selected for every non-static class type (or array of class type) member of T is trivial;

A trivial move constructor is a constructor that performs the same action as the trivial copy constructor, that is, makes a copy of the
object representation as if by std::memmove. All data types
compatible with the C language (POD types) are trivially movable.

(Emphasis mine)

The two member variables are POD types and therefore are trivially movable. Since your class is not virtual and it holds no non-trivial members it is therefore trivial and all the data members will be copied. As mentioned in the comments, this will lead to double deleting your pointer and UB.

Since this is the case, you need to implement your move semantics properly, by taking ownership of the moved objects pointer and setting it to nullptr. Or better yet, just use std::vector or even std::unique_ptr.

Move assignment in overloaded vector summation

There is a move elision takes place.

According to the C++ 17 Standard (12.8 Copying and moving class objects)

31 When certain criteria are met, an implementation is allowed to omit
the copy/move construction of a class object, even if the constructor
selected for the copy/move operation and/or the destructor for the
object have side effects. In such cases, the implementation treats
the source and target of the omitted copy/move operation as simply two
different ways of referring to the same object, and the destruction of
that object occurs at the later of the times when the two objects
would have been destroyed without the optimization.122 This elision of
copy/move operations, called copy elision, is permitted in the
following circumstances (which may be combined to eliminate multiple
copies):

(31.3) — when a temporary class object that has not been bound to a
reference (12.2) would be copied/moved to a class object with the same
cv-unqualified type, the copy/move operation can be omitted by
constructing the temporary object directly into the target of the
omitted copy/move

So the move constructor is omitted. The second temporary object created by the expression x + y + z; is directly built as the obejct r.

Vector r = x + y + z;

Also take into account that there is move elision in the return statement of the operator

(31.1) — in a return statement in a function with a class return type,
when the expression is the name of a non-volatile automatic object
(other than a function or catch-clause parameter) with the same
cvunqualified type as the function return type, the copy/move
operation can be omitted by constructing the automatic object directly
into the function’s return value

So the temporary object created in the expression x + y (within the operator) will be moved (that is there will be elision relative to the return statement - preceding quote) in the expression ( x + y ) + z where it will be used by constant lvalue reference and the new temporary obejct created in this expression will be built as r.

In whole in this code snippet

Vector x{1,1,1,1,1};
Vector y{2,2,2,2,2};
Vector z{3,3,3,3,3};
Vector r = x + y + z;

due to the move elision there will be created 5 objects of the class Vector.

---

Related Topics