双向链表 std::unique_ptr 类在节点删除时无法按预期工作

Posted 2023-02-18

【中文标题】双向链表 std::unique_ptr 类在节点删除时无法按预期工作

【英文标题】：Doubly linked list std::unique_ptr class doesn't work as expected on node removal

【发布时间】：2019-01-04 21:05:58

【问题描述】：

灵感来自于 Herb Sutter 在 CppCon2016 上的演讲，which can be found in this link. 我决定使用智能指针实现视频中所示的双向链表。 以下实现几乎与 remove() 方法中的一行代码不同。 我调试了这段代码，之前的节点在删除后没有更新为 null（应该是头节点）。 就好像智能指针之间的所有权转移是错误的。 下面是头文件和测试main()的代码：

LinkedList.h

#ifndef LINKEDLIST_H
#define LINKEDLIST_H

#include <iostream>
#include <memory>
#include <initializer_list>

namespace DLL 
    template <typename T> class LinkedList
        private:
            struct ListNode
                std::unique_ptr<ListNode> next; //2 uniq_ptr can't point to one another.
                ListNode* prev = nullptr; //weakptr needs to be cast back to a shared_ptr to check its state.
                T data; //Initialize empty;

            ListNode(const T& element)
                this->data = element;
            
        ;
    public:
        std::unique_ptr<ListNode> head;
        ListNode* tail = nullptr;

        LinkedList()
        ~LinkedList()

        void append(const T& element)
            ListNode* curr = nullptr;
            if (head.get() == nullptr) //If list is empty.
                head = std::make_unique<ListNode>(element);
            
            else if(head.get() -> next.get() == nullptr) //If list has one element
                 head.get() -> next = std::make_unique<ListNode>(element);
                 curr = head.get() -> next.get(); //Sets raw pointer to the first element.
                 curr -> prev = head.get();
                 tail = curr;
            
            else
                tail -> next = std::make_unique<ListNode>(element);
                curr = tail -> next.get(); //Sets raw pointer to the last element.
                curr -> prev = tail;
                tail = curr;// The new last element is the tail.
            
        

        int remove(const T& element)
            ListNode* curr = nullptr;
            if (head.get() == nullptr) //If list is empty.
                return -1; //Error: Can't remove from empty list.
            
            //List has one or more elements.
            curr = head.get();
            while(curr != nullptr)
                if(curr -> data == element) //Found element
                    if(curr -> prev == nullptr) //is head
                    //head.reset(head.get()->next.get()); Doesn't work
                    //Line below doesn't work too
                    head = std::move(curr->next); //Head now points to the next element
                    //New head's previous element doesn't point to nothing, as it should.
                    
                    else if(curr -> next.get() == nullptr) //is tail
                        tail = curr -> prev; //Reference the previous element
                        tail -> next.release(); //Release the old tail element
                        if(head.get() == tail)
                            tail = nullptr; //tail and head should not be the same.
                         //List contains one element
                    
                    else//is intermediate
                        //The next node should point to the previous one
                        curr -> next -> prev = curr -> prev;
                        curr -> prev -> next = std::move(curr -> next);
                        //The prev node now points to the next one of current.
                    
                    return 1; //Element found in list
                
                curr = curr -> next.get(); //Traverse the next element
            
            return 0; //Element not found in list
        

        void print() 
            ListNode* curr = head.get(); //Start from the start of the list.
            std::cout << "[ ";
            while (curr != nullptr) 
                std::cout << curr -> data << " ";
                curr = curr -> next.get();
            
            std::cout << "]" << std::endl;
        
    ;


#endif

ma​​in.cpp

int main()  //Temporary Test Main will be split from the implementation file in the future
    DLL::LinkedList <int> list; //Empty list
    list.append(1);
    list.append(4);
    list.append(5);
    list.append(6);
    list.print();
    list.remove(5);
    list.remove(1); //When 1 is removed the 4 doesn't properly update as head, meaning the previous pointer of 4 is not null
    list.remove(4);
    list.remove(6);
    list.print();
    retunn 0;

我很抱歉这种问题，我搜索了很多，但找不到类似的东西。我调试了好几天，但无法修复所有权线。 我尝试包含最少量的代码，以重现错误如果标题是长代码段，我很抱歉。

我使用 g++:g++ -std=c++14 main.cpp -o out 和 VS2015 编译器编译。 make_unique 调用需要 C++14 标志

【问题讨论】：

@user4581301 它并没有过时，我很快就证明了使用原始指针和弱指针是合理的。

谢谢。我知道你现在要去哪里。

【参考方案1】：

在您检查迭代器的前一个指针是否为空（基本上检查头部）的部分中，您有以下行：

head = std::move(curr->next);

将标头指针移动到元素的下一个指针。但是，您无法将新头指针的前一个指针更新为空。所以这段代码应该是这样的：

if(curr -> data == element) //Found element
    if(curr -> prev == nullptr) //is head
        head = std::move(curr->next); //Head now points to the next element
        if (head)
            head->prev = nullptr;
        else
            tail = nullptr;
    

由于您在成为新头指针的项目上使用std::move（这是正确的），因此您基本上保持该节点中包含的数据相同。在这种情况下，您需要明确说明 - std::unique_ptr 包装器对其拥有的对象的底层实现一无所知，因此在这种情况下它无法知道更新 prev 指针。

【讨论】：

值得注意的是，如果列表只有一个元素，该函数现在将失败。 head = std::move(curr->next); next 为 null，因此 head->prev = nullptr; 取消引用 null。

不是 100% 确定，但我认为这可能是提问者有 //New head's previous element doesn't point to nothing, as it should. 评论的原因。试图弄清楚为什么会有评论是我注意到这一点的原因。

我认为他们期望智能指针在移动时自动清理；无论如何，这就是我阅读该评论的方式。鉴于各种智能指针的不透明性，在开始时会出现一些混乱......

我从重写路径开始——我认为还有其他更有效的双链表方法（链表的少数好处之一是快速插入/删除，你应该不需要迭代）。但我认为在这种情况下最好保持简单的答案。

再次同意。为了完整起见，我建议将答案中的代码更新为 head = std::move(curr->next); if (head) head->prev = nullptr; 之类的内容。