Generic Single Linked List using std::unique_ptr, unknown uncompilation errors in Microsoft Visual Studio C++

Question

I am very new to Microsoft visual studio C++ as well as std::unique_ptr. On CodeReview I was recommended to rewrite using std::unique_ptr. You can find the question I am referencing here.

Here are the following errors I am receiving:

1>main.cpp
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(26): error C2760: syntax error: unexpected token 'identifier', expected ';'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(61): note: see reference to class template instantiation 'SingleLinkedList<T>' being compiled
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(65): error C2760: syntax error: unexpected token 'identifier', expected ';'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(104): error C2760: syntax error: unexpected token 'identifier', expected ';'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(104): error C7510: 'make_unique': use of dependent type name must be prefixed with 'typename'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(120): error C2760: syntax error: unexpected token 'identifier', expected ';'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(120): error C7510: 'make_unique': use of dependent type name must be prefixed with 'typename'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(136): error C2760: syntax error: unexpected token 'identifier', expected ';'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(136): error C7510: 'make_unique': use of dependent type name must be prefixed with 'typename'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(145): error C2760: syntax error: unexpected token 'identifier', expected ';'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(145): error C7510: 'make_unique': use of dependent type name must be prefixed with 'typename'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(152): error C2760: syntax error: unexpected token 'identifier', expected ';'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(152): error C7510: 'make_unique': use of dependent type name must be prefixed with 'typename'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(164): error C2760: syntax error: unexpected token 'identifier', expected ';'
1>c:\dev\linkedlist\linkedlist\singlelinkedlist.h(164): error C7510: 'make_unique': use of dependent type name must be prefixed with 'typename'
1>Done building project "LinkedList.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

Here is the header file:

#ifndef SingleLinkedList_h
#define SingleLinkedList_h

#include <iostream>

template <class T>
class SingleLinkedList {
private:

    struct Node {
        T data;
        std::unique_ptr<Node> next = nullptr;
        Node(T x) : data(x), next(nullptr) {}
    };
    std::unique_ptr<Node> head = nullptr;
    std::unique_ptr<Node> tail = nullptr;

    // This function is for the overloaded operator << 
    void display(std::ostream &str) const {
        for (std::make_unique<Node> loop = head; loop != nullptr; loop = loop->next) {
            str << loop->data << "\t";
        }
        str << "\n";
    }

public:
    // Constructors
    SingleLinkedList() = default;                                           // empty constructor 
    SingleLinkedList(SingleLinkedList const &source);                       // copy constructor

    // Rule of 5
    SingleLinkedList(SingleLinkedList &&move) noexcept;                     // move constructor
    SingleLinkedList& operator=(SingleLinkedList &&move) noexcept;          // move assignment operator
    ~SingleLinkedList();                                    

    // Overload operators
    SingleLinkedList& operator=(SingleLinkedList const &rhs);
    friend std::ostream& operator<<(std::ostream &str, SingleLinkedList &data) {
        data.display(str);
        return str;
    }

    // Memeber functions
    void swap(SingleLinkedList &other) noexcept;
    void push(const T &theData);                            
    void push(T &&theData);
    void display() const;
    void insertHead(const T &theData);
    void insertTail(const T &theData);
    void insertPosition(int pos, const T &theData);
    void deleteHead();
    void deleteTail();
    void deletePosition(int pos);
    bool search(const T &x);
};

template <class T>
SingleLinkedList<T>::SingleLinkedList(SingleLinkedList<T> const &source) {
    for(std::make_unique<Node> loop = source->head; loop != nullptr; loop = loop->next) {
        push(loop->data);
    }
}

template <class T>
SingleLinkedList<T>::SingleLinkedList(SingleLinkedList<T>&& move) noexcept {
    move.swap(*this);
}

template <class T>
SingleLinkedList<T>& SingleLinkedList<T>::operator=(SingleLinkedList<T> &&move) noexcept {
    move.swap(*this);
    return *this;
}

template <class T>
SingleLinkedList<T>::~SingleLinkedList() {
    while (head != nullptr) {
        deleteHead();
    }
}

template <class T>
SingleLinkedList<T>& SingleLinkedList<T>::operator=(SingleLinkedList const &rhs) {
    SingleLinkedList copy{ rhs };
    swap(copy);
    return *this;
}

template <class T>
void SingleLinkedList<T>::swap(SingleLinkedList &other) noexcept {
    using std::swap;
    swap(head, other.head);
    swap(tail, other.tail);
}

template <class T>
void SingleLinkedList<T>::push(const T &theData) {
    std::make_unique<Node> newNode = Node(theData);

    if (head == nullptr) {
        head = newNode;
        tail = newNode;
        newNode = nullptr;
    }

    else {
        tail->next = newNode;
        tail = newNode;
    }
}

template <class T>
void SingleLinkedList<T>::push(T &&theData) {
    std::make_unique<Node> newNode = Node(std::move(theData));

    if (head == nullptr) {
        head = newNode;
        tail = newNode;
        newNode = nullptr;
    }

    else {
        tail->next = newNode;
        tail = newNode;
    }
}

