BST with std::unique_ptr: What's difference in using rvalue reference in parameters?

Question

For practice, I'm playing with a binary search tree and a red-black tree with smart pointers.

The Node is as follows:

template <typename T>
struct BSTNode {
    T key;
    std::unique_ptr<BSTNode<T>> left;
    std::unique_ptr<BSTNode<T>> right;
    BSTNode<T>* succ;

    BSTNode(const T& key) : key {key}, succ {nullptr} {}
};

Here, succ means inorder successor, because I'm solving CLRS 12.3-5 which said to do exercise without having a link to parent.

Anyway, this code works happily:

void Transplant(BSTNode<T>* u, std::unique_ptr<BSTNode<T>>&& v) {
        auto p = Parent(u);
        if (!p) {
            root = std::move(v);
        } else if (u == p->left.get()) {
            p->left = std::move(v);
        } else {
            p->right = std::move(v);
        }
    }

    void Delete(BSTNode<T>* z) {
        if (!z) {
            return;
        }
        BSTNode<T>* pred = nullptr;
        if (z->left) {
            pred = Maximum(z->left.get());
        } else {
            auto y = Parent(z);
            auto x = z;
            while (y && x == y->left.get()) {
                x = y;
                y = Parent(y);
            }
            pred = y;
        }
        pred->succ = z->succ;
        if (!z->left) {
            Transplant(z, std::move(z->right));
        } else if (!z->right) {
            Transplant(z, std::move(z->left));
        } else {
            auto y1 = z->right.get();
            std::unique_ptr<BSTNode<T>> y;
            if (!y1->left) {
                y = std::move(z->right);
            } else {
                while (y1->left) {
                    y1 = y1->left.get();
                }
                y = std::move(Parent(y1)->left);
            }
            if (Parent(y.get()) != z) {
                Transplant(y.get(), std::move(y->right));
                y->right = std::move(z->right);
            }
            y->left = std::move(z->left);
            Transplant(z, std::move(y));
        }
    }

In contrast, when I remove the rvalue reference in the subroutine "Transplant", it gives segfault:

   void Transplant(BSTNode<T>* u, std::unique_ptr<BSTNode<T>> v) { // crashes
        auto p = Parent(u);
        if (!p) {
            root = std::move(v);
        } else if (u == p->left.get()) {
            p->left = std::move(v);
        } else {
            p->right = std::move(v);
        }
    }

the crash occurs at the 18th line of Delete, Transplant(z, std::move(z->right));.

What is the difference here? I've been told that std::unique_ptrs are usually passed by values.

Moreover, the code of Transplant without rvalue reference has been worked in the BST code that a node has a link to its parent. I don't see why.

Full code: https://wandbox.org/permlink/zB67AyI43kY6EVqV

Btw, be careful with unique pointers. A tree is destructed recursively and if it is a large unbalanced tree, stack overflow can happen in the process of destruction. — Evg, Jul 16 '20 at 07:42

Evg · Accepted Answer · 2020-07-16T07:47:22.020

When you pass a unique_ptr by value, you lose the original pointer:

void use_ptr(std::unique_ptr<int>) {}

std::unique_ptr<int> ptr = std::make_unique<int>(999);
use_ptr(std::move(ptr));
// here ptr no longer contains a pointer to 999
assert(!ptr);
std::cout << *ptr; // this is UB

When you pass it by rvalue reference, the original pointer remains non-null after function invocation:

void use_ptr(std::unique_ptr<int>&&) {}

std::unique_ptr<int> ptr = std::make_unique<int>(999);
use_ptr(std::move(ptr));
// ptr still manages a pointer to `999`
assert(ptr);
std::cout << *ptr; // this is not UB

std::move itself doesn't move anything, it's just a cast to make something an xvalue. The actual move occurs when you construct std::unique_ptr<int> argument from that xvalue - once you've constructed a new std::unique_ptr<int>, you've lost the original one. When you pass by && you don't construct anything, you simply pass a reference.

Consider this call:

Transplant(z, std::move(z->right));

With

void Transplant(BSTNode<T>*, std::unique_ptr<BSTNode<T>>)

inside Transplant, z->right is a unique_ptr that doesn't own anything, z->right.get() == nullptr, because you've just moved from it into the second Transplant argument.

But I didn't use ```z->right``` after passing it by value. And, even if ```z->right``` is passed as rvalue reference, ```Transplant``` moves it anyway. — frozenca, Jul 16 '20 at 07:09
@frozenca, check the line `while (x != y->right.get())`: `runtime error: member access within null pointer of type 'struct BSTNode'`. — Evg, Jul 16 '20 at 07:14
Oh, I see. Thanks. ```Parent``` uses ```z->right``` afterwards. — frozenca, Jul 16 '20 at 07:19

BST with std::unique_ptr: What's difference in using rvalue reference in parameters?

1 Answers1