67

This code is what I want to do:

Tony& Movie::addTony()
{
    Tony *newTony = new Tony;
    std::unique_ptr<Tony> tony(newTony);
    attachActor(std::move(tony));
    return *newTony;
}

I am wondering if I could do this instead:

Tony& Movie::addTony()
{
    std::unique_ptr<Tony> tony(new Tony);
    attachActor(std::move(tony));
    return *tony.get();
}

But will *tony.get() be the same pointer or null? I know I could verify, but what is the standard thing for it to do?

user3496846
  • 1,627
  • 3
  • 16
  • 28
  • 1
    Why are you `move()`'ing the `unique_ptr` into `attachActor()` to begin with? What does `attachActor()` actually do? – Remy Lebeau Mar 17 '16 at 20:45

3 Answers3

53

No, you cannot do that instead. Moving the unique_ptr nulls it. If it didn't, then it would not be unique. I am of course assuming that attachActor doesn't do something silly like this:

attachActor(std::unique_ptr<Tony>&&) {
    // take the unique_ptr by r-value reference,
    // and then don't move from it, leaving the
    // original intact
}

Section 20.8.1 paragraph 4.

Additionally, u (the unique_ptr object) can, upon request, transfer ownership to another unique pointer u2. Upon completion of such a transfer, the following postconditions hold:
   -- u2.p is equal to the pre-transfer u.p,
   -- u.p is equal to nullptr, and
   -- if the pre-transfer u.d maintained state, such state has been transferred to u2.d.

Benjamin Lindley
  • 101,917
  • 9
  • 204
  • 274
  • Though, it's probably not a great idea to have a function that takes `unique_ptr&&` anyway (aside from move-constructor and move-assigners). – Rufflewind Mar 17 '16 at 20:48
  • @BenjaminLindley, thank you for the answer! Yeah, attachActor() takes std::unique_ptr, nothing fancy. – user3496846 Mar 17 '16 at 21:00
  • @Rufflewind Not necessary, it might be handy if the function can throw and you don't want to lose the object you passed to it. Basically every container `insert`/`push_back` function. – sbabbi Mar 17 '16 at 21:06
21

The standard says (§ 20.8.1.2.1 ¶ 16, emphasis added) that the move constructor of std::unique_ptr

unique_ptr(unique_ptr&& u) noexcept;

Constructs a unique_ptr by transferring ownership from u to *this.

Therefore, after you move-construct the temporary object that gets passed as argument to attachActor form your tony, tony no longer owns the object and hence tony.get() == nullptr. (This is one of the few cases where the standard library actually makes assertions about the state of a moved-away-from object.)

However, the desire to return the reference can be fulfilled without resorting to naked new and raw pointers.

Tony&
Movie::addTony()
{
  auto tony = std::make_unique<Tony>();
  auto p = tony.get();
  attachActor(std::move(tony));
  return *p;
}

This code assumes that attachActor will not drop its argument on the floor. Otherwise, the pointer p would dangle after attachActor has returned. If this cannot be relied upon, you'll have to re-design your interface and use shared pointers instead.

std::shared_ptr<Tony>
Movie::addTony()
{
  auto tony = std::make_shared<Tony>();
  attachActor(tony);
  return tony;
}
5gon12eder
  • 24,280
  • 5
  • 45
  • 92
  • 1
    Transfer of ownership does not imply that `u` does not own anything, only that it does no longer own the object it owned before. If this was the only requirement, the transfer could happen by just **swapping** object ownership between `u` and `*this` (which is what some other move constructors do to prevent any allocation and deallocation). But as Benjamin Lindley notes, there are additional requirements on std::unique_ptr beyond just the textual "transferring ownership". – Dreamer Jan 24 '17 at 12:58
4

After move, unique_ptrs are set to nullptr. Finally, I think it depends on what attachActor is doing, however, in many cases, a good approach would be to use move semantics to guarantee single ownership for Tony at all times which is a way to reduce the risks of some types of bugs. My idea is to try to mimic ownership and borrowing from Rust.

    #include <string>
    #include <memory>
    #include <vector>
    #include <iostream>
    
    
    using namespace std;
    
    class Tony {
        public:
            string GetFullName(){
                return "Tony " + last_name_;
            }
            void SetLastName(string lastname) {
                last_name_ = lastname;
            }
        private:
            string last_name_;
    };
    
    class Movie {
        public:
            unique_ptr<Tony> MakeTony() {
                auto tony_ptr = make_unique<Tony>();
                auto attached_tony_ptr = AttachActor(move(tony_ptr));
                return attached_tony_ptr;
            }
            vector<string> GetActorsList(){
                return actors_list_;
            }
    
        private:
            unique_ptr<Tony> AttachActor(unique_ptr<Tony> tony_ptr) {
                tony_ptr->SetLastName("Garcia");
                actors_list_.push_back(tony_ptr->GetFullName());
                return tony_ptr;   // Implicit move
            }
    
            vector<string> actors_list_;
    };
    
    
    int main (int argc, char** argv) {
        auto movie = make_unique<Movie>();
        auto tony = movie->MakeTony();
        cout << "Newly added: " << tony->GetFullName() << endl;
        for(const auto& actor_name: movie->GetActorsList()) {
            cout << "Movie actors: " << actor_name << endl;
        }
    }
Albino Cordeiro
  • 1,554
  • 1
  • 17
  • 25