13
$\begingroup$

In Rust, the Drop trait is defined like this:

trait Drop {
    fn drop(&mut self);
}

This has a number of issues:

  • If you would call the method manually, it would cause the object to be in a partially destructed state, that might be unsafe. Rust special cases this method and doesn't allow it to be called. If drop took self as an argument instead there would still need to be a special case to prevent it from being called twice but that feels less arbitrary.
  • You can't control the order different fields are destructed, or prevent their destruction. This would be trivial if the method took self as an argument as you could forget or drop each field with whatever order or method desired, or they could still be automatically dropped if you didn't do anything with them.

Are there any reasons I'm missing why drop taking self would hurt soundness? Is there any source for why the trait was designed this way?

$\endgroup$
6
  • $\begingroup$ I can think of two simple reasons ─ one is that transferring ownership implies a move, which might not always be optimised out; the other is that ownership would allow a custom implementation to not drop the value, instead keeping it alive. That wouldn't be unsound (a custom implementation could instead keep a copy alive, with essentially the same results), but it would be perverse and potentially could be done by accident. $\endgroup$
    – kaya3
    Commented Jul 5 at 13:09
  • $\begingroup$ @kaya3 Not droping a value is, in Rust, not considered unsafe though. $\endgroup$
    – mousetail
    Commented Jul 5 at 13:13
  • $\begingroup$ Yes, so it's not unsafe/unsound, but it would be strange if a custom implementation of the Drop trait was allowed to not drop the value. $\endgroup$
    – kaya3
    Commented Jul 5 at 13:24
  • 1
    $\begingroup$ @kaya3 The entire point though is to allow not dropping a field, or dropping it in a custom way other than the default. Right now that requires using ManuallyDrop<T> and unsafe code. Sure the default should be to drop the fields but there are good reasons why you might not want to, and needing to reach for unsafe seems odd since doing this does not actually risk UB. $\endgroup$
    – mousetail
    Commented Jul 5 at 13:52
  • 1
    $\begingroup$ Not sure if this is the answer, but if drop took self by value, it would drop it at the end by default, triggering an infinite recursion. $\endgroup$ Commented Jul 5 at 19:28

2 Answers 2

5
$\begingroup$

Let's go spelunking!

TL;DR: Too many special rules would be necessary.

A quick Google search leads to #4330: drop could take self by-value, and allow early destruction, Jan 2nd, 2013.

drop was already taking a reference at this point, and @thestinger asked whether it should be switched to by value. @nikomatsakis was in favor, but the change did not go through as per the comment by @alexcrichton:

We decided in today's meeting (https://github.com/mozilla/rust/wiki/Meeting-weekly-2013-10-22) that drop by value requires too many special rules and semantics about it to warrant the convenience of taking self by value instead of by reference [emphasis mine]. For that reason, I'm de-milestoning the bug, removing the P-backcompat-lang label, and removing the I-completion label.

While we don't plan on doing this for 1.0, there is always the possibility of doing it afterwards. It would be a little painful, but types could implement a DropValue trait if necessary and the compiler would require that both Drop and DropValue are not implemented for the same type. This means that I don't want to close this bug, because it would continue to be an interesting issue to investigate. In the meantime, however, this is not going to be a priority.

Unfortunately, the link to the meeting notes is dead, and the Wayback machine only has 1 single save from Nov. 2020 where it was already a 301 redirect to a non-existing page.


On a personal note, I don't see many alternatives to &mut self:

  • By value will indeed require some rules to avoid infinite recursions, especially when the value is further moved to regular functions. The fabled &own self mode would run into the same issues.
  • Splitting the value into its fields would prevent calling methods on the value. For example, a Drop for Vec will first call .clear() to destroy all elements prior to releasing the memory, which requires the value as a whole.
$\endgroup$
5
$\begingroup$

If Drop::drop took self by value, it would infinitely recurse.

In the simplest case, self would be dropped again at the end of Drop::drop's scope, calling Drop::drop again, causing self to be dropped again, and so on.


But wait, couldn't Drop::drop be special-cased, so that it destroys self regularly? The problem is, it's not necessarily Drop::drop that would be destroying self. Consider:

struct Foo { 
    // ... some fields
}

fn do_something(foo: Foo) {
    // ... some code
    foo;
}

impl Drop for Foo {
    fn drop(self) {
        do_something(self);
    }
}

Here, the drop code for Foo isn't in Drop::drop, it's in do_something.

