Fixing the `Drop` trait bound

What would I intuitively expect the Drop trait to behave like:

When copying values, there are three categories of types:

  1. Cannot be copied (no traits)
  2. Can be copied using used-defined code (implements Clone)
  3. Can be trivially copied with mem-copy (implements Copy which derives from Clone)

For dropping the situation should be similar, with the equivalent categories:

  1. Cannot be dropped (linear types). Those currently do not exist in Rust.
  2. Can be dropped using user-defined code (implements Drop).
  3. Can be trivially dropped without executing any user-defined code (implements a hypothetical TrivialDrop trait which derives from Drop)

This way additional traits represent additional constraints on the type.

What is the current behaviour?

  • Drop is implemented only by types which manually implement it. For example Vec<T> implements Drop
  • Drop is not implemented for trivially droppable types (like &mut T and all Copy types)
  • Drop is not implemented for types which run user defined code when getting dropped, but where the drop code is associated with a contained field and not the type itself. (e.g. (Vec<i32>, ) and String)

The current design has a number of issues:

  • Drop is effectively a negative trait bound

  • Drop and Copy are effectively exclusive traits

  • Using Drop as bound is useless, because it doesnā€™t add any capabilities (every type can be dropped).

    At first glance it looks like a Drop bound would be useful for mem::drop to notify the programmer if theyā€™re accidentally using it on a type which doesnā€™t need to be dropped. But that would prevent us from using it on generic types which might or might not implement Drop, so this isnā€™t possible either.

  • Moving ownership of a resource from a public type to a private holder-type stored in a field is a breaking-change. This should be an implementation detail. For example Vec<u8> is Drop but the nearly equivalent String is not.

  • Itā€™s un-intuitive and invites mistakes

I propose fixing this over several steps:

  1. Do a crater run to find all known usages of Drop as bound/parent trait. Check if any of them have a good reason (I canā€™t think of one).

  2. Add a compiler warning whenever Drop is used as trait bound or when another trait is derived from Drop. (Older proposal)

    This should be an actual compiler warning, not just a clippy lint, so even people who donā€™t use clippy can prepare for Step 2. This warning is easy and efficient to emit, easy to fix and has no false positives.

  3. Fix the Drop trait a) have the Drop bound match all types This is technically a breaking change, but I expect breakage to be extremely rare. It might even be acceptable to do this outside an edition change. b) turn the warning into an error (obviously a breaking change)

    This was proposed earlier, but the issue has since been closed.

    Iā€™d start with option a), since it causes very little breakage and would consider adding option b) as part of the next rust edition.

  4. Introduce a TrivialDrop trait as an analog to Copy. This step will likely happen much later, or not at all if the benefits do not outweigh the costs.

How would TrivialDrop work?

  • Copy derives from TrivialDrop
  • The compiler automatically derives TrivialDrop for all types which donā€™t implement Drop manually and all of whose field are TrivialDrop (similar to Sync/Send). Since implementing an empty Drop is already an opt-out, we donā€™t need impl !Drop for T {}.
  • This could allow generic functions to take advantage of non lexical lifetimes, since the call to drop at the end of the scope isnā€™t required if T implements TrivialDrop.

A Similar proposal, which also adds a Destruct trait in addition to Forget/TrivialDrop. That trait doesnā€™t seem very useful to me, but could be added later if desired.

If linear types are ever added:

  • All normal types would implement Drop, linear types would not
  • Drop would be a default trait bound which can be opted out of using ?Drop. Just like Sized works today.
  • TrivialDrop would derive from Drop.
  • Traits would not automatically derive from Drop.

I donā€™t expect linear types to ever be integrated into Rust, but them being nicely compatible with this design is a good sign.

9 Likes

From what I can tell, truly linear types would need to be explicitly/manually dropped; they are not un-droppable. (However, this can be a matter of definition.)

Wouldn't this be a wildly breaking change? Also, if you deem Drop useless, why would you want a TrivialDrop trait that means even less?

My first reaction to this was "citation needed". I literally couldn't imagine anything more intuitive for defining custom destruction code than a single trait with a single method that is automatically run when destruction happens. I don't see how that is considered unintuitive.

If you never want to place Drop as a trait bound, or if you even want to stop people from using Drop as a trait bound, this shouldn't be an issue either. I also don't think it's purely an implementation detail: it's entirely reasonable that modifying a public type changes its public API.

All in all, this proposal introduces a lot of complexity and breakage, and its motivation is really weak, so I don't see a good reason for adding either.

