std::unique_ptr::release() vs std::move()

Question

I have a class that represents a runtime context and builds a tree, the tree root is held in a unique_ptr. When building the tree is done I want to extract the tree. This is how it looks (not runnable, this is not a debugging question):

class Context {
    private:
    std::unique_ptr<Node> root{new Node{}};
    public:
    // imagine a constructor, attributes and methods to build a tree
    std::unique_ptr<Node> extractTree() {
        return std::move(this->root);
    }
};

So I used std::move() to extract the root node from the Context instance.

However there are alternatives to using std::move() e.g.:

std::unique_ptr<Node> extractTree() {
    // This seems less intuitive to me
    return std::unique_ptr<Node>{this->root.release()};
}

Is std::move() the best choice?

You need to call `reset` from `this->root` after it is moved. You'd better read this answer as well http://stackoverflow.com/a/20850223/555515 — neuront, Mar 16 '16 at 09:16
If you find yourself calling `release()` on a unique_ptr, it's a warning that your logic is likely to be unsound. — Richard Hodges, Mar 16 '16 at 09:22
@DavidHaim A C API taking an owning pointer is likely to `free` it, so passing `unique_ptr.release()` to a C API means the object should have been `malloc`ed` and the `unique_ptr` better have `free` as its custom deleter, which is visible in the type. — nwp, Mar 16 '16 at 10:09
@nwp. nope. for example, IOCP and EPOLL need some pointer to work on, and they won't free it. so you do whatever you need as unique_ptr , pass it as raw pointer, get it some time later and restore it back to unique_ptr. another example is the ThreadPool api on Windows API. you can send context as raw pointer (PVOID) along with your task, and get it back on another thread without the thread pool freeing it. I'm talking out of real experience — David Haim, Mar 16 '16 at 10:11

score 12 · Accepted Answer · edited Dec 13 '18 at 07:02

You should definitely go with the first version as the second one basically does the everything the first version does with more code and less readability.

Philosophically speaking, the second version moves the unique pointer, no more, no less. so why going around the table instead of using the already existing, more readable and more idiomatic std::unique_ptr(std::unique_ptr&&) ?

And lastly, if your smart pointer somewhere in the future will hold custom deleter , the first version will make sure the deleter moves as well, the second version does not moves the deleter. I can definitely imagine a programmer building non-custom deleter unique pointer out of custom-deleter unique_pointer with using release. With move semantics, the program will fail to compile.

IGarFieldI · Answer 2 · 2016-03-16T09:16:15.347

2

What you're doing is dangerous. Once you've called getTree(), you must not call it a second time, which is not clear from the interface. You may want to reconsider your design (e.g. a shared_ptr might do a better job, or simply store the root as a raw pointer and manually take care of the deallocation).

Anyway, using std::move is the better option of the two if you want to stick with your design as it makes your intent more clear.

EDIT: apparently 'must not' has a special meaning of forbideness in English I was not aware of. It is fine to call the function twice or as many times as you want, but will not return a pointer to a valid object if done consecutively.

edited Mar 16 '16 at 09:16

answered Mar 16 '16 at 08:47

IGarFieldI

515
3
12

1

There's no problem with calling `getTree()` a second time. It will just return a null `unique_ptr`. – Benjamin Lindley Mar 16 '16 at 08:56
2

Why must you not call `getTree()` twice? Once the root is moved it is set to `nullptr`, so moving from it again gets you a `nullptr`, which seems like a very reasonable behavior for an empty object. – nwp Mar 16 '16 at 08:57
Would you generally assume, from the function signature and name, that your whole object basically becomes unusable? I wouldn't. If he wants to keep it that way and the behaviour is what he wanted, all is fine, but he may not have been aware of the consequences. – IGarFieldI Mar 16 '16 at 09:01
Who says the whole object becomes unusable? – Benjamin Lindley Mar 16 '16 at 09:09
@BenjaminLindley: I would fully expect to be able to call `getTree()` multiple times, getting the same tree each time, because I did nothing to change it in the meantime (that I know of, without looking at the implementation of `getTree()`). I agree with IGarFieldl that this is surprising behaviour (a.k.a. A Bad Thing (tm)). – DevSolar Mar 16 '16 at 09:11
I'm with @BenjaminLindley here. My expectation is that successive calls return a `nullptr`, which is what I want to happen. I don't think there is an expectation that `unique_ptr` relieves you from the burden of making `nullptr` checks. – kamikaze Mar 16 '16 at 09:12
@DevSolar: Even so, the statement *"you must not call it a second time"* implies that something bad will happen just from that action alone, like a null pointer dereference or some other kind of undefined behavior. – Benjamin Lindley Mar 16 '16 at 09:14
@DevSolar When you get a `unique_ptr` you take over possession of an object. You cannot expect to be able to get a second `unique_ptr` to the same object from the same source. This is exactly the kind of use-after-free scenario `unique_ptr`s are supposed to protect you from. – kamikaze Mar 16 '16 at 09:15
6

I think that it is reasonable to expect `get...()` calls to have a `const` behavior. I think that renaming the function to something like `extractTree()` will convey more clearly the fact that the `Context` will no longer have a tree after this call. – mindriot Mar 16 '16 at 09:17
1

@kamikaze: That's why I find it surprising to *get* a `unique_ptr` from such a function. ;-) Also note that I could do `getTree()->something()` and never even realize I just nuked the tree, wondering why the statement blows the second time around. – DevSolar Mar 16 '16 at 09:17
@mindriot I like your suggestion of renaming the method, I will do that. Even though I think the whole point of a `unique_ptr` is that it's only movable and thus it should be clear what happens. You are right, that I'm violating what one expects from a get-function. Glancing over my code, most of the return copies or const references. – kamikaze Mar 16 '16 at 09:27
So it's the `unique_ptr( unique_ptr&& u );` constructor that releases root when returning using `std::move` ? – Zitrax Dec 15 '16 at 10:13

std::unique_ptr::release() vs std::move()

2 Answers2

Linked