In this particular example, Drop::drop could call a special version of do_something that doesn't drop Foo. But the general problem is unsolvable. Consider:

struct Foo {
    fun: fn()
}

static FOO: Mutex<Option<Foo>> = Mutex::new(None);

fn do_something(foo: Foo) {
    let fun = foo.fun;
    FOO.lock().insert(foo);
    fun();
}

impl Drop for Foo {
    fn drop(self) {
        do_something(self);
    }
}

fn do_something_else() {
    FOO.lock().take();
}

fn main() {
    let foo = Foo { fun: do_something_else };
}

Here, the call to main drops foo = Foo { fun: do_something_else }, which calls Drop::drop, which calls do_something, which inserts foo into FOO, then calls do_something_else, which takes foo out of FOO, then drops foo again...infinite recursion. And the static analysis to detect and prevent this is hard to the point of being infeasible (there are possible examples that are more convoluted).


One possible workaround is to implicitly add a boolean in every Drop-implementing type that is set when the instance gets dropped the first time, and dropping when already set regularly destroys the instance. But this goes against Rust's goal to have maximum efficiency and minimum hidden complexity; no other impl block changes a type's structure. Furthermore, this would prevent Drop from being implemented on repr(transparent) types.

Another workaround is to prevent Drop::drop from wholly moving self, only individual fields. In fact, I could see the developers going with instead of &mut self. Nonetheless, it adds complexity to an already complex language, especially the Drop trait which already has extra restrictions.


Meanwhile, Drop::drop taking &mut self isn't a big problem, because it can be worked around in multiple ways:

  • In safe Rust: by replacing self with dummy data.
#[derive(Default)]
struct Foo {
    // ... some fields implementing `Default`
}

impl Drop for Foo {
    fn drop(&mut self) {
        let this = std::mem::take(self);
        // ... now you have a value `this`
    }
}

If you don't want to implement Default, you can use std::mem::replace and some random values. If a field's type doesn't permit "dummy" instances, you can sacrifice a bit of efficiency and cleanness by wrapping the field in Option, where it will always be Some except in Drop::drop.

struct Foo(ManuallyDrop<FooRepr>);

struct FooRepr {
    // ... some fields implementing `Default`
}

impl Foo {
    pub fn new(
        // ... FooRepr fields
    ) -> Self {
        Self(ManuallyDrop::new(FooRepr {
            // ...
        }))
    }
}

impl Deref for Foo {
    type Target = FooRepr;

    fn deref(&self) -> &Self::Target {
        self.0.deref()
    }
}

impl DerefMut for Foo {
    fn deref_mut(&mut self) -> &mut Self::Target {
        self.0.deref_mut()
    }
}

impl Drop for Foo {
    fn drop(&mut self) {
        let foo = unsafe { ManuallyDrop::take(&mut self.0) };
    }
}

Also note that, if you only care about moving some of self's fields, you can replace-with-stub or wrap the individual fields instead of the entire value.

$\endgroup$
9
  • $\begingroup$ I think this is most likely true, but since the question is about why the designers chose this approach it would be ideal to point at some supporting information on that side (which presumably does exist, given the Rust development process). $\endgroup$
    – Michael Homer
    Commented Jul 6 at 8:22
  • $\begingroup$ Swift made deinit take self as an immutable borrow, because mutating methods are allowed to consume self and then assign a new value to it (which would cause deinit to recurse). Is this not an issue in Rust? $\endgroup$
    – Bbrk24
    Commented Jul 6 at 13:05
  • $\begingroup$ @Bbrk24 it is possible in Rust with std::mem::swap. Likewise, it's possible to get an infinitely-recursing Drop::drop(&mut self), the easiest way just doing self.clone() since then the clone will be dropped. But it's not an issue the same way Drop::drop(self) is. The thing with Drop::drop(self), is that it would have to be special-cased because even an empty function would drop. And then it's not immediately clear what to do when self is passed to another function; if that is disallowed, it means the body of Drop::drop has special rules that no other function has. $\endgroup$
    – tarzh
    Commented Jul 8 at 3:41
  • $\begingroup$ Although Drop::drop already has special rules for how its called... $\endgroup$
    – tarzh
    Commented Jul 8 at 5:01
  • $\begingroup$ @MichaelHomer other than a stack exchange post with roughly the same answer, and some Reddit and forum posts asking how to move dropped fields, the only thing I found were the the commits where drop was added. It was really early in development (before the Drop trait, there were drop { ... } blocks). $\endgroup$
    – tarzh
    Commented Jul 8 at 5:23

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .