rust-unofficial / too-many-lists

Learn Rust by writing Entirely Too Many linked lists
https://rust-unofficial.github.io/too-many-lists/
MIT License
3.18k stars 276 forks source link

# 2.8 Drop manully implementation problems #239

Closed kobeHub closed 2 years ago

kobeHub commented 2 years ago

I'm confused about the 2.8 list drop manully implementation. The drop code for Box<T> runs, this first calls drop on the T.

impl Drop for List {
    fn drop(&mut self) {
        // NOTE: you can't actually explicitly call `drop` in real Rust code;
        // we're pretending to be the compiler!
        self.head.drop(); // tail recursive - good!
    }
}

impl Drop for Link {
    fn drop(&mut self) {
        match *self {
            Link::Empty => {} // Done!
            Link::More(ref mut boxed_node) => {
                boxed_node.drop(); // tail recursive - good!
            }
        }
    }
}

impl Drop for Box<Node> {
    fn drop(&mut self) {
        // Shall Box ptr pointed data drop first? then drop `node.next`, isn't it a tail recursive? 
        // self.Node.drop(); ???
        self.ptr.drop(); // uh oh, not tail recursive!
        deallocate(self.ptr);
    }
}

impl Drop for Node {
    fn drop(&mut self) {
        self.next.drop();
    }
}

Anyway Box should clear the data properly, is there something wrong or I misunderstand? Can someone explain that a little bit, thank you.

makisevon commented 2 years ago

Assuming that

Then

Drop L
    Drop A
        Drop A.Data
        Drop A.Next->B
            Drop A.Next->B.Data
            Drop A.Next->B.Next->...
            ...
            Drop A.Next->B.Next
        Drop A.Next

Not

Drop L
    Drop A
        Drop A.Data
        Drop A.Next
        Drop A.Next->B
            Drop B.Data
            Drop B.Next
            Drop B.Next->...
            ...

In standard library, you will see

unsafe impl<#[may_dangle] T: ?Sized, A: Allocator> Drop for Box<T, A> {}

Note the #[may_dangle].

nicarran commented 2 years ago

The way I understand it: the list can clear the data properly using the default drop implementation if the list size is not large enough to blow the stack because it is recursive. But if we can be sure that the default drop implementation is tail-recursive then the compiler/runtime magic supposedly can avoid blowing the stack... but it isn't tail-recursive, as explained above by @makisevon , so a custom drop implementation is needed to avoid blowing the stack.

kobeHub commented 2 years ago

I got it, thanks for your explaination!