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.

Sign up for GitHub

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

Jump to bottom

Once confident in dropck: remove #[unsafe_destructor] attribute #22196

Closed
pnkfelix opened this issue Feb 11, 2015 · 7 comments · Fixed by #23819
Closed

Once confident in dropck: remove #[unsafe_destructor] attribute #22196

pnkfelix opened this issue Feb 11, 2015 · 7 comments · Fixed by #23819
Assignees
@pnkfelix
Labels
P-medium Medium priority
Milestone
1.0 beta

Comments

@pnkfelix
Copy link
Member

Spawned off of #8861, this is the last step in RFC #769, Sound Generic Drop.

Basically, once we are relatively confident that:

  • the Drop-Check rule is sound,
  • the dropck code is at least a sound approximation of that rule, and
  • we have audited APIs in libstd for destructor safety,

then we should remove the #[unsafe_destructor] attribute and all the uses of it.
@pnkfelix
Copy link
Member Author

Nominating; we should decide whether we are going to do this for 1.0 beta, or if it can wait until the 1.0 release itself as a "polish issue." (or if it can wait even beyond that, i.e. to 1.x. But I am not advocating that position.)

(However, given that #21972 just landed today, it may be a good idea to let the code settle for a week before we actually make a decision about this. Its super easy to remove #[unsafe_destructor], as one can see from the two commits here, its just a matter of having the will to do it. :) )

@pnkfelix pnkfelix changed the title Once confident in Sound Generic Drop: remove #[unsafe_destructor] attribute Once confident in dropck: remove #[unsafe_destructor] attribute Feb 11, 2015
@nikomatsakis nikomatsakis mentioned this issue Feb 12, 2015
Closed
7 tasks
@pnkfelix pnkfelix mentioned this issue Feb 12, 2015
Closed
@pnkfelix
Copy link
Member Author

1.0 beta, P-high.

@pnkfelix pnkfelix added this to the 1.0 beta milestone Feb 12, 2015
@pnkfelix pnkfelix added P-medium Medium priority and removed I-nominated labels Feb 12, 2015
@tomjakubowski tomjakubowski mentioned this issue Feb 15, 2015
Closed
@SSheldon
Copy link
Contributor

@pnkfelix, is this still slated for the 1.0 beta? unsafe_destructor is the last feature my library uses, and I'd love to be able to run it stable on the beta. (Some discussion in #8142 seems to indicate that's a blocker but won't be completed for the beta.)

@pnkfelix
Copy link
Member Author

@SSheldon I'm going to try, but I don't know if I'll be able to do everything necessary to unfeature gate it by the beta. We will see, I'm reviewing the remaining tasks now.

@SSheldon
Copy link
Contributor

@pnkfelix, thanks! For what it's worth, I was able to remove the unsafe_destructor feature in my library by moving some of the fields of my struct into a new, non-generic struct and implementing Drop on that instead of the generic struct: SSheldon/rust-objc@b2bd3e0. (For anyone else that runs into this, that could potentially be a workaround.)

@diwic
Copy link
Contributor

diwic commented Mar 23, 2015

If you can't unfeature gate the entire unsafe_destructor, would it be possible to only unfeature gate those Drop impls we're confident with? Like, the simple and obvious ones (for various values of "simple" and "obvious" :-) ). I suspect a majority of Drop impls would be simple and obvious.

@pnkfelix
Copy link
Member Author

at this point I have a patch for the main known issue : PR #23638.

An audit of libstd still needs to happen, but I would be okay with doing that in between the beta release and 1.0 (and unfeature-gating #[unsafe_destructor] in the meantime. I'll bring it up with the team.

pnkfelix added a commit to pnkfelix/rust that referenced this issue Mar 28, 2015
@pnkfelix
Port of pcwalton removal of #[unsafe_destructor] check.
64c48f3 
Earlier commits impose rules on lifetimes that make generic
destructors safe; thus we no longer need the `#[unsafe_destructor]`
attribute nor its associated check.

----

So remove the check for the unsafe_destructor attribute.

And remove outdated compile-fail tests from when lifetime-parameteric
dtors were disallowed/unsafe.

In addition, when one uses the attribute without the associated
feature, report that the attribute is deprecated.

However, I do not think this is a breaking-change, because the
attribute and feature are still currently accepted by the compiler.
(After the next snapshot that has this commit, we can remove the
feature itself and the attribute as well.)

----

I consider this to:

Fix rust-lang#22196

(techincally there is still the post snapshot work of removing the
last remants of the feature and the attribute, but the ticket can
still be closed in my opinion).
@pnkfelix pnkfelix mentioned this issue Mar 28, 2015
Merged
bors added a commit that referenced this issue Mar 29, 2015
@bors
Auto merge of #23819 - pnkfelix:kill-unsafe-dtor, r=pnkfelix
27af78c 
Port of pcwalton removal of `#[unsafe_destructor]` check.

Earlier commits impose rules on lifetimes that make generic destructors safe; thus we no longer need the `#[unsafe_destructor]` attribute nor its associated check.

----

So remove the check for the unsafe_destructor attribute.

And remove outdated compile-fail tests from when lifetime-parameteric dtors were disallowed/unsafe.

In addition, when one uses the attribute without the associated feature, report that the attribute is deprecated.

However, I do not think this is a breaking-change, because the attribute and feature are still currently accepted by the compiler. (After the next snapshot that has this commit, we can remove the feature itself and the attribute as well.)

----

I consider this to:

Fix #22196

(technically there is still the post snapshot work of removing the last remnants of the feature and the attribute, but the ticket can still be closed in my opinion).
cadencemarseille added a commit to cadencemarseille/rust-pcre that referenced this issue May 16, 2015
@cadencemarseille
Remove #[unsafe_destructor]
7d2cfc6 
See rust-lang/rust#22196
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pnkfelix pnkfelix

Labels
P-medium Medium priority
Projects
None yet
Milestone
1.0 beta
Development

Successfully merging a pull request may close this issue.

Port of pcwalton removal of #[unsafe_destructor] check. pnkfelix/rust
3 participants
@pnkfelix @SSheldon @diwic