Stabilize feature(trait_upcasting)

[WaffleLapkin]

This feature was "done" for a while now, I think it's finally time to stabilize it! (stabilization report pending)
cc reference PR: rust-lang/reference#1622, tracking issue: #65991

r? compiler-errors

[rustbot]

Some changes occurred in tests/ui/sanitizer

cc @rust-lang/project-exploit-mitigations, @rcvalle

[compiler-errors]

Could you please provide some links that specify what has been done since the last stabilization? Ideally both the issues and the resulting fix PRs.

Ty for doing this tho :)

Gna nominate for T-lang but this should be an easy decision.

[compiler-errors]

And maybe link to the last stabilization too

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

[WaffleLapkin]

Stabilization report

The core of the feature hasn't changed since the last attempt to stabilize it.

But as a reminder this feature allows "upcasting" dyn Sub to dyn Super, such as in this example:

trait Sub: Super {}
trait Super {}

fn upcast(x: &dyn Sub) -> &dyn Super {
    x // implicit coercion
}

This is a long wanted feature that people used workarounds for for a long while now.

One possible downside is that this forces us into including more data in the vtables. However, our measurements show that the overhead is mostly negligible. Also note that we ate already including this overhead on stable for countless versions and no one ever complained.

Another possible downside is that this feature this allows upcasting of raw trait pointers in safe code. That puts constraints on their library invariant (safety invariant) -- specifically, even *const dyn Trait must always come with a vtable that is valid for Trait. This was also discussed during previous stabilization attempts, but still, something to keep in mind.

I believe that the feature is well tested and is ready for stabilization.

Previous stabilization attempt problems

After the last attempt to stabilize this feature @steffahn found two unsound interactions between trait upcasting and pointer casting (one of which also required feature(arbitrary_self_types)): #120222 and #120217. This caused a revert of the stabilization PR.

Both issues were since fixed in #120248 by adding additional checks for casting pointers, to uphold the library invariant of pointers to trait objects which is needed for this feature.

No new issues were found since.

[WaffleLapkin]

(@RalfJung I'm pretty sure I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination implies nomination and you don't need I-lang-nominated Nominated for discussion during a lang team meeting. )

[RalfJung]

@traviscross told me the opposite -- it still needs to be nominated with the regular label.

The label description should probably clarify or else people will inevitably keep getting this wrong.

[RalfJung]

The only possible downside

That's a very strong statement.^^

I'd say there is at least one other downside, or point worth mentioning: this allows upcasting of raw trait pointers in safe code. That puts constraints on their library invariant (safety invariant) -- specifically, even *const dyn Trait must always come with a vtable that is valid for Trait.

I'm fine with that, and AFAIK @rust-lang/types agrees, but it is a choice we are making here that should be called out explicitly. @rust-lang/opsem is still discussing what to do with the language invariant for raw trait pointers; having that be different from the library invariant is likely surprising but OTOH we still might want a weaker invariant here.

[WaffleLapkin]

@RalfJung I did call this out, but maybe not explicitly enough ("the only" slipped from an earlier draft). I've updated the wording to more clearly highlight this.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[WaffleLapkin]

Current status (so I don't forget): this is waiting on chalk/r-a properly handling upcasting and on T-lang to make a decision about the lint.

[bors]

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

[theemathas]

Trait upcasting has a soundness/miscompilation bug. Oops. #135315

[steffahn]

I kind-of hate to once again be the one finding new issues at the last minute that could be further delaying this feature, sowwy 🫠

but soundness is soundness! 😇

[nikomatsakis]

Better now than later. =)

…

On Thu, Jan 9, 2025, at 10:51 PM, Frank Steffahn wrote: I kind-of hate to **once again** be the one finding new issues at the last minute that could be further delaying this feature, sowwy 🫠 *but soundness is soundness!* 😇 — Reply to this email directly, view it on GitHub <#134367 (comment)>, or unsubscribe </~https://github.com/notifications/unsubscribe-auth/AABF4ZTDY6O4L5GMLWW6EIT2J47TZAVCNFSM6AAAAABTVG5QT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOBRGY4DQMRTGA>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[WaffleLapkin]

But on a more serious note, thanks for finding the bugs (so we can fix them!). And also, it's nice to know about them before merging stabilization, so we don't have to revert anything. <3 @steffahn

[WaffleLapkin]

Status update: I've implemented the feature in chalk/r-a.

This is now waiting on t-lang decision about the lint and some soundness fixes to the issues @steffahn found.

[WaffleLapkin]

(sorry for the ping, I accidentally deleted the whole rustc dev guide; I reverted this, so yeah...)