pre-RFC: the Destruct trait

23

Yep, as far as I know the only way to get better behaviour for pinning would be if Drop::drop took Pin<&mut Self> and most implementations relied on being able to ignore the pinning because they are Unpin. I haven’t thought about this proposal with pinning much, but I think the only issue is the one noted that it would be unsound to implement Destruct for a type that is !Unpin if it is auto-magically called.

24

Something nobody in the thread seems to have mentioned yet… But if you change destructors from by-reference to by-value, how is one supposed to implement Drop (or, in this case, Destruct) for unsized types which can’t be moved or live on stack but are still important part of the type system?

2 Likes
25

Along these lines - how are you supposed to call drop_in_place on these types? copy them out?

26

I knew I was missing something, it seemed too simple.

I guess this needs &own or similar, then. (An owned self that you can move out of and are in charge of dropping, but is still tied to the lifetime of someone else who owns the backing (potentially stack) allocation.)

27

This starts sounding more complicated than current &mut TBH.

2 Likes
28

has anyone talked about the possibility of destructuring Drop types, using a marker trait, and changing the Drop for such types such that:

  • fn Drop::drop(&mut self) where Self: !Destructurable
  • fn Drop::drop(self) where Self: Destructurable
  • lint if Drop::drop(self) doesn’t irrefutably destruct self, either through let Self { ... } = self; or match self { Self::... }. (this can even be checked with a regex if we require the use of Self over other ways of referring to the type!)

This also allows destructuring outside Drop.

29

Let me start with a simple and very valid requirement: wrap a FnOnce() into a struct, so we can have the Finally semantics.

Today we write:

struct Finally<F:FnOnce()>(F);
impl<F:FnOnce()> Drop for Finally<F> {
    fn drop(&mut self) {
        unsafe {
            (std::mem::replace(&mut self.0, std::mem::uninitialized()))()
        }
    }
}

fn main() {
    let finally = Finally(|| println!("finally!"));
}

To allow suppress the function call though, it is being very tricky. I think the best you can do is use another trait rather than FnOnce.

Now with this rfc, we write

struct Finally<F>(F) where F:FnOnce();
impl<F:FnOnce()> Destruct for Finally<F> {
    #[match] 
    fn destruct(self) {
        (self.0)();
    }
}

fn main() {
    let finally = Finally(|| println!("finally!"));
}

This is clearly much easier and intuitive. To suppress the call is also easy:

let finally = Finally(|| println!("finally!"));
let f = finally.0; // the closure was moved out, and so the call is suppressed
30

Bonus: your Finally has a soundness bug.

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

(Choose Tools>MIRI)

The defer crate uses an Option internally.

The fact that “fake” by-move is so easy to get wrong is why I think a by-value Drop is useful. Unsized types make it too complicated to solve simply, though, so I’m definitely going to try my hand at a “AfterDrop” as sketched in a previous thread.

1 Like
31

Hopefully not. That means the uninitialized value would be dropped (by the drop glue after drop returns).

IMO that is exactly a counterargument – I think it's mainly an education problem. If you understand that (some of) the drop glue happens after Drop::drop returns, it should be clear that replacing arbitrary parts of self with an uninitialized value isn't going to do any good. By-value drop "solves" this problem by hiding the fact, but in the process it actually makes the drop model yet more complex. In the end, that's what matters in the long run.

And those who don't fully understand how drop works could just refrain from using unsafe (and that's a good idea anyway), or learn the (currently quite simple and understandable) logic behind drop if they need to. Probably its internals should be better documented in the book (currently only a very high-level description is there) – it's so fundamental that it would be worth digging deeper into. (Yes, yes, I know, beginner friendliness and stuff. But it's probably not the complete beginners who will write custom drop impls, and so I don't think we would scare people away by explaining its internals.)

1 Like
32

We already have an approved RFC (1909) for passing unsized objects by value.

33

Even with that, it would be quite inefficient to move large structures / arrays from heap to stack just for the sake of dropping, compared to current API.

1 Like
34

I believe that this RFC adds more problems/difficulties then it should:

  • It does two thinks with one(+1) RFCs

    1. having a “drop-ish” method which accepts self
    2. allow pattern destruction of Drop types (I’m actually publishing a pre-RFC wrt. that tody, I wrote it a while ago just didn’t hade time to write it down formally).

  • It doesn’t play well with drop_in_place and dynamic sized types. Yes you can pass them by value in the future and you can (unsafe) move the value in case of drop_in_place. But that can be externally (yes not just verry) unefficient if combined with very large datastructures, i.e. exactly such on which you might more likely use drop_in_place on. (I mean we speak about a additional unnecessary copy of potentially megabytes in some (very rare) edge cases).

  • It theoretically is splitting drop into two phases (drop->destruct) but practically it’s more tricky. Mainly you can’t really do anything anymore in Drop as people might accidentally “sidestep” drop by explicitly calling destruct you now should put everything into destruct. But in practice people likely will use different approaches to what is done in drop and what in destruct (it even quite easy to do so accidentally when multiple people work on the same type) leading to likely subtle bugs.

  • The only real benefit of this RFC is cases where you either need specific drop order or need to move out thinks on drop and “send” them somewhere else. Which both are rare cases and (while sometimes needed) bad practice[1] (destruction of drop types can be made much simpler then shown in this RFC).

So in conclusion I don’t think the RFC is worth it in the way it currently is, it has to many drawbacks for common cases while bringing little benefit (after you extract the part of pattern destructing Drop traits, which is conceptually unrelated to Destruct except that destruct needs it to be implementable).

[1]: Expect freeing resources drop normally should (if possible) not have side-effects, also even if it has dorp order only matters if you either did something very creepy/dangerous wrt. refactoring with unsafe code or you have multiple side-effects which depend on their order which sometimes happens but tends to be bad design. In my experience most times you need either of this there is a api/struct layout/etc. which doesn’t have this problem and is better due to other design reason. Through without question there are cases where it’s needed but they should be rare enough that that small amount of manual drop doesn’t matter.

35

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