template <class T>
void SingleLinkedList<T>::display() const {
    std::make_unique<Node> newNode = head;
    while (newNode != nullptr) {
        std::cout << newNode->data << "\t";
        newNode = newNode->next;
    }
}

template <class T>
void SingleLinkedList<T>::insertHead(const T &theData) {
    std::make_unique<Node> newNode = Node(theData);
    newNode->next = head;
    head = newNode;
}

template <class T>
void SingleLinkedList<T>::insertTail(const T &theData) {
    std::make_unique<Node> newNode = Node(theData);
    tail->next = newNode;
    tail = newNode;
}

template <class T>
void SingleLinkedList<T>::insertPosition(int pos, const T &theData) {

}

template <class T>
void SingleLinkedList<T>::deleteHead() {
    std::make_unique<Node> old = head;
    head = head->next;
    delete old;
}

template <class T>
void SingleLinkedList<T>::deleteTail() {

}

template <class T>
void SingleLinkedList<T>::deletePosition(int pos) {

}

template <class T>
bool SingleLinkedList<T>::search(const T &x) {

}




#endif /* SingleLinkedList_h*/

Here is the main.cpp file:

#include <algorithm>
#include <cassert>
#include <iostream>
#include <ostream>
#include <iosfwd>
#include "SingleLinkedList.h"


    int main(int argc, const char * argv[]) {


        ///////////////////////////////////////////////////////////////////////
        ///////////////////////////// Single Linked List //////////////////////
        ///////////////////////////////////////////////////////////////////////
        SingleLinkedList<int> obj;
        obj.push(2);
        obj.push(4);
        obj.push(6);
        obj.push(8);
        obj.push(10);
        std::cout<<"\n--------------------------------------------------\n";
        std::cout<<"---------------displaying all nodes---------------";
        std::cout<<"\n--------------------------------------------------\n";
        std::cout << obj << std::endl;

        //
        //    std::cout<<"\n--------------------------------------------------\n";
        //    std::cout<<"-----------------Inserting At End-----------------";
        //    std::cout<<"\n--------------------------------------------------\n";
        //    obj.insertTail(20);
        //    std::cout << obj << std::endl;
        //
        //    std::cout<<"\n--------------------------------------------------\n";
        //    std::cout<<"----------------Inserting At Start----------------";
        //    std::cout<<"\n--------------------------------------------------\n";
        //    obj.insertHead(50);
        //    std::cout << obj << std::endl;
        //
        //    std::cout<<"\n--------------------------------------------------\n";
        //    std::cout<<"-------------Inserting At Particular--------------";
        //    std::cout<<"\n--------------------------------------------------\n";
        //    obj.insertPosition(5,60);
        //    std::cout << obj << std::endl;
        //
        //    std::cout<<"\n--------------------------------------------------\n";
        //    std::cout<<"----------------Deleting At Start-----------------";
        //    std::cout<<"\n--------------------------------------------------\n";
        //    obj.deleteHead();
        //    std::cout << obj << std::endl;
        //
        //    std::cout<<"\n--------------------------------------------------\n";
        //    std::cout<<"----------------Deleting At End-----------------";
        //    std::cout<<"\n--------------------------------------------------\n";
        //    obj.deleteTail();
        //    std::cout << obj << std::endl;
        //
        //
        //    std::cout<<"\n--------------------------------------------------\n";
        //    std::cout<<"--------------Deleting At Particular--------------";
        //    std::cout<<"\n--------------------------------------------------\n";
        //    obj.deletePosition(4);
        //    std::cout << obj << std::endl;
        //    std::cout << std::endl;
        //
        //    obj.search(8) ? printf("Yes"):printf("No");







        std::cin.get();
}

I assume most of the errors are syntax errors or that I just made very careless mistakes. Thanks.

Answer 1

The bulk of your syntax errors are derivatives of the same mistake, so lets look at the first one. It's extra-instructive because it points out TWO mistakes. Any time you can kill two stones with one bird, I say go for it. It's damn hard to kill a stone at the best of times.

void display(std::ostream &str) const {
    for (std::make_unique<Node> loop = head; loop != nullptr; loop = loop->next) {
        str << loop->data << "\t";
    }
    str << "\n";
}

In std::make_unique<Node> loop = head, std::make_uniqueis a function that gives you a std::unique_ptr. It is not a type you can use to declare a variable. This mistake is repeated several times through the code and cleaning up all of them will reveal a bunch of new errors. Yay fun.

Step one, let's replace the function with the correct type.

void display(std::ostream &str) const {
    for (std::unique_ptr<Node> loop = head; loop != nullptr; loop = loop->next) {
        str << loop->data << "\t";
    }
    str << "\n";
}

Groovy. Simple. Unfortunately it doesn't work. std::unique_ptr<Node> loop = head means you now have two unique_ptrs to the same object. That doesn't sound particularly unique and the compiler won't allow it. You'll probably get some arcane error message about a deleted function. That's because unique_ptr's copy constructor and = operator have been banished to the dark dimension to make it harder to accidentally wind up with multiple unique_ptrs.

