Support upcasting of dyn Trait values

[nikomatsakis]

Summary

Enable upcasts from dyn Trait1 to dyn Trait2 if Trait1 is a subtrait of Trait2.

This RFC does not enable dyn (Trait1 + Trait2) for arbitrary traits. If Trait1 has multiple supertraits, you can upcast to any one of them, but not to all of them.

This RFC has already been implemented in the nightly compiled with the feature gate trait_upcasting.

Credit

@crlf0710 has driven the implementation work for this.

Rendered

[nikomatsakis]

I'm nominating this for the lang-team meeting. This feature has been under discussion for some time.

[afetisov]

I didn't see any mention of marker traits. I can have many marker traits as supertraits, and the algorithm as described in the RFC would produce a lot of spurious entries and vtables. For example, this

trait A {}
trait B {}
trait C {}
trait D: A + B + C {}

would produce, if I understand the algorithm correctly,

Vtable entries for `<S as D>`: [
    MetadataDropInPlace,
    MetadataSize,
    MetadataAlign,
    TraitVPtr(<S as B>),
    TraitVPtr(<S as C>),
]

Vtable entries for `<S as B>`: [
    MetadataDropInPlace,
    MetadataSize,
    MetadataAlign,
]

even though we could use a single vtable for all marker traits and their marker subtraits.

Also, I suppose the compiler knows that autotraits don't affect the vtables? And how would this feature interact with specialization?

[oxalica]

It's worth mentioning in "Drawbacks" that current impl has extra memory cost for traits with multiple supertraits, even though the user does not really use upcasting. In other words, it's not zero-cost. This can potentially increase cache pressure due to spread out of vtable ptrs.

[nikomatsakis]

@afetisov

Also, I suppose the compiler knows that autotraits don't affect the vtables?

Correct. I don't know how we treat traits like marker traits that have no methods at present, but certainly it would be possible to optimize those as well.

And how would this feature interact with specialization?

It doesn't in particular. Specialization would affect what happens when methods are called, but that's an orthogonal consideration.

[nikomatsakis]

@oxalica I added a mention of that to the downsides, thanks.

[nikomatsakis]

Discussing in the lang-team meeting. This proposal has been under discussion for some time, and is always something we expected to work, so even though the RFC was only recently opened, I am going to move to merge...

@rfcbot fcp merge

[rfcbot]

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged team members:

• @cramertj
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[kupiakos]

A third option: The vtable for Trait1 + Trait2 contains entries for the vtables for Trait1 and Trait2. It's an extra layer of indirection, but much smaller and doesn't explode as quickly, while keeping upcasts trivial.

[nikomatsakis]

That is what I meant by the first option, I think. You wind up needing each subset of the various traits involved -- well, maybe not EVERY subset, since the vtable for A + B might have A as a prefix.

I think this is what we will wind up doing, regardless.

[Aaron1011]

One drawback to the vtable layout design: The ordering of super traits (and where clauses) now has performance implications for both the defining crate and downstream crates. While we'll probably never stabilize the vtable layout we may want to tell users to put the "most commonly used" supertrait first when defining a trait.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[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.

[oxalica]

The extra cost that I mentioned in #3324 (comment) seems still not included?

It's worth mentioning in "Drawbacks" that current impl has extra memory cost for traits with multiple supertraits, even though the user does not really use upcasting.

I'm quite concern about this since embedded-system users may find their memory footprint unexpectedly grown without a single code change.
If we cannot solve this. I'd rather suggest disable upcasting for traits have multiple supertraits, and only add TraitVPtr when explicitly noted via something like #[upcastable]

[crlf0710]

@oxalica The vtable format has long been in its current format since Jun 2021 (1.56), and is considered an implementation detail. So there won't be any memory footprint grown with this RFC accepted. It's only the feature defined in this RFC will make use of this implementation detail to supply new operations.

Although embedded-system users usually won't really write such complex trait code, if they do and they cares, i'd suggest they request a thinner vtable with some attribute with a new RFC, so we can more aggressively tailor the actual layout of the vtable entries(like removing vacant entries), instead of the opposite.

[nbdd0121]

@oxalica The vtable format has long been in its current format since Jun 2021 (1.56), and is considered an implementation detail. So there won't be any memory footprint grown with this RFC accepted. It's only the feature defined in this RFC will make use of this implementation detail to supply new operations.

No, this is not just implementation detail. While dyn trait upcasting stabilisation itself does not change the vtable format, it commits us to a specific format where it'll no longer be backward compatible to remove TraitVPtr from the vtable since it'll remove the ability to upcast some traits.

Furthermore, in one sense, the 1.56 vtable format change could be considered a I-heavy regression because it introduces space penalty for everyone even though nobody can use trait upcasting on stable. IMO it's not a valid argument to say "the regression is already there" as a reason to commit to the regression.

Although embedded-system users usually won't really write such complex trait code, if they do and they cares, i'd suggest they request a thinner vtable with some attribute with a new RFC, so we can more aggressively tailor the actual layout of the vtable entries(like removing vacant entries), instead of the opposite.

