Proposal: drop with panic argument

[joshlf]

Currently, in order to detect in a Drop impl whether the program is currently panicking, the drop function must access thread-local storage, which is expensive. This slows down certain key primitives such as MutexGuard.

I propose that we add a new method to Drop that takes a bool argument indicating whether the function is being called due to unwinding. It would look something like this:

trait Drop {
    fn drop(&mut self);

    fn drop_unwinding(&mut self, unwinding: bool) {
        Drop::drop(self);
    }
}

The code that performs unwinding would be altered to call Drop::drop_unwinding with the unwinding argument set to true. The code that drops values in the normal, non-unwinding case would be altered to call Drop::drop_unwinding with the unwinding argument set to false.

cc @sfackler @alexcrichton

[CAD97]

Alternative: why not have

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

instead, with the semantics being that drop_unwinding is called when unwinding, and drop when not?

The case where you want to handle dropping during an unwind already need to implement both, so there’s no added “cost” there, unless you’re intending to see one of the following:

impl Drop for _ {
    fn drop(&mut self) { unreachable!() }
    // Or
    fn drop(&mut self) {
        Drop::drop_unwinding(self, probably_not);
    }

    fn drop_unwinding(&mut self, unwinding: bool) { ... }
}

Keep in mind also the language rules that make Drop::drop uncallable due to the special rule about moving out of &mut self in this case, so just delegating from one to the other is not trivial in std and would have to be special-cased (again) to be callable from the other drop function.

The advantage of redelegating to .drop(unwinding) being able to share the deconstruction code that’s the same between unwinding and not, I suppose.

---

I’m in favor of something like this, though!

[joshlf]

Ah, good point; that’d work too. Do you know if, in either of these approaches, there’s a way to skirt the issue of needing to special case in libcore in order to avoid the no-calling-drop rule?

[CAD97]

Making a rule that both Drop::drop and Drop::drop_unwinding are allowed to call each other if that’s the only thing they do would avoid special casing libcore only at the cost of making Drop more special. And then it would support Drop::drop delegating to Drop::drop_unwinding, though really if we’re going the full magic route, just have drop and drop_unwinding, with a requirement to implement exactly one of them and the compiler calling whichever one is defined

I think the “best” solution would be more magic on Drop to support passing the unwinding parameter if requested, with the “simplest” being magic such that drop_unwinding gets an lang default to just call drop, though that would require unreachable!()ing drop when implementing drop_unwinding.

[joshlf]

Is there any way to do something like trait Drop: !DropUnreachable {} and trait DropUnreachable: !Drop {}? I don’t believe you can, but if you could figure out a way to do that in vanilla Rust, you could express the mutual exclusion without any compiler magic.

[Darsstar]

Wasn’t there a comment, which I wasn’t able to find after searching for a few minutes, in one of the many places where discussion about the pinning API took place about how we should use a time machine to redefine Drop to be drop(this: Pin<&mut T>); if we had one?

Not that that necessarily means a new method on Drop should have such a signature, but I thought it should be mentioned.

cc @RalfJung

[RalfJung]

Yeah if we change the signature anyway – which will be tough on compatibility – then ideally we make it take Pin<&mut T>, not &mut T.

[earthengine]

which will be tough on compatibility

I am curious about the real difficulty here - as drop is not directly callable by the user, if we just change Drop to include new methods and make some magics such that if the new methods are not defined it is default to the old method when available; otherwise the old method is default to the new “required” method.

Making special cases usually not a good thing; but object destruction is so special by itself already…

[ExpHP]

You know… Drop as a trait doesn’t even make sense in the first place.

You can use T: Drop as a trait bound, but why would you? It tells you nothing of value. Specializing on it would not only be painful, but also incorrect, because a type like struct Bar(Vec<i32>); technically does not implement Drop. (one should specialize on T: Copy instead).

If destructors used another special language syntax you had to learn—one which justified not only its existing peculiarities, but also the sort of magical things we’re talking about here—would it really be that bad?

[drXor]

Well, Drop being a trait is mostly useless if you want to bound by it… but it is useful as a negative bound. In a world where we have negative bounds that don’t wreck inference (haha like that’ll happen), T: !Drop is really useful; you could imagine that union U<T> { t: T } would implicitly have T: Sized + !Drop.

Though, I think the more reasonable situation is a #[dtor] attribute to put on the dtor method, and a lang-item trait Forget that embodies “trivially destructable” (i.e., mem::forget(x) is a semantic no-op since no resources are leaked), in analogy to how Copy means “trivially copyable”… as a bonus, we would get to write trait Copy: Clone + Forget and remove the silly "Copy and Drop can’t coexist" magic.

[rkruppe]

Knowing that a type does or doesn’t implementing Drop is not useful, it doesn’t tell you anything. As @ExpHP explained, a type can transitively contain a type with a Drop impl (and thus need code to run when it goes out of scope) without itself implementing Drop. In fact, neither Vec<T> nor String implement Drop (at the moment – this is basically an implementation detail).

In my opinion the only reason Drop is a trait is so that we don’t need special syntax or attributes to implement custom logic, and to reuse a few properties of trait impls that are also relevant to drop code, such as coherence. It is a mechanism for associating code with a type, not for generic programming.

[burakumin]

It is a mechanism for associating code with a type, not for generic programming.

The question is: is this really an acceptable status quo? It has always seemed to me that having a true destructor concept would enlarge the design space by offering new possibilities for interesting extensions that currently are restrained or invalidated by the fact Drop is a trait (destructors as patterns for example) for almost no good reasons

[drXor]

Knowing that a type does or doesn’t implementing Drop is not useful,

Right, I went through this argument in another thread already and forgot about this part. You want a transitive auto thing going on for it to actually make sense:

// of course, how auto traits work isn't really nailed down, 
// but let's not get bogged down in details.
unsafe auto trait Forget {}
impl<D> !Forget for D where D: Drop {}
impl Forget for $primitive {}
impl Forget for $empty_struct {}

In a perfect world, we don’t have Drop and have some alternative way to declare dtors, and a Forget trait (which the compiler would have to attach to types magically… unfortunately) to embody trivial destruction, which is actually useful. I think that destructors are too fundamental to shove into the trait system; for example, the magic restrictions that ban Drop + Copy and Drop::drop(x).

[system]

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