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 upcasting coercion #65991

Open
19 of 21 tasks
Tracked by #165 ...
alexreg opened this issue Oct 31, 2019 · 84 comments · Fixed by #118133
Open
19 of 21 tasks
Tracked by #165 ...

Tracking issue for dyn upcasting coercion #65991

alexreg opened this issue Oct 31, 2019 · 84 comments · Fixed by #118133
Assignees
Labels
A-trait-system Area: Trait system C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-trait_upcasting `#![feature(trait_upcasting)]` finished-final-comment-period The final comment period is finished for this PR / Issue. S-tracking-design-concerns Status: There are blocking design concerns. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@alexreg
Copy link
Contributor

alexreg commented Oct 31, 2019

This is a tracking issue for RFC3324. Corresponding MCP is here.
The feature gate for the issue is #![feature(trait_upcasting)].

STATUS UPDATE

We are in the process of stabilizing.

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Previous discussions

Unresolved Questions

  • Currently CoerceUnsized trait cannot express this case: a smart pointers that wrap a raw pointer and don't guarantee via a custom invariant that it is valid. Maybe a separate CoerceUnsizedUnsafe trait is needed. (see Trait upcasting coercion (part4) #88010 (comment)).
  • Should we make upcasting opt-in in some form to limit vtable size by default? The current inclination of the lang-team is "no", but it would be useful to gather data on how much supporting upcasting contributors to overall binary size.
  • Before stabilizing it we should check that libs-api is ok with upcasting for all dyn-allowed traits in the library, since those we can't change. (addressed by Tracking issue for dyn upcasting coercion #65991 (comment))
  • Should we add an opt-out mechanism, and extend library stabilization checklist with "do we want to opt-out for now"?

Implementation history

@alexreg alexreg added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Oct 31, 2019
@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Oct 31, 2019
@Centril Centril added the F-trait_upcasting `#![feature(trait_upcasting)]` label Oct 31, 2019
@alexreg alexreg added A-trait-system Area: Trait system T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 31, 2019
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 16, 2021
Refactor vtable codegen

This refactor the codegen of vtables of miri interpreter, llvm, cranelift codegen backends.

This is preparation for the implementation of trait upcasting feature. cc rust-lang#65991

Note that aside from code reorganization, there's an internal behavior change here that now InstanceDef::Virtual's index now include the three metadata slots, and now the first method is with index 3.

cc  `@RalfJung` `@bjorn3`
bjorn3 pushed a commit to bjorn3/rust that referenced this issue Jul 7, 2021
Refactor vtable codegen

This refactor the codegen of vtables of miri interpreter, llvm, cranelift codegen backends.

This is preparation for the implementation of trait upcasting feature. cc rust-lang#65991

Note that aside from code reorganization, there's an internal behavior change here that now InstanceDef::Virtual's index now include the three metadata slots, and now the first method is with index 3.

cc  `@RalfJung` `@bjorn3`
@crlf0710 crlf0710 changed the title Tracking issue for trait upcasting Tracking issue for trait upcasting coercion Jul 8, 2021
@nikomatsakis
Copy link
Contributor

This is currently blocked on resolving the safety conditions for upcasting. The following updated document describes my current thinking, looking for feedback:

https://rust-lang.github.io/dyn-upcasting-coercion-initiative/design-discussions/upcast-safety-2.html

@dimpolo
Copy link
Contributor

dimpolo commented Feb 4, 2022

I was playing around with this and discovered that the feature gate (and its incomplete_features warning) can be bypassed using #![feature(unsize)] instead.

This will upcast a &dyn Bar where Foo is a supertrait of Bar.

fn upcast<Dyn: ?Sized + Unsize<dyn Foo>>(bar: &Dyn) -> &dyn Foo { bar }

Playground

Would this mean the unsize feature should be incomplete as well?

@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Feb 11, 2022

Came to comment the same observation that @dimpolo has made: I think the impl of Unsize<dyn '_ + Super> for dyn '_ + Sub ought, itself, to be feature-gated by trait_upcasting.

@nikomatsakis
Copy link
Contributor

That makes sense, although really I just want to stabilize this.

@joshtriplett joshtriplett added S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Jun 22, 2022
@Mark-Simulacrum
Copy link
Member

cc #89460, which is tracking a future-compat lint that seems related to this feature.

