Potential generalization of destructing struct destructuring

[steveklabnik]

Issue by glaebhoerl Monday Jan 27, 2014 at 21:17 GMT

For earlier discussion, see https://github.com/rust-lang/rust/issues/11855

This issue was labelled with: in the Rust repository

---

Currently structs with destructors are (sensibly) not allowed to be move-destructured (#3147), because then when their destructor runs, it would access deinitialized values.

This could potentially be generalized if we recognize that a destructor, in other words drop glue, is not an indivisible thing. It consists of the Drop impl for the struct itself, and the destructors for each of its fields. Therefore if we write:

struct S { a: A, b: B }
impl Drop for S { ... }
let s = make_some_s();
let S { a, b } = s;

when the destructuring happens we could run the Drop impl for S, but not the drop glue for A and B. Those are then moved out, and their destructors will run later, whenever they go out of scope.

I believe this is sound: Drop::drop() takes &mut self, so it can mutate the components of S but not deinitialize them. If we broaden our considerations to unsafe code, then if the fields of a struct S have destructors themselves, then the Drop impl for S must, even today, leave them in a valid state, because those destructors will then access it. It is only cases where the fields of S do not have their own destructors, and the Drop impl for S uses unsafe code to put them in an invalid state, which would become dangerous, and we would have to be very careful about.

We'd obviously have to think about whether we actually want this (I haven't thought of any use cases yet), but as a theoretical possibility, I think it checks out.

[spiveeworks]

I have a use case.. I'm trying to make polydimensional array DSTs, which is all well and good, but I would like to implement movable-reference semantics, which means making a Box variant with a drop flag.

The relevance to destructuring is I'd like to implement fn result(self: OptionalFlatBox<T>) -> Result<FlatBox<T>, FlatBoxSpace<T>>, and using this feature I could do so:

struct OptionalFlatBox<T: SizeInfo> {
    is_init: bool,
    space: FlatBoxSpace<T>,
}
impl<T: SizeInfo> OptionalFlatBox<T> {
    fn result(mut self: Self) -> Result<FlatBox<T>, FlatBoxSpace<T>> {
        let is_init = self.is_init;
        self.is_init = false;
        let OptionalFlatBox { space, .. } = self;
        if is_init {
            Ok(FlatBox { space })
        } else {
            Err(space)
        }
    }
}

So what this code does is it sets the drop flag to false and destructures, this way the referant is not dropped, then if the drop flag was true it returns a wrapper that always drops its content, and if it was false it returns the object responsible for allocating.

Without this feature I would need to construct a new FlatBoxSpace and then mem::forget(self).... but it gets worse, FlatBoxSpace contains a size_info: T field, where T: SizeInfo, and this type is not necessarily Copy, in fact it could be a DST specifying the widths of a jagged array! So in order to move the SizeInfo I would need to ptr::read(&self.space.size_info) and then leak the original, which is quite complicated for something so clearly sound :(

[Lucretiel]

I run into this a lot with RAII wrapper types. A trivial example:

struct RunOnCleanup<F: FnOnce()> {
    on_drop: F
}

impl<F: FnOnce()> Drop for RunOnCleanup<F> {
    fn drop(&mut self) {
        let on_drop = self.on_drop; // fails here, can't move out of borrow
        on_drop();
    }
}

I've also seen it happen where, when dropping, I want to call into_iter on one of my fields, because I need to iterate over its T (not its &T). Usually I end up having to wrap them in an Option and do Option.take() in drop, which seems silly.

It seems to me that, if drop took self by value, the compiler could implicitly destructure it (to ensure it doesn't drop itself recursively at the end of drop(self)) and then use the normal rule for dropping each individual field (that is, drop them when they go out of scope, unless ownership was transferred into something else).

[RustyYato]

@Lucretiel On nightly you can use ManuallyDrop::take to do this

[moulins]

I have stumbled onto an other use case for this feature, where ManuallyDrop isn't enough:

pub struct Unescaper<'a> {
    buf: &'a mut String,
    old_len: usize,
    has_errored: bool,
}

pub struct UnescaperError;

impl<'a> Unescaper<'a> {
    pub fn new(buf: &'a mut String) -> Self {
        Self {
            old_len: buf.len(),
            buf,
            has_errored: false,
        }
    }
   
    pub fn unescape_fragment(&mut self, fragment: &str) {
        // unescape the content of the string fragment,
        // write it to buf, and update internal state...
    }

    pub fn finish(self) -> Result<&'a str, UnescaperError> {
        if self.has_errored {
            // The Drop impl will clear out the partially unescaped string
            Err(UnescaperError)
        } else {
            // Disable clear-on-drop and returns the unescaped string to the user
            let this = std::mem::ManuallyDrop::new(self);
            &this.buf[this.old_len..] // <--+
            //                              |
            // error: cannot return value referencing local variable `this`
        }
    }
}

// Clear what we've written out on drop
impl<'a> Drop for Unescaper<'a> {
    fn drop(&mut self) {
        self.buf.truncate(self.old_len)
    }
}

Currently, there is no way to write this without using unsafe code to transmute the lifetime of the returned buffer

If destructuring was always allowed, and disabled the Drop impl, I could simply write:

pub fn finish(self) -> Result<&'a str, UnescaperError> {
    if self.has_errored {
        Err(UnescaperError)
    } else {
        let Self { buf, old_len, .. } = self; // note: this disables clear-on-drop
        &buf[old_len..]
    }
}