Fix range overflow checking

Nadrieril · Nadrieril · commit b4e4ead0f20d · 2023-10-11T03:04:53.000+02:00
diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl
@@ -173,7 +173,7 @@ mir_build_leading_irrefutable_let_patterns = leading irrefutable {$count ->
 
 mir_build_literal_in_range_out_of_bounds =
     literal out of range for `{$ty}`
-    .label = this value doesn't fit in `{$ty}` whose maximum value is `{$max}`
+    .label = this value does not fit into the type `{$ty}` whose range is `{$min}..={$max}`
 
 mir_build_lower_range_bound_must_be_less_than_or_equal_to_upper =
     lower range bound must be less than or equal to upper
diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs
@@ -551,6 +551,7 @@ pub struct LiteralOutOfRange<'tcx> {
     #[label]
     pub span: Span,
     pub ty: Ty<'tcx>,
+    pub min: i128,
     pub max: u128,
 }
 
diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs
@@ -20,15 +20,15 @@ use rustc_index::Idx;
 use rustc_middle::mir::interpret::{
     ErrorHandled, GlobalId, LitToConstError, LitToConstInput, Scalar,
 };
-use rustc_middle::mir::{self, Const, UserTypeProjection};
-use rustc_middle::mir::{BorrowKind, Mutability};
+use rustc_middle::mir::{self, BorrowKind, Const, Mutability, UserTypeProjection};
 use rustc_middle::thir::{Ascription, BindingMode, FieldPat, LocalVarId, Pat, PatKind, PatRange};
-use rustc_middle::ty::CanonicalUserTypeAnnotation;
-use rustc_middle::ty::TypeVisitableExt;
-use rustc_middle::ty::{self, AdtDef, Region, Ty, TyCtxt, UserType};
-use rustc_middle::ty::{GenericArg, GenericArgsRef};
+use rustc_middle::ty::layout::IntegerExt;
+use rustc_middle::ty::{
+    self, AdtDef, CanonicalUserTypeAnnotation, GenericArg, GenericArgsRef, Region, Ty, TyCtxt,
+    TypeVisitableExt, UserType,
+};
 use rustc_span::{ErrorGuaranteed, Span, Symbol};
-use rustc_target::abi::FieldIdx;
+use rustc_target::abi::{FieldIdx, Integer};
 
 use std::cmp::Ordering;
 
@@ -111,6 +111,59 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
         }
     }
 
