Pre-RFC: allow by-value drop

internals ideas (deprecated)
1

Proposal

Currently, Drop in rust is always by-reference - the object still needs to be valid for drop-glue after the call to Drop::drop. An elegant way to fix this is to add a trait (e.g. Destruct):

#[lang="destruct"]
pub trait Destruct {
    fn destroy(self) {
        // This isn't really legal Rust, but the "implicit"
        // impl should be this.
        <Drop as Self>::drop(&mut self);
        let Self { .. } = self;
    }
}

The trait should work like Drop, except that you can destructure self on objects that have it (of course, to avoid infinite recursion, the implementation of destroy has to eventually destructure or leak self).

Privacy Issues

Currently, in Rust, you can destructure a destructor-less object, even if it has private fields:

pub mod m { pub struct S(u32); }
mod b { use m::S; pub fn foo(s: S) { let S { .. } = s; } }

This is problematic with Destroy, because other modules can call this, and lose the structure without calling the destructor. The problem is avoided with Drop by preventing all destructuring, which basically breaks Destroy (unless you do a manual destructuring by getting a raw pointer to self, ptr::read-ing all fields, and then forgetting self, but this is quite ugly).

To allow for sane usage of Destroy, we can either force destructuring of a type with an explicit Destroy impl to mention all fields, or just forbid destructuring of types with an explicit Destroy impl and inaccessible (because of privacy) fields.

Misc. Details

The same destructor-lifetime rules that apply to impls of Drop should also apply to impls of Destroy. A single type implementing both Drop and Destroy shall cause a compile-time error, as if there was an impl<T:⁠ Drop> Destroy for T.

Currently, the trait Destroy isn’t object-safe, as it has a by-value self. However, vtables (for Box<Trait>) need to have a way to call it. Because of ABI issues, this could require a shim. We already have a drop-glue shim, through, so we can just adapt it.

Issues

I don’t like the solutions to field privacy. If we have proper DST, we need to add a nice way to call the destructor by-value (maybe add a fn destroy<T: ?Sized>(t: *mut T) that explicitly calls the drop-glue?)

1 Like
2

Very nice, I’ve thought about this as well. In fact I’d go as far as actually replacing the Drop trait with that.

As for destructuring: I propose to change the destructuring rules so that you can only destructure if either

  1. the struct is local to the module (important for Drop implementation) or
  2. the struct does not possess private fields (important for the other use cases).

This means that you can take the object’s contents by value in the destructor and fixes the mess that is otherwise done with mem::forget and ptr::read.

3

Oh, and I propose that the Rust community takes a look at this before 1.0, because this really has the chance of fixing a few warts with the current destructor system.

4

cc @pnkfelix @nikomatsakis

5

I am only still keeping Drop because of backwards-compatability. Edited to include your destructuring rules.

6

Is the idea that destructuring would prevent the Destruct glue from being invoked? Is there an idea that you could destructure enums that have Destruct glue? I guess the syntax for that would be something like:

match self { _ => () };

But that would change current behavior. Is there an alternative syntax for enums that doesn’t rely on knowing each variant? (This would have the desired destructuring behavior, I think, but relies on knowing each available variant for the enum.)

FWIW, @eddyb also had an Interior<T> proposal which I probably won’t do justice to here, but which would allow an unsafe move from a value T into a value Interior<T>. Interior<T> does not implement Drop, so that by-value moves are allowed from Interior<T>. A DropValue trait(?) would take Interior<T> as an argument. This would work with enums that implement Drop very naturally.

Previous discussion.

7

This is irrellevant - a trivial match is a by-ref match and doesn’t consume the discriminant. Actually, trivial destructurings also behave this way in today’s Rust:

fn main() {
    let v = vec![4u32];
    let Vec { .. } = v;
    println!("{:?}", v); // No "use of moved value" error
}

If you want to consume the enum/struct, you need to make the match/destructuring by-value. Currently (e.g. http://is.gd/SQIOOU), rustc complains “cannot move out of type Foo, which defines the Drop trait”. I want this complaint to not be a problem with Destroy.

8

As for the requirement of destructuring in the destructor: We already have a (#[deny]) lint today that triggers if a method unconditionally recurses. I guess that would catch most of the errors about not destructuring self in drop.

9

I probably wasn't clear enough, but I was trying to call that out as a possible problem: if we want the destruct hook to require a destructure to prevent an infinite recursion, then it'd be good if that destructuring could be ergonomic. There is also the issue of Drop impls on enums that don't easily get destructed:

enum Foo { Bar, Baz }
impl std::ops::Drop for Foo {
  fn drop(&mut self) {
    println!("drop");
  }
}

What does a consuming destructuring of this Foo look like? How do we make sure that we avoid a recursive drop loop for this enum type? unsafe { std::mem::forget(self) }?

10

I think when ideas like this have been discussed in the past, we've tended to try to identify if the ideas could be adopted post 1.0 -- that is, are they compatible with the current Drop system. The principle being that we could have a smooth transition where we deprecate the Drop trait and transition to a new system like this one.

As far as I can tell, the proposal here falls into that category (since @arielb1 has stated that Drop remains solely for backwards-compatibility).

11

I think in this case you would have to use std::mem::forget. Actually, this applies to all cases where all members of Self are Copy, which is slightly annoying. You could add a NoCopy marker, through.

12

Thanks, @arielb1, that’s how it seemed to me, too. I suppose an alternative would be to use a move keyword to mark a match as consuming (or possibly to override Copy semantics, and force a move?):

match move x { _ => () };

but difficulty in handling cases where a consuming destructuring isn’t easy is the main reason I stopped investigating this sort of mechanism when I looked at the problem.

13

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.