TrivialDrop, as it is named here, does mean something: it means the type and all its transitive constituents have no custom disposal logic, and to destroy a value of the type it suffices to deallocate its backing memory with no other actions performed. This property is currently impossible to express in the trait system except indirectly, through a Copy bound, but that has other consequences that may not always be desirable. OP links to a comment of mine under an RFC where having a way to express this property was quite crucial.

The same functionality was proposed by myself in the above-mentioned comment under the name Forget, and earlier by @canndrew under the same name; @glaebhoerl also sketched the basic idea on Reddit, with the name DropIsForget. I think it's telling that multiple people stumbled upon this idea independently.

3 Likes

For the purpose of this post, a droppable type can be assigned to _ or go out of scope without causing a compiler error.

Since Copy already requires the absence of a destructor and TrivialDrop is proposed as an auto-implemented trait, this shouldn't be a breaking change. It just splits out part of the requirements of Copy into a separate trait.

I would never have expected the difference between user implemented Drop and a compiler implemented one that calls the user-implement Drop of the type's fields to affect which traits a type implements.

For a fun exercise, try to predict which standard collections fulfill the Drop bound, and which do not.

I think refactoring a type to move its resource ownershop into an inner holder type should not be a breaking change. So

struct Collection{
    
}

impl Drop for Collection
{
    fn drop(&mut self)
    {
        
    }
}

could be turned into:

impl Drop for Inner
{
    fn drop(&mut self)
    {
        
    }
}

struct Collection{
    inner:Inner
}

without breaking consumers.

Disallowing Drop as trait bound introduces breakage, but only in cases that no sane code will hit.

Introducing TrivialDrop should not result in any breakage.

Introducing TrivialDrop shouldn't be too complex, since deriving Copy already requires the same logic. And it might even reduce the complexity of NLL slightly, since it could take advantage of already implemented traits, instead of duplicating the logic.

1 Like

I say we should start with a lint for bounding by Drop, then add TriviallyDrop as a separate RFC.

3 Likes

Considering how broken Drop bounds are, Iā€™d prefer a compiler warning which gets turned into an error in the next edition upgrade over a clippy lint.

3 Likes

Could this be replaced by non-trivial EagerDrop instead?

I'd like to be able to state that a drop implementation isn't sensitive to ordering and doesn't have side effects (like locks do). I presume that would also allow NLL for eagerly-droppable types.

2 Likes

I agree that this should eventually become a compiler lint, but starting out in Clippy means I was able to implement it without having to screw with multiple-hour-long compiler compiles.

I'll move it into rustc itself once it's been tested in clippy.

8 Likes

See related discussion on RFC #2632 about Drop bounds in const functions. It's looking like that feature may need some sort of ConstDrop trait (TrivialDrop):


Edit: Based on discussion within the last 24h it looks like ConstDrop isn't good enough either. See this amusing snippet:

if we start requiring T: ConstDrop then this will presumably creep into the APIs of the standard library and cause breakages so it may not be feasible?

Oh right, this doesn't do the effect systemy trickery where the bound loses its constness.

Actually. we can do this. We'll make const ConstDrop be implemented for the types as the rules say in this RFC, but we also implement ConstDrop (without the impl const ConstDrop, just impl ConstDrop) for all types

ConstDrop is not exactly this proposal, although it addresses a similar sub-problem of expressing ā€˜all transitively constituent Drop implementations have property Xā€™. But I think overhauling Drop as proposed here could make the other proposal simpler as well: const ConstDrop would simply become const Drop, with the following additional rules:

  • TrivialDrop/Forget implies const Drop.
  • Automatically-generated inductive/destructuring impls of Drop are const whenever all constituentsā€™ impls are const

The RFC was actually recently changed to propose basically what the OP proposed: T: Drop as a bound changes nothing (because all types can be dropped), and T: const Drop (well, really const fn foo<T: Drop>) means ā€œcan be dropped in const codeā€, which basically means that (recursively) all involved Drop instances must be impl const Drop.

Cc @oli-obk

1 Like

@RalfJung Thatā€™s a little strangeā€¦ If Drop actually means ā€œrunning destructorā€, then the existing Drop::drop is actually before_drop or somethingā€¦

Itā€™s a little strange, yes ā€“ but itā€™s not the only way in which Drop is magic. Like, you cannot even call Drop::drop (in safe code at least).

The more I use ManuallyDrop the more I wish there was a static analysis that detects that I've forgotten to properly drop these in all code paths.

1 Like

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