Arbitrary self types v2: pointers feature gate.

[adetaylor]

The main arbitrary_self_types feature gate will shortly be reused for a new version of arbitrary self types which we are amending per this RFC. The main amendments are:

• do support self types which can't safely implement Deref
• do not support generic self types
• do not support raw pointers as self types.

This PR relates to the last of those bullet points: this strips pointer support from the current arbitrary_self_types feature. We expect this to cause some amount of breakage for crates using this unstable feature to allow raw pointer self types. If that's the case, we want to know about it, and we want crate authors to know of the upcoming changes.

For now, this can be resolved by adding the new
arbitrary_self_types_pointers feature to such crates. If we determine that use of raw pointers as self types is common, then we may maintain that as an unstable feature even if we come to stabilize the rest of the arbitrary_self_types support in future. If we don't hear that this PR is causing breakage, then perhaps we don't need it at all, even behind an unstable feature gate.

Tracking issue

This is step 4 of the plan outlined here

[rustbot]

r? @fmease

rustbot has assigned @fmease.
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

[adetaylor]

r? @wesleywiser but don't review it yet until I'm sure it passes tests

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

[wesleywiser]

Just a few minor suggestions 🙂

[wesleywiser]

Looking at results for arbitrary_self_types on GH search, it seems the majority of hits are from forks of PyO3 (which mentions the feature in a comment, does not currently use it) and forks of rustc. Therefore, I don't think we need to worry too much about trying to migrate current users of the unstable feature to (potentially) the newly introduced pointer feature gate.

If you concur, I'm happy to r+!

[adetaylor]

OK, I've been told that it's pretty expensive to do a crater run and not all that useful for nightly-only breakage, so I'll take the r+ then!

[wesleywiser]

We're right at the beginning of the release cycle so if there is unexpected, widespread breakage we have plenty of time to fix or revert before beta branches so I think this is the appropriate time to:

@bors r+

[bors]

📌 Commit 8e20b66 has been approved by wesleywiser

It is now in the queue for this repository.

[adetaylor]

This didn't work as planned - I'm seeing an unexpected error in one of my crates which relies upon the nightly arbitrary_self_types feature. I'm going to look into why, but we might need to revert this if a fix isn't immediately apparent. Looking into it now.

Phew, panic over. It's because I was doing a completely gross hack in autocxx to work around the absence of arbitrary self types, and it's expected that it broke.

In case anyone else hits the same thing, the gross hack was:

// CppRef<T> is some smart pointer type

impl<'a, T: ?Sized> Deref for CppRef<'a, T> {
    type Target = *const T;   // note the pointer type here
    #[inline]
    fn deref(&self) -> &Self::Target {
        // contents don't matter
    }
}

This hack tricked the deref algorithm to jumping from CppRef<T> to T without us having to have a UB-prone &T at any moment. This workaround obviously relied on the actual *const T dereferencing part of the former arbitrary self types work, which is just the bit we intentionally disabled in this PR. It's unsurprising that this broke.

[purplesyringa]

This broke crossmist. crossmist needs to serialize and deserialize objects all the time, including Box<dyn Trait>, sort of like serde_traitobject (after a quick look, that crait likely broke too).

While serializing through dyn Trait is easy if the trait contains something like a serialize_me method, deserializing requires somehow creating a valid fat pointer to call a method on from thin air. Both crossmist and serde_traitobject achieve this by storing a vtable pointer prior to object data during serialization, and invoke the virtual method on a dangling fat pointer (with the right vtable) during deserialization.

Crucially, this is only valid if pointers are used, not references, because references can't be dangling or null.

There are ways to workaround this, e.g. by using ZSTs or adding a fn get_deserializing_fn(&self) -> fn(&mut Deserializer) -> Self function to the trait and prepending the "deserializing fn" to data instead of the vtable. However, all these workarounds introduce indirection and slow code down when really the expected behavior is quite simple.

I think the breakage is going to be more significant than expected and I hope arbitrary_self_types_pointers is not removed.

[adetaylor]

@purplesyringa thanks for the report, good to know. First off, from my side, there's no particular pressure to remove arbitrary_self_types_pointers, it's just that we're hoping to eventually stabilize the rest of arbitrary self types, so that's why it's been split into a separate unstable feature gate. Others may have different views, but that's my current understanding.

Maybe your use-case could be accommodated using "regular" arbitrary_self_types by transmuting your fat pointer into a transparent newtype wrapper which implements the upcoming Receiver trait, and then using that as the receiver? But maybe not if you're trying to call existing functions on existing types, and don't have an opportunity to insert an additional type wrapper.

[purplesyringa]

Sure, I just saw this in the OP and thought I should weigh in to prevent this outcome:

If we don't hear that this PR is causing breakage, then perhaps we don't need it at all, even behind an unstable feature gate.

---

Wrapping the fat pointer in a newtype is fine, the more troublesome part is object safety. The method fn deserialize_me(self: NewTypeWrapper<Self>) would have to be callable via Self as Deserialize's vtable. This interacts with DispatchFromDyn, which I'm unfamiliar with, and it's not 100% clear to me how stable it is to stabilization and if I can easily wrap a raw pointer using it.

[adetaylor]

DispatchFromDyn is on its way to partial stabilization via the #[derive(SmartPointer)] work in case that works for you?

[purplesyringa]

It might work, thanks!