Stabilize feature(trait_upcasting)

[WaffleLapkin]

This feature was "done" for a while now, I think it's finally time to stabilize it! Stabilization report: #134367 (comment).
cc reference PR: rust-lang/reference#1622.

Closes #65991 (tracking issue), closes #89460 (the lint is no longer future incompat).

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.

After this comment @steffahn found another soundness issue: #135315, which was then promptly fixed by @compiler-errors in #135318.

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.

[gabrielecastellano]

Hello! As this feature was initially stabilized and merged more than 1 year ago #118133, I am a bit confused about the workflow of rust toolchains. I was wondering if someone could explain how it works, i mean why this ended up to be tracked for milestone 1.86 instead, of e.g., 1.76 one year ago. Thanks!

[Kobzol]

Because the initial stabilization was later reverted: #120233

[gabrielecastellano]

Because the initial stabilization was later reverted: #120233

Completely missed that. Now it is very clear :)