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.

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

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

Closed
wants to merge 14 commits into from
33 changes: 33 additions & 0 deletions library/core/src/num/nonzero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -895,3 +895,36 @@ 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_min_max", issue = "89065")]
#[doc = concat!("The maximum value for a`", stringify!($Ty), "` is the same as `", stringify!($Int), "`")]
Copy link
Contributor

Choose a reason for hiding this comment

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

In for a`", there should be a space between a and `, and the sentence needs a period at the end.

#[doc = concat!("assert_eq!(", stringify!($Ty), "::MAX, ", stringify!($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.

I think this and the below example need a ``` ... ``` code block, for formatting (though they're not very interesting as doctests).

I think including some blank lines as done above for nonzero_unsigned_signed_operations (such as ``` above and below the Examples header, and a normal blank line between the MAX and MIN definitions) but before would also make the macro code more readable even if it doesn't affect the documentation output.

pub const MAX : $Ty = $Ty::new(<$Int>::MAX).unwrap() ;
mjclements marked this conversation as resolved.
Show resolved Hide resolved
#[unstable(feature = "nonzero_min_max", issue = "89065")]
#[doc = concat!("The minimum value for a`", stringify!($Ty), "`.")]
/// # Examples
#[doc = concat!("assert_eq!(", stringify!($Ty), "::MIN, ", stringify!($MinVal), ";")]
pub const MIN : $Ty = $Ty::new($MinVal).unwrap();
}
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);
}
Copy link
Member

@ibraheemdev ibraheemdev Sep 21, 2021

Choose a reason for hiding this comment

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

Suggested change
macro_rules! nonzero_min_max {
( $( $MinVal:expr , $Ty: ident($Int: ty); )+ ) => {
$(
impl $Ty {
#[unstable(feature = "nonzero_min_max", issue = "89065")]
#[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);")]
pub const MAX : $Ty = $Ty::new(<$Int>::MAX).unwrap() ;
#[unstable(feature = "nonzero_min_max", issue = "89065")]
#[doc = concat!("The minimum value for a`", stringify!($Ty), "`.")]
/// # Examples
#[doc = concat!("assert_eq!(", stringify!($Ty), "::MIN, ", stringify!($MinVal), ";")]
pub const MIN : $Ty = $Ty::new($MinVal).unwrap();
}
)+
}
}
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);
}
macro_rules! nonzero_min_max {
( $( $Ty:ident($Int:ty) => $min_val:expr, $max_val:expr )+ ) => {
$(
impl $Ty {
#[unstable(feature = "nonzero_min_max", issue = "89065")]
#[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);")]
pub const MAX : $Ty = $Ty::new($max_val).unwrap() ;
#[unstable(feature = "nonzero_min_max", issue = "89065")]
#[doc = concat!("The minimum value for a`", stringify!($Ty), "`.")]
/// # Examples
#[doc = concat!("assert_eq!(", stringify!($Ty), "::MIN, ", stringify!($MinVal), ";")]
pub const MIN: $Ty = $Ty::new($min_val).unwrap();
}
)+
}
}
nonzero_min_max! {
NonZeroU8 => 1, u8::MAX;
NonZeroU16 => 1, u16::MAX;
NonZeroU32 => 1, u32::MAX;
NonZeroU64 => 1, u64::MAX;
NonZeroU128 => 1, u128::MAX;
NonZeroUsize => 1, usize::MAX;
NonZeroI8 => i8::MIN, i8::MAX;
NonZeroI16 => i16::MIN, i16::MAX;
NonZeroI32 => i32::MIN, i32::MAX;
NonZeroI64 => i64::MIN, i64::MAX;
NonZeroI128 => i128::MIN, i128::MAX;
NonZeroIsize => isize::MIN, isize::MAX;
}

Copy link
Member

Choose a reason for hiding this comment

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

Formatting is up to you, but I'd probably do something like this.