diff options
| author | Dylan DPC <dylan.dpc@gmail.com> | 2020-11-09 01:13:25 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2020-11-09 01:13:25 +0100 |
| commit | d69ee57f977d03644acd8fbfd1799410e2c02db3 (patch) | |
| tree | 127bcfb0132eabba68e01335aaf1b2b30c43a689 | |
| parent | 1773f60ea5d42e86b8fdf78d2fc5221ead222bc1 (diff) | |
| parent | e750238404f59f3f6d94142f3e12ffc4c5c0b366 (diff) | |
| download | rust-d69ee57f977d03644acd8fbfd1799410e2c02db3.tar.gz rust-d69ee57f977d03644acd8fbfd1799410e2c02db3.zip | |
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```
| -rw-r--r-- | compiler/rustc_middle/src/middle/limits.rs | 6 | ||||
| -rw-r--r-- | library/core/src/lib.rs | 1 | ||||
| -rw-r--r-- | library/core/src/num/error.rs | 17 | ||||
| -rw-r--r-- | library/core/src/num/mod.rs | 22 | ||||
| -rw-r--r-- | library/core/tests/nonzero.rs | 4 | ||||
| -rw-r--r-- | library/core/tests/num/mod.rs | 65 |
6 files changed, 69 insertions, 46 deletions
diff --git a/compiler/rustc_middle/src/middle/limits.rs b/compiler/rustc_middle/src/middle/limits.rs index def9e5ebb52..41342764ba7 100644 --- a/compiler/rustc_middle/src/middle/limits.rs +++ b/compiler/rustc_middle/src/middle/limits.rs @@ -48,10 +48,12 @@ fn update_limit( .unwrap_or(attr.span); 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::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`"), kind => bug!("unimplemented IntErrorKind variant: {:?}", kind), }; diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index ea560288c28..41202546566 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -159,6 +159,7 @@ #![feature(slice_ptr_get)] #![feature(no_niche)] // rust-lang/rust#68303 #![feature(unsafe_block_in_unsafe_fn)] +#![feature(int_error_matching)] #![deny(unsafe_op_in_unsafe_fn)] #[prelude_import] diff --git a/library/core/src/num/error.rs b/library/core/src/num/error.rs index aab17155186..9d8c8c86291 100644 --- a/library/core/src/num/error.rs +++ b/library/core/src/num/error.rs @@ -98,15 +98,18 @@ pub enum IntErrorKind { /// /// Among other causes, this variant will be constructed when parsing an empty string. 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 - /// 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, /// Integer is too large to store in target integer type. - Overflow, + PosOverflow, /// Integer is too small to store in target integer type. - Underflow, + NegOverflow, /// Value was Zero /// /// This variant will be emitted when the parsing string has a value of zero, which @@ -119,7 +122,7 @@ impl ParseIntError { #[unstable( feature = "int_error_matching", reason = "it can be useful to match errors when making error messages \ - for integer parsing", + for integer parsing", issue = "22639" )] pub fn kind(&self) -> &IntErrorKind { @@ -136,8 +139,8 @@ impl ParseIntError { match self.kind { IntErrorKind::Empty => "cannot parse integer from empty string", IntErrorKind::InvalidDigit => "invalid digit found in string", - IntErrorKind::Overflow => "number too large to fit in target type", - IntErrorKind::Underflow => "number too small to fit in target type", + IntErrorKind::PosOverflow => "number too large 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", } } diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs index c90887b89d0..9f5ae57b74a 100644 --- a/library/core/src/num/mod.rs +++ b/library/core/src/num/mod.rs @@ -63,7 +63,12 @@ pub use nonzero::{NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, No #[stable(feature = "try_from", since = "1.34.0")] 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; 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 (is_positive, digits) = match src[0] { + b'+' | b'-' if src[1..].is_empty() => { + return Err(PIE { kind: InvalidDigit }); + } b'+' => (true, &src[1..]), b'-' if is_signed_ty => (false, &src[1..]), _ => (true, src), }; - if digits.is_empty() { - return Err(PIE { kind: Empty }); - } - let mut result = T::from_u32(0); if 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) { Some(result) => result, - None => return Err(PIE { kind: Overflow }), + None => return Err(PIE { kind: PosOverflow }), }; result = match result.checked_add(x) { Some(result) => result, - None => return Err(PIE { kind: Overflow }), + None => return Err(PIE { kind: PosOverflow }), }; } } else { @@ -865,11 +869,11 @@ fn from_str_radix<T: FromStrRadixHelper>(src: &str, radix: u32) -> Result<T, Par }; result = match result.checked_mul(radix) { Some(result) => result, - None => return Err(PIE { kind: Underflow }), + None => return Err(PIE { kind: NegOverflow }), }; result = match result.checked_sub(x) { Some(result) => result, - None => return Err(PIE { kind: Underflow }), + None => return Err(PIE { kind: NegOverflow }), }; } } diff --git a/library/core/tests/nonzero.rs b/library/core/tests/nonzero.rs index 825e5e63b59..fb1293c99bb 100644 --- a/library/core/tests/nonzero.rs +++ b/library/core/tests/nonzero.rs @@ -135,11 +135,11 @@ fn test_from_str() { ); assert_eq!( "-129".parse::<NonZeroI8>().err().map(|e| e.kind().clone()), - Some(IntErrorKind::Underflow) + Some(IntErrorKind::NegOverflow) ); assert_eq!( "257".parse::<NonZeroU8>().err().map(|e| e.kind().clone()), - Some(IntErrorKind::Overflow) + Some(IntErrorKind::PosOverflow) ); } diff --git a/library/core/tests/num/mod.rs b/library/core/tests/num/mod.rs index 190528fd445..49e5cc0eaa5 100644 --- a/library/core/tests/num/mod.rs +++ b/library/core/tests/num/mod.rs @@ -2,10 +2,11 @@ use core::cmp::PartialEq; use core::convert::{TryFrom, TryInto}; use core::fmt::Debug; use core::marker::Copy; -use core::num::TryFromIntError; +use core::num::{IntErrorKind, ParseIntError, TryFromIntError}; use core::ops::{Add, Div, Mul, Rem, Sub}; use core::option::Option; -use core::option::Option::{None, Some}; +use core::option::Option::None; +use core::str::FromStr; #[macro_use] mod int_macros; @@ -67,6 +68,15 @@ where 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] fn from_str_issue7588() { let u: Option<u8> = u8::from_str_radix("1000", 10).ok(); @@ -77,49 +87,52 @@ fn from_str_issue7588() { #[test] fn test_int_from_str_overflow() { - assert_eq!("127".parse::<i8>().ok(), Some(127i8)); - assert_eq!("128".parse::<i8>().ok(), None); + test_parse::<i8>("127", Ok(127)); + test_parse::<i8>("128", Err(IntErrorKind::PosOverflow)); - assert_eq!("-128".parse::<i8>().ok(), Some(-128i8)); - assert_eq!("-129".parse::<i8>().ok(), None); + test_parse::<i8>("-128", Ok(-128)); + test_parse::<i8>("-129", Err(IntErrorKind::NegOverflow)); - assert_eq!("32767".parse::<i16>().ok(), Some(32_767i16)); - assert_eq!("32768".parse::<i16>().ok(), None); + test_parse::<i16>("32767", Ok(32_767)); + test_parse::<i16>("32768", Err(IntErrorKind::PosOverflow)); - assert_eq!("-32768".parse::<i16>().ok(), Some(-32_768i16)); - assert_eq!("-32769".parse::<i16>().ok(), None); + test_parse::<i16>("-32768", Ok(-32_768)); + test_parse::<i16>("-32769", Err(IntErrorKind::NegOverflow)); - assert_eq!("2147483647".parse::<i32>().ok(), Some(2_147_483_647i32)); - assert_eq!("2147483648".parse::<i32>().ok(), None); + test_parse::<i32>("2147483647", Ok(2_147_483_647)); + test_parse::<i32>("2147483648", Err(IntErrorKind::PosOverflow)); - assert_eq!("-2147483648".parse::<i32>().ok(), Some(-2_147_483_648i32)); - assert_eq!("-2147483649".parse::<i32>().ok(), None); + test_parse::<i32>("-2147483648", Ok(-2_147_483_648)); + test_parse::<i32>("-2147483649", Err(IntErrorKind::NegOverflow)); - assert_eq!("9223372036854775807".parse::<i64>().ok(), Some(9_223_372_036_854_775_807i64)); - assert_eq!("9223372036854775808".parse::<i64>().ok(), None); + test_parse::<i64>("9223372036854775807", Ok(9_223_372_036_854_775_807)); + test_parse::<i64>("9223372036854775808", Err(IntErrorKind::PosOverflow)); - assert_eq!("-9223372036854775808".parse::<i64>().ok(), Some(-9_223_372_036_854_775_808i64)); - assert_eq!("-9223372036854775809".parse::<i64>().ok(), None); + test_parse::<i64>("-9223372036854775808", Ok(-9_223_372_036_854_775_808)); + test_parse::<i64>("-9223372036854775809", Err(IntErrorKind::NegOverflow)); } #[test] fn test_leading_plus() { - assert_eq!("+127".parse::<u8>().ok(), Some(127)); - assert_eq!("+9223372036854775807".parse::<i64>().ok(), Some(9223372036854775807)); + test_parse::<u8>("+127", Ok(127)); + test_parse::<i64>("+9223372036854775807", Ok(9223372036854775807)); } #[test] fn test_invalid() { - assert_eq!("--129".parse::<i8>().ok(), None); - assert_eq!("++129".parse::<i8>().ok(), None); - assert_eq!("Съешь".parse::<u8>().ok(), None); + test_parse::<i8>("--129", Err(IntErrorKind::InvalidDigit)); + test_parse::<i8>("++129", Err(IntErrorKind::InvalidDigit)); + 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] fn test_empty() { - assert_eq!("-".parse::<i8>().ok(), None); - assert_eq!("+".parse::<i8>().ok(), None); - assert_eq!("".parse::<u8>().ok(), None); + test_parse::<u8>("", Err(IntErrorKind::Empty)); } #[test] |
