1
Fork 0

Rollup merge of #77640 - ethanboxx:int_error_matching_attempt_2, r=KodrAus

Refactor IntErrorKind to avoid "underflow" terminology

This PR is a continuation of #76455

# Changes

- `Overflow` renamed to `PosOverflow` and `Underflow` renamed to `NegOverflow` after discussion in #76455
- Changed some of the parsing code to return `InvalidDigit` rather than `Empty` for strings "+" and "-". https://users.rust-lang.org/t/misleading-error-in-str-parse-for-int-types/49178
- Carry the problem `char` with the `InvalidDigit` variant.
- Necessary changes were made to the compiler as it depends on `int_error_matching`.
- Redid tests to match on specific errors.

r? ```@KodrAus```
This commit is contained in:
Dylan DPC 2020-11-09 01:13:25 +01:00 committed by GitHub
commit d69ee57f97
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 69 additions and 46 deletions

View file

@ -48,10 +48,12 @@ fn update_limit(
.unwrap_or(attr.span); .unwrap_or(attr.span);
let error_str = match e.kind() { let error_str = match e.kind() {
IntErrorKind::Overflow => "`limit` is too large", IntErrorKind::PosOverflow => "`limit` is too large",
IntErrorKind::Empty => "`limit` must be a non-negative integer", IntErrorKind::Empty => "`limit` must be a non-negative integer",
IntErrorKind::InvalidDigit => "not a valid integer", IntErrorKind::InvalidDigit => "not a valid integer",
IntErrorKind::Underflow => bug!("`limit` should never underflow"), IntErrorKind::NegOverflow => {
bug!("`limit` should never negatively overflow")
}
IntErrorKind::Zero => bug!("zero is a valid `limit`"), IntErrorKind::Zero => bug!("zero is a valid `limit`"),
kind => bug!("unimplemented IntErrorKind variant: {:?}", kind), kind => bug!("unimplemented IntErrorKind variant: {:?}", kind),
}; };

View file

@ -159,6 +159,7 @@
#![feature(slice_ptr_get)] #![feature(slice_ptr_get)]
#![feature(no_niche)] // rust-lang/rust#68303 #![feature(no_niche)] // rust-lang/rust#68303
#![feature(unsafe_block_in_unsafe_fn)] #![feature(unsafe_block_in_unsafe_fn)]
#![feature(int_error_matching)]
#![deny(unsafe_op_in_unsafe_fn)] #![deny(unsafe_op_in_unsafe_fn)]
#[prelude_import] #[prelude_import]

View file

@ -98,15 +98,18 @@ pub enum IntErrorKind {
/// ///
/// Among other causes, this variant will be constructed when parsing an empty string. /// Among other causes, this variant will be constructed when parsing an empty string.
Empty, Empty,
/// Contains an invalid digit. /// Contains an invalid digit in its context.
/// ///
/// Among other causes, this variant will be constructed when parsing a string that /// Among other causes, this variant will be constructed when parsing a string that
/// contains a letter. /// contains a non-ASCII char.
///
/// This variant is also constructed when a `+` or `-` is misplaced within a string
/// either on its own or in the middle of a number.
InvalidDigit, InvalidDigit,
/// Integer is too large to store in target integer type. /// Integer is too large to store in target integer type.
Overflow, PosOverflow,
/// Integer is too small to store in target integer type. /// Integer is too small to store in target integer type.
Underflow, NegOverflow,
/// Value was Zero /// Value was Zero
/// ///
/// This variant will be emitted when the parsing string has a value of zero, which /// This variant will be emitted when the parsing string has a value of zero, which
@ -119,7 +122,7 @@ impl ParseIntError {
#[unstable( #[unstable(
feature = "int_error_matching", feature = "int_error_matching",
reason = "it can be useful to match errors when making error messages \ reason = "it can be useful to match errors when making error messages \
for integer parsing", for integer parsing",
issue = "22639" issue = "22639"
)] )]
pub fn kind(&self) -> &IntErrorKind { pub fn kind(&self) -> &IntErrorKind {
@ -136,8 +139,8 @@ impl ParseIntError {
match self.kind { match self.kind {
IntErrorKind::Empty => "cannot parse integer from empty string", IntErrorKind::Empty => "cannot parse integer from empty string",
IntErrorKind::InvalidDigit => "invalid digit found in string", IntErrorKind::InvalidDigit => "invalid digit found in string",
IntErrorKind::Overflow => "number too large to fit in target type", IntErrorKind::PosOverflow => "number too large to fit in target type",
IntErrorKind::Underflow => "number too small to fit in target type", IntErrorKind::NegOverflow => "number too small to fit in target type",
IntErrorKind::Zero => "number would be zero for non-zero type", IntErrorKind::Zero => "number would be zero for non-zero type",
} }
} }

View file

@ -63,7 +63,12 @@ pub use nonzero::{NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, No
#[stable(feature = "try_from", since = "1.34.0")] #[stable(feature = "try_from", since = "1.34.0")]
pub use error::TryFromIntError; pub use error::TryFromIntError;
#[unstable(feature = "int_error_matching", issue = "22639")] #[unstable(
feature = "int_error_matching",
reason = "it can be useful to match errors when making error messages \
for integer parsing",
issue = "22639"
)]
pub use error::IntErrorKind; pub use error::IntErrorKind;
macro_rules! usize_isize_to_xe_bytes_doc { macro_rules! usize_isize_to_xe_bytes_doc {
@ -830,15 +835,14 @@ fn from_str_radix<T: FromStrRadixHelper>(src: &str, radix: u32) -> Result<T, Par
let src = src.as_bytes(); let src = src.as_bytes();
let (is_positive, digits) = match src[0] { let (is_positive, digits) = match src[0] {
b'+' | b'-' if src[1..].is_empty() => {
return Err(PIE { kind: InvalidDigit });
}
b'+' => (true, &src[1..]), b'+' => (true, &src[1..]),
b'-' if is_signed_ty => (false, &src[1..]), b'-' if is_signed_ty => (false, &src[1..]),
_ => (true, src), _ => (true, src),
}; };
if digits.is_empty() {
return Err(PIE { kind: Empty });
}
let mut result = T::from_u32(0); let mut result = T::from_u32(0);
if is_positive { if is_positive {
// The number is positive // The number is positive
@ -849,11 +853,11 @@ fn from_str_radix<T: FromStrRadixHelper>(src: &str, radix: u32) -> Result<T, Par
}; };
result = match result.checked_mul(radix) { result = match result.checked_mul(radix) {
Some(result) => result, Some(result) => result,
None => return Err(PIE { kind: Overflow }), None => return Err(PIE { kind: PosOverflow }),
}; };
result = match result.checked_add(x) { result = match result.checked_add(x) {
Some(result) => result, Some(result) => result,
None => return Err(PIE { kind: Overflow }), None => return Err(PIE { kind: PosOverflow }),
}; };
} }
} else { } else {
@ -865,11 +869,11 @@ fn from_str_radix<T: FromStrRadixHelper>(src: &str, radix: u32) -> Result<T, Par
}; };
result = match result.checked_mul(radix) { result = match result.checked_mul(radix) {
Some(result) => result, Some(result) => result,
None => return Err(PIE { kind: Underflow }), None => return Err(PIE { kind: NegOverflow }),
}; };
result = match result.checked_sub(x) { result = match result.checked_sub(x) {
Some(result) => result, Some(result) => result,
None => return Err(PIE { kind: Underflow }), None => return Err(PIE { kind: NegOverflow }),
}; };
} }
} }

