Make `dyn` casts into a coercion, allow `dyn` upcasting#101832

compiler-errors · 2022-09-14T23:43:48Z

I know that dyn* is likely not going to be a feature exposed to surface Rust, but this makes it slightly more ergonomic to write tests for these types anyways. ... and this was just fun to implement anyways.

Make dyn* into a coercion instead of a cast
Enable dyn* upcasting since we basically get it for free
Simplify some of the cast checking code since we're using the coercion path now

r? @eholk but feel free to reassign
cc @nikomatsakis and @tmandry who might care about making dyn* casts into a coercion

rustbot · 2022-09-14T23:43:51Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

eholk

Looks good to me. I added a few questions and suggestions, but feel free to accept the suggestions or not and r=me.

compiler/rustc_typeck/src/check/coercion.rs

src/test/ui/dyn-star/const.rs

compiler/rustc_codegen_ssa/src/base.rs

bors · 2022-09-15T21:56:50Z

☔ The latest upstream changes (presumably #101858) made this pull request unmergeable. Please resolve the merge conflicts.

compiler/rustc_typeck/src/check/coercion.rs

compiler-errors · 2022-10-14T04:55:43Z

@eholk had to move things around so would appreciate another review, addressed the nits + added the test you asked for.

@rustbot ready

… gate

bjorn3 · 2022-10-14T11:05:01Z

compiler/rustc_codegen_ssa/src/base.rs

    }
 }

+// Returns the vtable pointer type of a `dyn` or `dyn*` type


This will need to be implemented in cg_clif too.

eholk · 2022-10-14T15:55:29Z

Looks good to me, thanks!

@bors r+

bors · 2022-10-14T15:55:32Z

📌 Commit 8c7e836 has been approved by eholk

It is now in the queue for this repository.

…holk Make `dyn*` casts into a coercion, allow `dyn*` upcasting I know that `dyn*` is likely not going to be a feature exposed to surface Rust, but this makes it slightly more ergonomic to write tests for these types anyways. ... and this was just fun to implement anyways. 1. Make `dyn*` into a coercion instead of a cast 2. Enable `dyn*` upcasting since we basically get it for free 3. Simplify some of the cast checking code since we're using the coercion path now r? `@eholk` but feel free to reassign cc `@nikomatsakis` and `@tmandry` who might care about making `dyn*` casts into a coercion

bors · 2022-10-15T07:36:41Z

⌛ Testing commit 8c7e836 with merge b15e2c1...

bors · 2022-10-15T10:17:08Z

☀️ Test successful - checks-actions
Approved by: eholk
Pushing b15e2c1 to master...

rust-timer · 2022-10-15T11:36:24Z

Finished benchmarking commit (b15e2c1): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.3%	[1.3%, 1.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	2.5%	[1.8%, 3.1%]	2
Regressions ❌ (secondary)	2.9%	[2.9%, 2.9%]	1
Improvements ✅ (primary)	-4.6%	[-4.6%, -4.6%]	1
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	0.1%	[-4.6%, 3.1%]	3

the arithmetic mean of the percent change ↩ ↩²
number of relevant changes ↩ ↩²

bjorn3 · 2022-10-23T13:16:28Z

Implemented in cg_clif as of bjorn3/rustc_codegen_cranelift@342bac9.

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 14, 2022

rust-highfive assigned eholk Sep 14, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 14, 2022

eholk approved these changes Sep 15, 2022

View reviewed changes

eholk reviewed Sep 15, 2022

View reviewed changes

compiler/rustc_typeck/src/check/coercion.rs Outdated Show resolved Hide resolved

crlf0710 added the F-dyn_star `#![feature(dyn_star)]` label Sep 17, 2022

compiler-errors 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 26, 2022

compiler-errors added 2 commits October 14, 2022 04:27

Make dyn* cast into a coercion

76386bd

Allow dyn* upcasting

feb4244

compiler-errors force-pushed the dyn-star-plus branch from aed5815 to 86ce082 Compare October 14, 2022 04:55

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 14, 2022

compiler-errors added 2 commits October 14, 2022 05:47

Remove CastCheckResult since it's unused

0cb217d

Address nits, add test for implicit dyn-star coercion without feature…

8c7e836

… gate

compiler-errors force-pushed the dyn-star-plus branch from 86ce082 to 8c7e836 Compare October 14, 2022 05:47

bjorn3 reviewed Oct 14, 2022

View reviewed changes

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 Oct 14, 2022

Dylan-DPC mentioned this pull request Oct 15, 2022

Rollup of 6 pull requests #103077

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 15, 2022

bors merged commit b15e2c1 into rust-lang:master Oct 15, 2022

rustbot added this to the 1.66.0 milestone Oct 15, 2022

compiler-errors deleted the dyn-star-plus branch November 2, 2022 02:57

compiler-errors mentioned this pull request Nov 24, 2022

dyn* does not support trait upcasting #104800

Closed

jruderman mentioned this pull request Nov 30, 2022

ICE with dyn_star: should be handled by try_coerce #105097

Closed

jruderman mentioned this pull request Dec 16, 2022

dyn * ICE when used in constant and static #105777

Closed

This was referenced Jan 7, 2024

ICE with dyn* and method that takes self by value #119694

Closed

ICE with dyn* when trait is implemented for union #119695

Closed

matthiaskrgr mentioned this pull request Oct 25, 2024

ICE: should be handled by coerce #132127

Closed

Uh oh!

Conversation

compiler-errors commented Sep 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 14, 2022

Uh oh!

eholk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bors commented Sep 15, 2022

Uh oh!

Uh oh!

compiler-errors commented Oct 14, 2022

Uh oh!

bjorn3 Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

eholk commented Oct 14, 2022

Uh oh!

bors commented Oct 14, 2022

Uh oh!

bors commented Oct 15, 2022

Uh oh!

bors commented Oct 15, 2022

Uh oh!

rust-timer commented Oct 15, 2022

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

Uh oh!

bjorn3 commented Oct 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

compiler-errors commented Sep 14, 2022 •

edited

Loading