Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

impl DispatchFromDyn for Cell and UnsafeCell #97373

Merged
merged 2 commits into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 32 additions & 1 deletion library/core/src/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ use crate::cmp::Ordering;
use crate::fmt::{self, Debug, Display};
use crate::marker::{PhantomData, Unsize};
use crate::mem;
use crate::ops::{CoerceUnsized, Deref, DerefMut};
use crate::ops::{CoerceUnsized, Deref, DerefMut, DispatchFromDyn};
use crate::ptr::{self, NonNull};

mod lazy;
Expand Down Expand Up @@ -571,6 +571,16 @@ impl<T: Default> Cell<T> {
#[unstable(feature = "coerce_unsized", issue = "18598")]
impl<T: CoerceUnsized<U>, U> CoerceUnsized<Cell<U>> for Cell<T> {}

// Allow types that wrap `Cell` to also implement `DispatchFromDyn`
// and become object safe method receivers.
// Note that currently `Cell` itself cannot be a method receiver
// because it does not implement Deref.
// In other words:
// `self: Cell<&Self>` won't work
// `self: CellWrapper<Self>` becomes possible
#[unstable(feature = "dispatch_from_dyn", issue = "none")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably ought to point at an actual issue, rather than none.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trait itself currently doesn't have an issue associated with it.

#[unstable(feature = "dispatch_from_dyn", issue = "none")]
#[lang = "dispatch_from_dyn"]
pub trait DispatchFromDyn<T> {
// Empty.
}

We could associate the trait and it's usages with arbitrary_self_types but I'm not sure if that is desired or should be done in this PR.

impl<T: DispatchFromDyn<U>, U> DispatchFromDyn<Cell<U>> for Cell<T> {}

impl<T> Cell<[T]> {
/// Returns a `&[Cell<T>]` from a `&Cell<[T]>`
///
Expand Down Expand Up @@ -2078,6 +2088,16 @@ impl<T> const From<T> for UnsafeCell<T> {
#[unstable(feature = "coerce_unsized", issue = "18598")]
impl<T: CoerceUnsized<U>, U> CoerceUnsized<UnsafeCell<U>> for UnsafeCell<T> {}

// Allow types that wrap `UnsafeCell` to also implement `DispatchFromDyn`
// and become object safe method receivers.
// Note that currently `UnsafeCell` itself cannot be a method receiver
// because it does not implement Deref.
// In other words:
// `self: UnsafeCell<&Self>` won't work
// `self: UnsafeCellWrapper<Self>` becomes possible
#[unstable(feature = "dispatch_from_dyn", issue = "none")]
impl<T: DispatchFromDyn<U>, U> DispatchFromDyn<UnsafeCell<U>> for UnsafeCell<T> {}

/// [`UnsafeCell`], but [`Sync`].
///
/// This is just an `UnsafeCell`, except it implements `Sync`
Expand Down Expand Up @@ -2169,6 +2189,17 @@ impl<T> const From<T> for SyncUnsafeCell<T> {
//#[unstable(feature = "sync_unsafe_cell", issue = "95439")]
impl<T: CoerceUnsized<U>, U> CoerceUnsized<SyncUnsafeCell<U>> for SyncUnsafeCell<T> {}

// Allow types that wrap `SyncUnsafeCell` to also implement `DispatchFromDyn`
// and become object safe method receivers.
// Note that currently `SyncUnsafeCell` itself cannot be a method receiver
// because it does not implement Deref.
// In other words:
// `self: SyncUnsafeCell<&Self>` won't work
// `self: SyncUnsafeCellWrapper<Self>` becomes possible
#[unstable(feature = "dispatch_from_dyn", issue = "none")]
//#[unstable(feature = "sync_unsafe_cell", issue = "95439")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a stray line here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this from the above CoerceUnsized impl.
I assumed this was meant to signal that there are two unstable features involved here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah there has been a longstanding feature request for unstable APIs to be able to be covered by multiple tracking issues / feature gates. For example if there's a NonZeroI256 type, and an unstable wrapping_neg inherent method on it (which is also unstable on a bunch of stable integer types), the method should be covered by gated on both features. We have had a problem in the standard library in the past of accidentally over-stabilizing in similar situations. If NonZeroI256 were stabilized and its wrapping_neg was gated on the NonZeroI256 feature instead of the wrapping_neg feature, it might inadvertently get stabilized during the NonZeroI256 stabilization without adequate review.

I thought there was a feature request issue filed for this but I wasn't able to find it just now.

The way this is written in the PR is the best we can currently do.

impl<T: DispatchFromDyn<U>, U> DispatchFromDyn<SyncUnsafeCell<U>> for SyncUnsafeCell<T> {}

#[allow(unused)]
fn assert_coerce_unsized(
a: UnsafeCell<&i32>,
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/feature-gates/feature-gate-dispatch-from-dyn-cell.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Check that even though Cell: DispatchFromDyn it remains an invalid self parameter type

use std::cell::Cell;

trait Trait{
fn cell(self: Cell<&Self>); //~ ERROR invalid `self` parameter type: Cell<&Self>
}

fn main() {}
12 changes: 12 additions & 0 deletions tests/ui/feature-gates/feature-gate-dispatch-from-dyn-cell.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0307]: invalid `self` parameter type: Cell<&Self>
--> $DIR/feature-gate-dispatch-from-dyn-cell.rs:6:19
|
LL | fn cell(self: Cell<&Self>);
| ^^^^^^^^^^^
|
= note: type of `self` must be `Self` or a type that dereferences to it
= help: consider changing to `self`, `&self`, `&mut self`, `self: Box<Self>`, `self: Rc<Self>`, `self: Arc<Self>`, or `self: Pin<P>` (where P is one of the previous types except `Self`)

error: aborting due to previous error

For more information about this error, try `rustc --explain E0307`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Check that a self parameter type requires a DispatchFromDyn impl to be object safe

#![feature(arbitrary_self_types, unsize, coerce_unsized)]

use std::{
marker::Unsize,
ops::{CoerceUnsized, Deref},
};

struct Ptr<T: ?Sized>(Box<T>);

impl<T: ?Sized> Deref for Ptr<T> {
type Target = T;

fn deref(&self) -> &T {
&*self.0
}
}

impl<T: Unsize<U> + ?Sized, U: ?Sized> CoerceUnsized<Ptr<U>> for Ptr<T> {}
// Because this impl is missing the coercion below fails.
// impl<T: Unsize<U> + ?Sized, U: ?Sized> DispatchFromDyn<Ptr<U>> for Ptr<T> {}

trait Trait {
fn ptr(self: Ptr<Self>);
}
impl Trait for i32 {
fn ptr(self: Ptr<Self>) {}
}

fn main() {
Ptr(Box::new(4)) as Ptr<dyn Trait>;
//~^ ERROR the trait `Trait` cannot be made into an object
//~^^ ERROR the trait `Trait` cannot be made into an object
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
error[E0038]: the trait `Trait` cannot be made into an object
--> $DIR/feature-gate-dispatch-from-dyn-missing-impl.rs:32:25
|
LL | fn ptr(self: Ptr<Self>);
| --------- help: consider changing method `ptr`'s `self` parameter to be `&self`: `&Self`
...
LL | Ptr(Box::new(4)) as Ptr<dyn Trait>;
| ^^^^^^^^^^^^^^ `Trait` cannot be made into an object
|
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
--> $DIR/feature-gate-dispatch-from-dyn-missing-impl.rs:25:18
|
LL | trait Trait {
| ----- this trait cannot be made into an object...
LL | fn ptr(self: Ptr<Self>);
| ^^^^^^^^^ ...because method `ptr`'s `self` parameter cannot be dispatched on

error[E0038]: the trait `Trait` cannot be made into an object
--> $DIR/feature-gate-dispatch-from-dyn-missing-impl.rs:32:5
|
LL | fn ptr(self: Ptr<Self>);
| --------- help: consider changing method `ptr`'s `self` parameter to be `&self`: `&Self`
...
LL | Ptr(Box::new(4)) as Ptr<dyn Trait>;
| ^^^^^^^^^^^^^^^^ `Trait` cannot be made into an object
|
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
--> $DIR/feature-gate-dispatch-from-dyn-missing-impl.rs:25:18
|
LL | trait Trait {
| ----- this trait cannot be made into an object...
LL | fn ptr(self: Ptr<Self>);
| ^^^^^^^^^ ...because method `ptr`'s `self` parameter cannot be dispatched on
note: required for `Ptr<{integer}>` to implement `CoerceUnsized<Ptr<dyn Trait>>`
--> $DIR/feature-gate-dispatch-from-dyn-missing-impl.rs:20:40
|
LL | impl<T: Unsize<U> + ?Sized, U: ?Sized> CoerceUnsized<Ptr<U>> for Ptr<T> {}
| --------- ^^^^^^^^^^^^^^^^^^^^^ ^^^^^^
| |
| unsatisfied trait bound introduced here
= note: required by cast to type `Ptr<dyn Trait>`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0038`.
22 changes: 22 additions & 0 deletions tests/ui/self/arbitrary_self_types_pointers_and_wrappers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![feature(rustc_attrs)]

use std::{
cell::Cell,
ops::{Deref, CoerceUnsized, DispatchFromDyn},
marker::Unsize,
};
Expand All @@ -20,6 +21,20 @@ impl<T: ?Sized> Deref for Ptr<T> {
impl<T: Unsize<U> + ?Sized, U: ?Sized> CoerceUnsized<Ptr<U>> for Ptr<T> {}
impl<T: Unsize<U> + ?Sized, U: ?Sized> DispatchFromDyn<Ptr<U>> for Ptr<T> {}


struct CellPtr<'a, T: ?Sized>(Cell<&'a T>);

impl<'a, T: ?Sized> Deref for CellPtr<'a, T> {
type Target = T;

fn deref(&self) -> &T {
self.0.get()
}
}

impl<'a, T: Unsize<U> + ?Sized, U: ?Sized> CoerceUnsized<CellPtr<'a, U>> for CellPtr<'a, T> {}
impl<'a, T: Unsize<U> + ?Sized, U: ?Sized> DispatchFromDyn<CellPtr<'a, U>> for CellPtr<'a, T> {}

struct Wrapper<T: ?Sized>(T);

impl<T: ?Sized> Deref for Wrapper<T> {
Expand All @@ -42,6 +57,7 @@ trait Trait {
fn ptr_wrapper(self: Ptr<Wrapper<Self>>) -> i32;
fn wrapper_ptr(self: Wrapper<Ptr<Self>>) -> i32;
fn wrapper_ptr_wrapper(self: Wrapper<Ptr<Wrapper<Self>>>) -> i32;
fn cell(self: CellPtr<Self>) -> i32;
}

impl Trait for i32 {
Expand All @@ -54,6 +70,9 @@ impl Trait for i32 {
fn wrapper_ptr_wrapper(self: Wrapper<Ptr<Wrapper<Self>>>) -> i32 {
***self
}
fn cell(self: CellPtr<Self>) -> i32 {
*self
}
}

fn main() {
Expand All @@ -65,4 +84,7 @@ fn main() {

let wpw = Wrapper(Ptr(Box::new(Wrapper(7)))) as Wrapper<Ptr<Wrapper<dyn Trait>>>;
assert_eq!(wpw.wrapper_ptr_wrapper(), 7);

let c = CellPtr(Cell::new(&8)) as CellPtr<dyn Trait>;
assert_eq!(c.cell(), 8);
}