Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Implemented MIN and MAX constants for the non-zero integer types.#89065 #89077

Closed
wants to merge 14 commits into from
Closed

Implemented MIN and MAX constants for the non-zero integer types.#89065 #89077

Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
0156ef1
Max working.
mjclements Sep 18, 2021
b5c1fe1
nonzero min and nonzero max working.
mjclements Sep 18, 2021
84d88f2
Fixed some formatting issues.
mjclements Sep 18, 2021
b274022
Made a macro name consistent.
mjclements Sep 18, 2021
0f84d19
Fixed variable name _properly_
mjclements Sep 18, 2021
279f0bf
Fixed build errors.
mjclements Sep 18, 2021
591b89d
Addressing PR Comment.
mjclements Sep 20, 2021
56ce3a2
Removed {unsafe} since we are in constant context.
mjclements Sep 20, 2021
3cbfdec
Split macro, added documentation.
mjclements Sep 22, 2021
7a718e1
Trailing ws.
mjclements Sep 23, 2021
87d92f9
Tidy cleanup comment, trailing WS
mjclements Sep 23, 2021
2fecb01
Update library/core/src/num/nonzero.rs
mjclements Sep 24, 2021
5b48c54
Cleaned up some documentation.
mjclements Sep 25, 2021
5ac53ef
Fixes missing parenthesis in doc test.
mjclements Sep 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions library/core/src/num/nonzero.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,3 +895,39 @@ macro_rules! nonzero_unsigned_is_power_of_two {
}

nonzero_unsigned_is_power_of_two! { NonZeroU8 NonZeroU16 NonZeroU32 NonZeroU64 NonZeroU128 NonZeroUsize }

macro_rules! nonzero_min_max {
( $( $MinVal:expr , $Ty: ident($Int: ty); )+ ) => {
$(
impl $Ty {
#[unstable(feature = "nonzero_max", issue = "89065")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see a reason to use two separate feature gates for min and max.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. 591b89d

#[doc = concat!("The maximum value for a`", stringify!($Ty), "` is the same as `", stringify!($Int), "`")]
#[doc = concat!("assert_eq!(", stringify!($Ty), "::MAX, ", stringify!($Int), "::MAX);")]
// SAFETY: Since the MAX value, for any supported integer type, is greater than 0, the MAX will always be non-zero.
pub const MAX : $Ty = unsafe { $Ty::new_unchecked(<$Int>::MAX) };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// SAFETY: Since the MAX value, for any supported integer type, is greater than 0, the MAX will always be non-zero.
pub const MAX : $Ty = unsafe { $Ty::new_unchecked(<$Int>::MAX) };
pub const MAX : $Ty = $Ty::new(<$Int>::MAX).unwrap();

Since we're in a const context this is guaranteed to get evaluated (and potentially fail if you pass in a zero after all) at compile time. No need for unsafe.

Same for the min case below. Unfortunately unwrap_or isn't const yet or we could get rid of the $MinVal input entirely.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree! I really wanted to get rid of that $MinVal, but couldn't find a clean way. Fixed the unsafe.
591b89d

#[unstable(feature = "nonzero_min", issue = "89065")]
#[doc = concat!("The minimum value for a`", stringify!($Ty), "`.")]
/// # Examples
#[doc = concat!("assert_eq!(", stringify!($Ty), "::MIN, ", stringify!($MinVal), ";")]
// SAFETY: In the signed case, the minimum integer is negative, and therefore non-zero.
// SAFETY: In the unsignedd case, we use one, which is non-zero.
pub const MIN : $Ty = unsafe { $Ty::new_unchecked($MinVal)};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation looks off here. Did the formatter really demand it like that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a macro, it should be formatted manually.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have specific advice on how to format it? I noticed that macros needed manual formatting. Wasn't sure if I had it right though @ibraheemdev .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This closing brace, and the analogous one in nonzero_constants_signed should be unindented by one level = four spaces. The rest looks fine.

)+
}
}

nonzero_min_max! {
1 , NonZeroU8(u8);
1 , NonZeroU16(u16);
1 , NonZeroU32(u32);
1 , NonZeroU64(u64);
1 , NonZeroU128(u128);
1 , NonZeroUsize(usize);
i8::MIN , NonZeroI8(i8);
i16::MIN , NonZeroI16(i16);
i32::MIN , NonZeroI32(i32);
i64::MIN , NonZeroI64(i64);
i128::MIN , NonZeroI128(i128);
isize::MIN , NonZeroIsize(isize);
}