Making Drop more magic to make it less magic

[CAD97]

Prompted by discussion in this thread.

There’s probably some problem that I’m missing that makes this not work, but:

Redefine Drop to be

trait Drop: !Copy {
    fn drop(self);
}

(Compatibility with current impls is provided with a magic legacy_drop(&mut self).)

The magic of this definition is that you may move out of self even though Self: Drop, and that Drop is not called on self at the end of scope. (It’s still called on any members that aren’t moved.)

We no longer require the magic that you can’t refer to this fn directly.

Calling any function that takes Self is handled by the unconditional_recursion lint. (As it would unconditionally recurse back into Drop::drop.)

---

Pre-1.0 issue about by-value drop.

I think this trait definition, modulo supporting current trait impls, is less magic than the current. (Though I accept if changing this definition like this is undesirable or impossible.)

Current Drop:

• Cannot refer to Drop::drop directly
• Cannot move out of Drop types
  • Semantically moving out of self in Drop::drop requires unsafe and wrapping members in mem::ManuallyDrop
• Calls to drop glue emitted at the end of every block that has types by-value

This Drop:

• Support for two ways of implementing the trait
• Can move out of Drop types in Drop::drop (but not elsewhere)
• Drop glue in Drop::drop skips covering self, other by-value scopes emit full drop glue
• unconditional_recursion lint has to learn about drop recursion

(Implementation note: this drop glue behavior can be implemented by handling drop glue for Self as if it were not Drop. Rather than the recursion lint, you could also consider Self to be a distinct type here only, so you cannot recurse (but also cannot mem::forget).)

---

The other proposal in the linked topic was to have a Drop version that required pattern matching self in the drop fn declaration. This seems like more magic than this formulation, but would also work.

Both ideas could also be applied to using a new DropMove type instead, and having magic interop instead of magic translation.

[Centril]

@CAD97

Redefine Drop to be

Why is this backwards compatible?

[CAD97]

Drop::drop isn’t directly callable currently.

There is required magic to allow implementations in the current style.

This proposal is predicted on the idea that a move-capable drop is desired, which would require either this “dual” Drop trait, or another DropMove trait with similar magic to glue the two together such that one implies the other.

I posit that the “dual” trait is less magic than entangling two Drop traits, one by-ref and one by-val. I additionally posit that this formulation of the trait, minus the “dual” nature, is less magic than the current one. (The magic is in the drop glue.)

Actually, I think the “dual” trait of {Drop::drop(self) while allowing implementations to implement Drop::drop(&mut self)} is actually equivalent to {DropMove::drop(self) and not-callable Drop::drop(&mut self) where implementing one also implements the other}.

[eaglgenes101]

The magic here can be reduced further by introducing the idea of a moribund state (which in some senses can be seen as already implemented through drop flags). At that point, the useful life of the object is over, and sooner or later the object has to be sent back to the allocator to have its used memory salvaged.

Then the drop glue can be seen as checking that the object is moribund, and if so, not performing the drop call that would cause the whole infinite recursion business that motivated Drop’s design in the first place. In practice, however, this would mostly not appear in the final result, as in most cases the compiler can reason that it already knows the object’s liveliness at the drop glue points, and can thus optimize out the check, or have compile-time checks to avoid having to consider emitting the liveliness checks in the first place.

After that, all that’s left of the magic is emitting drop glue everywhere (and internal compiler details).

[CAD97]

In the following example:

struct Resource {
    ..
}
impl Drop for Resource {
    fn drop(self) {
        use_resource(self);
    }
}
fn use_resource(resource: Resource) {
    ..
}
fn main() {
    let resource = Resource::create(..);
    use_resource(resource);
}

it’s illustrated that this couldn’t work how you seem to think it would. It would require passing the drop flag / moribund state everywhere, and checking it everywhere, because every function could be called with a live or moribund struct. It couldn’t even be elided the way drop flags are, as it’s an unknown parameter to the function (unless you’re implying every function should be generic over moribund state of each parameter which is Drop).

It’s for this reason that this by-move Drop proposal suggests the the use of the unconditional_recursion lint to prevent unconditional recursion, and confines the drop glue changes to Drop::drop.

[earthengine]

My proposal here, is to make the “move everything out of self” being the first step of any drop function body, making writing such a thing safer and intuitive. For this purpose, we can always do a pattern match in the calling site. As this requirement need to change the way we call a function, we need a new ABI for the calling convention.

---

the destruction-call ABI

trait Drop {
    extern "destruction-call" fn drop(self);
}

Demostration:

enum MyEnum {
    Variant1{ field: String },
    Variant2(String)
}
impl Drop for MyEnum {
    extern "destruction-call" fn drop(self) {
        match self {
            MyEnum::Variant1{ field } => println!("variant1: {}", field),
            MyEnum::Variant2(s) => println!("variant2: {}", s)
        }
    }
}

