Re-use struct fields on drop; was: Drop(&mut self) vs. drop(self)

Assume that a struct holds a precious resource. When that struct is being dropped, we’d like to to something more with that precious resource.

Currently, with Drop::drop(&mut self) the struct needs to wrap the resource into some other container to be able to move it out from within Drop::drop. Example:

fn do_something_more_with_precious_resource(resource: PreciousResource) {
}

struct PreciousResource;

struct Guard {
  resource: Option<PreciousResource>,
}

impl Guard {
  fn normal_and_frequent_access_to_resource(&self) {
    // For each access, have to pay runtime cost to check for presence:
    self.resource.as_ref().unwrap();
  }
}

impl Drop for Guard {
  fn drop(&mut self) {
    do_something_more_with_precious_resource(self.resource.take().unwrap())
  }
}

Wouldn’t it be more ergonomic and efficient to define a DropConsuming like this:

trait DropConsuming {
  fn drop_consuming(self);
}

struct Guard2 {
  resource: PreciousResource,
}

impl Guard2 {
  fn normal_and_frequent_access_to_resource(&self) {
    // Can access the resource without having to unwrap some Option first
    &self.resource;
  }
}

impl DropConsuming for Guard2 {
  fn drop_consuming(self) {
    // Can move out of self
    do_something_more_with_precious_resource(self.resource)
  }
}

What’s the reason behind this quite fundamental choice?

2 Likes

See https://github.com/rust-lang/rust/issues/4330#issuecomment-26852226

The root problem, as I understand it, is that if you have fn drop(self), you somehow need to not Drop the self the second time.

Like

fn foo(x: X) {
     // calling drop here
}

impl DropByValue for X {
    fn drop(self) {
        foo(self); // what does this mean?
    }
}
1 Like

Oh thanks, I see.

Maybe I’m looking then for some not-yet-existent functionality like:

struct Precious;

// Instead of dropping the fields of X on Drop each on their
// own, the compiler will pass them to a custom function
// where they can be used together for some clean-up.
#[do_with_fields_after_drop("X::after_drop", "precious", "sender")]
struct X {
  precious: Precious,
  sender: Sender<Precious>,
  not_so_important: u64,
}

impl X {
  // Just an example what I could do with those fields:
  fn after_drop(precious: Precious, sender: Sender<Precious>) {
    // Good: I can consume precious and sender, therefore I can call
    // Sender::send(self, T)
    sender.send(precious).unwrap()
  }
  fn do_something(&self) {
    // Good: I can use self.precious without any match or Option::unwrap
    self.precious.foo();
}

There are two ways you can “rescue” fields from an object being dropped: Option.take() (take the object and replace it with nothing), or mem::replace() (take the object and replace it with another one, usually a dummy one).

1 Like

I'm mostly using the Option-way currently, but as far as I can see, both these ways have drawbacks:

With the Option method I pay a runtime cost every time I want to access a member of that struct during the "regular" time of life of that struct. Also, there are more possible error-paths.

The mem::replace method is in a way just the Option method in disguise: The value that I create to replace the precious value must be created at that spot in the code, which usually means that it is some disfunctional state and kind of a None anyway (think a unopened file). The problem here is that, if it is possible to construct such a null-like value, then the member functions of the struct typically have to deal with that possibility too and check before each access, which is like doing if let Some(x) ....

Ultimately, I want to make useless/disfunctional state irrepresentable, but both the Option as well as the mem::replace method require a useless/disfunctional state to be possible.

With this method I can avoid disfunctional state, and re-use the fields after drop.

What if moving self out completely was forbidden in drop(self)? Typically, you only want to move out individual fields. So at the end of drop(self) the remaining fields (fields that haven't been moved out) will be dropped, and self itself won't be. So only drop(self) would be special-cased but it's already special-cased now.

1 Like

I think it’s true what @matklad points out. The compiler would not only need to prevent moving out self but also need to track which fields got moved out, and which still are to be dropped. This seems very hard.

But I still think that an additional functionality like I sketched here could be realistic and would be beneficial to the language.

You probably can't escape that problem; drop is already not something which can be tracked completely at compile time (cf. drop flags). In this sense, using an Option is probably not any less efficient.

1 Like

My main concern is actually not the ultimate performance, sorry if I gave that impression. Much more important for me is making the code simpler and avoiding the possiblity of disfunctional state.

The only purpose of using Option<T> in my original post is to be able to move fields out in drop(&mut self). But I pay a huge price in terms of complexity (and a little runtime cost) for that because all my member functions need to check for Some every time they want to actually use that field.

What I sketched here could in my opinion be a viable alternative: The fields are no longer Option<T>, so all member functions can access them unconditionally. Still, I'll be able to re-use the fields when the struct gets dropped.

That also uses basically the drop machinery, so yes, I can't and won't avoid that. But I think that it would be possible to avoid the additional complexity, error handling, mental overhead and runtime cost due to having to use Option<T> for everything which I want to move out in drop(&mut self).

I do not propose to replace the existing Drop. What I sketched here is meant to be an addition.

For most value semantic types u64, you don’t even need to introduce an Option since picking any random valid value will still happily do nothing on drop. So just to make sure, PreciousResource is a more interesting type than that either has non-trivial Drop logic for all values, or for which no valid value can be quickly and easily pulled out of thin air, right?

Sure, in the example you can see how the not_so_important: u64 is actually not even considered in the after_drop. And yes, the Precious would be in my use case non-copy, non-clone and non-trivial drop semantics.

Looking at this more generally, I think you're exaggerating a bit the hugeness of the price in proportion to the scope of the problem. If you have a flag that indicates the validity of the state of your data, it is natural that you have to check that flag every time you use that data, even if you expect that flag to only be set for some small amount of time. The only way to not have to check the flag is to eliminate it, and the only way to do that is to never change its value.

So really it sounds like you want to just put that flag somewhere else. It's not clear how you would do this while still being able to associate the flag with the specific object it refers to. Either way, you have to signal a relationship between the object and the flag, and that association will be present in the type of the object.

What you could do, if you know for certain that the object will never be invalid in some branches of code, is to make two types: one that is undroppable, and another that is, and have a way to convert between them when you enter or exit the region of code that needs the object. So all your methods that check the Option could eliminate that check by only existing on the type without Option, and all your drop semantics would be present in the object that uses Option. This effectively pushes your 'drop flag' into a compile-time structure (partially), but it is less flexible overall.

EDIT: There may be a way to automatically do this kind of transformation (specifically for drop) with something like NoDrop.

1 Like

My point is that X::precious in the example can be a Precious, and does not have to be a Option<Precious>. I would like to be able to statically express that X can not exist without a valid Precious.

The only reason why I'm forced to use a Option<Precious> in today's Rust is because I wan't to move out Precious when X is dropped.

With some functionality like this I avoid the Option<T> altogether and still move out the Precious from X.

This simplifies the code substantially because member functions can rely on the presence of Precious because there's no Option.

Or to replace it with another valid value. But I can unfortunately not pull such a value out of thin air. And I want to avoid null-like values like None for which I otherwise have no purpose at all during runtime.

No, in the proposed example I would not need such a flag because I ride on the already existing drop-functionality in the compiler. On drop, the compiler will simply pass the selected fields from that struct which is in the process of being dropped to a function of my choice. There's no runtime flag needed, this will always happen during a drop of that type X.

You're misunderstanding me, I think.

My point is that X::precious in the example can be a Precious , and does not have to be a Option<Precious> . I would like to be able to statically express that X can not exist without a valid Precious .

This is what I was explaining how to do. You can use Option to make drop work, so you could get rid of Option and only add it back in before you actually do the drop. (Some variation of this.) I am skeptical it will be convenient, however.

I'm talking about the drop flag here, not the Precious. If you can always replace one Precious with another, then you don't need the flag because you never change its value. This is basically an information theory problem.

No, in the proposed example I would not need such a flag because I ride on the already existing drop-functionality in the compiler. On drop, the compiler will simply pass the selected fields from that struct which is in the process of being dropped to a function of my choice. There’s no runtime flag needed, this will always happen during a drop of that type X .

The flag would still have to exist, it would just not be in your code, it would be automatically accounted for by the compiler's drop machinery. This is what I meant when I said you wanted the flag moved.

In either case, to solve the problem of double-drop, you'd need to signal that your special functions don't call drop, even though they take ownership. So you're making a compile-time distinction, and I suggested that you might be able to achieve the same result with NoDrop, by telling the compiler to never automatically drop Precious, so that you can safely pass it into your own drop function, process it, and then unwrap it to tell the compiler to finish up.

EDIT: This is also starting to sound a bit like an X-Y problem; what are you actually trying to do with Precious that can't be done in an &mut method using normal drop machinery?

EDIT 2: playing around with NoDrop, and I don't see an easy way to get it to do this either...

This sounds like exactly why ManuallyDrop exists.

It safely Derefs to its contents, and it's easy to be confident that one has maintained its invariants if the only place you use its unsafeness is in Drop::drop.

4 Likes

Ah, I see now what you meant. But yes, that forces the user of X to be aware of that make_droppable which is not very convenient.

Ah, ok. I'm not very familiar with the implementation details of the drop flag. But yes, X::precious would always be a Precious. Precious is non-copy, non-clone and has an impl Drop.

Is there in the example a probem of double-drop? The compiler knows exactly which fields it moves as parameters into my custom function, and which not.

Well, one of the things I want to do is actually in the example. I do that nowadays by using Option in order to be able to move out in drop. But as pointed out in that example, this could be solved much more elegantly by such a custom after_drop. It is more elegant because I statically know that precious is present. I simply don't want to bother with a possible self.precious == None even though that should never happen, but if I make a mistake, I would only find out at runtime. Checking this at compile time is simply better. The compiler has all the required knowledge to support this.

I don't think that ManuallyDrop allows me to do what I would like. I can simply not use ManuallyDrop::into_inner(slot: ManuallyDrop<T>) -> T from a drop(&mut self) because I can not move slot.

How doesn’t it? Here’s your first motivating example:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=edf81d6ed0bb4aca1359d9c9f138a252

use std::mem::{self, ManuallyDrop};

fn do_something_more_with_precious_resource(resource: PreciousResource) {}

struct PreciousResource;

struct Guard {
    resource: ManuallyDrop<PreciousResource>,
}

impl Guard {
    fn normal_and_frequent_access_to_resource(&self) {
        // For each access, just Deref
        let _: &PreciousResource = &*self.resource;
    }
}

impl Drop for Guard {
    fn drop(&mut self) {
        let mut out = unsafe { mem::uninitialized() };
        mem::swap(&mut out, &mut *self.resource);
        do_something_more_with_precious_resource(out)
    }
}

(This could also potentially do ptr::copy instead of mem::swap but I did the easy option)

4 Likes

Oh true, I can leave the memory in invalid state behind. Great! Thanks!

Just to clarify: this is specifically because of the semantics of ManuallyDrop. I’ve still put it into an unsafe state in Guard::drop, but because it cannot be accessed afterwards, and ManuallyDrop suppresses the drop impl, this is fine (so long as you don’t use it after invalidation).

That’s why it requires the unsafe block.