@crlf0710 crlf0710 changed the title Tracking issue for trait upcasting coercion Tracking issue for dyn upcasting coercion Jul 28, 2022
ltratt added a commit to ltratt/yk that referenced this issue Mar 7, 2024
Unfortunately, trait upcasting is still
[incomplete/unstable](rust-lang/rust#65991).
Forcing every implementation of `CompiledTrace` to implement `as_any` is
a manual way of allow upcasting.

Counter-intuitively, we use the upcast to `Any` to allow us to downcast
to a concrete implementation of `CompiledTrace`. `depot.rs` can now call
`downcast::<LLVMCompiledTrace>` to get at the `LLVMCompiledTrace`.

This will then allow us to push methods out of `CompiledTrace` and into
`LLVMCompiledTrace`.
ltratt added a commit to ltratt/yk that referenced this issue Mar 7, 2024
Unfortunately, trait upcasting is still
[incomplete/unstable](rust-lang/rust#65991).
Forcing every implementation of `CompiledTrace` to implement `as_any` is
a manual way of allow upcasting.

Counter-intuitively, we use the upcast to `Any` to allow us to downcast
to a concrete implementation of `CompiledTrace`. `depot.rs` can now call
`downcast::<LLVMCompiledTrace>` to get at the `LLVMCompiledTrace`.

This will then allow us to push methods out of `CompiledTrace` and into
`LLVMCompiledTrace`.
@PoignardAzur
Copy link
Contributor

Can we have a status update with regards to the reverted stabilization? The current status update says "We are in the process of stabilizing" since January, with no mention of the recent soundness issues.

@WaffleLapkin
Copy link
Member

The stabilization is currently blocked on the fix of the unsoundness bug tracked in #120222. The fix is in #120248, but needs a bit more work. I've been occupied by the never type stuff which is urgent because some of the changes are edition based. I hope I'll have time soon to finish #120248.

This is also blocked on rust reference PR, which someone needs to take over: rust-lang/reference#1259 (comment).

mark-beck added a commit to cep-sose2024/rust-crypto-netwatch that referenced this issue May 27, 2024
When using Box<dyn ProviderConfig>, you would have to upcast the dyn ProviderConfig to dyn Any. This feature is currently experimental (rust-lang/rust#65991). As ProviderConfig is always going to be turned into an Any, it has no real purpose anyways.
BREAKING
ngussek pushed a commit to nmshd/rust-crypto that referenced this issue Jun 4, 2024
* feat: added internal work to fork

* fix: added missing changes

* feat: implemented config

* fix: added lib cratetype

* fix: AndroidConfig has to be public

* fix: missing match in instance

* fix: don't load java vm in initialize_module, as it will be called before the config was passed

* feat: added default logging implementation for android

* feat: implemented symmetric algorithm for encryption, *not tested

* fix: change traits to use Box<dyn Any>

When using Box<dyn ProviderConfig>, you would have to upcast the dyn ProviderConfig to dyn Any. This feature is currently experimental (rust-lang/rust#65991). As ProviderConfig is always going to be turned into an Any, it has no real purpose anyways.
BREAKING

* test(mod.rs): adding the tests again

* feat: Add set_key_size and set_algorithm_parameter_spec methods to Builder

* fix: Update set_algorithm_parameter_spec method in Builder

* feat: docs for set_block_modes, set_key_size and set_algorithm_parameter_spec

* fix: builder methods using wrong parameters

* feat: wrapped KeyGenerator for symmetric encryption

* feat: added symetrical encryption for android

* fix: added more paddings

* fix: readded common tests

* fix: readded other tests

* fix: mod.rs

* fix: wrang hash names for signature

---------

Co-authored-by: markoisus <[email protected]>
Co-authored-by: Dexter <[email protected]>
Co-authored-by: Baibars <[email protected]>
@Kish29
Copy link

Kish29 commented Jul 16, 2024

It looks like the blocking bug #120222 have been fixed out, and what's next for this stabilization?

@WaffleLapkin
Copy link
Member

@Kish29 the next steps are changing the reference (cc rust-lang/reference#1259) and doing a new stabilization PR :)

@noclue

This comment was marked as off-topic.

Zalathar added a commit to Zalathar/rust that referenced this issue Sep 15, 2024
unstable-book: `trait_upcasting` example should not have `#![allow(incomplete_features)]`

Tracking issue: rust-lang#65991

`trait_upcasting` is not currently an incomplete feature; therefore examples of its use do not require `#![allow(incomplete_features)]`.
Zalathar added a commit to Zalathar/rust that referenced this issue Sep 15, 2024
unstable-book: `trait_upcasting` example should not have `#![allow(incomplete_features)]`

Tracking issue: rust-lang#65991

`trait_upcasting` is not currently an incomplete feature; therefore examples of its use do not require `#![allow(incomplete_features)]`.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 15, 2024
Rollup merge of rust-lang#130370 - kpreid:patch-2, r=compiler-errors

unstable-book: `trait_upcasting` example should not have `#![allow(incomplete_features)]`

Tracking issue: rust-lang#65991

`trait_upcasting` is not currently an incomplete feature; therefore examples of its use do not require `#![allow(incomplete_features)]`.
jieyouxu added a commit to jieyouxu/rust that referenced this issue Oct 18, 2024
…pkin

Never emit `vptr` for empty/auto traits

Emiting `vptr`s for empty/auto traits is unnecessary (rust-lang#114942) and causes unsoundness in `trait_upcasting` (rust-lang#131813). This PR should ensure that we never emit vtables for such traits. See the linked issues for more details.

I'm not sure if I can add tests for the vtable layout. So this PR only adds tests for the soundness hole (i.e., the segmentation fault will disappear after this PR).

Fixes rust-lang#114942
Fixes rust-lang#131813

Cc rust-lang#65991 (tracking issue for `trait_upcasting`)

r? `@WaffleLapkin`  (per rust-lang#131813 (comment))
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 18, 2024
Rollup merge of rust-lang#131864 - lrh2000:upcast_reorder, r=WaffleLapkin

Never emit `vptr` for empty/auto traits

Emiting `vptr`s for empty/auto traits is unnecessary (rust-lang#114942) and causes unsoundness in `trait_upcasting` (rust-lang#131813). This PR should ensure that we never emit vtables for such traits. See the linked issues for more details.

I'm not sure if I can add tests for the vtable layout. So this PR only adds tests for the soundness hole (i.e., the segmentation fault will disappear after this PR).

Fixes rust-lang#114942
Fixes rust-lang#131813

Cc rust-lang#65991 (tracking issue for `trait_upcasting`)

r? `@WaffleLapkin`  (per rust-lang#131813 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-system Area: Trait system C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-trait_upcasting `#![feature(trait_upcasting)]` finished-final-comment-period The final comment period is finished for this PR / Issue. S-tracking-design-concerns Status: There are blocking design concerns. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet