-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Update a bunch of library types for MCP807 #135236
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
Conversation
This comment has been minimized.
This comment has been minimized.
9afba99 to
32606ca
Compare
This comment has been minimized.
This comment has been minimized.
32606ca to
91c7fde
Compare
This comment has been minimized.
This comment has been minimized.
91c7fde to
1df19d1
Compare
This comment has been minimized.
This comment has been minimized.
| #[derive(Clone, Copy)] | ||
| #[repr(transparent)] | ||
| #[rustc_layout_scalar_valid_range_start(1)] | ||
| #[rustc_nonnull_optimization_guaranteed] |
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 didn't see this attribute in the new code for the non zero integers
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.
Good catch! Thank you.
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.
Oh, actually, I think it's fine. The inner ones don't need it because they're not exactly the guaranteed ones, rather the NonZero wrapper struct has it:
rust/library/core/src/num/nonzero.rs
Lines 122 to 126 in 62bf38f
| #[stable(feature = "generic_nonzero", since = "1.79.0")] | |
| #[repr(transparent)] | |
| #[rustc_nonnull_optimization_guaranteed] | |
| #[rustc_diagnostic_item = "NonZero"] | |
| pub struct NonZero<T: ZeroablePrimitive>(T::NonZeroInner); |
1df19d1 to
6eb893e
Compare
This comment has been minimized.
This comment has been minimized.
6eb893e to
f10ae9d
Compare
This comment has been minimized.
This comment has been minimized.
This greatly reduces the number of places that actually use the `rustc_layout_scalar_valid_range_*` attributes down to just 3: ``` library/core\src\ptr\non_null.rs 68:#[rustc_layout_scalar_valid_range_start(1)] library/core\src\num\niche_types.rs 19: #[rustc_layout_scalar_valid_range_start($low)] 20: #[rustc_layout_scalar_valid_range_end($high)] ``` Everything else -- PAL Nanoseconds, alloc's `Cap`, niched FDs, etc -- all just wrap those `niche_types` types.
f10ae9d to
6f2a783
Compare
|
Yay, finally got through CI 🎉 Hmm, since PAL changes, how about @rustbot ready |
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 looks great! And thanks to juntyr for reviewing too.
| /// 100% perma-unstable | ||
| #[doc(hidden)] | ||
| pub mod niche_types; |
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.
It's a shame there isn't a way to say "this is pub only for alloc/std". But I guess having it be unstable is good enough.
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.
Yeah, there's a bunch of issue = "none" stuff so that alloc and core can both use it :/
library/core/src/num/niche_types.rs
Outdated
| impl $name { | ||
| #[inline] | ||
| pub const unsafe fn new_unchecked(val: $int) -> Self { | ||
| // SAFETY: same precondition |
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.
It's kinda weird to write "same precondition" here as it isn't entirely clear what "same" is referring to. In any case, I think a bit of repetition here would be fine. Especially if things get moved around in the future.
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.
Agreed, especially now that it's used cross-crates. (An earlier version of this it was pub(super) and I didn't care as much.) Put proper /// docs and SAFETY on it.
|
Thanks! @bors r+ |
This greatly reduces the number of places that actually use the
rustc_layout_scalar_valid_range_*attributes down to just 3:Everything else -- PAL Nanoseconds, alloc's
Cap, niched FDs, etc -- all just wrap thoseniche_typestypes.r? ghost