Should this trait be unsafe?

extern { type Erased; }
type ErasedPtr = ptr::NonNull<Erased>;

unsafe trait Eraseable {
    /// Turn this erasable pointer into an erased pointer.
    fn erase(this: Self) -> ErasedPtr;
    /// Unerase this erased pointer.
    ///
    /// # Safety
    ///
    /// The erased pointer must have been created by `erase`.
    unsafe fn unerase(this: ErasedPtr) -> Self;
}

Unsafe code (and safe code!) relies on Eraseable::unerase(Eraseable::erase(_)) to be valid. But I think that is encoded entirely within the type—an implementation that fulfills the type requirements is safe to use—thus the trait doesn't need to be an unsafe trait. I'm not exactly certain, though; the exact meaning of a non-unsafe trait with an unsafe method is unclear.

This trait would be implemented for things like Box<impl Sized> and Box<impl ?Sized + StoresLengthInline>.

This is basically the key. It sounds like the properties of this trait represent a trusted assertion -- that unerase is a round-trip operation for erase. If implementing this trait unfaithfully would cause other code to become unsafe, for example, then the trait should be unsafe to implement. So you've done the right thing here by making it an unsafe trait.

I disagree that the property you're aiming for is encoded in the type. It's encoded in the comments, but that's different. It's pretty easy to write an unfaithful implementation that compiles:

struct Evil<'a>(&'a mut u8);

impl<'a> Eraseable for Evil<'a> {
    // This method is horribly wrong but safe and it compiles
    fn erase(this: Self) -> ErasedPtr {
        NonNull::dangling()
    }

    // This method is basically correct, but wrong because of erase.
    unsafe fn unerase(this: ErasedPtr) -> Self {
        Evil(this.cast().as_mut())
    }
}
5 Likes

I was focused on unerase when I should have thought about erase, whoops! Thanks for catching that.

I think it's part of Eraseable's contract. You should be able to run Foo::unerase(my_foo.erase()) and end up with the same object and no UB... That's probably an indicator that Eraseable should be an unsafe trait.

1 Like

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