Skip to content

Commit

Permalink
Rollup merge of #128001 - Krappa322:master, r=scottmcm
Browse files Browse the repository at this point in the history
Improve documentation for <integer>::from_str_radix

Two improvements to the documentation:
- Document `-` as a valid character for signed integer destinations
- Make the documentation even more clear that extra whitespace and non-digit characters is invalid. Many other languages, e.g. c++, are very permissive in string to integer routines and simply try to consume as much as they can, ignoring the rest. This is trying to make the transition for developers who are used to the conversion semantics in these languages a bit easier.
  • Loading branch information
matthiaskrgr authored Sep 19, 2024
2 parents 749f80a + 467dbcb commit 767ae2b
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 26 deletions.
69 changes: 53 additions & 16 deletions library/core/src/num/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ macro_rules! unlikely {
};
}

// Use this when the generated code should differ between signed and unsigned types.
macro_rules! sign_dependent_expr {
(signed ? if signed { $signed_case:expr } if unsigned { $unsigned_case:expr } ) => {
$signed_case
};
(unsigned ? if signed { $signed_case:expr } if unsigned { $unsigned_case:expr } ) => {
$unsigned_case
};
}

// All these modules are technically private and only exposed for coretests:
#[cfg(not(no_fp_fmt_parse))]
pub mod bignum;
Expand Down Expand Up @@ -1410,15 +1420,25 @@ const fn from_str_radix_panic(radix: u32) {
}

macro_rules! from_str_radix {
($($int_ty:ty)+) => {$(
($signedness:ident $($int_ty:ty)+) => {$(
impl $int_ty {
/// Converts a string slice in a given base to an integer.
///
/// The string is expected to be an optional `+` sign
/// followed by digits.
/// Leading and trailing whitespace represent an error.
/// Digits are a subset of these characters, depending on `radix`:
/// The string is expected to be an optional
#[doc = sign_dependent_expr!{
$signedness ?
if signed {
" `+` or `-` "
}
if unsigned {
" `+` "
}
}]
/// sign followed by only digits. Leading and trailing non-digit characters (including
/// whitespace) represent an error. Underscores (which are accepted in rust literals)
/// also represent an error.
///
/// Digits are a subset of these characters, depending on `radix`:
/// * `0-9`
/// * `a-z`
/// * `A-Z`
Expand All @@ -1430,10 +1450,13 @@ macro_rules! from_str_radix {
/// # Examples
///
/// Basic usage:
///
/// ```
#[doc = concat!("assert_eq!(", stringify!($int_ty), "::from_str_radix(\"A\", 16), Ok(10));")]
/// ```
/// Trailing space returns error:
/// ```
#[doc = concat!("assert!(", stringify!($int_ty), "::from_str_radix(\"1 \", 10).is_err());")]
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "const_int_from_str", since = "1.82.0")]
pub const fn from_str_radix(src: &str, radix: u32) -> Result<$int_ty, ParseIntError> {
Expand Down Expand Up @@ -1535,20 +1558,31 @@ macro_rules! from_str_radix {
)+}
}

from_str_radix! { i8 u8 i16 u16 i32 u32 i64 u64 i128 u128 }
from_str_radix! { unsigned u8 u16 u32 u64 u128 }
from_str_radix! { signed i8 i16 i32 i64 i128 }

// Re-use the relevant implementation of from_str_radix for isize and usize to avoid outputting two
// identical functions.
macro_rules! from_str_radix_size_impl {
($($t:ident $size:ty),*) => {$(
($($signedness:ident $t:ident $size:ty),*) => {$(
impl $size {
/// Converts a string slice in a given base to an integer.
///
/// The string is expected to be an optional `+` sign
/// followed by digits.
/// Leading and trailing whitespace represent an error.
/// Digits are a subset of these characters, depending on `radix`:
/// The string is expected to be an optional
#[doc = sign_dependent_expr!{
$signedness ?
if signed {
" `+` or `-` "
}
if unsigned {
" `+` "
}
}]
/// sign followed by only digits. Leading and trailing non-digit characters (including
/// whitespace) represent an error. Underscores (which are accepted in rust literals)
/// also represent an error.
///
/// Digits are a subset of these characters, depending on `radix`:
/// * `0-9`
/// * `a-z`
/// * `A-Z`
Expand All @@ -1560,10 +1594,13 @@ macro_rules! from_str_radix_size_impl {
/// # Examples
///
/// Basic usage:
///
/// ```
#[doc = concat!("assert_eq!(", stringify!($size), "::from_str_radix(\"A\", 16), Ok(10));")]
/// ```
/// Trailing space returns error:
/// ```
#[doc = concat!("assert!(", stringify!($size), "::from_str_radix(\"1 \", 10).is_err());")]
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "const_int_from_str", since = "1.82.0")]
pub const fn from_str_radix(src: &str, radix: u32) -> Result<$size, ParseIntError> {
Expand All @@ -1576,8 +1613,8 @@ macro_rules! from_str_radix_size_impl {
}

#[cfg(target_pointer_width = "16")]
from_str_radix_size_impl! { i16 isize, u16 usize }
from_str_radix_size_impl! { signed i16 isize, unsigned u16 usize }
#[cfg(target_pointer_width = "32")]
from_str_radix_size_impl! { i32 isize, u32 usize }
from_str_radix_size_impl! { signed i32 isize, unsigned u32 usize }
#[cfg(target_pointer_width = "64")]
from_str_radix_size_impl! { i64 isize, u64 usize }
from_str_radix_size_impl! { signed i64 isize, unsigned u64 usize }
10 changes: 0 additions & 10 deletions library/core/src/num/nonzero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1972,16 +1972,6 @@ macro_rules! nonzero_integer_signedness_dependent_methods {
};
}

// Use this when the generated code should differ between signed and unsigned types.
macro_rules! sign_dependent_expr {
(signed ? if signed { $signed_case:expr } if unsigned { $unsigned_case:expr } ) => {
$signed_case
};
(unsigned ? if signed { $signed_case:expr } if unsigned { $unsigned_case:expr } ) => {
$unsigned_case
};
}

nonzero_integer! {
Self = NonZeroU8,
Primitive = unsigned u8,
Expand Down
2 changes: 2 additions & 0 deletions library/core/tests/num/int_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,8 @@ macro_rules! int_module {

assert_eq!($T::from_str_radix("Z", 35).ok(), None::<$T>);
assert_eq!($T::from_str_radix("-9", 2).ok(), None::<$T>);
assert_eq!($T::from_str_radix("10_0", 10).ok(), None::<$T>);
assert_eq!(u32::from_str_radix("-9", 10).ok(), None::<u32>);
}

#[test]
Expand Down

0 comments on commit 767ae2b

Please sign in to comment.