rust-lang
Drop type destructuring
Rust does not allow destructuring types which implement the Drop trait. This means that moving data out of such types is hard and error prone. The rationale is that once fields are moved out, the type's Drop implementation cannot run, which can be undesired. This RFC proposes to allow destructuring anyway, in certain situations.
drop_type_destructuring adds a new built-in macro, destructure. You can use this macro to destructure types that implement Drop. For example, assuming Foo is a type that implements Drop and has fields a and b, you could write the following:
fn example(x: Foo) {
destructure!(let Foo { a, b: _ } = x);
// yay we own `a` now, we can even drop it!
drop(a);
}
Written together with @WaffleLapkin
Macros in patterns are allowed, so this can be:
fn example(x: Foo) {
let destructure!(Foo { a, b: _ }) = x;
// yay we own `a` now, we can even drop it!
drop(a);
}
Which also allows nested patterns.
We talk about this in alternatives section, and highlight why we don't think we should do that.
I think there’s a 3rd alternative. Instead of statement or pattern, destructure! could be an expression!
In that version, your examples look like:
fn example(x: Foo) {
let Foo { a, b: _ } = destructure!(x);
// yay we own `a` now, we can even drop it!
drop(a);
}
fn example(x: Foo) {
let mut a = vec![1, 2, 3];
Foo { a, b: _ } = destructure!(x);
// yay we own `a` now, we can even drop it!
drop(a);
}
etc…
The motivating example could also be adapted. Either in a way that matches your “new” implementation
pub fn into_parts(self) -> (W, Result<Vec<u8>, WriterPanicked>) {
let Self { buf, inner, panicked } = destructure!(self);
let buf = if !this.panicked { Ok(buf) } else { Err(WriterPanicked { buf }) };
(inner, buf)
}
or in a way that closely resembles the original code, but without unsafe:
pub fn into_parts(self) -> (W, Result<Vec<u8>, WriterPanicked>) {
let this = destructure!(self);
let buf = this.buf;
let buf = if !this.panicked { Ok(buf) } else { Err(WriterPanicked { buf }) };
let inner = this.inner;
(inner, buf)
}
which one would probably further simplify to
pub fn into_parts(self) -> (W, Result<Vec<u8>, WriterPanicked>) {
let this = destructure!(self);
let buf = if !this.panicked { Ok(this.buf) } else { Err(WriterPanicked { this.buf }) };
(this.inner, buf)
}
In this version, destructure!(…expr…) wraps the value of expr: Foo into a value of type Destructure<Foo>. The place destructure! is use must fulfill the relevant visibility restrictions (all fields visible, no external #[non_exhaustive] struct…).
The type Destructure<T: ?Sized> is a transparent wrapper around T that acts partially like ManuallyDrop, except it only disable the outermost drop implementation of T itself, but keeps all drop glue. Furthermore, it dereferences to T in an owned-access manner (like Box), but with the additional power that when T is a struct or enum, one can move out of fields of T even when T implements Drop.
The code examples above assume pattern matching can transparently “dereference” this layer of Destructure, otherwise it’d have to be written like
let Foo { a, b: _ } = *destructure!(x);
why not have destructure!(…) itself expand to the equivalent of *destructure!(…) above?
I like that the let this = destructure!(self); approach with subsequent field access is also an option; and given match ergonomics exist, and implicit dereferencing of Box and other deref-patterns seems desirable (IMO), I think enabling matching Deref<Foo> directly against a Foo { …… } pattern seems a good design after all.
Finally, such a Destructure type is essentially all we need in order to offer a way to implement Drop in a by-value manner. (There probably exist many prior discussions on that kind of point, one in particular I could find right now would be #1180 in this very repo.)
(some code examples on this point)
The trait would be
trait DestructuringDrop {
fn drop(self: Destructure<Self>);
}
and then you can do stuff like
struct S {
value: Owned,
other_fields: Something
}
impl DestructuringDrop for S {
fn drop(self: Destructure<Self>) {
// whatever custom drop code
// we van get ownership of the `value` by just
let v: Owned = this.value;
// can also pass elsewhere
function_taking_owner(v);
// other_fields will still drop automatically
// insofar they haven’t been moved out of
}
}
Similar to Drop, this is only meant to be implemented, not as a trait bound. For backwards compatibility (e.g. the tricks some macros like pin-project play to prevent Drop impls), implementing DestructuringDrop for a type will also make the type fulfill T: Drop bounds; also implementing either is mutually exclusive. Finally, existing Drop implementations will effectively desugar to DestructuringDrop, roughly as if the following impl existed
impl<T: ?Sized + Drop> DestructuringDrop for `T` {
fn drop(mut self: Destructure<Self>) {
// dereferences `Destructure<Self>` to `Self` and takes a mutable reference of that place
<T as Drop>::drop(&mut *this);
// after `Drop::drop` is called, no fields of `Self` are moved out yet here, so at the end of this function,
// all destructors of fields run recursively
}
}
as you can see DestructuringDrop replaces Drop and all drop glue (for types with a DestructuringDrop implementation, that is). Finally, the above “desugaring” only truly works if there’s a #[feature(unsized_fn_params)] style support to handle ?Sized and furthermore assumes the argument passed by value will not actually move to a different place in memory, so &mut *this can still refer to the dropped value in its original place.
Of course, this Drop in a by-value is only supposed to be meant as a future possibility, however a quite nice one, in my opinion.
Also note that the naming of the type Destructure<T> is a placeholder – with this interpretation of the feature, of course other names might be more fitting. I might even call it ManuallyDrop if we didn’t have that already[^1].
[^1]: That being said, since ManuallyDrop also disables the Drop code of a type, as another future possibility, ManuallyDrop<T> could gain the same capabilities of moving fields values by-value like Destructure<T> does above.
In fact, when first writing this comment I wanted to propose that `ManuallyDrop` itself fills the role described here – that is until I realized that it’s far too easy to accidentally leave some fields behind in the `ManuallyDrop` (in particular matching against a pattern that doesn’t bind all fields would do that) even though the *intention* is just to disable the outermost layer of `Drop` code, and still take proper care of all the fields without leaking them, too.
@steffahn my concern is that implementing and defining Destructure is a lot more complex, it's quite a bit of magic. At the same time I see some of the appeal. I think we should at least add your proposal as an alternative.
Upd: see 2a7cfb066d326224b007900215652c0a3ef53ce8
I don't like the name destructure!(), because the name does not convey the purpose (preventing Drop from being called). A better name would be forget!().
An alternative is to just allow moving fields out of ManuallyDrop<T>. I think this would be much more elegant, and doesn't require macro magic, or any other new syntax. It is also more versatile:
let mut foo = ManuallyDrop::new(foo);
// you can move fields by destructuring:
let Foo { some_field, .. } = *foo;
// or, you can move fields by assigning them:
let some_field = foo.some_field;
// or, you can move fields by passing them to a function:
do_something(foo.some_field)
Box allows moving out fields as well, so adding the same feature to ManuallyDrop seems reasonable.
@Aloso forget! does not convey the meaning either IMO. the purpose is to get fields out, which requires not running drop...
As for moving fields out of ManuallyDrop... This has multiple issues.
First of all, it's a bit of a footgun, as you need to not forget to move out all fields with significant drops. Second of all, you kind-of still need to have a macro, because we still want to check that all fields are accessible from current scope.
You can read more details about a similar approach here: https://github.com/jdonszelmann/rfcs/blob/drop-type-destructuring/text/3738-drop-type-destructuring.md#macro-expression-and-magic-type
I have rarely needed this and I also sort of blinked at the name. I don't know that we'll find a good one but I will propose steal! if we are bikeshedding. Or maybe destructure_drop!, since destructuring assignment is already a concept.
I'm sure Manually Drop is special in some way that I don't know, but making it more special probably isn't the way. I can implement my own ManuallyDrop today, minus perhaps aliasing concerns: forget(self.value.take()) where value is Option<T>.
TLDR of the below: sometimes you need this, but it replaces the problem of not having it with the problem of having it. I support it because I don't believe in hamstringing capabilities and have needed it, but fear the day someone inexperienced uses it and I get paged at 2 AM because prod is down.
There might be a good case for saying "don't use this unless you are sure you need it" in learning materials. Consider:
- Some crate publishes some sort of guard with public fields (say, internal to a monorepo. There aren't good reasons IMO to put such a thing on crates.io, but for better or worse crate is also synonymous with something like "build unit". I have committed such sins myself).
- Someone new to Rust tries to move out of this guard, and gets an error.
- But
destructure!saves the day. - Next month someone else gets to find out that actually no that broke in prod.
Given that Drop impls are often RAII-like things, and those things are often concurrency etc. that'd almost certainly not show up in unit tests for example. I'm not sure how one would spot such mistakes other than to say every use is suspect. That's not even talking about leaks yet. Leaks are a theoretical concern in the sense that they are safe, but not in the sense that a well-meaning dev who doesn't know Rust well may fail to understand the implications and put this somewhere important.
I have seen even senior-level devs, all the way up to someone who is alone worth my entire team by themself, end up getting stuck in the position of trying to use Rust professionally and having to make compromises on a deadline while they learn. Today most of such compromises are relatively harmless: you might overuse Option or maybe you do Arc in async code or whatever. It produces meh but working code. This however kind of doesn't maintain that property. It gets rid of the compiler screaming in an insidious way.
Related to @ahicks92 's concerns, I wonder if in addition to having all fields public, this should also require that the struct implements a, possibly unsafe, marker trait to opt in to this.
In fact, that may be necessary for backwards compatibility, since there could be existing structs where all fields are public that have a Drop implementation that it relies on being called.
I don't see a backward compatibility concern because the macro itself is new. As long as you can't have code which would change behavior, the macro doesn't cause a problem. A marker trait might be interesting but broadly speaking I can't think of an example of something on crates.io which has public fields because really, mostly, Rust devs don't do that as a whole. It is very easy to opt out: add a PhantomData<()>. There are already lots of ways adding a private field to a struct whose fields were previously all public breaks API compatibility.
This is the kind of thing where I'd use it and I don't see a problem with it other than it being a weird corner of the language, but I've had to teach others and watch qualified people struggle in ways where this could be the tool reached for. Semantically it's fine, in so far as the question being whether or not we want this. I will leave that to "official" people in other words; as far as that aspect I've needed it on occasion but all those times have been memorable and annoying.
I also have trouble thinking of a case where if you intentionally use it you break something. It feels like finding a way for this to be a footgun for people who know why it exists is really hard and you'd have to do it on purpose. I've seen concerns around unsafe code raised, but today you have to deal with mem::forget already and figuring out some way to write code which both makes all fields public in a public API and also relies on drop order or something for safety seems like quite the tall order.
Perhaps it'd be less magic from the language perspective if destructuring a type with Drop was a deny-by-default error that can be disabled with allow/expect? After all, forgetting is safe, so the concern here is only checking with the user did you really mean to forget?, and that's what lints do.
fn example(x: Foo) {
+ #[allow(forget_drop)]
let Foo { a } = x;
}
I think with the just-allow-let approach (or, to a lesser degree, #[allow(forget_drop)] approach) it’s much less obvious which usage disables Drop.
These usages might be expected to disable Drop, but they can never do it, that would be a breaking change:
let Foo { .. } = x;
let Foo { ref y } = x;
How about this one?
let Foo { y } = x;
This can only disable Drop if type of y is not Copy — otherwise it’s currently valid (it copies the field and then runs the destructor), so we can’t change it to disabling Drop (or maybe we can over an edition, but that would be extremely confusing and error-prone).
In other words, allowing plain let (or even let with #[allow]) to disable destructor means that we can’t locally determine whether this particular let does that. An explicit operation to disable Drop is much more readable IMHO.
With just-allow-let + the forget_drop lint, I'd write this:
#[expect(forget_drop)]
let Foo { y } = x;
which would error if I am not in fact disabling Drop there. EDIT: oops that was written just above.
That’s fair, and that’s why I said that forget_drop suffers from this problem to a lesser degree. Maybe we could even additionally lint against allow(forget_drop), since it is a footgun.
It still means there’s no way to disable drop by destructuring when all the fields are Copy though.
@ahicks92 I think your concern is valid.
However, I've seen people give up on using Rust entirely and decide to do all their major work in C, for projects that would benefit from memory safety, because they happened to not be able to figure out the bizarre workarounds that Rust requires in these cases. So, certainly, teaching people how to do something correctly is desirable, but I think it's important to not overstate the cost of this next to "do nothing", if the "do nothing" is "people do in fact do nothing, and the current state of affairs is abominable". If you evaluate this entirely in the context of a workplace that has already chosen Rust, sure, but other people are out there looking at the complications of this case... yes, specifically this case... and walking away from the language because of it.
@kornelski (re this), I don't think this works as a lint. destructure! is fundamentally different from normal destructuring (sort of). Since it stops drop from happening.
Also note that there are cases where you can "destructure" a Drop type today -- specifically if all the fields you access are Copy. It would be somewhat confusing if turning off or not drop dependent on Copy-ness of the moved fields, even with a lint. A lint would just be a crutch, I really do believe we need a separate operation here. (although I've also thought of the lint approach at some point, it makes sense, but I don't think it's a good idea in the end)
@workingjubilee I adopted Rust for a few reasons many of which are personal interest. I brought Rust into work however for reliability. From my perspective reliability is the first thing. We were able to write services that worked almost the first time and ran for months even on less-than-ideal sets of dependencies. I care more about preserving this property than I do encouraging antipatterns for the sake of adoption. Leaks etc. aren't unsafe, but they are unreliable. Saying this now, I almost feel like Rust discussions should almost adopt unreliable as a concrete term.
There is a C mindset. I am skeptical that solving this minor problem would get someone in a C mindset to adopt Rust. Maybe though the people in charge have a different opinion on the important trade-off. In any case we aren't debating having it, we're debating (over)teaching it. Changing how it's taught isn't a breaking change, let us say. I do find your counterpoint valid though. I just don't agree with it.
@ahicks92 I don't feel like this RFC hinders Rust's reliability. The only potential footgun with destructure! is not running a type's drop. But this can already happen in a number of ways — Box::leak, mem::forget, ManuallyDrop, Rc cycles, double-panics... Adding one more way to forget a value doesn't feel important — if you can catch all existing things in review, you can probably catch this one too.
Not only that, this can only realistically happen in the same module as the type definition (Drop types almost always have private fields), which means the programmer/reviewer are more aware of it, probably.
If you are still afraid this might negatively affect your codebase, I'd recommend clippy, you can configure it to warn against certain macros: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros.
@WaffleLapkin I have only so far made a point about misuse by new Rust coders, and that I'm concerned about teaching it. I was going to say that almost the first thing I said is that I support adding it but reading back, while this is factually true, I see how it's unclear and I should probably have pulled it out more obviously. Apologies for the lack of clarity. If I was going to phrase my stance in a sentence: I support adding it but view it more like an intrinsic in the C sense, such that it is "special" and I would want people new to Rust to understand that.
I'm currently between jobs and also my last team was great and I'd have been able to say "no bad" and been listened to. Indeed the reason I draw from my experience is because it was interesting to see an otherwise experienced native-level team (in many cases far my senior) struggle, and just how they did so. That said I did not know clippy could flag macros, it is good to know that this arising won't mean that I am all alone manually code reviewing, and I'm sure I will find some other use for that eventually in any case. For something like this in so far as it would affect me personally being able to mitigate it is indeed the important part.
The most flexible syntax would somehow decorate the pattern, not the statement or expression.
match foo {
// just a ref so this arm is okay
Foo(ref bar) => ..,
no_drop!(Foo(bar)) => ..,
}
Although it doesn't really need to be that flexible. Limiting this feature to let statements seems reasonable. But I don't think wrapping the expression with a macro makes sense - that puts a value in some new magical temporary state that is hard to grok. Maybe my preference then is
#[no_drop]
let Foo { bar } = foo;
Thinking more, a no_drop!(..) macro for patterns seems right. It would remove ambiguity in the case of complex nested patterns since the effect would only be applied to the immediately contained struct pattern. The compiler can enforce that the contained struct implements Drop and that the pattern moves the struct.
@camsteffen Well you can't exactly enforce that the struct has a drop impl because then conditional drop impls become a footgun that authors won't find. E.g. only some platforms needing cleanup of a resource. Authors aren't going to compile their code with all feature combinations all of the time.
We can do this today if you have access to the original struct. This is very UB and only happens to work but a second struct with exactly the same fields but no Drop impls, a transmute, and some proc macros would do it. That relies on the layouts being the same which I don't believe is guaranteed, however, but point being the macro is only kind of special. If we are allowed to use repr(c) I believe it actually may not be UB, but given what it is and my history trying to participate in unsafe edge case discussions I won't say I'm confident making that statement. Still, it does imply:
#[derive(Consumable)]
#[consumable(other_name="FooConsumed")]
struct Foo { ... }
// and
trait Consumable {
type Consumed;
fn consume(self) --> Self::Consumed;
}
// later
match foo.consume() { ... }
can almost be written, and can probably be completely written if using an attribute macro instead of a derive so that it may add repr(C).
My RFC #3466 is an alternative to this, written in terms of ManuallyDrop. I hope to write up a comparison at some point.
Having now read both RFCs: my #3466 is similar to the Destructure proposal in the alternatives section of this RFC—except that it introduces no new types, using ManuallyDrop instead. Because of this, fields that are not explicitly moved out of are implicitly forgotten, not dropped. This allows destructuring Drop structs with private fields soundly, a capability not offered by this RFC. But the downside is that a user could easily forget to move out of a field, and accidentally leak it.
Overall, my RFC is both simpler and more powerful compared to this one, but also easier to misuse. There is no conflict between them, we could pursue both.
a user could easily forget to move out of a field, and accidentally leak it
Or, if that field is private, it may not even be possible not to leak it when destructuring.
This feature is not supposed to allow destructuring types with private fields when you don't have access to them (the type must be constructible), so leaking of private fields can't happen.
This feature is not supposed to allow destructuring types with private fields […] so leaking of private fields can't happen.
Certainly accidental leakage of this sort is undesirable, but I see no reason to forbid users from deliberately leaking things if they really need to. mem::forget exists for a reason. And even with this RFC in its current form, there is a risk of accidental leakage, if the Drop impl being skipped actually did something important that the user didn’t take into account.
This feature is not supposed to allow destructuring types with private fields when you don't have access to them
right, but @Jules-Bertholet's proposal does:
This allows destructuring Drop structs with private fields soundly, a capability not offered by this RFC.
from https://github.com/rust-lang/rfcs/pull/3738#issuecomment-2579205078
I've just been playing around with datastructures with custom Drop impls, and I discovered that the current limitation is a massive pain. The proposed destruct!(pattern = value) is quite insufficient for my case: I want to be able to move out of a field, or move out of an enum variant in a match.
destruct!(pattern) would be sufficient to remove most of the pain. A wrapper-type-based solution would be even more convenient because we can make moving out of ManuallyDrop(myvalue).0 work.
@Nadrieril can you provide a specific example of where destructure!(pattern = value) is not enough? Does that come up in real code?
Here's more-or-less the code it came up in:
type HalfRc<T> = StaticRc<T, 1, 2>;
struct Node {
val: RefCell<u32>,
next: RefCell<Option<HalfRc<Node>>>,
prev: RefCell<Option<HalfRc<Node>>>,
}
enum DoublyLinkedList {
Empty,
NonEmpty {
first_node: HalfRc<Node>,
last_node: HalfRc<Node>,
},
}
impl DoublyLinkedList {
fn into_vec(self) -> Vec<T> {
match self {
Empty => vec![],
// `destructure!(pattern = value)` not sufficient here.
destructure!(NonEmpty { first_node, last_node }) => ...,
}
}
}
impl Drop for DoublyLinkedList { ... }
Basically, adding the Drop impl broke a bunch of things, which is quite annoying. I've also had that for structs, where suddently self.foo stopped being allowed. For this usecase, the solution that minimizes the pain of adding a Drop impl is the ManuallyDrop one:
fn into_vec(self) -> Vec<T> {
match ManuallyDrop::new(self) {
Empty => vec![],
// magic
NonEmpty { first_node, last_node } => ...,
}
}