View file

@ -135,11 +135,11 @@ fn test_from_str() {
); );
assert_eq!( assert_eq!(
"-129".parse::<NonZeroI8>().err().map(|e| e.kind().clone()), "-129".parse::<NonZeroI8>().err().map(|e| e.kind().clone()),
Some(IntErrorKind::Underflow) Some(IntErrorKind::NegOverflow)
); );
assert_eq!( assert_eq!(
"257".parse::<NonZeroU8>().err().map(|e| e.kind().clone()), "257".parse::<NonZeroU8>().err().map(|e| e.kind().clone()),
Some(IntErrorKind::Overflow) Some(IntErrorKind::PosOverflow)
); );
} }

View file

@ -2,10 +2,11 @@ use core::cmp::PartialEq;
use core::convert::{TryFrom, TryInto}; use core::convert::{TryFrom, TryInto};
use core::fmt::Debug; use core::fmt::Debug;
use core::marker::Copy; use core::marker::Copy;
use core::num::TryFromIntError; use core::num::{IntErrorKind, ParseIntError, TryFromIntError};
use core::ops::{Add, Div, Mul, Rem, Sub}; use core::ops::{Add, Div, Mul, Rem, Sub};
use core::option::Option; use core::option::Option;
use core::option::Option::{None, Some}; use core::option::Option::None;
use core::str::FromStr;
#[macro_use] #[macro_use]
mod int_macros; mod int_macros;
@ -67,6 +68,15 @@ where
assert_eq!(ten.rem(two), ten % two); assert_eq!(ten.rem(two), ten % two);
} }
/// Helper function for asserting number parsing returns a specific error
fn test_parse<T>(num_str: &str, expected: Result<T, IntErrorKind>)
where
T: FromStr<Err = ParseIntError>,
Result<T, IntErrorKind>: PartialEq + Debug,
{
assert_eq!(num_str.parse::<T>().map_err(|e| e.kind().clone()), expected)
}
#[test] #[test]
fn from_str_issue7588() { fn from_str_issue7588() {
let u: Option<u8> = u8::from_str_radix("1000", 10).ok(); let u: Option<u8> = u8::from_str_radix("1000", 10).ok();
@ -77,49 +87,52 @@ fn from_str_issue7588() {
#[test] #[test]
fn test_int_from_str_overflow() { fn test_int_from_str_overflow() {
assert_eq!("127".parse::<i8>().ok(), Some(127i8)); test_parse::<i8>("127", Ok(127));
assert_eq!("128".parse::<i8>().ok(), None); test_parse::<i8>("128", Err(IntErrorKind::PosOverflow));
assert_eq!("-128".parse::<i8>().ok(), Some(-128i8)); test_parse::<i8>("-128", Ok(-128));
assert_eq!("-129".parse::<i8>().ok(), None); test_parse::<i8>("-129", Err(IntErrorKind::NegOverflow));
assert_eq!("32767".parse::<i16>().ok(), Some(32_767i16)); test_parse::<i16>("32767", Ok(32_767));
assert_eq!("32768".parse::<i16>().ok(), None); test_parse::<i16>("32768", Err(IntErrorKind::PosOverflow));
assert_eq!("-32768".parse::<i16>().ok(), Some(-32_768i16)); test_parse::<i16>("-32768", Ok(-32_768));
assert_eq!("-32769".parse::<i16>().ok(), None); test_parse::<i16>("-32769", Err(IntErrorKind::NegOverflow));
assert_eq!("2147483647".parse::<i32>().ok(), Some(2_147_483_647i32)); test_parse::<i32>("2147483647", Ok(2_147_483_647));
assert_eq!("2147483648".parse::<i32>().ok(), None); test_parse::<i32>("2147483648", Err(IntErrorKind::PosOverflow));
assert_eq!("-2147483648".parse::<i32>().ok(), Some(-2_147_483_648i32)); test_parse::<i32>("-2147483648", Ok(-2_147_483_648));
assert_eq!("-2147483649".parse::<i32>().ok(), None); test_parse::<i32>("-2147483649", Err(IntErrorKind::NegOverflow));
assert_eq!("9223372036854775807".parse::<i64>().ok(), Some(9_223_372_036_854_775_807i64)); test_parse::<i64>("9223372036854775807", Ok(9_223_372_036_854_775_807));
assert_eq!("9223372036854775808".parse::<i64>().ok(), None); test_parse::<i64>("9223372036854775808", Err(IntErrorKind::PosOverflow));
assert_eq!("-9223372036854775808".parse::<i64>().ok(), Some(-9_223_372_036_854_775_808i64)); test_parse::<i64>("-9223372036854775808", Ok(-9_223_372_036_854_775_808));
assert_eq!("-9223372036854775809".parse::<i64>().ok(), None); test_parse::<i64>("-9223372036854775809", Err(IntErrorKind::NegOverflow));
} }
#[test] #[test]
fn test_leading_plus() { fn test_leading_plus() {
assert_eq!("+127".parse::<u8>().ok(), Some(127)); test_parse::<u8>("+127", Ok(127));
assert_eq!("+9223372036854775807".parse::<i64>().ok(), Some(9223372036854775807)); test_parse::<i64>("+9223372036854775807", Ok(9223372036854775807));
} }
#[test] #[test]
fn test_invalid() { fn test_invalid() {
assert_eq!("--129".parse::<i8>().ok(), None); test_parse::<i8>("--129", Err(IntErrorKind::InvalidDigit));
assert_eq!("++129".parse::<i8>().ok(), None); test_parse::<i8>("++129", Err(IntErrorKind::InvalidDigit));
assert_eq!("Съешь".parse::<u8>().ok(), None); test_parse::<u8>("Съешь", Err(IntErrorKind::InvalidDigit));
test_parse::<u8>("123Hello", Err(IntErrorKind::InvalidDigit));
test_parse::<i8>("--", Err(IntErrorKind::InvalidDigit));
test_parse::<i8>("-", Err(IntErrorKind::InvalidDigit));
test_parse::<i8>("+", Err(IntErrorKind::InvalidDigit));
test_parse::<u8>("-1", Err(IntErrorKind::InvalidDigit));
} }
#[test] #[test]
fn test_empty() { fn test_empty() {
assert_eq!("-".parse::<i8>().ok(), None); test_parse::<u8>("", Err(IntErrorKind::Empty));
assert_eq!("+".parse::<i8>().ok(), None);
assert_eq!("".parse::<u8>().ok(), None);
} }
#[test] #[test]