-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Stabilize offset_of_slice
#139673
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
Stabilize offset_of_slice
#139673
Conversation
This comment has been minimized.
This comment has been minimized.
Not sure why you are assigning this to me, I have little to do with this feature and with these parts of the compiler. Someone from @rust-lang/types should have a look regarding this "statically known alignment" check.
r? compiler
Since you may have more context...
r? @WaffleLapkin
This should have a proper stabilization report written up for it. I think there's a template somewhere for some of the questions to ask--I'm on my phone so I cannot find it.
I can take a look at the HIR typeck implementation later too.
@RalfJung mostly because you wrote the tracking issue, but maybe someone else is better suited yea. @compiler-errors looked for a template but couldn't find one, so I wrote something informal. However, I'm happy to extend it! Let me take a look at some more other stabilization reports around. @WaffleLapkin agreed with the statically known alignment, though if we want to avoid that term we can keep it to just mentioning slices for now.
The stabilization template draft is at rust-lang/rustc-dev-guide#2219.
c2dc5a8 to
2fabbb6
Compare
Alright, I'm working on a proper stabilization report using that template, thanks Ralf!
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.
Could you please add a comment onto require_type_has_static_alignment that notes that changing the behavior will extend the functionality of offset_of and is insta-stable?
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
---- library/core/src/mem/mod.rs - mem::offset_of (line 1271) stdout ----
error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
--> library/core/src/mem/mod.rs:1285:12
|
17 | assert_eq!(mem::offset_of!(Struct<[u8]>, b), 1); // OK — the last field is a slice
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `[u8]`
= note: this error originates in the macro `mem::offset_of` (in Nightly builds, run with -Z macro-backtrace for more info)
ugh, I believe the CI failure is because it tries to compile the docs with an older compiler? like a #[cfg(bootstrap)] issue? could be wrong but I'm unsure how to fix that if that's the case.
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.
Duplicate this item, mark one as #[cfg(bootstrap)] and one as #[cfg(not(bootstrap))]. Give the #[cfg(bootstrap)] one dummy documentation like /// This item will not be documented so it doesn't get doc-tested in stage 0.
@rustbot author
Reminder, once the PR becomes ready for a review, use @rustbot ready.
@rustbot labels -I-lang-nominated
Please renominate when the stabilization report is available.
Tracking issue: #126151
This stabilizes using
offset_of!for dynamically sized fields (i.e. the last field of a DST) which are a slice or a wrapper around a slice. If the field is, for example,dyn Traitwe cannot know ifs offset, because the offset can depend on the alignment of the underlying type. But if the dynamically sized field is a slice, the alignment is constant and the offset of that field can be calculated.There did not seem to be any problems with the implementation of this feature, and no known issues. Discussed with @WaffleLapkin to move for stabilization. I updated the documentation for the
offset_of!().r? @RalfJung