A quick note on why all this matters: Ownership. One of the biggest problems with pointers is deciding who deletes what and when. To sort this out you establish ownership of a pointer. Perhaps the pointer is to an Automatic variable and the stack or whatever's being used to manage Automatic data will take care of it. You delete NOTHING! Muhuhahahaha! Maybe someone newed the variable. Someone has to delete it. That someone is the owner. Some insane fool may even have malloced the object. Someone has to free it instead of deleteing it. Again, that's the owner.

There is no practical way to determine how the object was allocated and how it needs to be disposed of from a pointer. All a pointer does is point. The programmer needs to establish a contract that outlines who, if anybody, looks after it and how. In the old days this could be a painstaking process.

History has shown that contract-based pointer management is difficult to get right. And if you do get it right, the next programmer to come along can completely smurf it up. Pointer management works a lot better when the contract has some sort of automated enforcement.

Today we use a smart pointer. There are a bunch of smart pointers. We're sticking to unique_ptr this time. If you want to know more, read What is a smart pointer and when should I use one?

unique_ptr is the owner. It is a plain old Automatic variable that happens to contain a pointer that it will release as soon as the unique_ptr goes out of scope. No more worries about ownership. The owner is explicit. And like the Highlander, There can be only one.. If you have two unique_ptrs to the same object, it's not all that unique, is it? With two owners, one would go out of scope first and destroy the object, leaving the other owner a ticking timebomb.

Do not new a unique_ptr. That completely defeats the point.

You cannot copy a unique_ptr, but you can transfer ownership with std::move. Note that the object, if any, currently owned by the the destination unique_ptr will be destroyed.

If you call a function that receives a unique_ptr, you will have to transfer ownership to the receiving function parameter. The pointer will now be destroyed the function returns, assuming it doesn't transfer ownership of the pointer elsewhere. Since you often don't want this to happen, but still want the ownership of the pointer to remain clear, pass the unique_ptr by reference.

You can also use the unique_ptr::get method to get a raw, un-owned pointer, but whoever you give it to must know to leave the pointer alone, and that brings us back to contract- based pointer management.

Huh. That wasn't all that <expletive deleted> quick, was it? Sorry about that.

Getting back on topic, your pointer is owned by a unique_ptr. You can't just make a new copy with std::unique_ptr<Node> loop = head;, Highlander's law, above. Transferring ownership to a tightly scoped Automatic variable that will die and destroy the pointer at the end of the loop with std::unique_ptr<Node> loop = std::move(head); would leave the head pointer pointing to nothing. Not useful. The linked list is ruined, but at least everything in it was "correctly" destroyed.

As discussed above you can use a reference or get. You cannot reseat the reference with loop = loop->next, so reference cannot work. That leaves get

void display(std::ostream &str) const {
    for (Node* loop = head.get(); loop != nullptr; loop = loop->next.get()) {
        str << loop->data << "\t";
    }
    str << "\n";
}

There is very little room in which the programmer can get confused by the raw pointer here and ownership is still guaranteed by the unique_ptr, so all should be good.

This problem is repeated in

template <class T>
SingleLinkedList<T>::SingleLinkedList(SingleLinkedList<T> const &source) {
    for(std::make_unique<Node> loop = source->head; loop != nullptr; loop = loop->next) {
        push(loop->data);
    }
}

Same solution.

There is a variant in the two push methods

template <class T>
void SingleLinkedList<T>::push(const T &theData) {
    std::make_unique<Node> newNode = Node(theData);

    if (head == nullptr) {
        head = newNode;
        tail = newNode;
        newNode = nullptr;
    }

    else {
        tail->next = newNode;
        tail = newNode;
    }
}

Solution (sort of):

template <class T>
void SingleLinkedList<T>::push(const T &theData) {
    std::unique_ptr<Node> newNode = std::make_unique<Node>(theData);

    if (head == nullptr) {
        head = newNode;
        tail = newNode;
        newNode = nullptr;
    }

    else {
        tail->next = newNode;
        tail = newNode;
    }
}

But note the assignments to head and tail. That violates Highlander and needs to be fixed. You can transfer ownership of newNode into head, but then there is nothing left to give tail. And why should tail get ownership of anything? Whatever tail points at should be owned by head or some Node's next. It's ownership is guaranteed. Since it's confined within SingleLinkedList it is entering into a contract with itself and can get away with just being a plain old pointer.

That means

Node* tail = nullptr;

and

template <class T>
void SingleLinkedList<T>::push(const T &theData) {
    std::unique_ptr<Node> newNode = std::make_unique<Node>(theData);

    if (head == nullptr) {
        head = std::move(newNode);
        tail = head.get();
    }
    else {
        tail->next = std::move(newNode);
        tail = tail->next.get();
    }
}

I'm out of time, but

void SingleLinkedList<T>::insertHead(const T &theData)

and

void SingleLinkedList<T>::insertTail(const T &theData) {

need to be updated to the new world order and take into account "What if the list is empty?"