let v = MyEnum::Variant2("123".into());
v.drop();

is desugar (not quite; magics needed) to

fn drop_myenum_variant1(field: String) {
    println!("{}", field);
}
fn drop_myenum_variant2(s: String) {
    println!("{}", s);
}


let v = MyEnum::Variant2("123".into());
match v { 
    MyEnum::Variant1{ field=field } => drop_myenum_variant1(field),
    MyEnum::Variant2(s) => drop_myenum_variant2(s),
}; 

The destruction-call ABI following the rules below:

• receive a single argument
• if the argument type is an enum, the function body must be a single match statement on its argument
• in each match arms, the variables in the patterns are the things to be moved to the stack frame
• if the argument type is a struct, a single armed match statement is assumed and is optional (if not written in a match, the self.field path will be required, as it is today)
• the match will be performed in the caller site

Magics

The only magic here is the need to create multiple code entries and count as a single “function”. The calling site destruction is just sugar.

Meanwhile, the new ABI can be used to declare other functions as well. So this “magic” makes Drop strictly less magical. Now the Drop trait is only magic in the sense it is called automatically.

[CAD97]

Another possibility that I failed to mention:

Keep Drop::drop(&mut self), but add a special rule that this function only is allowed to move out of &mut self. This is probably a comparible level of magic to other solutions, but seems less tasteful to me than magicing up a solution that looks like it takes ownership.

I do like the idea of marking it extern "rust-drop" fn though.

[scottmcm]

@CAD97

It’s for this reason that this by-move Drop proposal suggests the the use of the unconditional_recursion lint to prevent unconditional recursion, and confines the drop glue changes to Drop::drop .

unconditional_recursion only catches direct recursion, not mutual recursion. So it wouldn't even catch

impl Drop for Resource {
    fn drop(self) {
        drop(self);
    }
}

@earthengine

is to make the “move everything out of self” being the first step of any drop function body

That's what I mentioned in the thread @CAD97 linked in the OP: we already have syntax for "destructure the type", so we could just require that implementations of the new drop method uses that in the parameter pattern. It's a simple and clear restriction that's entirely local.

On back-compat: we can have drop glue work by calling existing drop with &mut Self, then new drop with Self. People could implement either, both, or neither.

[earthengine]

@scottmcm

so we could just require that implementations of the new drop method uses that in the parameter pattern.

Unfortunately this "parameter pattern" only works for structs. For enums you still have to write match arms. This is what my post implies.

[stepancheg]

Can I offer an idea?

I think drop can be safe and flexible with little help of intrinsic type.

Rust can have a magic container:

// holds a value of type T, and performs "default" drop
// on destruction, i. e. invokes drop on all fields
// this object is instantiated by compiler only, not directly
struct DefaultDrop<T>(T);

impl<T> DefaultDrop<T> {
  // get a reference to contained object
  fn get_mut(&mut self) -> &mut T { ... }
  // however contained object can be recovered
  fn recover(self) -> T { ... }
}

// and then drop signature provided to user could look like this:

trait BetterDrop {
  fn drop(self_holder: DefaultDrop<Self>);
}

// so by default to close resource or something get_mut can be used

impl BetterDrop for MyObjbect {
  fn drop(self_holder: DefaultDrop<MyObject>) {
    self_holder.get_mut().close_something();
    // and the default destructor will be called at the end of this function
    // when self_holder is be destroyed
  }
}

// however this API allows data recovery

impl BetterDrop for AnotherObject {
  fn drop(self_holder: DefaultDrop<AnotherObject) {
    let AnotherObject { field1, field2 } = self_holder.recover();
    // self_holder no longer exists, so no destructor will be called
    // except for destructors of individual fields
    // (unless those fields be moved elsewhere)
  }
}

[scottmcm]

@earthengine

Unfortunately this “parameter pattern” only works for structs. For enums you still have to write match arms.

That's a good point. I guess it could be done differently, either by requiring that the first thing in the HIR be a match on self, or a MIR-level check that _1 is never used as a whole; only its fields.

[earthengine]

@scottmcm

requiring that the first thing in the HIR be a match on self

This is exactly what I proposed.

[H2CO3]

This (along with most ideas in the linked thread) smells very much like it would be much more painful to use and implement than the current design. I understand that there use cases where it’s useful to move out of self in Drop, but I really don’t think it’s typical and that it’s worth it.

I think it’s acceptable better to keep Drop as-is which fits the vast majority of use cases and is a lot simpler and a lot less magical (incidentally, I don’t think this makes Drop any less magical – the converse), and in exchange, maybe some Drop impls will need unsafe.

That’s a better tradeoff than trying to introduce more constraints and perform analyses on an fn body that are hard to even formulate (let alone implement) correctly and exhaustively. Especially since Drop is a so fundamental part of the language, I would expect radical changes like this to be a fertile source of compiler bugs and unsoundness.

[literda]

Soundness, safety and correctness are of course most important.