Rust's zero-cost abstraction principle, if anything, suggests that any feature with cost should be opt-in instead of opt-out. Plus, opting out from upcasting will be much more difficult from opting-in. It's not backward compatible to opt a super trait relation out from upcasting, but it's backward compatible to opt into upcasting.

Removing vacant entries is not relevant; it is an optimisation that all Rust user will benefit.

[nikomatsakis]

Hi everyone! Thanks for the feedback. As discussed in recent @rust-lang/lang meetings, I've pushed text that (a) identifies the size impact of supporting upcasting as a downside, (b) discusses possible future ideas for how to improve it and (c) indicates why the RFC does not include "opt-in" at the trait level. However, I also added an unresolved question to re-evaluate that decision prior to stabilization.

I do have a request. It would be really helpful to have some data for this discussion. What would be ideal is some examples of the amount of extra data generated by these vtables for some particular Rust applications, and some other comparison (such as how this compares to the overall size of the binary, or to the impact of panic=abort).

[nikomatsakis]

Also, I'd appreciate it if people could review what I wrote for accuracy.

[nikomatsakis]

@afetisov Ah, I just saw your comment, that's helpful context. I'll try to summarize it in the RFC. I believe the compilation time blowup is likely related to trait resolution and independent from vtable layout.

[afetisov]

@nikomatsakis From the model described in the RFC, it doesn't make sense to me that vtable size would not grow exponentially. Could it be that LLVM is doing some deduplication?

[juntyr]

Suggested change

It would be very useful to support `dyn Trait1 + Trait2` for arbitrary sets of traits. Doing so would require us to decide how to describe the vtable for the combination of two traits. There is an intefaction between this feature and upcasting, because if we support upcasting, then we must be able to handle upcasting from some subtrait to some arbitrary combination of supertraits. For example a `&mut dyn Subtrait`...

It would be very useful to support `dyn Trait1 + Trait2` for arbitrary sets of traits. Doing so would require us to decide how to describe the vtable for the combination of two traits. There is an interaction between this feature and upcasting, because if we support upcasting, then we must be able to handle upcasting from some subtrait to some arbitrary combination of supertraits. For example a `&mut dyn Subtrait`...

[burdges]

I've not grasped why safe code should be able to cast raw pointers, like removing autotraits sure, but surely *const dyn T2 as *const dyn T1 could just be an unsafe operation with no real consequences? I'd expect the current formulation winds up fine, and likely the restrictions upon emitting a raw pointer with invalid vtable would make sense anyways, but why does safe code benefit from this cast?

[nbdd0121]

One motivating factor is that we currently have no unsafe coercions, and I don't think forbidding this cast in safe code worth the addition language complexity of introducing an entire new class of coercion.

[nikomatsakis]

@afetisov I'm not sure, I'd defer to @crlf0710 on that point. I've weakened the text in the RFC though accordingly.

[nikomatsakis]

Huzzah! The @rust-lang/lang team has decided to accept this RFC. Further comments can be placed on the tracking issue, rust-lang/rust#65991.

[nikomatsakis]

I've not grasped why safe code should be able to cast raw pointers, like removing autotraits sure, but surely *const dyn T2 as *const dyn T1 could just be an unsafe operation with no real consequences? I'd expect the current formulation winds up fine, and likely the restrictions upon emitting a raw pointer with invalid vtable would make sense anyways, but why does safe code benefit from this cast?

@burdges, to me the main considerations are:

• compatible with method dispatch in the future
• simpler casting model, and consistent with the fact that *dyn Foo + Send is currently upcastable to *dyn Foo

In contrast, the downside seems pretty minor. There's not a very strong reason to create *const dyn Foo with invalid metadata, and the restriction that such a pointer should not be released to "the wild" seems quite reasonable to me. If there are public-facing APIs that may be returning garbage *const dyn values, I think it would be good to return an Option.

There are a number of write-ups on the design repository as well. This one seems to cover a lot of the tradeoffs.

[burdges]

We've TraitObject for anytime we'd actually fiddle with the metadata pointer I guess.

[joshka]

Typos Food trait should be Eat

[joshka]

Suggested change

	trait Eat { fn eat(&mut self); }
	trait Grab { fn grab(&mut self); }
	trait Sandwich: Food + Grab { }
	trait Eat { fn eat(&mut self); }
	trait Grab { fn grab(&mut self); }
	trait Sandwich: Eat + Grab { }

[joshka]

Suggested change

	let f: &mut dyn Food = s; // coercion
	let f: &mut dyn Eat = s; // coercion

[joshka]

Suggested change

	Note that you cannot, currently, upcast to *multiple* supertraits. That is, an `&mut dyn Sandwich` can be coerced to a `&mut dyn Food` or a `&mut dyn Grab`, but `&mut (dyn Food + Grab)` is not yet a legal type (you cannot combine two arbitrary traits) and this coercion is not possible.
	Note that you cannot, currently, upcast to *multiple* supertraits. That is, an `&mut dyn Sandwich` can be coerced to a `&mut dyn Eat` or a `&mut dyn Grab`, but `&mut (dyn Eat + Grab)` is not yet a legal type (you cannot combine two arbitrary traits) and this coercion is not possible.