PinCoerceUnsized trait into core

[dingxiangfei2009]

cc @Darksonn @wedsonaf @ojeda

This is a PR to introduce a PinCoerceUnsized trait in order to make trait impls generated by the proc-macro #[derive(SmartPointer)], proposed by RFC, sound. There you may find explanation, justification and discussion about the alternatives.

Note that we do not seek stabilization of this PinCoerceUnsized trait in the near future. The stabilisation of this trait does not block the eventual stabilization process of the #[derive(SmartPointer)] macro. Ideally, use of DerefPure is more preferrable except this will actually constitute a breaking change. PinCoerceUnsized emerges as a solution to the said soundness hole while avoiding the breaking change. More details on the DerefPure option have been described in this section of the RFC linked above.

Earlier discussion can be found in this Zulip stream and rust-for-linux thread.

[rustbot]

r? @m-ou-se

rustbot has assigned @m-ou-se.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Nilstrieb]

"same value at the same address" here is pretty vague, and it's unclear to me without reading the RFC thread what exactly that entails and how it compares to the stable deref trait crate. notably, the preconditions on the crate are currently not satisfied by Box because of the special strict aliasing rules imposed by it.

[Darksonn]

The meaning of "same value" is that the concrete type must not change. I copied the explanation in below. Was it unclear?

Here, "same value" means that if deref returns a trait object, then the actual type behind that trait object must not change. Additionally, when you unsize coerce from Self to Unsized, then if you call deref on Unsized and get a trait object, then the underlying type of that trait object must be <Self as Deref>::Target.

Analogous requirements apply to other unsized types. E.g., if deref returns [T], then the length must not change. In other words, the underlying type must not change from [T; N] to [T; M].

The motivation for this requirement is that with trait objects, you could otherwise first return one struct, and then later return some wrapper struct that wraps the original struct using #[repr(transparent)].

[compiler-errors]

This shouldn't be made into a lang item until it's actually used by the compiler. I feel like having unused lang items is not desirable.

[workingjubilee]

is there any chance of us somehow managing to check and error in that case?

[workingjubilee]

thinks okay the moment this left my hands the answer came to me as "probably not usefully" (it would only be one more formality-tier listing somewhere and someone will just add it to that).

[dingxiangfei2009]

I think the use of it is rather imminent. Ideally with #123472 this would go into the trait bounds that #[derive(SmartPointer)] generates.

[dingxiangfei2009]

I came to my right sense! I have dropped the lang term here.

[Darksonn]

It would make sense to add the following as a test:

use core::cell::{Cell, RefCell, UnsafeCell};
use core::pin::Pin;

pub trait MyTrait {}
impl MyTrait for String {}

pub fn cell(arg: Pin<Cell<Box<String>>>) -> Pin<Cell<Box<dyn MyTrait>>> {
    arg
}
pub fn refcell(arg: Pin<RefCell<Box<String>>>) -> Pin<RefCell<Box<dyn MyTrait>>> {
    arg
}
pub fn ucell(arg: Pin<UnsafeCell<Box<String>>>) -> Pin<UnsafeCell<Box<dyn MyTrait>>> {
    arg
}

This compiles today, so we shouldn't break it.

[dingxiangfei2009]

@Darksonn test cases have been added.

[traviscross]

@rustbot labels +I-lang-nominated +T-lang

Nominating as @dingxiangfei2009 requested lang review.

[nikomatsakis]

Nit

[nikomatsakis]

Suggested change

	/// If this type alos implements `DerefMut`, then the same guarantee must be upheld by
	/// If this type also implements `DerefMut`, then the same guarantee must be upheld by

[dingxiangfei2009]

Applied

[traviscross]

r? @scottmcm

[traviscross]

@rustbot labels -I-lang-nominated

We talked about this in the meeting between members of the lang team and RfL. We felt this work was sufficiently covered by the RFC that no lang team decision was needed here for this nightly work. It just needs review, and we'll leave that to @scottmcm as he has context here.

[Darksonn]

I think you should also implement for raw pointers and NonNull. (And add a test.)

[Darksonn]

It looks like this is using the old safety comment. The RFC was updated to use these safety requirements instead:

/// Trait that indicates that this is a pointer or a wrapper for one, where
/// unsizing can be performed on the pointee when it is pinned.
///
/// # Safety
///
/// If this type implements `Deref`, then the concrete type returned by `deref`
/// and `deref_mut` must not change without a modification. The following
/// operations are not considered modifications:
///
/// * Moving the pointer.
/// * Performing unsizing coercions on the pointer.
/// * Performing dynamic dispatch with the pointer.
/// * Calling `deref` or `deref_mut` on the pointer.
///
/// The concrete type of a trait object is the type that the vtable corresponds
/// to. The concrete type of a slice is an array of the same element type and
/// the length specified in the metadata. The concrete type of a sized type
/// is the type itself.

It gets rid of the requirement that the address is the same, and simplifies it to just requiring that the concrete type is unchanged.