I’m also against performing some analysis on a fn body, that feels brittle.

As stated in the original thread, I think that both drop(&mut self), as well as the ability to re-use the fields of a struct have their valid use cases.

How is drop and field-reuse different:

drop(&mut self) gives us the ability to do some final work with the still fully operational and alive instance of that struct.

Field-reuse allows us to use parts of the no longer existent struct for something else.

Do not change Drop, but add better field-reuse

Personally, I’d like to keep Drop exactly as it is.

On top of that, I proposed to add the (optional) possibility to instruct the compiler to pass a sub-set of the fields of the no longer existent struct to a function of my choice.

What is possible in today’s Rust?

As proposed here one can use a derive to achieve almost everything in today’s Rust (please forgive possibly bad naming, it’s a first rough prototype):

#[derive(RescueOnDrop)]
#[rescue_with="FileCell::recycle"]

struct FileCell {

  // NOTE  Sadly, I have to spell out `ManuallyDrop` here:
  #[rescue_me]  file: ManuallyDrop<File>,
  #[rescue_me]  sender: ManuallyDrop<Sender<File>>,

  this_will_not_be_rescued: UnimportantType,
}

impl FileCell {
  fn recycle(file: File, sender: Sender<File>) {
    sender.send(file).unwrap();
  }
}

I’ve implemented a first very rough derive and I’m very happy with it, except that I have to spell out ManuallyDrop in the declaration of the struct FileCell, and therefore also have to manually spell out ManuallyDrop::new(...).

I could let the proc macro insert the ManuallyDrop in the field declaration, but that’s not nice either.

What could be slightly improved IMHO

With a bit more integration into the language, I should be able to declare the struct simply as:

#[derive(RescueOnDrop)]
#[rescue_with="FileCell::recycle"]

struct FileCell {
  #[rescue_me]  file: File,
  #[rescue_me]  sender: Sender<File>,
  this_will_not_be_rescued: UnimportantType,
}

Sure, I can achieve moving out of self via ManuallyDrop. It works. But, I personally feel that the ability to rescue fields from a struct instead of discarding them would be fundamental enough to deserve better support in the compiler.

[Ixrec]

Despite reading the "original thread", I actually have no idea what the motivation/argument in favor of this change is supposed to be. It feels like there's a lot of context missing.

@CAD97

We no longer require the magic that you can’t refer to this fn directly.

Do we know why this magic exists today? Is it to prevent double-drop? If so, I don't see how this change affects the need for it.

@CAD97

Calling any function that takes Self is handled by the unconditional_recursion lint. (As it would unconditionally recurse back into Drop::drop .)

But... it's just a lint. It can't actually guarantee no double-drops occur via sufficiently convoluted recursion (because that requires solving the halting problem), which means this is fundamentally unsound... right?

More generally, I don't get how this is an improvement over using Option or ManuallyDrop on the field you want to move out of on drop() (and possibly wrapping that in some macro magic like @literda's doing). Both require understanding this issue enough to opt-in to the non-default behavior, the opt-in is not hard, and in the case of ManuallyDrop it's zero-overhead. To the extent that I even understand what's being proposed (which I'm not at all sure I do), this sounds like it's just a different way to make the compiler generate the same code you'd get by using one of those wrappers today.

[Centril]

@stepancheg

Rust can have a magic container:

impl BetterDrop for MyObjbect {
    fn drop(self_holder: DefaultDrop<Self>) {
        drop(self_holder.recover());
    }
}

are we not back to square one?

[earthengine]

@Centril

are we not back to square one?

Technically there is no way to prevent people writing unconditional recursions. Even with my "destructing-call" proposal, people can still construct the destructed data again, and then call drop again.

The reason to prevent direct recursion on drop is the possibility of partial drop, and so causes UB when calling drop with an invalid object.

In this sense, "destruction-call" will not be affected - if the object was recovered, it is restored to a valid state, so calling drop on them cannot UB.

for @stepancheg 's "magic container" idea, I am not quite sure how this can be handled: is it possible to do partial move out of Self? If so, would that recover returns an invalid object?

[Soni]

Can you use a macro instead?

[literda]

@Soni

Can you use a macro instead?

As I mentioned here a macro can solve almost everything except for having to manually spell out ManuallyDrop in the declaration of the struct.

[eaglgenes101]

It couldn’t even be elided the way drop flags are, as it’s an unknown parameter to the function (unless you’re implying every function should be generic over moribund state of each parameter which is Drop ).

Monomorphizing over liveliness is probably practical for the vast majority of drop impls, which simply release resources other than memory associated with the object. And code bloat, which simply causes large binary sizes and can be reliably detected and then warned against by rustc observing that it has to generate a large number of liveliness-monomorphized functions (and cannot optimize them away), is preferrable to the drop-recursion footguns that plague the other suggestions and would require the compiler to solve the halting problem to identify and warn against with accuracy.