Unique Pointer attempting to reference a deleted function

Question

Hello I am trying to use pointers and learning the basics on unique pointers in C++. Below is my code I have commented the line of code in main function. to debug the problem However, I am unable to do so. What am I missing ? Is my move() in the insertNode() incorrect ? The error I get is below the code :

#include<memory>
#include<iostream>

struct node{
    int data;
    std::unique_ptr<node> next;
};


void print(std::unique_ptr<node>head){
        while (head)
            std::cout << head->data<<std::endl;
    }


std::unique_ptr<node> insertNode(std::unique_ptr<node>head, int value){
        node newNode;
        newNode.data = value;
        //head is empty
        if (!head){
            return std::make_unique<node>(newNode);
        }
        else{
            //head points to an existing list
            newNode.next = move(head->next);
            return std::make_unique<node>(newNode);
        }
    }



auto main() -> int
{
    //std::unique_ptr<node>head;
    //for (int i = 1; i < 10; i++){
    //  //head = insertNode(head, i);
    //}
}

ERROR std::unique_ptr>::unique_ptr(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)' : attempting to reference a deleted function

Solution

Aside from other small problems, the main issue is this line:

return std::make_unique<node>(newNode);

You are trying to construct a unique pointer to a new node, passing newNode to the copy constructor of node. However, the copy constructor of node is deleted, since node contains a non-copyable type (i.e. std::unique_ptr<node>).

You should pass a std::move(newNode) instead, but this is problematic since you create the node on the stack and it will be destroyed at the exit from the function.

Using a std::unique_ptr here is a bad idea in my opinion, since, for example, to print the list (or insert into the list), you need to std::move the head (so you lose it) and so on. I think you're much better off with a std::shared_ptr.

Comments

[Neil Kirk]

You can't copy unique_ptrs. Your print function can't work without moving the unique_ptr you use as a parameter, making it empty. I would make the parameter const node&. As print function doesn't keep any reference to the parameter after it returns, it doesn't need to know about how it is managed.

[Ben Voigt]

print is broken in other ways too -- right now it is an infinite loop. And insertNode needs to be pass-by-reference also

[szulak]

auto main() -> int. really?

[Julian]

You shouldn't really be using unique_ptr like this. For example, by passing a unique_ptr to your print function, you're implying that it is taking ownership of the pointer; when really that isn't the case. The enclosing scope of a unique_ptr is said to be the sole owner of it (which is why it's called unique); you can move this ownership but not share/copy it. A shared_ptr, on the other hand, represents ownership that is shared; a raw/weak pointer or reference (probably what your print function should accept... const qualified) generally indicates a lack of ownership.

[Ben Voigt]

Diagnosis is correct, but recommendation to use shared_ptr is baseless. There's no reason that any of this code should be copying smart pointers.

[vsoftco]

@BenVoigt I actually agree with your comment, I should probably remove my answer.

[Ben Voigt]

Perhaps just the last paragraph should be changed or improved. The rest is helpful.

[vsoftco]

@BenVoigt I'd like to make this answer better, can you suggest how? I grant you the right to edit and add it to my answer, without any complaint from my side. In general, you'd need to copy at least the "head" of the list, so what kind of pointer would you'd use for?

[Ben Voigt]

I rather liked WhozCraig's method in his now-deleted answer, that the unique_ptr that used to hold the head now holds the new head. There isn't really a reason to copy the head pointer, because the action of linking another node in front makes it no longer the head. Actually pass-by-value to insertHead, as used in the question, is not so bad after all, since it forces the caller to be aware that the old head pointer is being taken away from them.