Skip to content

Commit

Permalink
Add more alternatives
Browse files Browse the repository at this point in the history
  • Loading branch information
@Amanieu
Amanieu committed Jul 16, 2022
1 parent 120bb41 commit c43d065
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions text/0000-panic-in-drop.md
View file
Expand Up @@ -275,6 +275,16 @@ This mirrors the behavior of C++ where a throwing destructor can be opted into u

Unfortunately this doesn't really fit Rust's safety model: such a type would have to be unsafe to create and use since it would be easy to invoke UB by using this type with an API that doesn't support unwinding drops.

## Allow individual `Drop` impls to opt-out of unwinding

This is a reverse of the previous point: the default behavior of drops could stay as it is, but individual `Drop` impls could opt-in to aborting if an unwind escapes it.

However this doesn't help generic code such as collections which still need to be able to handle user-defined types which do abort on drop.

This comment has been minimized.

Sign in to view

Copy link
@Lokathor

Lokathor Jul 16, 2022

Contributor

This rebuttal seems strange. If the idea is to have an attribute for when you need to abort on pain in drop, why does it matter to a collection if it's holding elements of a type using that attribute? Each drop of an element that it performs just runs the drop code of that element, so why does the generic collection need to know or care?

This comment has been minimized.

Sign in to view

Copy link
@Amanieu

Amanieu Jul 16, 2022

Author Member

With this RFC, the collection wouldn't need to deal with the possibility of unwinds from drops. But if it's something that types can opt into then generic code still needs to handle this.

This comment has been minimized.

Sign in to view

Copy link
@Lokathor

Lokathor Jul 16, 2022

Contributor

Well, yes? That's the entire point? If they want to deal with an unwind they can (eg: some sort of Pre-crap Your Pants design), or they can just slap the "you can't unwind out of this drop" attribute on their drop and call it a day.

This comment has been minimized.

Sign in to view

Copy link
@Amanieu

Amanieu Jul 17, 2022

Author Member

As an example, this wouldn't have helped with rust-lang/hashbrown#348: clone_from starts by dropping all existing elements before cloning the elements over.

This comment has been minimized.

Sign in to view

Copy link
@Lokathor

Lokathor Jul 17, 2022

Contributor

So couldn't hashbrown have just put the #[abort_if_my_drop_unwinds] attribute on its own Drop impl and moved on? I'm unclear why it couldn't do that.

This comment has been minimized.

Sign in to view

Copy link
@Amanieu

Amanieu Jul 17, 2022

Author Member

That wouldn't have helped since the problematic panic is in clone_from, not in drop.

## Only abort for unwinds out of implicit drops

This would still allow unwinds to escape calls to `drop_in_place`. Abort guards would be generated for implicit drops at the end of a scope.

## Only address the `catch_unwind` issue

Several solutions have been proposed ([disabling unwinding for unwind payloads](https://github.com/rust-lang/rust/pull/99032), [`drop_unwind`](https://github.com/rust-lang/rust/pull/85927), [`catch_unwind_v2`](https://internals.rust-lang.org/t/some-thoughts-on-a-less-slippery-catch-unwind/16902)) to specifically address the [issue](https://github.com/rust-lang/rust/issues/86027) with `catch_unwind`. However these increase API complexity and do not address the remaining issues.
Expand Down

0 comments on commit c43d065

Please sign in to comment.