Introduce MovingPtr as a safer alternative to moving typed values by raw pointer

[james7132]

Objective

Make moving potentially large values, like those seen in #20571 and those seen by #20772, safer and easier to review.

Solution

Introduce MovingPtr<'a, T> as a wrapper around NonNull<T>. This type:

• Wraps a pointer and is thus cheap to pass through to functions.
• Acts like a Box<T> that does not own the allocation it points to. It will drop the value it points to when it's dropped, but will not deallocate when it's dropped.
• Acts like a OwningPtr in that it owns the values it points to and has an associated lifetime, but it has a concrete type.
• As it owns the value, it does not implement Clone or Copy.
• Does not support arbitrary pointer arithmetic other than to get MovingPtrs of the value's fields.
• Does not support casting to types other than ManuallyDrop<T> and MaybeUninit<T>.
• Has methods that consume the MovingPtr that copies the value into a target pointer or reads it onto the stack.
• Provide unsafe functions for partially moving values of members out and returns a MovingPtr<'a, MaybeUninit<T>> in its stead.
• Optionally supports unaligned pointers like OwningPtr for use cases like Introduce Command::apply_raw #20593.
• Provides From impl for converting to OwningPtr to type erasure without loss of the lifetime or alignment requirements.
• Provides a TryFrom impl to attempt to convert an unaligned instance into a aligned one. Can be combined with DebugCheckedUnwrap to assert that the conversion is sound.
• The deconstruct_moving_ptr provides a less error-prone way of decomposing a MovingPtr into separate MovingPtr for its fields.

This design is loosely based on the outptr proposal for in-place construction, but currently eschews the requirements for a derive macro.

Testing

CI, new doc tests pass.

[hymm]

I did a quick pass over the safety comments and they mostly seem reasonable. partial_move and move_field are a bit complex to reason about though and I need to think about them a bit more.

[hymm]

where do you need this nasty function in the other pr?

[james7132]

The other PR splits the bundle in two: components without effects to be inserted and related components (i.e. SpawnRelated) that only have effects. The latter would need to operate on the partially moved instance of the bundle, and hence why it returns a MovingPtr<'a, MaybeUninit<T>>.

[james7132]

I added an example for both this and move_field on how to use this.

[hymm]

So the goal here is to destructure a struct without moving the parts. So ideally this is only done on !Drop structs, just like normal destructuring. Seems like it would be pretty hard to make this safer without a macro. You could return an iterator of OwningPtr's from a iterator of offsets, but since this is typed I can't think of a way to make it work.

[james7132]

I agree. The original intent was to use this as a building block for a macro like the one shown here: #20877 (comment), but given that we're already primarily using this in a derive macro/all_tuples tower, I wanted to eschew that extra compilation overhead.

---

The point about !Drop is pretty strong. In their current construction derive(Bundle) types that implement Drop may access values that were already moved out of.

EDIT: It looks like it's currently a compile failure. #20772 would allow B: Drop bundles to be derived, but also force them to be forgotten.

[hymm]

is 'a the correct lifetime here or should it be tied to &self and be '_?

[james7132]

The 'a lifetime is accurate. This function's use is meant for decomposition, and I'm not actually sure how to best structure this. This needs to not take ownership of self, as to not drop whole value, but needs to extract all fields as MovingPtrs and then immediately forget the top level MovingPtr before any of the fields are used. Ideally this would be done without extra traits and yet another all_tuples impl tower. Example:

let field_a = parent.move_field::<FieldAType>(offset_of!(Self, field_a));
let field_b = parent.move_field::<FieldBType>(offset_of!(Self, field_b));
let field_c = parent.move_field::<FieldCType>(offset_of!(Self, field_c));
mem::forget(parent);
insert(field_a);
insert(field_b);
insert(field_c);

This must be done in this order or else we risk double-dropping already moved fields if an intermediate call to the hypothetical insert panics.

[james7132]

Ideally, we should have a macro like the following to simplify the above to:

decompose_moving_ptr!(parent, 
   field_a: FieldAType => insert(field_a),
   field_b: FieldBType => insert(field_b),
   field_c: FieldCType => insert(field_c),
);

[james7132]

I added an example for both this and partial_move on how to use this.

[james7132]

To try to make this a bit easier, I added a macro via deconstruct_moving_ptr to make sure its usage patterns are obeyed.

[james7132]

tbh I think the move_field stuff is really ugly because it leaves the "parent" in a very strange state where all fields must be moved out and the parent cannot be dropped, kind of negating it's usefulness. maybe I'm overlooking something here.

Have you already implemented the stack overflow pr with this new type? I'm not really seeing how the move_field stuff will alleviate the unsafe virality of that pr.

To try to make this a bit easier, I added a macro via deconstruct_moving_ptr to make sure its usage patterns are obeyed. It should be pretty hard to mess that one up, and generally looks close to a mix of a struct deconstruction and a match expression. I'm not 100% sure how well this is going to work with our proc macros though.

Otherwise, I think this is in a good enough spot for use in #20772 now.

[janis-bhm]

Maybe this macro should have something like this to force the user to declare every field, though it might be overkill for fields that are !Drop

[james7132]

I was going to suggest that this would be handled separately. The derive(Bundle) types might need something like this, but the all_tuples impls wouldn't require this assertion.

[SkiFire13]

Just a small nit