Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dropping #125

Closed
markcsaving opened this issue May 14, 2019 · 4 comments
Closed

Dropping #125

markcsaving opened this issue May 14, 2019 · 4 comments

Comments

@markcsaving
Copy link

I'm new to Rust, but I'm curious as to why we don't implement drop for the first two lists like this:

fn drop(&mut self) {
while let Some(_) = self.pop() {}
}

This seems like a very obvious implementation, and it seems more elegant than the given implementations. I ran a test which creates a list of a million elements and lets said list go out of scope, and everything went fine (the test resulted in stack overflow when I ran it before implementing drop).

Is there some reason I'm not seeing to prefer the given implementations?

@Gankra
Copy link
Collaborator

Gankra commented May 14, 2019

I believe you are correct, it just didn't occur to me to do so when I wrote it. It doesn't really hurt to have some more motivation to write some more non-trivial code, though.

As a minor point in my favour, the pop-based implementation is semantically demanding that the payload of each node be copied out of the allocation and onto the stack to be destroyed. The manual implementation I provide avoids this, instead only copying around Boxes (pointers). This could be fairly important if the generic case ends up having a rather large type with an actual destructor to run. I wouldn't be surprised if this difference had a tendency to evaporate in optimized builds, though.

@Gankra
Copy link
Collaborator

Gankra commented May 14, 2019

(an implementation which wanted the best of both worlds would introduce a pop_node method that returns Option<Box<Node>>, from-which pop and drop are derived)

@Gankra
Copy link
Collaborator

Gankra commented May 14, 2019

@Gankra Gankra closed this as completed May 14, 2019
@markcsaving
Copy link
Author

An excellent answer! Your book is a great resource, and I'm glad to have indirectly contributed to part of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants