ACP: Add core::mem::WithDrop to associate a custom drop fn with some data

[orlp]

Proposal

Problem statement

It is a common need to require something to run when a value gets destroyed. For example to clean up some resource, or to restore some invariant. This is especially important when writing panic-safe code.

Rust has a tool in the language for this purpose: the Drop trait. This allows you, for a specific type, to specify what should happen when a value of this type gets destroyed. However, this only allows one such implementation per type. This leads to a common but unwieldy pattern where one defines a dummy type wrapping some data for the sole purpose of adding a Drop implementation to the data.

This pattern gets doubly annoying when generics are involved, as an inner struct definition may not access the generics of its surroundings, forcing you to repeat all those again.

Motivating examples or use cases

There are many such cases of this, even in the standard library. For example in library/alloc/src/collections/btree/mem.rs:

struct PanicGuard;
impl Drop for PanicGuard {
    fn drop(&mut self) {
        intrinsics::abort()
    }
}
let guard = PanicGuard;
...
mem::forget(guard);

Or compiler/rustc_serialize/src/opaque.rs:

struct SetOnDrop<'a, 'guarded> {
    decoder: &'guarded mut MemDecoder<'a>,
    current: *const u8,
}
impl Drop for SetOnDrop<'_, '_> {
    fn drop(&mut self) {
        self.decoder.current = self.current;
    }
}

Or library/std/src/sys/pal/unix/sync/condvar.rs:

struct AttrGuard<'a>(pub &'a mut MaybeUninit<libc::pthread_condattr_t>);
impl Drop for AttrGuard<'_> {
    fn drop(&mut self) {
        unsafe {
            let result = libc::pthread_condattr_destroy(self.0.as_mut_ptr());
            assert_eq!(result, 0);
        }
    }
}

Or an example of repeating generics from library/alloc/src/vec/drain.rs:

struct DropGuard<'r, 'a, T, A: Allocator>(&'r mut Drain<'a, T, A>);

impl<'r, 'a, T, A: Allocator> Drop for DropGuard<'r, 'a, T, A> {
    fn drop(&mut self) {
        // ...
    }
}

Solution sketch

I propose we add a parallel to the core::mem::ManuallyDrop type to control the drop implementation of a specific value, rather than a type. This type which I propose we call WithDrop (also in core::mem) is similar to ManuallyDrop, but rather than foregoing the drop it calls a specific function on Drop with the value instead. To be concrete:

#[derive(Clone)]
pub struct WithDrop<T, F: FnOnce(T) = fn(T)> {
    value: ManuallyDrop<T>,
    on_drop: ManuallyDrop<F>,
}

impl<T, F: FnOnce(T)> WithDrop<T, F> {
    /// Wrap a value so that it is passed to `on_drop` when this `WithDrop` is dropped.
    #[must_use]
    pub const fn new(value: T, on_drop: F) -> Self {
        Self {
            value: ManuallyDrop::new(value),
            on_drop: ManuallyDrop::new(on_drop),
        }
    }
    
    /// Return the contained value without calling `on_drop`.
    pub fn into_inner(self) -> T {
        unsafe {
            let mut slf = ManuallyDrop::new(self);
            let ret = ManuallyDrop::take(&mut slf.value);
            ManuallyDrop::drop(&mut slf.on_drop);
            ret
        }
    }
}

impl<T, F: FnOnce(T)> Drop for WithDrop<T, F> {
    fn drop(&mut self) {
        unsafe {
            let val = ManuallyDrop::take(&mut self.value);
            let on_drop = ManuallyDrop::take(&mut self.on_drop);
            on_drop(val);
        }
    }
}

impl<T, F: FnOnce(T)> Deref for WithDrop<T, F> {
    type Target = T;

    #[inline(always)]
    fn deref(&self) -> &T {
        &self.value
    }
}

