8000 impl DispatchFromDyn for Cell and UnsafeCell by dimpolo · Pull Request #97373 · rust-lang/rust · GitHub
[go: up one dir, main page]
Skip to content

impl DispatchFromDyn for Cell and UnsafeCell #97373

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 26, 2023
Merged

impl DispatchFromDyn for Cell and UnsafeCell #97373

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2d1cb2
impl DispatchFromDyn for Cell and UnsafeCell
dimpolo May 24, 2022
af58854
add feature gate tests for DispatchFromDyn
dimpolo Oct 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
impl DispatchFromDyn for Cell and UnsafeCell
  • Loading branch information
@dimpolo
dimpolo committed Jan 24, 2023
commit a2d1cb2c22016ae003ab51d9654746cc4fc5200a
33 changes: 32 additions & 1 deletion library/core/src/cell.rs
View file
Open in desktop
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.

rust/library/core/src/ops/unsize.rs

Lines 115 to 119 in bed4ad6

#[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
22 changes: 22 additions & 0 deletions tests/ui/self/arbitrary_self_types_pointers_and_wrappers.rs
View file
Open in desktop
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);
}
0