Making Drop more magic to make it less magic

[literda]

I try to summarize the thoughts in this work-in-progress pre-RFC.

[Soni]

I mean a macro foo!(bar: Baz) that expands to #[fixme] bar: ManuallyDrop<Baz>

[literda]

@Soni

I mean a macro foo!(bar: Baz) that expands to #[fixme] bar: ManuallyDrop<Baz>

Oh I see, yes sure, that works, but we could do better IMHO.

[Ixrec]

I think the motivation section needs to talk about how you end up wanting to move things out of a struct during drop in the first place. This is a situation I’ve never personally run into, and for some reason none of these threads have talked about how you end up running into it, so I can’t shake the feeling that this is really an XY problem or a symptom of an architecture issue somewhere above this specific struct. Which is something we’d want to clear up before adding anything to the core language.

But once compelling motivation is established, the rest is all bikeshedding I have no strong opinion on.

[stepancheg]

for @stepancheg 's “magic container” idea, I am not quite sure how this can be handled: is it possible to do partial move out of Self?

There are no partial moves in my proposal.

If so, would that recover returns an invalid object?

DefaultDrop<T> contains a proper valid object, because constructor did not run yet (default destructor of <T> is called in the destructor of DefaultDrop<T> and the custom destructor is running now.)

Let me start with an illustration with current rust:

struct MyStruct(u32);

impl Drop for MyStruct { fn drop(&mut self) { panic!(); } }

fn my() {
  let foo = MyStruct(10);
  let MyStruct(ten) = foo;
}

This code won't panic because MyStruct destructor won't be executed, because MyStruct was destructured.

There are not "partial moves" in my proposal. Either object fields are destroyed altogether by DefaultDrop destructor (as if there was no custom destructor). Or object is "recovered", and it is a regular object as before destructor was started.

So if recover is called:

• object can be kept intact, then destructor will be executed again leading to an infinite loop. That is expected. If you explicitly call recover, you have to understand that. Most users won't need to call recover.
• object can be destructured to recover fields, so no destructor will be called
• object can be "forgotten" with (forget function), so rust won't call the destructor
• object can be saved elsewhere (I don't know why that can be useful, but technically it's possible)

A developer who calls recover should be aware that returned object is a valid regular object that will be destroyed again unless something is done with it.

A couple of examples.

fn drop(self_holder: DefaultDrop<My>) {
    // do nothing: field destructors will be called
}

fn drop(self_holder: DefaultDrop<My>) {
    self_holder.get_mut().close_something();
    // do nothing: field destructors will be called
}

fn drop(self_holder: DefaultDrop<My>) {
    let recovered = self_holder.recover();
    let My { field1, field2, .. } = recovered;
    // destructors of `field1` and `field2` and other fields will be called
    // as with usual destructuring
}

fn drop(self_holder: DefaultDrop<My>) {
    forget(self_holder);
    // completely skip the destructor
}

fn drop(self_holder: DefaultDrop<My>) {
    forget(self_holder.recover());
    // completely skip the destructor
}

fn drop(self_holder: DefaultDrop<My>) {
    let my = self_holder.recover();
    // infinite loop, because `my` is a regular object that will be destroyed
}

[earthengine]

@stepancheg

This code won’t panic because MyStruct destructor won’t be executed, because MyStruct was destructured.

This code DOES panic. Try it in playground.

If the match only do partial move like in the following though,

enum MyEnum {
    V1(String),
    V2
}

impl Drop for MyEnum { fn drop(&mut self) { panic!(); } }

fn main() {
  let foo = MyEnum::V1("v1".into());
  if let MyEnum::V1(s) = foo { println!("{}", s) }
}

You get

error[E0509]: cannot move out of type `MyEnum`, which implements the `Drop` trait
  --> src/main.rs:10:10
   |
10 |   if let MyEnum::V1(s) = foo { println!("{}", s) }
   |          ^^^^^^^^^^^-^
   |          |          |
   |          |          hint: to prevent move, use `ref s` or `ref mut s`
   |          cannot move out of here

Deeper thoughts

I believe dropping an object includes two related but different things:

1. Fulfill the invariant for an object to be dropped (for example, call some FFI to release external resources). After that, the drop clue for the main object is considered being used and so should not be run again
2. Destruct the object into its parts, and allows further dropping clue on child objects to run

I believe today's Drop is good for step 1, as it expect a fully functional object, including its drop clue. However, trying to do partial move in such a drop method is trying to mix it up with step 2, which is the source of the problems we have today.

Having it in consideration, we can go forward to the right thing without breaking any backward compatibility issues. Here is the proposal:

New trait: Destruct

trait Destruct {
    #[destruct]
    fn destruct(self);    
}
impl<T:Destruct> for Option<T> {
    #[destruct]
    fn destruct(self) {
        match self {
            Some(v) => v.destruct(),
            None => ()
        }
    }
}

Rules:

• A function/method have #[destruct] attribute must accept a single argument and its first HIR operation must be a pattern match that moves things from the arguments out (we call this "moving match"). This guarantees no references to the destructing object can occur in user code.
• destruct will be called when the object was implicitly dropped. But if it is destructed through moving matches it is not called.
• If an object implements both Drop and Destruct, it does not cause E0509: a moving match will call drop before the match, to justify the call. However, partial move in drop will be unconditionally UB.
• If an object implements only Destruct, it can also be Copy. I don't think this is useful, but there is no reason to stop this.
• For objects implements only Drop, it behaves exactly the same as today: partial moving match results in E0509. drop can do partial move without UB. We may later consider lint against it and finally obsolete this style.
• #[destruct] can apply to any functions, as long as it follows the same rule. Especially, this allows closures being declared like this, making it possible to pass a "pattern match snippet" across. However, for methods that call-by-reference rather than call-by-move, the match need not be a moving match.

Compare to the DefaultDrop<T> proposal:

I understand @stepancheg 's DefaultDrop<T> means a T with the drop clue removed. However, it have more magic here, and didn't resolve E0509 at all.