- Feature Name: no_drop_destruction
UPDATE: disallowed _
binding pattern (see below for reasons)
The initial version did allow _
bindings and had a part which was
slightly ambiguous over wether or not the behavior of _
is changed.
While cleaning this up I became aware that it is better to not enable
pattern matching for destructive patterns containing _
.
Summary
Allow, but lint, using destruction patterns with Drop
types in some cases.
In code:
#[allow(no_drop_destruction)]
let ImplementsDrop { some, fields } = implements_drop;
Motivation
There are many places where it can be use full to pattern destruct a type implementing
Drop
(without it calling drop
) to e.g. implement type conversion.
A pretty common use-case are wrapper types which implement Drop
and have a
into_inner
method. Currently this can be implemented by either:
- Put the inner type into a
Option
even if the only place where the option can becomeNone
is theinto_inner
function which then drops self.- But this requires calling
unwrap()
at all other places, which is a mess.
- But this requires calling
- Use unsafe code (awful but should work) :
- Make sure the struct is on the stack
- Then use something like
let Type { ref mut field, ...} = &mut self
to get&mut
's to all fields. - use
std::ptr::read
to unsafely move out all fields - use
std::mem::forget
(which will leave just a bit a now irrelevant bytes on the stack). - return from the
into_inner
function to “clean up” the stack
A example for a type affected by this is std::io::BufWriter
.
This also doesn’t only affect wrapper types (which are pretty common, e.g. see combinators of iterators/futures) but also some other cases where you have a function which consumes self but doesn’t “end it” but instead convert it to another type.
Guide-level explanation
Types implementing Drop
normally do not allow moving out any values from them,
as they need to run some logic on when they leave the scope, which is not possible
as it could easily lead to leaked resources and similar problems.
Still sometimes you might want to decompose a type implementing Drop
into all it’s parts
to then recompose it into a different type. You can do so by disabling the no_drop_destruction
line.
For example if you have some wrapper type:
struct MyWrapper<A> {
inner: A,
field2: &'static str
}
impl<A> MyWrapper<A> {
fn into_inner(self) -> A {
#[allow(no_drop_destruction)]
let MyWrapper { inner, field2 } = self;
// We use `drop` for clarity and to not have a unused variable.
drop(field2);
inner
}
}
impl<A> Drop for MyWrapper<A> {
fn drop(&mut self) { println!("it dropped"); }
}
Note that if you pattern matching to destruct a Drop
types you do have the responsibility
all fields appropriately, even if they are pointers. This is also the reason why only
destruction patterns work which map all fields.
For example you can use:
let MyWrapper { inner, field2 } = wrapper;
match wrapper { MyWrapper { inner, field2 } => {...} }
if you annotate the pattern with #[allow(no_drop_destruction)]
.
But you can not use this with following pattern:
-
..
(e.g.let MyWrapper { inner, .. } = wrapper;
) -
_
(e.g.let MyWrapper { inner, field2:_ } = wrapper;
)
The reason for this is at it would allow partial movement
on Drop
types. The reason for it is that in rust destructive patterns
are basically just syntax sugar for moving out all fields on-by-one.
But assigning to _
doesn’t move anything. E.g. let (a, _) = c;
can
be roughly seen as let a = c.0; let _ = c.1;
which doesn’t move
the second value out of c
so you still can access c.1
after it.
But when doing a pattern destruction of a type implementing Drop
the you have the responsibility to handle all fields.
Additionally if ..
would be allowed it would be just to easy to add some
leaks by adding a field if it would be allowed to use ..
to discard all remaining fields.
Note that this only applies to the type(s) which implement Drop
so
if you have a pattern containing patterns the non drop types in it
still can use those, e.g. following is fine:
let StructWhichIsNotDrop { wrapper: MyWrapper { inner, field2:_f }, .. } = foobar
let MyWrapper { inner: Some(_), field2:_f } = wrapper;
So using you can only use pattern destruction on types implementing Drop
if you move each field of the drop type into a variable (_
isn’t a variable)
Reference-level explanation
Allowing destruction patterns for types impl. Drop
-
Allow destructive patterns which cover all fields for types implementing
Drop
.- in more complex patterns patterns consisting of multiple
“sub-patterns” only the fields belonging to types implementing
Drop
need to be covered
- in more complex patterns patterns consisting of multiple
“sub-patterns” only the fields belonging to types implementing
-
A pattern covers all fields if each field is has is assigned to a variable.
-
_
is not a variable. -
..
in a struct doesn’t assign the fields behind it.
-
-
Matching a whole type with
_
/..
which implementsDrop
is still allowed- This is already allowed ( e.g.
let _ = MyWrapper { inner }
) - Matching the whole type is more then matching all fields of it, so
using
MyWrapper {..}
as a pattern is still not allowed ifMyWrapper
implementsDrop
- This is already allowed ( e.g.
-
If a pattern is used on a type implementing
Drop
and the patterns moves out values from that type, then the typesDrop::drop
implementation will not be run- Naturally you can only use patterns which cover all fields like mentioned above
For example:
Type(_f)
Type { field: _ f}
Enum::Variant(_f)
Enum::Variant { field:_f}
Due to 3. it is also possible to have a _
match arm, e.g.:
match enum_with_drop {
Enum::Variant(_f) => { unimplemented!(); },
_ => { unimplemented!(); }
}
Which means that following would be possible in the same way it is in current rust for non drop types:
match enum_with_drop {
Enum::Variant(f) => { return Some(f); },
_ => {}
}
drop(enum_with_drop);
return None;
This explicitly does not allow moving out single fields of a struct, it’s strictly limited to patterns which cover all fields, even through rust internally might just move fields out one-by-one.
Restricting usage of new destructive Drop
patterns (lint)
When a struct implementing Drop
is pattern destructed the code
doing so has the responsibility to handle all fields, which can include
handling some pointers e.g. to clear up some resources no longer
needed. As such adding a field to the struct should make all
destructive patterns on it fail to compile, which is why ..
in
struct patterns is excluded.
Additionally there should be some form of “opt-in” as people
should preferably be aware that they are destructing a drop type.
The simplest way to implement this is to have a lint which by
default produces an error if a Drop
implementing type is
destructed. This allows the usage of #[allow(no_drop_destruction)]
to “opt-in” for a specific expression and has the benefit of
not needing any new attributes, and being compatible
with all lint related tooling.
Edge Cases
There is a number of edge-cases mainly wrt. using a lint as a “opt-in” mechanism.
Which is that if you attribute a expression with #[allow(no_drop_destruction)]
it
allows it for the whole expression even if it contains multiple drop destruction
patterns, which I believe is fine (see below), but for the case it get’s replaced
with a new attribute here are some additional parts:
- using a destruction pattern on a drop types should be fine independent of where the pattern appear, it can be in another pattern even if it is another drop destruction pattern.
- in a destruction pattern, you should be able to further match/destruct on the fields of the struct/(variants of the enum/ items of the tuple struct) like normally, just hiding some fields of a drop impl. struct is not allowed
- For a pattern containing potentially multiple sub-patterns of drop implementing structs a single annotations should be enough.
Drawbacks
(this section needs some updates wrt. the updates wrt. _
and
the added open questions)
I do not believe that this RFC has any drawbacks compared to the current status because of the reasons listed next. Still there are some drawbacks to using lint’s as a “opt-in” mechanism which are listed further below.
The original restriction on moving values out for drop types was based on:
- The fact that it’s really easy to mess it up wrt. to moving out
fields and partial destruction
- which doesn’t apply as only full pattern based destruction is allowed).
- It is possible to circumvent drop being run without unsafe
- Which rust learned is always possible, i.e.
mem::forget
is stable + safe by now - Types still can “protect” them self as against
no_drop_destruction
as it requires all fields to be visible for the calling code and mostDrop
types have at last some private fields, which means that only code in the modules (and sub-modules) of the type impl.Drop
can use the destruction patterns. (Again another reason to not allow the..
pattern.
- Which rust learned is always possible, i.e.
Drawbacks of using lints as “opt-in”
The draw back of using lints as a “opt-in” mechanism is that programmers can enable
it is that they always apply to a full scope, i.e. they can just enable it crate wide and
if it’s enabled for a match statement it’s enabled for the whole statement including
the match arm “bodies”, not just a patterns in the match statement. This could
be avoided by using a custom attribut e.g. #[drop_destruction]
instead.
I do not believe that this should be a problem in practice as I believe that due to the constraints this RFC putts on the patterns it would still be reasonable if there would be no lint or similar. _I.e. I believe that having a linte/“opt-in” is just a small improvement and not required on itself for this rfc.
Rationale and alternatives
I believe that this approach to allowing some limited form
of pattern destruction is preferable to some alternatives a
it is clear, simple and straight forward. It does not require
any new syntax, trait’s or attributes (but a single additional
no_drop_destruction
lint is recommended).
Due to the limitations it has wrt. the patterns it avoids the
problems which originally caused moving out of fields of
types implementing Drop
to be disallowed. (As far as I
can tell).
Alternatives
- Do not change anything and force people to add overhead in form of (should be) unnecessary options or suboptimal unsafe code.
- A modified version of this proposal which behaves different wrt. the “opt-in” lint (see unresolved questions).
There is another proposal,
which would add pattern destruction of types but is
focuses on a alternative version of Drop
which accepts self
instead of &mut self
,
because of this I would say that it is less a alternative but more another proposal
which could benefit from this proposal. It also should be noted, that it requires
the type to implement a specific trait
, which is sub-optimal for simple by
pattern-destruction of Drop
types as wether or not it’s ok to do so is a property
of the code doing so (does it handle all fields) and not the type on it self.
Prior art
This RFC is about lifting a rust specific constraint, there might be some priority art but I’m not aware of any.
Unresolved questions
The current pre-RFC does not specify wether or not you can
bind some or the values by reference. And how this interacts
which the ergonomic parts of rust which allow omitting ref
and
similar in some places. Both approaches of allowing and disallowing
it have benefits and drawbacks and I will update this section once
I’m more clear about them.
While this RFC includes a restriction of using pattern-destruction on drop types through a lint, it can be seen as a somewhat open question wether:
- We should drop the lint part.
- Make the lint warn by default instead of error.
- Replace the lint with a custom attribute
- which produces an error if not given
- or a warning if not given
I prefer a lint as it works with all the lint tooling and prefer to start with defaulting to error as we can always later on make the lint only warn if it seems to be a better solution.
Future possibilities
This RFC could be used as basis for some possible future RFCs like
e.g. have a alternative to Drop
which accepts self
instead of &mut self
.
But this is widely out of the scope of this RFC and not the motivation why
this RFC is required.