impl<T, F: FnOnce(T)> DerefMut for WithDrop<T, F> {
    #[inline(always)]
    fn deref_mut(&mut self) -> &mut T {
        &mut self.value
    }
}

impl<T: fmt::Debug, F: FnOnce(T)> fmt::Debug for WithDrop<T, F> {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.debug_tuple("WithDrop").field(&self.value).finish()
    }
}

With this type the above patterns are easy to implement. For example:

let abort_on_panic = WithDrop::new((), |_| intrinsics::abort());
...
abort_on_panic.into_inner();

Because WithDrop implements Deref and DerefMut you can still easily access the underlying data:

let mut file = std::fs::File::create("foo.txt")?;
...
{
    let mut f = WithDrop::new(&mut file, |f| f.flush().unwrap());
    write!(f, "important data")?;
    may_panic();
    write!(f, "more important data")?;
}

Alternatives

There are two main alternatives to this proposal. One is some kind of defer keyword, e.g. discussed here. That would require full-on language support, and is also limited to strictly scoping-based cleanup. The latter means that even if we later end up accepting some kind of defer language mechanic in Rust, WithDrop isn't necessarily obsolete as it allows moving the entire WithDrop around to do non-scoped per-value cleanup.

The second alternative is simply for a user to implement the above, or use one of the crates that provides this or something similar. The most popular crate providing something similar is the scopeguard crate. While acceptable to some both feel unsatisfying to me. Adding a custom dummy Drop struct is annoying but just isn't annoying enough for me to justify adding a dependency on a microcrate. I personally feel if the need is fairly universal† and the implementation simple and uncontroversial it should be part of the standard library.

†The scopeguard crate has 385 million downloads all-time. It is in the top 50 most downloaded crates of all time.

While this proposal is inspired by scopeguard, I simplified it to its core: simply associating a drop function with some data. I did not include the macros it contains or the panicking strategies it exposes. I find the latter simply unnecessary as you can call std::thread::panicking yourself in the drop handler to do something different based on whether or not we're unwinding. To be specific for the former, scopeguard also contains the defer! macro (and variants for panicking strategies), which is (paraphrased) defined as

macro_rules! defer {
    ($($t:tt)*) => {
        let _guard = WithDrop::new((), |()| { $($t)* });
    };
}

While it is able to solve some of the same problems as this proposal I believe a macro like this is less flexible than the type (it's limited to the current scope, might run into borrowing issues, can't cancel the defer like WithDrop::into_inner can), and clashes with a potential future defer language feature. Should the defer language feature ever officially be considered and properly denied I wouldn't be fully opposed to adding it in addition to WithDrop, but I think it makes for a poor alternative.

---

As for alternatives within this proposal, I considered the names CustomDrop or OnDrop as well. I preferred WithDrop, but I'm open to suggestions.

Links and related work

Defer discussion: https://internals.rust-lang.org/t/a-defer-discussion/20387
scopeguard: https://docs.rs/scopeguard/latest/scopeguard

[pitaj]

For name, why not DropGuard? I've heard many people refer to this pattern as "drop guards" and several of your examples either match it exactly or contain at least one of the words.

[orlp]

@pitaj Because that's only one of it's possible applications, and not what the type itself does. The type itself just calls a function on drop, it doesn't necessarily guard anything.

[scottmcm]

I've always liked the "bomb" phrasing of this.

impl<T, F: FnOnce(T)> DropBomb<T, F> {
    /// Wrap a value so that it is passed to `explode` when this `DropBomb` is dropped.
    pub const fn arm(payload: T, explode: F) -> Self { ... }
    
    /// Return the contained value without calling `explode`.
    pub fn defuse(self) -> T { ... }
}

where I think the metaphor makes it clearer than into_inner that it's not going to call the closure.

(It might be too "cute", though.)

---

Pondering: should the type be must_use instead of the constructor being must_use?

[kennytm]

