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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue for dyn-star #102425

Open
7 of 9 tasks
eholk opened this issue Sep 28, 2022 · 16 comments
Open
7 of 9 tasks

Tracking issue for dyn-star #102425

eholk opened this issue Sep 28, 2022 · 16 comments
Assignees
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-dyn_star `#![feature(dyn_star)]` T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@eholk
Copy link
Contributor

eholk commented Sep 28, 2022

This issue tracks known issues or missing functionality with support for dyn* Trait objects. These are an experimental feature that are meant to support async function in dyn traits.

In addition to the issues linked here, you can also find things that need to be done by greping the code for // FIXME(dyn-star) or looking at the F-dyn_star label.

Associated PRs

About this issue

This is a lang-team experiment -- there hasn't been an RFC for this functionality, we're exploring it both as a potential implementation detail and (more speculatively) a possible language feature in the future. You can read more about dyn* at this blog post.

Lang-team champion: @nikomatsakis

@eholk eholk added the F-dyn_star `#![feature(dyn_star)]` label Sep 28, 2022
@Rageking8
Copy link
Contributor

@rustbot label +C-tracking-issue

@rustbot rustbot added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Sep 29, 2022
@inquisitivecrystal inquisitivecrystal added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Sep 29, 2022
@nikomatsakis nikomatsakis moved this to Experimental in Initiatives Oct 4, 2022
@nikomatsakis nikomatsakis self-assigned this Oct 4, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 21, 2022
…er-errors

Add a test case for async dyn* traits

This adds a test case that approximates async functions in dyn traits using `dyn*`. The purpose is to have an example of where we are with `dyn*` and the goal of using it for dyn traits.

Issue rust-lang#102425

r? `@compiler-errors`
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 1, 2023
…sue, r=jackh726

Use the correct tracking issue for `dyn_star`

`#![feature(dyn_star)]` now has its own tracking issue, rust-lang#102425.
@RalfJung
Copy link
Member

RalfJung commented Feb 6, 2023

Currently the dynamic type hidden by dyn* is required to match usize in size and alignment. In principle it seems possible to allow smaller alignment, and possibly even smaller size (though in the latter case care has to be taken that the vtable always ends up at the same offset -- this would be easier if the vtable was the first component of the pair, not the 2nd). Are there plans to allow that?

@ds84182
Copy link

ds84182 commented Feb 6, 2023

Right now you can actually place smaller size & align types with dyn* by stuffing it inside a usize with a little bit of unsafe. Due to the existing alignment requirements for the vtable ptr, it will always end up at the same offset as long as size_of::<T>() <= size_of::<usize>() && align_of::<T>() <= align_of::<usize>().

@RalfJung
Copy link
Member

RalfJung commented Feb 6, 2023

Due to the existing alignment requirements for the vtable ptr, it will always end up at the same offset as long as size_of::() <= size_of::() && align_of::() <= align_of::().

This is not true for size_of::<T>() == 0. Also, this assumes that for a usize (and pointers), size == align.

@ds84182
Copy link

ds84182 commented Feb 6, 2023

This is not true for size_of::<T>() == 0. Also, this assumes that for a usize (and pointers), size == align.

Ah, right. I guess a better way to describe the requirements is something like

union DynStarCompatible<T> {
  true_value: T,
  size_align: usize,
}

where DynStarCompatible<T> matches the size and align of usize. (of course, this still has the same assumption about usize and ptrs, but this could be trivially replaced with *const ())

@eholk
Copy link
Contributor Author

eholk commented Feb 7, 2023

Currently the dynamic type hidden by dyn* is required to match usize in size and alignment. In principle it seems possible to allow smaller alignment, and possibly even smaller size (though in the latter case care has to be taken that the vtable always ends up at the same offset -- this would be easier if the vtable was the first component of the pair, not the 2nd). Are there plans to allow that?

Yes, this is something I would like to support.

@RalfJung
Copy link
Member

From the initial PR:

Note that this is never intended to become stable surface syntax for Rust, but rather dyn* is planned to be used as an implementation detail for async functions in dyn traits.

Honestly that seems like a shame -- this could be useful way beyond async fn I think. (But then there are other proposals that are even more general so indeed stabilizing this particular one might be the wrong call.)

@l-Luna
Copy link

l-Luna commented Feb 11, 2023

From the initial PR:

Note that this is never intended to become stable surface syntax for Rust, but rather dyn* is planned to be used as an implementation detail for async functions in dyn traits.

Honestly that seems like a shame -- this could be useful way beyond async fn I think. (But then there are other proposals that are even more general so indeed stabilizing this particular one might be the wrong call.)

