Rollup merge of rust-lang#77640 - ethanboxx:int_error_matching_attempt_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`

Author: Dylan-DPC

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),
                     };

library/core/src/lib.rs

@@ -158,6 +158,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]

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",
         }
     }

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 }),
             };
         }
     }

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)
     );
 }
 

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]