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

rustc: Implement the Drop trait #3925

Closed
wants to merge 1 commit into from

Conversation

pcwalton
Copy link
Contributor

@pcwalton pcwalton commented Nov 6, 2012

r? @brson

@brson
Copy link
Contributor

brson commented Nov 6, 2012

@jruderman The plan is to remove drop for 0.5, so all the existing tests will get converted to the trait. With that in mind, the test coverage here looks very good to me. Two things that I can think of that aren't covered here are parameterizing over the Drop trait and using it as an object - these things probably work fine, but Drop is special, and these are scenarios that real code will rarely use, so regression tests might be appropriate.

I'm also curious about how trait/impl visibility works, and how that releates to Drop.

@brson
Copy link
Contributor

brson commented Nov 6, 2012

r+

@pcwalton
Copy link
Contributor Author

pcwalton commented Nov 7, 2012

It raises annoying issues about where to place the drop flag, and I didn't want to put too much in this patch.

@nikomatsakis
Copy link
Contributor

Regarding enums, that's fine. It seems like we can (eventually) treat them the same we treat structs. That is, we translate a struct Foo { ... } to { {...} }. If there is a drop implementation, it becomes { {...}, flag }. Presumably we can wrap enums in an extra struct layer in the same way. This means that you can access fields or contents without caring about whether there is a destructor or not.

@pcwalton pcwalton closed this Nov 7, 2012
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

Successfully merging this pull request may close these issues.

5 participants