Scope pinning `&mut T`

code review
1

Hi, I had an idea for a primitive like that that will allow safely pinning &mut T. What do you think about it?

#![no_std]

use core::mem::{forget, ManuallyDrop};
use core::pin::Pin;
use core::ptr;

pub fn scope_pin<T, R, F>(place: &mut T, replacement: T, f: F) -> R
where
    F: FnOnce(Pin<&mut T>) -> R,
{
    struct Restore<T> {
        ptr: *mut T,
        replacement: ManuallyDrop<T>,
    }

    struct Bomb;

    impl Drop for Bomb {
        fn drop(&mut self) {
            panic!("drop unwound while restoring a scope-pinned place");
        }
    }

    impl<T> Drop for Restore<T> {
        fn drop(&mut self) {
            let bomb = Bomb;
            unsafe {
                ptr::drop_in_place(self.ptr);
                ptr::write(self.ptr, ManuallyDrop::take(&mut self.replacement));
            }
            forget(bomb);
        }
    }

    let ptr: *mut T = place;
    let restore = Restore {
        ptr,
        replacement: ManuallyDrop::new(replacement),
    };

    let value = f(unsafe { Pin::new_unchecked(&mut *ptr) });

    drop(restore);

    value
}
2

I would invite you to explain more about your idea first. For example:

  • why do you think it's safe?
  • why do you think this would be useful?

I believe you can make this just unsafe { *self.ptr = ManuallyDrop::take(&mut self.replacement) }. The compiler will take care of dropping the value at *self.ptr and ensure that it will be replaced by ManuallyDrop::take(&mut self.replacement) even if the dropping panics. Moreover there's no need for a panic bomb in that case because *self.ptr will be replaced in any case.

3

I think that in case the drop impl of T panics it would leave the place in a bad state. I also believe it is kind of more clear what is going on if the drop is explicit.

Why I believe it is safe? Unless you longjump, the T from &mut T is dropped, fulfilling the drop guarantee of Pin constructed. A replacement is just to not leave the reference in an inconsistent state, it is fairly trivial to get one in most cases imo. Idk about use cases, just sharing an idea, as I think it is both interesting and sound.

I took an inspiration from the fact the if a !Forgetis borrowing the value, it is effectively pinned until this !Forget value is dropped. For !Destruct(linear?) types the guarantee seems even more powerful.

4

I agree, it looks sound and the underlying idea is interesting (it's a very similar reasoning to the one used by Pin::set!)

However I believe this is unfortunately rather useless given it can be implemented using safe code and other primitives:

pub fn scope_pin<T, R, F>(place: &mut T, replacement: T, f: F) -> R
where
    F: FnOnce(Pin<&mut T>) -> R,
{
    f(pin!(std::mem::replace(place, replacement)))
}
5

It looks like the difference is whether the structure in place is moved before it’s pinned or not. But if the value is address-sensitive, presumably it should’ve already been pinned. So I can’t see a use case either.

6

Yeah, that's a good way to look at it...

7

I guess the only substantial difference is that the unsafe version only does one write (from the replacement into place) while the safe one must more one than one to swap the values at place and replacement, which could be less efficient.

8

irrespective of potential use cases, i would prefer this. no need for superfluous unsafe

#![no_std]

use core::mem::{forget, ManuallyDrop};
use core::pin::Pin;
use core::ptr;

pub fn scope_pin<T, R, F>(place: &mut T, replacement: T, f: F) -> R
where
    F: FnOnce(Pin<&mut T>) -> R,
{
    struct Restore<'a, T> {
        ptr: &'a mut T,
        replacement: ManuallyDrop<T>,
    }

    struct Bomb;

    impl Drop for Bomb {
        fn drop(&mut self) {
            panic!("drop unwound while restoring a scope-pinned place");
        }
    }

    impl<T> Drop for Restore<'_, T> {
        fn drop(&mut self) {
            let bomb = Bomb;
            *self.ptr = unsafe { ManuallyDrop::take(&mut self.replacement) };
            forget(bomb);
        }
    }
    
    let restore = Restore {
        ptr : place,
        replacement: ManuallyDrop::new(replacement),
    };

    let value = f(unsafe { Pin::new_unchecked(restore.ptr) });

    drop(restore);

    value
}

ofc drop_with_owned_fields would be even better

9

hey, could please why did you create a Bomb with a custom Drop and then used forget(bomb) to not drop it? Are you using it to check something? thanks!

10

Assigning to *self.ptr will drop its value, which runs its Drop code and that might panic. In that case the forget won't be reached and the bomb will be dropped.

This is a common patter when you want to run some cleanup code, or need to handle a panic differently than a normal return.

That said I don't think this bomb is needed here.

11

Honestly, why don't you use simple approach like this?

pub fn scope_pin<T, R, F>(place: &mut T, replacement: T, f: F) -> R
where
    F: FnOnce(Pin<&mut T>) -> R,
{
    f(std::pin::pin!(std::mem::replace(place, replacement)))
}
12