might be useful to point out that the post you linked intentionally ignores any non-Box types, considering them not useful for supporting Pin

bors added a commit to rust-lang-ci/rust that referenced this issue Feb 21, 2023
…li-obk

Miri: basic dyn* support

As usual I am very unsure about the dynamic dispatch stuff, but it passes even the `Pin<&mut dyn* Trait>` test so that is something.

TBH I think it was a mistake to make `dyn Trait` and `dyn* Trait` part of the same `TyKind` variant. Almost everywhere in Miri this lead to the wrong default behavior, resulting in strange ICEs instead of nice "unimplemented" messages. The two types describe pretty different runtime data layout after all.

Strangely I did not need to do the equivalent of [this diff](rust-lang#106532 (comment)) in Miri. Maybe that is because the unsizing logic matches on `ty::Dynamic(.., ty::Dyn)` already? In `unsized_info` I don't think the `target_dyn_kind` can be `DynStar`, since then it wouldn't be unsized!

r? `@oli-obk` Cc `@eholk` (dyn-star) rust-lang#102425
RalfJung pushed a commit to RalfJung/miri that referenced this issue Feb 21, 2023
Miri: basic dyn* support

As usual I am very unsure about the dynamic dispatch stuff, but it passes even the `Pin<&mut dyn* Trait>` test so that is something.

TBH I think it was a mistake to make `dyn Trait` and `dyn* Trait` part of the same `TyKind` variant. Almost everywhere in Miri this lead to the wrong default behavior, resulting in strange ICEs instead of nice "unimplemented" messages. The two types describe pretty different runtime data layout after all.

Strangely I did not need to do the equivalent of [this diff](rust-lang/rust#106532 (comment)) in Miri. Maybe that is because the unsizing logic matches on `ty::Dynamic(.., ty::Dyn)` already? In `unsized_info` I don't think the `target_dyn_kind` can be `DynStar`, since then it wouldn't be unsized!

r? `@oli-obk` Cc `@eholk` (dyn-star) rust-lang/rust#102425
@eholk
Copy link
Contributor Author

eholk commented Mar 29, 2023

From the initial PR:

Note that this is never intended to become stable surface syntax for Rust, but rather dyn* is planned to be used as an implementation detail for async functions in dyn traits.

Honestly that seems like a shame -- this could be useful way beyond async fn I think. (But then there are other proposals that are even more general so indeed stabilizing this particular one might be the wrong call.)

Maybe "never" was too strong in the original PR. I think dyn* will likely turn out to be a more ergonomic version of dyn, so hopefully we can find a version that makes sense to make part of the surface language at some point. I think the point was we picked dyn* as a placeholder syntax that we didn't want to bikeshed just yet, and if the feature becomes part of Rust proper we'd probably do it under a different syntax.

github-actions bot pushed a commit to rust-lang/glacier that referenced this issue Apr 7, 2023
=== stdout ===
=== stderr ===
warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
 --> /home/runner/work/glacier/glacier/ices/105777.rs:1:12
  |
1 | #![feature(dyn_star)]
  |            ^^^^^^^^
  |
  = note: see issue #102425 <rust-lang/rust#102425> for more information
  = note: `#[warn(incomplete_features)]` on by default

warning: 1 warning emitted

==============
github-actions bot pushed a commit to rust-lang/glacier that referenced this issue Apr 10, 2023
=== stdout ===
=== stderr ===
warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
 --> /home/runner/work/glacier/glacier/ices/104631.rs:3:12
  |
3 | #![feature(dyn_star)]
  |            ^^^^^^^^
  |
  = note: see issue #102425 <rust-lang/rust#102425> for more information
  = note: `#[warn(incomplete_features)]` on by default

error[E0277]: `AlignedUsize` needs to have the same ABI as a pointer
  --> /home/runner/work/glacier/glacier/ices/104631.rs:14:13
   |
14 |     let x = AlignedUsize(12) as dyn* Debug;
   |             ^^^^^^^^^^^^^^^^ `AlignedUsize` needs to be a pointer-like type
   |
   = help: the trait `PointerLike` is not implemented for `AlignedUsize`

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0277`.
==============
@oxalica
Copy link
Contributor

oxalica commented Apr 24, 2023

I doubt that if this needs to be a language feature instead of a library impl.
We can implement dyn* using ptr_metadata:

// Or `Dyn: Pointee + ?Sized`, but we'll tolerant non `dyn` unsized types. 
pub struct InlinedDyn<Dyn: Pointee<Metadata = ptr::DynMetadata<Dyn>> + ?Sized> {
    data: usize, // Or anything if you like, or even with a const generic size.
    metadata: ptr::DynMetadata<Dyn>,
    _marker: PhantomData<Dyn>,
}
impl<Dyn: Pointee<Metadata = ptr::DynMetadata<Dyn>> + ?Sized> InlinedDyn<Dyn> {
    pub fn new<T: Unsize<Dyn>>(value: T) -> Self;
}
impl<Dyn: Pointee<Metadata = ptr::DynMetadata<Dyn>> + ?Sized> ops::Deref for InlinedDyn<Dyn> {
    type Target = Dyn;
}
impl<Dyn: Pointee<Metadata = ptr::DynMetadata<Dyn>> + ?Sized> ops::DerefMut for InlinedDyn<Dyn> {}

Is there any feature I missed that cannot be implemented like this?

@bjorn3
Copy link
Member

bjorn3 commented Apr 24, 2023

That doesn't allow coercing to dyn* or using it as receiver. It also doesn't allow an automatic impl Trait for dyn* Trait for every trait.

@compiler-errors
Copy link
Member

Noting here (and left an unchecked checkbox above) that dyn and dyn* should (probably?) have their TyKind representation split from one another. #107908 was an attempt at this, but the PR got stale, and rebasing it would've probably been a pain.

@oxalica
Copy link
Contributor

oxalica commented May 8, 2023

That doesn't allow coercing to dyn* or using it as receiver.

That's fair, but I still think a struct-like lang-item is more pretty, just like Box<T>. So we can more easily extend it with more methods or parameterized it with const-generic custom size or alignment.

It also doesn't allow an automatic impl Trait for dyn* Trait for every trait.

Is this really necessary? Box<T> doesn't do this also, but we are still able do make Box<dyn FnOnce()> work.

@bjorn3
Copy link
Member

bjorn3 commented May 8, 2023

Box<T> doesn't do this also, but we are still able do make Box<dyn FnOnce()> work.

Through an unstable feature to allow unsized arguments that is. Not something that can be replicated by end users for other traits.

@eholk
Copy link
Contributor Author

eholk commented May 9, 2023

That doesn't allow coercing to dyn* or using it as receiver.

That's fair, but I still think a struct-like lang-item is more pretty, just like Box<T>. So we can more easily extend it with more methods or parameterized it with const-generic custom size or alignment.

For what it's worth, we consider dyn* to be a placeholder syntax while we develop the idea. If we were to stabilize something like as part of user-facing Rust, we'd probably spell it something like Dyn<T> instead.

@RalfJung
Copy link
Member

RalfJung commented Sep 9, 2023

dyn and dyn* should (probably?) have their TyKind representation split from one another

#115699 on the side fixes yet another bug caused by those two being the same TyKind.

RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 20, 2024
Miri: basic dyn* support

As usual I am very unsure about the dynamic dispatch stuff, but it passes even the `Pin<&mut dyn* Trait>` test so that is something.

TBH I think it was a mistake to make `dyn Trait` and `dyn* Trait` part of the same `TyKind` variant. Almost everywhere in Miri this lead to the wrong default behavior, resulting in strange ICEs instead of nice "unimplemented" messages. The two types describe pretty different runtime data layout after all.

Strangely I did not need to do the equivalent of [this diff](rust-lang/rust#106532 (comment)) in Miri. Maybe that is because the unsizing logic matches on `ty::Dynamic(.., ty::Dyn)` already? In `unsized_info` I don't think the `target_dyn_kind` can be `DynStar`, since then it wouldn't be unsized!

r? `@oli-obk` Cc `@eholk` (dyn-star) rust-lang/rust#102425
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Miri: basic dyn* support

As usual I am very unsure about the dynamic dispatch stuff, but it passes even the `Pin<&mut dyn* Trait>` test so that is something.

TBH I think it was a mistake to make `dyn Trait` and `dyn* Trait` part of the same `TyKind` variant. Almost everywhere in Miri this lead to the wrong default behavior, resulting in strange ICEs instead of nice "unimplemented" messages. The two types describe pretty different runtime data layout after all.

Strangely I did not need to do the equivalent of [this diff](rust-lang/rust#106532 (comment)) in Miri. Maybe that is because the unsizing logic matches on `ty::Dynamic(.., ty::Dyn)` already? In `unsized_info` I don't think the `target_dyn_kind` can be `DynStar`, since then it wouldn't be unsized!

r? `@oli-obk` Cc `@eholk` (dyn-star) rust-lang/rust#102425
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-dyn_star `#![feature(dyn_star)]` T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
Status: Experimental
Development

No branches or pull requests