search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.23k stars 12.7k forks source link

`ManuallyDrop` should be a dyn-compatible receiver type #64351

Open withoutboats opened 5 years ago

withoutboats commented 5 years ago

We should extend the set of library-defined self types on stable to include types involving ManuallyDrop. ManuallyDrop is a transparent wrapper around another type which is completely unconstrained; there should be no technical limitations on making it an dyn-compatible[^1] receiver type.

To be explicit, I am proposing to make the following types valid self types on stable, and all but the first would be dyn-compatible:

This is similar to but distinct from #56193, which is about making coercions valid involving a manually drop wrapped around a pointer type.

In particular, I am personally interested in having Pin<&mut ManuallyDrop<Self>> as a dyn-compatible receiver type for an unsafe trait method in my code, as it assists users in understanding the guarantees the caller is giving them - that it will never access or move this value again, even to drop it, do with that what you will.

cc @mikeyhew @rust-lang/lang @rust-lang/wg-unsafe-code-guidelines

[^1]: Formerly known as "object safe".

withoutboats commented 5 years ago

(Note: I believe for now, the way to express the method I'm interested in so that it is object safe is self: Pin<&mut Self> and document that it is safe to ptr::read from Self if Self: Unpin, but I'm not 100% clear on whether its actually safe to ptr::read from Self with the way unsafe code guidelines are shaking out.)

comex commented 5 years ago

👎 . We should focus on stabilizing arbitrary_self_types rather than giving the standard library more special privileges.

mikeyhew commented 5 years ago

I'm not sure I agree with @comex, I don't think adding another special case would get in the way of stabilizing arbitrary_self_types. But I do get his/her/their frustration. All of the above listed receiver types are usable with the arbitrary_self_types feature, and they are object-safe. I don't necessarily think that the entire feature should be stabilized in one go, but we should work toward getting an RFC merged and stabilizing parts of it.

If we do add ManuallyDrop as a special case, I'm pretty sure all we have to do is implement the Receiver trait for it.

withoutboats commented 5 years ago

These things don't block one another - stabilizing the ability for users to define custom receivers doesn't have any kind of ordering with which std types can be custom receivers. We've already given std the special privilege to use this feature beyond what other crates can do (as we have with many other features), these types were just overlooked from the set we stabilized for no particular reason. Blocking progress in one way to create pressure to make progress in another is filibuster logic.

Thanks for pointing out that its as easy as adding impls @mikeyhew!

comex commented 5 years ago

The reason I care about ordering is that every new std-exclusive feature makes it harder to migrate from std functionality to alternatives.

In fact, the ordering is the only aspect of these features I care about. I'm not personally interested in arbitrary_self_types for its own sake. Rather, my sole interest is in minimizing the amount of std-exclusive functionality. Stabilizing arbitrary_self_types would reduce it; adding this would increase it.

That said, this is admittedly only a small extension to an existing set of functionality. If it were up to me, the existing set wouldn't have been stabilized until arbitrary_self_types was ready, but it's obviously not up to me. :)

mikeyhew commented 5 years ago

If we do add ManuallyDrop as a special case, I'm pretty sure all we have to do is implement the Receiver trait for it.

I just realized that's not true. Right now, if the feature flag is not enabled, we require that the receiver type implements Receiver + Deref<Target=Self>. &ManuallyDrop<Self> implements Deref<Target=ManuallyDrop<Self>> and does not work. I had a PR (https://github.com/rust-lang/rust/pull/55880) that allowed composed receiver types like &&Self in stable, but we ended up going with these more strict requirements in https://github.com/rust-lang/rust/pull/56805. Allowing composed receivers in general would probably be the easiest way to allow &ManuallyDrop<Self>. To allow &ManuallyDrop<Self> but not allow &&Self would be more challenging and probably not worth it.

withoutboats commented 5 years ago

@mikeyhew is it challenging to implement the work for composed receivers but not stabilize them? I would think that's just a matter of the Receiver impls. Composed receivers are a bit more uncertain to mebecause they make the set of receivers infinite with absurdity like self: &&&mut &Self

cramertj commented 5 years ago

I'm in the process of working to stabilize composed receivers in https://github.com/rust-lang/rust/pull/64325

nikomatsakis commented 5 years ago

I personally have no objection to extending the set of stabilized types a bit farther. That said, I would appreciate a few more details on the proposed use cases.

Otherwise:

would all seem to be in order.

Seems like it might also make sense to combine this effort with #64325?

RalfJung commented 5 years ago

What are the rules a type has to satisfy to be a "well-behaved" object-safe receiver type? Is there a "checklist" such that we can be sure a type that checks all the boxes is good to go as objcect-safe receiver type? This should be documented somewhere persistently, IMO before we go on stabilizing more of those. I am, naturally, particularly interested in any interaction with unsafe code and guarantees it would have to promise to uphold.

Yes, such docs work is not a lot of fun, but it is important (and with Pin, when we all agreed we'd improve the docs before stabilization, nobody did it... so based on that experience, I think such docs should be a blocker for landing stabilizations).

withoutboats commented 5 years ago

What are the rules a type has to satisfy to be a "well-behaved" object-safe receiver type? Is there a "checklist" such that we can be sure a type that checks all the boxes is good to go as objcect-safe receiver type?

Being represented as a pointer to the type and (transitively) safely dereferencing to it. There are some additional traits involved. I agree the present situation is not great, but I think this has to do with stabilizing the arbitrary_self_types feature much more than with extending the set of std-privileged types.

RalfJung commented 5 years ago

Being represented as a pointer to the type and (transitively) safely dereferencing to it.

ManuallyDrop is not a pointer type though. How does it determine how many indirections to take? Like, ManuallyDrop<&Self> and Box<&Self> look "syntactically similar" but are obviously different in terms of representation.

I agree the present situation is not great, but I think this has to do with stabilizing the arbitrary_self_types feature much more than with extending the set of std-privileged types.

Well, we better make sure the std-privileged types we add are actually safe. That's hard to do until we have a proper handle of what that actually means.

withoutboats commented 5 years ago

@RalfJung yes, ManuallyDrop<Self> is not object safe as a receiver just like self is not.

There are types we have held off on adding because they would involve making UCG related decisions - nonnull and raw ptrs, for example, because we haven't determined if the vtable pointer needs to be valid and how that relates to calling trait object methods, etc. Which is why the requirements now are narrowly types that we are confident would guarantee a valid vtable and data pointer (which includes ManuallyDrop).

RalfJung commented 5 years ago

Oh, and receiver types don't "nest", right? So all of the ones you are listing in the OP are exactly one pointer indirection.

However, Rc and Arc do not dereference to &self -- at least not in the sense of the MIR-level * operator. Or do you mean that the Deref impl is called?

withoutboats commented 5 years ago

@RalfJung yea, I meant the deref impl, which in those cases has to also do the offset past the reference counts.. In terms of nesting, currently they don't, but cramertj has a PR to allow them, though any that are double indirection (or more) would not be object safe.

My understanding is that if the type guarantees that Self is valid, there shouldn't be an intersection with UCG. That's why we've excluded types that don't necessarily guarantee that so far.

RalfJung commented 5 years ago

though any that are double indirection (or more) would not be object safe.

Why that?

My understanding is that if the type guarantees that Self is valid, there shouldn't be an intersection with UCG.

On first sight, that sounds like a reasonable assumption.

withoutboats commented 5 years ago

Why that?

I think its just an implementation limitation on how we access the vtable right now, @mikeyhew would know more.