30

I am trying some new C++11 features on visual studio 11, started with the move constructor. I wrote a simple class called "MyClass" containing a move constructor:

class MyClass { public: explicit MyClass( int aiCount ) : mpiSize( new int( aiCount ) ), miSize2( aiCount) { } MyClass( MyClass&& rcOther ) : mpiSize( rcOther.mpiSize ) , miSize2( *rcOther.mpiSize ) { rcOther.mpiSize = 0; rcOther.miSize2 = 0; } ~MyClass() { delete mpiSize; } private: int *mpiSize; int miSize2; };

I got there questions here:

  1. I assumed that the compiler would generate a move constructor for MyClass if I don't implement one - but it doesn't seems so?
  2. Is the implementation of the move constructor correct for MyClass?
  3. Is there a better way to implement the move constructor for MyClass?
Improve this question

3 Answers 3

Reset to default
39

  1. MSVC++ implemented move constructors before the final version of the standard was out. In the version of the standard MSVC++'s implementation was based on, the rules for generating a default move constructor were ridiculously more strict than they are in the final version of the standard. See here: Why is this code trying to call the copy constructor? (specifically this answer and the comments on it) for more info on that. This has not been and will not be fixed in Visual Studio 11, for some unknown stupid reason because they had other priorities.

  2. No, you need to call std::move on the members of rcOther, and you initialise members with the corresponding members from the dying object (you misnamed miSize):

    MyClass( MyClass&& rcOther ) : mpiSize( std::move(rcOther.mpiSize) ) , miSize2( std::move(rcOther.miSize2) ) { rcOther.mpiSize = 0; }

    It doesn't make a difference for built in types like int and int*, but it definitely makes a difference for user-defined types.

    • The reason for this is that std::move just returns the argument casted into a T&&, an rvalue-reference, so that the correct constructor (the move constructor, T(T&&)) is called for each of the sub-objects. If you don't use std::move on the members of the dying object, they will be treated like T&, and the copy constructor of your subobjects (T(T&)) will be called instead of the move constructor. That is very bad and thwarts almost the entire purpose of you having written a move constructor.


3. You are doing some unnecessary things, like setting the integer to 0. You only need to set the pointer to 0 so that deleteing it won't delete the resource of the new object you created.

Also, if this is not a didactic exercise, you may want to consider using std::unique_ptr instead of managing the lifetime of your own object. This way you don't even have to write a destructor for your class. Note that if you do that, using std::move to initialise the member from the dying member in the move constructor would be mandatory.

  • Konrad Rudolph in his answer has caught the fact that your class manages a non-automatic resource but does not follow the Rule of Five Three, Four, or Five. See his answer for more details on this.
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

As for the "unknown, stupid reason", Herb Sutter covers the release schedule for Microsoft's C++ team in his talk at Going Native 2012. Although you might disagree with their priorities, the reason seems neither stupid nor unknown (although it could probably be made more obvious on Microsoft's website).
@AndréCaron they don't have a release date set for VS11, so they could easily take a little more time and fix this issue.
@SethCarnegie: they could equally well make it VS12 and finish implementing the whole standard. My comment stands: you can disagree with their priorities (I know I do), but the reason is neither unknown nor stupid.
@AndréCaron there is a difference between introducing a new feature and fixing a bug in a previous version. But you are right, and I have edited my answer to reflect that. My original answer was subjectively influenced by my annoyance at their decision not to fix it.
15

Why doesn’t the compiler auto-generate a move constructor?

The compiler does generate a move constructor if you don’t do so – after a fashion. However, the compiler cannot second-guess your motives so it doesn’t know what the pointer in your class does. In particular, it doesn’t know that the pointer confers ownership of memory and needs to be nulled out.

Is the implementation of the move constructor correct?

The move constructor is correct1 but the rest of the class isn’t, you are violating the rule of three: your class needs an appropriate copy constructor and copy assignment operator.

Is there a better way to implement the move constructor?

A better way to write the move constructor looks as follows:

MyClass(MyClass&& rcOther) : mpiSize(std::move(rcOther.mpiSize)) , miSize2(std::move(rcOther.miSize2)) { rcOther.mpiSize = 0; }

Two comments:

  • Why did you not directly copy the members, instead dereferencing rcOther.mpiSize? While this isn’t wrong, it also makes no sense and is misleading.
  • You do not need to zero out the integer, and since it’s unnecessary it shouldn’t be done: The only modification your move constructor should perform on the moved-from object is to relinquish ownership of its resources so that it can be destroyed without causing resources to be doubly deleted.

But an even better way is to rely on pre-existing facilities. In this case, you want to model memory ownership. A naked pointer does this poorly, you should use a std::unique_ptr instead. This way, you don’t need to implement either destructor nor move constructor since the the auto-generated methods do the right thing.

1 Caveat: See Seth’s answer for a better explanation that mentions std::move (which is a no-op in this particular case, however).

Improve this answer

12 Comments

I declared mpiSize as a raw int pointer for testing purpose. You are right, rcOther.miSize = 0; is unecessary and can be omitted. What does the rule of three means? I only know the rule of two :) (implement the assignment operator if a copy constructor is defined, and vice versa).
@jisaak The “rule of two” does not exist. I’ve added a link explaining the rule of three.
@Seth That’s a matter of debate. In terms of correctness, the rule of three remains the rule of three. I do agree that as a design guideline, we should always use the rule of five.
@jisaak No, that is fine: the important part is that you prevent the compiler from auto-generating these methods for you, because those would be broken. Declaring them without defining them is OK, although C++11 offers a better mechanism, by deleting them explicitly, e.g. MyClass(MyClass const&) = delete;
Ah, there's a typo in a previous comment that i misunderstood. " rcOther.miSize = 0; is unecessary" should be " rcOther.miSize2 = 0; is unecessary"
|
5

Since C++14 you can take advantage of the std::exchange() convenience function template for defining the move constructor. This may result in a more concise move constructor definition:

MyClass(MyClass&& other) noexcept: mpiSize(std::exchange(other.mpiSize, nullptr)), miSize2(std::exchange(other.miSize2, 0)) {}

std::exchange(other.mpiSize, nullptr) replaces the value of other.mpiSize with nullptr but returns the value other.mpiSize had prior to the replacement.

Improve this answer

1 Comment

This approach is awkward for maintenance, because when the default values for mpiSize and miSize2 change, you need to change them in multiple places.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.