+    /// Overflowing literals are linted against in a late pass. This is mostly fine, except when we
+    /// encounter a range pattern like `-130i8..2`: if we believe `eval_bits`, this looks like a
+    /// range where the endpoints are in the wrong order. To avoid a confusing error message, we
+    /// check for overflow then.
+    /// This is only called when the range is already known to be malformed.
+    fn error_on_literal_overflow(
+        &self,
+        expr: Option<&'tcx hir::Expr<'tcx>>,
+        ty: Ty<'tcx>,
+    ) -> Result<(), ErrorGuaranteed> {
+        use hir::{ExprKind, UnOp};
+        use rustc_ast::ast::LitKind;
+
+        let Some(mut expr) = expr else {
+            return Ok(());
+        };
+        let span = expr.span;
+
+        // We need to inspect the original expression, because if we only inspect the output of
+        // `eval_bits`, an overflowed value has already been wrapped around.
+        // We mostly copy the logic from the `rustc_lint::OVERFLOWING_LITERALS` lint.
+        let mut negated = false;
+        if let ExprKind::Unary(UnOp::Neg, sub_expr) = expr.kind {
+            negated = true;
+            expr = sub_expr;
+        }
+        let ExprKind::Lit(lit) = expr.kind else {
+            return Ok(());
+        };
+        let LitKind::Int(lit_val, _) = lit.node else {
+            return Ok(());
+        };
+        let (min, max): (i128, u128) = match ty.kind() {
+            ty::Int(ity) => {
+                let size = Integer::from_int_ty(&self.tcx, *ity).size();
+                (size.signed_int_min(), size.signed_int_max() as u128)
+            }
+            ty::Uint(uty) => {
+                let size = Integer::from_uint_ty(&self.tcx, *uty).size();
+                (0, size.unsigned_int_max())
+            }
+            _ => {
+                return Ok(());
+            }
+        };
+        // Detect literal value out of range `[min, max]` inclusive, avoiding use of `-min` to
+        // prevent overflow/panic.
+        if (negated && lit_val > max + 1) || (!negated && lit_val > max) {
+            return Err(self.tcx.sess.emit_err(LiteralOutOfRange { span, ty, min, max }));
+        }
+        Ok(())
+    }
+
     fn lower_pattern_range(
         &mut self,
         lo_expr: Option<&'tcx hir::Expr<'tcx>>,
@@ -155,29 +208,9 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
             }
             // `x..y` where `x >= y`, or `x..=y` where `x > y`. The range is empty => error.
             _ => {
-                let max = || {
-                    self.tcx
-                        .layout_of(self.param_env.with_reveal_all_normalized(self.tcx).and(ty))
-                        .ok()
-                        .unwrap()
-                        .size
-                        .unsigned_int_max()
-                };
-                // Emit a different message if there was overflow.
-                if let Some(hir::Expr { kind: hir::ExprKind::Lit(lit), .. }) = lo_expr
-                    && let rustc_ast::ast::LitKind::Int(val, _) = lit.node
-                {
-                    if lo.eval_bits(self.tcx, self.param_env) != val {
-                        return Err(self.tcx.sess.emit_err(LiteralOutOfRange { span: lit.span, ty, max: max() }));
-                    }
-                }
-                if let Some(hir::Expr { kind: hir::ExprKind::Lit(lit), .. }) = hi_expr
-                    && let rustc_ast::ast::LitKind::Int(val, _) = lit.node
-                {
-                    if hi.eval_bits(self.tcx, self.param_env) != val {
-                        return Err(self.tcx.sess.emit_err(LiteralOutOfRange { span: lit.span, ty, max: max() }));
-                    }
-                }
+                // Emit a more appropriate message if there was overflow.
+                self.error_on_literal_overflow(lo_expr, ty)?;
+                self.error_on_literal_overflow(hi_expr, ty)?;
                 let e = match end {
                     RangeEnd::Included => {
                         self.tcx.sess.emit_err(LowerRangeBoundMustBeLessThanOrEqualToUpper {
@@ -219,7 +252,6 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
 
             hir::PatKind::Range(ref lo_expr, ref hi_expr, end) => {
                 let (lo_expr, hi_expr) = (lo_expr.as_deref(), hi_expr.as_deref());
-                let span = lo_expr.map_or(span, |e| e.span);
                 // FIXME?: returning `_` can cause inaccurate "unreachable" warnings. This can be
                 // fixed by returning `PatKind::Const(ConstKind::Error(...))` if #115937 gets
                 // merged.
diff --git a/tests/ui/match/match-range-fail-2.stderr b/tests/ui/match/match-range-fail-2.stderr
@@ -2,19 +2,19 @@ error[E0030]: lower range bound must be less than or equal to upper
   --> $DIR/match-range-fail-2.rs:5:9
    |
 LL |         6 ..= 1 => { }
-   |         ^ lower bound larger than upper bound
+   |         ^^^^^^^ lower bound larger than upper bound
 
 error[E0579]: lower range bound must be less than upper
   --> $DIR/match-range-fail-2.rs:11:9
    |
 LL |         0 .. 0 => { }
-   |         ^
+   |         ^^^^^^
 
 error[E0030]: lower range bound must be less than or equal to upper
   --> $DIR/match-range-fail-2.rs:17:9
    |
 LL |         0xFFFF_FFFF_FFFF_FFFF ..= 1 => { }
-   |         ^^^^^^^^^^^^^^^^^^^^^ lower bound larger than upper bound
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ lower bound larger than upper bound
 
 error: aborting due to 3 previous errors
 
diff --git a/tests/ui/match/validate-range-endpoints.rs b/tests/ui/match/validate-range-endpoints.rs
@@ -30,16 +30,15 @@ fn main() {
         _ => {}
     }
 
-    // FIXME: error message is confusing
     match 0i8 {
         0..129 => {}
-        //~^ ERROR lower range bound must be less than upper
+        //~^ ERROR literal out of range
         0..=128 => {}
-        //~^ ERROR lower range bound must be less than or equal to upper
+        //~^ ERROR literal out of range
         -129..0 => {}
-        //~^ ERROR lower range bound must be less than upper
+        //~^ ERROR literal out of range
         -10000..=-20 => {}
-        //~^ ERROR lower range bound must be less than or equal to upper
+        //~^ ERROR literal out of range
 
         // overflow is detected in a later pass for these
         128..=0 => {}
diff --git a/tests/ui/match/validate-range-endpoints.stderr b/tests/ui/match/validate-range-endpoints.stderr
@@ -2,58 +2,58 @@ error: literal out of range for `u8`
   --> $DIR/validate-range-endpoints.rs:9:12
    |
 LL |         1..257 => {}
-   |            ^^^ this value doesn't fit in `u8` whose maximum value is `255`
+   |            ^^^ this value does not fit into the type `u8` whose range is `0..=255`
 
 error: literal out of range for `u8`
   --> $DIR/validate-range-endpoints.rs:11:13
    |
 LL |         1..=256 => {}
-   |             ^^^ this value doesn't fit in `u8` whose maximum value is `255`
+   |             ^^^ this value does not fit into the type `u8` whose range is `0..=255`
 
 error[E0030]: lower range bound must be less than or equal to upper
   --> $DIR/validate-range-endpoints.rs:20:9
    |
 LL |         1..=TOO_BIG => {}
-   |         ^ lower bound larger than upper bound
+   |         ^^^^^^^^^^^ lower bound larger than upper bound
 
 error[E0030]: lower range bound must be less than or equal to upper
   --> $DIR/validate-range-endpoints.rs:22:9
    |
 LL |         1..=const { 256 } => {}
-   |         ^ lower bound larger than upper bound
+   |         ^^^^^^^^^^^^^^^^^ lower bound larger than upper bound
 
 error: literal out of range for `usize`
   --> $DIR/validate-range-endpoints.rs:28:32
    |
 LL |         10000000000000000000..=99999999999999999999 => {}
-   |                                ^^^^^^^^^^^^^^^^^^^^ this value doesn't fit in `usize` whose maximum value is `18446744073709551615`
+   |                                ^^^^^^^^^^^^^^^^^^^^ this value does not fit into the type `usize` whose range is `0..=18446744073709551615`
 
-error[E0579]: lower range bound must be less than upper
-  --> $DIR/validate-range-endpoints.rs:35:9
+error: literal out of range for `i8`
+  --> $DIR/validate-range-endpoints.rs:34:12
    |
 LL |         0..129 => {}
-   |         ^
+   |            ^^^ this value does not fit into the type `i8` whose range is `-128..=127`
 
-error[E0030]: lower range bound must be less than or equal to upper
-  --> $DIR/validate-range-endpoints.rs:37:9
+error: literal out of range for `i8`
+  --> $DIR/validate-range-endpoints.rs:36:13
    |
 LL |         0..=128 => {}
-   |         ^ lower bound larger than upper bound
+   |             ^^^ this value does not fit into the type `i8` whose range is `-128..=127`
 
-error[E0579]: lower range bound must be less than upper
-  --> $DIR/validate-range-endpoints.rs:39:9
+error: literal out of range for `i8`
+  --> $DIR/validate-range-endpoints.rs:38:9
    |
 LL |         -129..0 => {}
-   |         ^^^^
+   |         ^^^^ this value does not fit into the type `i8` whose range is `-128..=127`
 
-error[E0030]: lower range bound must be less than or equal to upper
-  --> $DIR/validate-range-endpoints.rs:41:9
+error: literal out of range for `i8`
+  --> $DIR/validate-range-endpoints.rs:40:9
    |
 LL |         -10000..=-20 => {}
-   |         ^^^^^^ lower bound larger than upper bound
+   |         ^^^^^^ this value does not fit into the type `i8` whose range is `-128..=127`
 
 error[E0004]: non-exhaustive patterns: `i8::MIN..=-17_i8` and `1_i8..=i8::MAX` not covered
-  --> $DIR/validate-range-endpoints.rs:52:11
+  --> $DIR/validate-range-endpoints.rs:51:11
    |
 LL |     match 0i8 {
    |           ^^^ patterns `i8::MIN..=-17_i8` and `1_i8..=i8::MAX` not covered
@@ -66,7 +66,7 @@ LL +         i8::MIN..=-17_i8 | 1_i8..=i8::MAX => todo!()
    |
 
 error[E0004]: non-exhaustive patterns: `i8::MIN..=-17_i8` not covered
-  --> $DIR/validate-range-endpoints.rs:56:11
+  --> $DIR/validate-range-endpoints.rs:55:11
    |
 LL |     match 0i8 {
    |           ^^^ pattern `i8::MIN..=-17_i8` not covered
@@ -80,5 +80,5 @@ LL +         i8::MIN..=-17_i8 => todo!()
 
 error: aborting due to 11 previous errors
 
-Some errors have detailed explanations: E0004, E0030, E0579.
+Some errors have detailed explanations: E0004, E0030.
 For more information about an error, try `rustc --explain E0004`.
diff --git a/tests/ui/range/range-pattern-out-of-bounds-issue-68972.stderr b/tests/ui/range/range-pattern-out-of-bounds-issue-68972.stderr
@@ -2,13 +2,13 @@ error: literal out of range for `u8`
   --> $DIR/range-pattern-out-of-bounds-issue-68972.rs:5:14
    |
 LL |         251..257 => {}
-   |              ^^^ this value doesn't fit in `u8` whose maximum value is `255`
+   |              ^^^ this value does not fit into the type `u8` whose range is `0..=255`
 
 error: literal out of range for `u8`
   --> $DIR/range-pattern-out-of-bounds-issue-68972.rs:7:15
    |
 LL |         251..=256 => {}
-   |               ^^^ this value doesn't fit in `u8` whose maximum value is `255`
+   |               ^^^ this value does not fit into the type `u8` whose range is `0..=255`
 
 error: aborting due to 2 previous errors
 

Original file line number	Diff line number	Diff line change
`@@ -551,6 +551,7 @@ pub struct LiteralOutOfRange<'tcx> {`
`551`	`551`	`#[label]`
`552`	`552`	`pub span: Span,`
`553`	`553`	`pub ty: Ty<'tcx>,`
	`554`	`+ pub min: i128,`
`554`	`555`	`pub max: u128,`
`555`	`556`	`}`
`556`	`557`
Original file line number	Diff line number	Diff line change
`@@ -2,19 +2,19 @@ error[E0030]: lower range bound must be less than or equal to upper`
`2`	`2`	`--> $DIR/match-range-fail-2.rs:5:9`
`3`	`3`	`\|`
`4`	`4`	`LL \| 6 ..= 1 => { }`
`5`		`- \| ^ lower bound larger than upper bound`
	`5`	`+ \| ^^^^^^^ lower bound larger than upper bound`
`6`	`6`
`7`	`7`	`error[E0579]: lower range bound must be less than upper`
`8`	`8`	`--> $DIR/match-range-fail-2.rs:11:9`
`9`	`9`	`\|`
`10`	`10`	`LL \| 0 .. 0 => { }`
`11`		`- \| ^`
	`11`	`+ \| ^^^^^^`
`12`	`12`
`13`	`13`	`error[E0030]: lower range bound must be less than or equal to upper`
`14`	`14`	`--> $DIR/match-range-fail-2.rs:17:9`
`15`	`15`	`\|`
`16`	`16`	`LL \| 0xFFFF_FFFF_FFFF_FFFF ..= 1 => { }`
`17`		`- \| ^^^^^^^^^^^^^^^^^^^^^ lower bound larger than upper bound`
	`17`	`+ \| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ lower bound larger than upper bound`
`18`	`18`
`19`	`19`	`error: aborting due to 3 previous errors`
`20`	`20`
Original file line number	Diff line number	Diff line change
@@ -2,13 +2,13 @@ error: literal out of range for `u8`
`2`	`2`	`--> $DIR/range-pattern-out-of-bounds-issue-68972.rs:5:14`
`3`	`3`	`\|`
`4`	`4`	`LL \| 251..257 => {}`
`5`		- \| ^^^ this value doesn't fit in `u8` whose maximum value is `255`
	`5`	+ \| ^^^ this value does not fit into the type `u8` whose range is `0..=255`
`6`	`6`
`7`	`7`	error: literal out of range for `u8`
`8`	`8`	`--> $DIR/range-pattern-out-of-bounds-issue-68972.rs:7:15`
`9`	`9`	`\|`
`10`	`10`	`LL \| 251..=256 => {}`
`11`		- \| ^^^ this value doesn't fit in `u8` whose maximum value is `255`
	`11`	+ \| ^^^ this value does not fit into the type `u8` whose range is `0..=255`
`12`	`12`
`13`	`13`	`error: aborting due to 2 previous errors`
`14`	`14`