-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
Hierarchy of Sized traits #3729
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: León Orell Valerian Liehr <[email protected]>
Wow, a funny (but good!) feeling to think that this might finally get resolved after a decade. :)
I think
Also, I agree with @RalfJung than the After we do that, we we can debate syntax. In particular, the "Why have Pointee?" is a bit misleading in that semantically there currently is indeed no reason, it is just there for syntactic purposes. However, if we reuse At the point I still think it is good education to talk about the empty trait set as:
Oh and a final thing on semantics, I might also also present the back compat / editions story this way:
In particular, the existing old syntax maps to the new semantics via the old semantics; we can cleanly factor that into those two steps, and I think this brings conceptual clarity. |
imo having an associated type on practically every type will be annoying, so I think it might be useful to require writing |
the trait's unstable, if it's just a matter of bikeshedding we can rename the type |
I've added this as one of the unresolved questions.
I've rewritten this section, it should have been updated earlier with the addition of the alternatives which keep the
I haven't made these changes at the moment - I've had good feedback about the explanations in the RFC and would prefer not to make major changes to that until the proposal itself needs changing. |
I've updated the previous summary comment so it is still accurate. |
I've pushed one small change with a reference to the now-upstream const traits RFC in #3762. |
Linux has at its syscall boundary There is even more exotic case: Note: manpage says that size of last field is 256. This is a lie. In fact, this is DST, and it can be both bigger and smaller than 256. Also, the string in the last field is always proper null-terminated C string, so we have two different ways of determining struct size: by calling
Not supporting both features will lead to horrible hacks, such as this: https://github.com/rust-lang/rust/blob/c1132470a6986b12503e8000e322e9164c1f03ac/library/std/src/sys/pal/unix/fs.rs#L730-L768 (this is from standard library!) And again: to support |
In the latest version, this hack is mostly gone. What remains is not so bad: https://github.com/rust-lang/rust/blob/58d6301cad7ac20407e708cd9d1ec499d1804015/library/std/src/sys/pal/unix/fs.rs#L726-L753 IMO it is not worth contorting the language to natively express esoteric types such as those, as long as we have some way of interfacing with C code using such types (and we do). |
Another summary comment! Here are the previous update summaries copied below so you don't need to uncollapse GitHub comments to find it: 2024-11-25Summary comment: #3729 (comment) For those following along or catching up, these are the notable the changes to the RFC since this was posted:
And these are all the other smaller changes that don't materially impact what is being proposed:
At the moment, I prefer the following alternatives to the primary proposal of the RFC, and may re-write to incorporate these as the primary proposal:
2024-12-04Summary comment: #3729 (comment) These are the major changes since the last summary:
These are the minor changes since the last summary:
These are the alternatives described in the RFC that I think are worth consideration as the primary proposal:
We had a design meeting with the language team yesterday (Feb 5th) which led to the changes described below, a rough summary:
I've started an implementation of this - I've more-or-less finished implementing the non-const parts of the proposal, and will proceed with the const parts. It won't land upstream until const traits implementors are okay with it. These are links to the compare with previous versions of the document (switch to the "Files changed" tab and then the rich diff):
These are the major changes since the last summary:
These are the minor changes since the last summary: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the summary comments they're incredibly useful. I've re-read this and looked through the updates and this proposal feels closer, however I still have a few comments...
Positive trait bounds are growing on me though, I like that they stop us having to worry about what "the empty set of traits" means, however I'm not convinced that's possible, because of supertrait bounds (as described in a comment).
|
||
This property trivially falls out of the previous proposals in this RFC: | ||
runtime-sized types are `Sized` (but not `const Sized`) and therefore can | ||
implement `Clone` and `Copy`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this RFC is implicitly dependent on unsized locals here, the only way I can see the Clone or Copy traits supporting Sized
types is with it. Consider <svint8_t as Clone>::clone
that returns an svint8_t but that doesn't have a stack size known at compile time, so you need a bunch of the unsized locals machinery to support it right?
``` | ||
|
||
Once this interaction with constness was realised, this RFC's dependency on const | ||
traits was introduced. Due to const traits (at the time of writing this RFC) being |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like surprisingly weak motivation for all the const stuff that's making this hierarchy complicated. I wonder if we can replace Clone
with:
trait Clone {
fn clone(&self) -> Self;
fn clone_from(&mut self, other: &Self) where Self: Sized { ... }
}
and replace Clone bounds with Clone + Sized
, admittedly this is more edition shenanigans that feels a bit dirty (and I haven't completely thought it through).
I do agree that having const trait stuff would give you the ability to do some stuff: size_of
on non-const Sized types, and have arrays&slices of non-const Sized types (although this would require some complier work). I'm just not clear how useful those are.
which is now equivalent to a `MetaSized` bound in non-`const fn`s and | ||
`~const MetaSized` in `const fn`s and will be deprecated in the next edition. | ||
|
||
Traits now have an implicit default bound on `Self` of `const MetaSized`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still very nervous about this, I believe this is done solely for backwards compatibility reasons but I want to make sure it's clear this is sacrificing forwards compatibility.
If traits gain (debatably keep) an implicit const MetaSized
bound then:
- Everything keeps working in a nice way
- People writing libraries will just write
trait Foo {}
rather thantrait Foo: Pointee {}
unless they're specifically thinking about unsized types for some reason making them a second class citizen in the ecosystem (and relaxing the bund when asked is a breaking change) - If we ever add another layer below
Pointee
(eg externref) then no existing traits will work with them and it would be breaking to relax them. I think this means no std traits could ever support them.
If traits have no implicit bound (also debatably keep, depending on how you imagine this) then:
- You'd have to do this over an edition, and add in
: const MetaSized
everywhere so that default method implementations keep working. (removing that bound is safe for std, and dubious for crates). - You'd need to write
: Pointee
etc if you need to write default method impls, but I think that's a good thing. - This does force us to work out what you can do with "the empty set of traits", because that what these types have in default trait impls (with no supertraits) (this makes me less favourable towards positive bounds)
| `Pointee` | `T: Pointee` | `T: ?(const Sized)` | | ||
|
||
In other words, `?(const Sized)` fully opts out of all default bounds and then one has to | ||
explicitly opt back in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was straw-manned slightly in the design meeting, I disagree with the phrasing:
'I don't want to think of the "next most precise thing"'
and
"one level below" in the hierarchy
Neither proposal makes you think like that (ignoring the ?MetaSized
below), the ?Sized
variant requires you to write the positive bound always (and I'd argue that the T: Pointee
case should be written T: ?(const Sized) + Pointee
) so the only negative thinking you have to do is "do I need to disable the default const Sized
bound?".
However, I agree this discussion can be punted until stabilisation given it has essentially no impact on the semantics of what's going on.
P.S The idea of negative trait aliases (trait Foo = ?Sized;
) terrifies me, it feels like it'd be incredibly confusing at the use-site.
All of Rust's types are either sized, which implement the
Sized
trait and have a statically known size during compilation, or unsized, which do not implement theSized
trait and are assumed to have a size which can be computed at runtime. However, this dichotomy misses two categories of type - types whose size is unknown during compilation but is a runtime constant, and types whose size can never be known. Supporting the former is a prerequisite to stable scalable vector types and supporting the latter is a prerequisite to unblocking extern types. This RFC proposes a hierarchy ofSized
traits in order to be able to support these use cases.This RFC relies on experimental, yet-to-be-RFC'd const traits, so this is blocked on that. I haven't squashed any of the previous revisions but can do so if/when this is approved. Already discussed in the 2024-11-13 t-lang design meeting with feedback incorporated.
See this comment for the most recent summary of changes to this RFC since it was opened.
Rendered