-1 DropBomb, the terminology is primarily for running those tests which the "defusing" is the expected behavior and dropping is not. In this ACP's examples (aside from PanicGuard), running to .drop() is intended. So I'd prefer more neutral names like WithDrop/OnDrop/DropGuard/ScopeGuard/Defer etc.

[orlp]

@kennytm I'm not a huge fan of any name which includes Guard, as in my opinion just like your argument against Bomb, that is simply one possible application of the type. Depending on the application the type may not actually be guarding anything.

I'm even less of a fan of any name which includes Scope. One of the advantages of this type over a potential defer language future is precisely that it's not limited to a scope, but to the lifetime of the object. It could be moved around, passed over channels, etc.

I really think a neutral name which simply describes what the type does or how it behaves rather than how it could be used or applied should be chosen.

[purplesyringa]

I find the latter simply unnecessary as you can call std::thread::panicking yourself in the drop handler to do something different based on whether or not we're unwinding

Since panicking is broken (see rust-lang/rust#143612), I wonder if it would be a good idea to reconsider this. I still need to do some research on whether that would cover a majority of uses of panicking, but I wanted to put it out there.

[kennytm]

I'm not a huge fan of any name which includes Guard

Well at least the suffix Guard is consistent with MutexGuard.

But the name of the type is important mainly because the entry point is designed as an inherent method of the type. If instead it is a function like Mutex::lock() -> MutexGuard

fn finalize<T, F: FnOnce(T)>(value: T, f: F) -> Bikeshed<T, F> {
    Bikeshed::new(value, f)
}

...
{
   let mut f = std::mem::finalize(&mut file, |f| f.flush().unwrap());
   ...
}

then most of the time it does not quite matter whether the type name contains "Guard" or not.

You do have to name the type in two scenarios:

1. When you do want to pass the value to other functions or persist the value in a structure, where there is no type-inference
2. When calling .into_inner() — because the WithDrop<T, _> type implements Deref, the Rust convention requires this method to be implemented as a static method taking this: Self rather than self to avoid confusion with any T::into_inner method.

[yoshuawuyts]

Oh hi, I just realized this proposal had been filed a few days ago. I just opened rust-lang/rust#144236, which brings in the mini-scopeguard crate. That's a crate I wrote up earlier this year specifically to upstream, and the recent lang team call motivated me to follow through on it. In the PR I'm proposing a different set of names than this ACP, for which I've explained my rationale here: rust-lang/rust#144236 (comment).

In reading this issue, I didn't realize we had this many different names for the same functionality in the stdlib. I only knew about the instances of DropGuard and the one lone instance of ScopeGuard. But we have a lot more, so I figured it might be helpful to link to their uses in rust-lang/rust and sort them by frequency:

• ScopeGuard 10 definitions
• WithDrop 6 definitions
• Defer 4 definitions
• PanicGuard 3 definitions
• DropBomb 2 definitions
• AttrGuard 2 definitions
• SetOnDrop 2 definitions
• ScopeGuard 1 definition
• OnDrop 1 definition

My main takeaway here is that we probably want to have something here sooner. This really feels like it could to with some consistency. I do find myself still being the most partial to core::mem::DropGuard. It feels it best fits in with all the other "drop" operations in core::mem, as well as with the other "guard" impls we have in the stdlib.

What I'm less sure about is the into_inner method. It seems easy to overlook, but will often be load-bearing. I'm not sure what else we should call it though, which is why I proposed in rust-lang/rust#144236 (comment) to mark that as an open question.

[programmerjake]

A name suggestion for into_inner that makes it stand out more: skip_drop

[kennytm]

.skip_drop() sounds like it will skip the drop of the inner value itself 😅

[programmerjake]

.skip_drop() sounds like it will skip the drop of the inner value itself 😅

that's why I didn't suggest forget... maybe into_without_drop?

[kennytm]

because the function must be called as let value = DropGuard::into_inner(guard) rather than let value = guard.into_inner(), I think keeping the name into_inner() is fine