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.

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

Jump to bottom

Allow codegen to unsize dyn* to dyn #106532

Merged
merged 1 commit into from
Jan 12, 2023
Merged

Allow codegen to unsize dyn* to dyn #106532

merged 1 commit into from
Jan 12, 2023

Conversation

compiler-errors
Copy link
Member

dyn* Trait is just another type that implements Trait, so we should be able to unsize &dyn* Trait into &dyn Trait perfectly fine, same for Box and other unsizeable types.

Fixes #106488

@rustbot
Copy link
Collaborator

rustbot commented Jan 6, 2023

r? @nagisa

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 6, 2023
@nagisa
Copy link
Member

nagisa commented Jan 9, 2023

r? rust-lang/compiler

@rustbot rustbot assigned jackh726 and unassigned nagisa Jan 9, 2023
@jackh726
Copy link
Member

jackh726 commented Jan 9, 2023

@bors r+

@compiler-errors
Copy link
Member Author

@bors r=jackh726

@bors
Copy link
Contributor

bors commented Jan 9, 2023

📌 Commit d4c4c4354adfe563e7ee00c907c2f029655f2ecf has been approved by jackh726

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 9, 2023
bjorn3
bjorn3 reviewed Jan 9, 2023
View reviewed changes
compiler/rustc_codegen_ssa/src/base.rs
) => {
assert_eq!(src_dyn_kind, target_dyn_kind);

) if src_dyn_kind == target_dyn_kind => {
Copy link
Member

Choose a reason for hiding this comment

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

Note to self: same change needs to be made to cg_clif

Copy link
Member

Choose a reason for hiding this comment

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

And also in Miri.

@compiler-errors
Copy link
Member Author

@bors r- wait, this needs to be a build-pass test

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 9, 2023
@compiler-errors
Copy link
Member Author

@bors r=jackh726

@bors
Copy link
Contributor

bors commented Jan 9, 2023

📌 Commit 170e942 has been approved by jackh726

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 9, 2023
JohnTitor pushed a commit to JohnTitor/rust that referenced this pull request Jan 10, 2023
Rollup merge of rust-lang#106532 - compiler-errors:dyn-star-to-dyn, r…
c86b7dd 
…=jackh726

Allow codegen to unsize `dyn*` to `dyn`

`dyn* Trait` is just another type that implements `Trait`, so we should be able to unsize `&dyn* Trait` into `&dyn Trait` perfectly fine, same for `Box` and other unsizeable types.

Fixes rust-lang#106488
@JohnTitor JohnTitor mentioned this pull request Jan 10, 2023
Closed
@crlf0710 crlf0710 added the F-dyn_star `#![feature(dyn_star)]` label Jan 11, 2023
@Noratrieb
Copy link
Member

Tests moved from src/tests to the tests directory in the root, can you rebase and move them?
@bors r-
@rustbot author

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 11, 2023
@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 11, 2023
@compiler-errors
Copy link
Member Author

@bors r=jackh726

@bors
Copy link
Contributor

bors commented Jan 11, 2023

📌 Commit 70a8d8d has been approved by jackh726

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 11, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 12, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 12, 2023
@bors
Auto merge of rust-lang#106757 - matthiaskrgr:rollup-9j8830g, r=matth…
2b8590e 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#106167 (Fix invalid syntax and incomplete suggestion in impl Trait parameter type suggestions for E0311)
 - rust-lang#106309 (Prefer non-`[type error]` candidates during selection)
 - rust-lang#106532 (Allow codegen to unsize `dyn*` to `dyn`)
 - rust-lang#106596 (Hide more of long types in E0271)
 - rust-lang#106638 (std tests: use __OsLocalKeyInner from realstd)
 - rust-lang#106676 (Test that we cannot use trait impl methods arguments as defining uses)
 - rust-lang#106702 (Conserve cause of `ImplDerivedObligation` in E0599)
 - rust-lang#106732 (rustc_llvm: replace llvm::makeArrayRef with ArrayRef constructors.)
 - rust-lang#106733 (Revert "warn newer available version of the x tool")
 - rust-lang#106748 (Clean up `OnUnimplementedFormatString::verify`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e664cb2 into rust-lang:master Jan 12, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 12, 2023
@RalfJung RalfJung mentioned this pull request Feb 6, 2023
Merged
bjorn3 added a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Feb 19, 2023
@bjorn3
Allow codegen to unsize dyn* to dyn
9e6dfba 
cg_clif counterpart to rust-lang/rust#106532
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 21, 2023
@bors
Auto merge of rust-lang#107728 - RalfJung:miri-dyn-star, r=RalfJung,o…
f715e43 
…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 pull request Feb 21, 2023
@bors
Auto merge of #107728 - RalfJung:miri-dyn-star, r=RalfJung,oli-obk
1232148 
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
@compiler-errors compiler-errors deleted the dyn-star-to-dyn branch August 11, 2023 19:59
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 20, 2024
@bors
Auto merge of #107728 - RalfJung:miri-dyn-star, r=RalfJung,oli-obk
c2dd4d8 
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 pull request Apr 27, 2024
@bors
Auto merge of #107728 - RalfJung:miri-dyn-star, r=RalfJung,oli-obk
c834084 
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
Reviewers

@bjorn3 bjorn3 bjorn3 left review comments

@RalfJung RalfJung RalfJung left review comments

Assignees

@jackh726 jackh726

Labels
F-dyn_star `#![feature(dyn_star)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.68.0
Development

Successfully merging this pull request may close these issues.

Box<dyn* Trait> to Box<dyn Trait> ICE
9 participants
@compiler-errors @rustbot @nagisa @jackh726 @bors @Noratrieb @RalfJung @bjorn3 @crlf0710