You Can Stop Writing Comments About Pointer Ownership

There's a certain thing that you have probably done at least once if you program in C++ for a living. To the best of my knowledge, a lot of C++ programmers still keep doing it, even though the new C++ standard offers a better way of accomplishing what they're trying to do.

The Problem

While working with C++ code, I sometimes come across methods that look something like this:

// the .h file: class BlackBox { public: //... // store() takes ownership of the object pointed to by ptr! void store(Gadget *ptr); //... private: Gadget *m_contents; }; // the .cpp file: void BlackBox::store(Gadget *ptr) { // ... m_contents = ptr; // ... } //... BlackBox::~BlackBox() { //... // when the box gets destroyed, so does its contents delete m_contents; //... }

I see this in both legacy and new code. The idea here is as follows: if we have an instance crate of BlackBox , and we call cr ate.store(gadget_ptr) , then the lifetime of the object that gadget_ptr points to is tied to the lifetime of crate : the destruction of crate will cause the destruction of the Gadget within it.

In the previous versions of C++, there was no way to convey this idea of "transfer of ownership" to the compiler, so the only remaining option was to write these comments. The comments can help other programmers understand what your intention is, but they mean nothing to the compiler, of course. It will happily make your program do the wrong thing if you ask it to. The comments do nothing.

The main thing that can go wrong here is that the client code can accidentally (or unknowingly) break the contract and attempt to destroy the instance of Gadget after its ownership has been transferred to an instance of BlackBox :

// crate is an instance of BlackBox // gadget_ptr is a pointer to an instance of Gadget crate.store(gadget_ptr); // store takes ownership of gadget_ptr's pointee... //...some other stuff happens that is not important here... delete gadget_ptr; // ...uh-oh, the programmer forgot or didn't know that store owns *gadget_ptr at this point!

This leads to undefined behaviour. For example, the memory, pointed to by gadget_ptr could be freed twice: the first time, it will be freed by the client code, and the second time it will be "freed" by the destructor of BlackBox . This is called "double-free", and you don't want this to happen in your program. But before that takes place, a method of BlackBox might try accessing the "dead" instance of Gadget and wreak havoc. Whatever the case, we don't want it to happen.

Another, less scary, but annoying thing is that even if everything is fine, we may still end up with suspicious-looking code like the following:

crate.store(new Gadget(beeper, flasher));

The above looks like a memory leak, but it really isn't. However, someone who is not familiar with how BlackBox::store works, might assume that it is!

The Solution

Fortunately, the new C++ standard gives us all the tools we need to solve this problem. These tools are: unique pointers and rvalue references. Let us revisit BlackBox::store . We can take advantage of the new language features and rewrite BlackBox like this:

// The .h file: class BlackBox { public: //... void store(std::unique_ptr<Gadget> &&ptr); //... private: std::unique_ptr<Gadget> m_contents; }; // The .cpp file: void BlackBox::store(std::unique_ptr<Gadget> &&ptr) { // ... m_contents = std::move(ptr); // ... }

The code that calls store will now look like this:

// Assuming gadget_ptr is also a unique_ptr: crate.store(std::move(gadget_ptr));

Or, if you want to create an object and immediately pass it to the method:

crate.store(std::make_unique<Gadget>(beeper, flasher));

This solves the problems that were mentioned earlier. It's basically impossible to double-delete: no matter how much you fiddle with the unique pointer, after passing it to the method it will have been moved from, so you can't destroy the object. The "create an object and pass it to the method" scenario is also looking much nicer, since it's much more obvious what's really going on.

Explanation

So let's break down what's going on here.

We replaced the regular plain pointer with a "smart" unique_ptr (and we no longer need to explicitly destroy the owned object in the destructor).

BlackBox::store 's only parameter is now an rvalue reference to a unique_ptr.

's only parameter is now an rvalue reference to a unique_ptr. The transfer of ownership happens inside BlackBox::store . The line m_contents = std::move(ptr); essentially amounts to taking whatever state was inside the object bound to ptr and transferring it into m_contents , after which the object bound to ptr will not point to anything. Side note: what is really going on is a tad more complicated. Even though ptr is an rvalue reference, when used in an expression, it acts as if it was an lvalue reference (don't ask). std::move doesn't actually move anything, it just returns an rvalue reference to its parameter, whatever the parameter may be. Given the above, the line m_contents = std::move(ptr); converts ptr to an rvalue reference and invokes the move assignment operator of unique_ptr, which does the job of transferring pointer ownership!

. The line essentially amounts to taking whatever state was inside the object bound to and transferring it into , after which the object bound to will not point to anything. The client code now has to use std::move if it wants to pass an lvalue into BlackBox::store . This is because lvalues cannot be bound to rvalue references, so a conversion is needed (and std::move does that conversion).

if it wants to pass an lvalue into . This is because lvalues cannot be bound to rvalue references, so a conversion is needed (and does that conversion). If the client code wants to pass an rvalue (like in the example with make_unique ), no call to std::move is needed.

Some people might ask why I chose to make the parameter of BlackBox::store an rvalue reference rather than an lvalue reference and force the client code to use std::move . While it is true that an lvalue reference would also have worked, if I did that, it would be impossible to write code like crate.store(std::make_unique<Gadget>(beeper, flasher)); . I also happen to think that forcing the client to use std::move at the call sites is good rather than bad, because it communicates the intended effect to the reader.

Conclusion

Stop writing comments about pointer ownership. Instead, let the compiler enforce your rules! Take advantage of unique pointers and move semantics.

Like this post? Follow this blog on Twitter for more!