1

I'm interested in updating an old personal project to modern C++. I appreciate how RAII simplifies cleanup: instead of making a new object and remembering to delete it before every return point in a function, just make_unique and it will be destroyed appropriately. But I have one nitpick when comparing the generated assembly.

Say there's a class method that replaces one of its unique_ptr members with a new value:

// std::unique_ptr<int> MyClass::m_foo;
void MyClass::refresh_foo(int x) {
    m_foo = std::make_unique<int>(x * 3 + 5);
}

This will create a new int, assign it to m_foo, and then delete the old value of m_foo. But that's not quite the same as the old behavior, which could delete the old value, then create a new one and assign it to m_foo:

// int *MyClass::m_foo;
void MyClass::refresh_foo(int x) {
    delete m_foo;
    m_foo = new int(x * 3 + 5);
}

From the Compiler Explorer, gcc, clang, and MSVC all generate less code for the old way than the new one. I understand why this is the case, since that's the order in which all expressions are evaluated; p = q; has to construct q first. And of course m_foo had an invalid value in between the delete and new lines. But is there a way I'm missing to have unique_ptr destroy its old value and then create a new one in one expression? A "replace_unique"? Seems like it would be helpful when dealing with large objects so two of them don't needlessly coexist.

Edit: To be clear, I'm just using int as a trivial example here. My actual use cases are with classes of my own or from a library. And m_foo.reset(new int(x * 3 + 5)); naturally has the same problem compared to the old way of doing things, since it too has to construct the new int before assigning it and deleting the previous one.

Remy
  • 401
  • 2
  • 19
  • How about m_foo.reset(new int(x * 3 + 5)) ? – Severin Pappadeux Sep 22 '21 at 02:35
  • Why are you allocating a new object at all, anyway? Why not just `*m_foo = x*3 + 5`? – user2357112 Sep 22 '21 at 02:35
  • @SeverinPappadeux both reset() and operator= update the held pointer to point at the new object before destroying the old object. The OP wants the opposite. – Remy Lebeau Sep 22 '21 at 04:17
  • 1
    Note that the two behaviors are not equivalent. If an exception occurs in `new int(...)`, the so-called "old behavior" leaves `m_foo = nullptr`, whereas the `m_foo = std::make_unique(...)` leaves `m_foo` unchanged. – Raymond Chen Sep 22 '21 at 04:24
  • 1
    @RaymondChen actually, in the "old behavior", an exception in `new int` will leave `m_foo` *dangling* since the previous `int` was destroyed but the pointer is unchanged, not null. – Remy Lebeau Sep 22 '21 at 15:37

2 Answers2

3

You can use unique_ptr::reset() to deterministically destroy the currently held object when you want. Update the unique_ptr with a nullptr pointer before then updating it with a new object pointer, eg:

// std::unique_ptr<int> MyClass::m_foo;
void MyClass::refresh_foo(int x) {
    m_foo.reset(); // <- int is destroyed here
    m_foo = std::make_unique<int>(x * 3 + 5);
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Thanks! m_foo.reset() and m_foo = nullptr both generate identical code. Interestingly, gcc and clang both leave a second delete call at the end, "just in case" m_foo needs destroying again. Which is just standards compliance according to stackoverflow.com/questions/45689690. – Remy Sep 22 '21 at 13:15
0

I accepted the answer which calls m_foo.reset() before m_foo = std::make_unique<int>(...) because it's the simplest way to solve the problem as stated: destroying the original value before constructing the new one. However, that does result in an extra delete call which shouldn't be necessary and is less optimal than the "pre-modern" way of using raw pointers.

This method, at least in Compiler Explorer with -Ofast, avoids the second delete call, and only has one extra mov instruction compared to the old way (explicitly setting m_foo to nullptr before assigning its newly constructed value):

// std::unique_ptr<int> MyClass::m_foo;
void refresh_smart(int x) {
    m_foo = nullptr;
    auto new_foo = std::make_unique<int>(x * 3 + 5);
    m_foo.swap(new_foo);
    new_foo.release(); // no memory leak since it's null
}

As a reusable template function:

template<typename T, typename... Ps>
void remake_unique(std::unique_ptr<T> &p, Ps... args) {
    p = nullptr;
    auto new_p = std::make_unique<T>(args...);
    p.swap(new_p);
    new_p.release();
}

// std::unique_ptr<int> MyClass::m_foo;
void refresh_smart(int x) {
    remake_unique(m_foo, x * 3 + 5);
}
Remy
  • 401
  • 2
  • 19
  • The `swap()` and `release()` is not necessary, since `m_foo` is `nullptr` at that point, so this is merely setting `m_foo` to the new object in `new_foo` and setting `new_foo` to `nullptr`, thus making the subsequent `release()` redundant. This is really no better than just moving the result of `make_unique()` into `m_foo` directly. You really shouldn't be striving to replicate the "old" behavior, as it was incorrect to begin with. Use `unique_ptr` the way it is meant to be used. If your code relies on the order of destructions, then you have bigger problems to deal with. – Remy Lebeau Sep 22 '21 at 15:35
  • I see, thanks. You're right, I'm not depending on the order of destructors here, and apart from that order, the assembly for `m_foo = make_unique()` is just as good as the "old" behavior (and better than if I do `new_foo = make_unique(); delete m_foo; m_foo = new_foo` to have the same construct-new/destroy-old order). – Remy Sep 22 '21 at 16:11
  • (Comparing all three approaches: https://godbolt.org/z/x3vr74b95) – Remy Sep 22 '21 at 16:18