Skip to content

Commit

Permalink
Rollup merge of rust-lang#77640 - ethanboxx:int_error_matching_attemp…
Browse files Browse the repository at this point in the history
…t_2, r=KodrAus

Refactor IntErrorKind to avoid "underflow" terminology

This PR is a continuation of rust-lang#76455

# Changes

- `Overflow` renamed to `PosOverflow` and `Underflow` renamed to `NegOverflow` after discussion in rust-lang#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``
  • Loading branch information
Dylan-DPC authored Nov 8, 2020
2 parents b1277d0 + e750238 commit a2c2720
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 46 deletions.
6 changes: 4 additions & 2 deletions compiler/rustc_middle/src/middle/limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
};
Expand Down
1 change: 1 addition & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
17 changes: 10 additions & 7 deletions library/core/src/num/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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",
}
}
Expand Down
22 changes: 13 additions & 9 deletions library/core/src/num/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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 }),
};
}
}
Expand Down
4 changes: 2 additions & 2 deletions library/core/tests/nonzero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}

Expand Down
65 changes: 39 additions & 26 deletions library/core/tests/num/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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]
Expand Down

0 comments on commit a2c2720

Please sign in to comment.