Fix overflow checking in range patterns

compiler/rustc_lint/src/types.rs

@@ -140,13 +140,15 @@ declare_lint! {
 pub struct TypeLimits {
     /// Id of the last visited negated expression
     negated_expr_id: Option<hir::HirId>,
+    /// Span of the last visited negated expression
+    negated_expr_span: Option<Span>,
 }
 
 impl_lint_pass!(TypeLimits => [UNUSED_COMPARISONS, OVERFLOWING_LITERALS, INVALID_NAN_COMPARISONS]);
 
 impl TypeLimits {
     pub fn new() -> TypeLimits {
-        TypeLimits { negated_expr_id: None }
+        TypeLimits { negated_expr_id: None, negated_expr_span: None }
     }
 }
 
@@ -426,17 +428,15 @@ fn lint_int_literal<'tcx>(
             return;
         }
 
-        let lit = cx
-            .sess()
-            .source_map()
-            .span_to_snippet(lit.span)
-            .expect("must get snippet from literal");
+        let span = if negative { type_limits.negated_expr_span.unwrap() } else { e.span };
+        let lit =
+            cx.sess().source_map().span_to_snippet(span).expect("must get snippet from literal");
         let help = get_type_suggestion(cx.typeck_results().node_type(e.hir_id), v, negative)
             .map(|suggestion_ty| OverflowingIntHelp { suggestion_ty });
 
         cx.emit_spanned_lint(
             OVERFLOWING_LITERALS,
-            e.span,
+            span,
             OverflowingInt { ty: t.name_str(), lit, min, max, help },
         );
     }
@@ -622,9 +622,10 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx hir::Expr<'tcx>) {
         match e.kind {
             hir::ExprKind::Unary(hir::UnOp::Neg, ref expr) => {
-                // propagate negation, if the negation itself isn't negated
+                // Propagate negation, if the negation itself isn't negated
                 if self.negated_expr_id != Some(e.hir_id) {
                     self.negated_expr_id = Some(expr.hir_id);
+                    self.negated_expr_span = Some(e.span);
                 }
             }
             hir::ExprKind::Binary(binop, ref l, ref r) => {

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

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

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_span::{Span, Symbol};
-use rustc_target::abi::FieldIdx;
+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, Integer};
 
 use std::cmp::Ordering;
 
@@ -85,127 +85,159 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
         )
     }
 
-    fn lower_range_expr(
+    fn lower_pattern_range_endpoint(
         &mut self,
-        expr: &'tcx hir::Expr<'tcx>,
-    ) -> (PatKind<'tcx>, Option<Ascription<'tcx>>) {
-        match self.lower_lit(expr) {
-            PatKind::AscribeUserType { ascription, subpattern: box Pat { kind, .. } } => {
-                (kind, Some(ascription))
+        expr: Option<&'tcx hir::Expr<'tcx>>,
+    ) -> Result<(Option<mir::Const<'tcx>>, Option<Ascription<'tcx>>), ErrorGuaranteed> {
+        match expr {
+            None => Ok((None, None)),
+            Some(expr) => {
+                let (kind, ascr) = match self.lower_lit(expr) {
+                    PatKind::AscribeUserType { ascription, subpattern: box Pat { kind, .. } } => {
+                        (kind, Some(ascription))
+                    }
+                    kind => (kind, None),
+                };
+                let value = if let PatKind::Constant { value } = kind {
+                    value
+                } else {
+                    let msg = format!(
+                        "found bad range pattern endpoint `{expr:?}` outside of error recovery"
+                    );
+                    return Err(self.tcx.sess.delay_span_bug(expr.span, msg));
+                };
+                Ok((Some(value), ascr))
             }
-            kind => (kind, None),
         }
     }
 
+    /// 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,
-        ty: Ty<'tcx>,
-        lo: mir::Const<'tcx>,
-        hi: mir::Const<'tcx>,
+        lo_expr: Option<&'tcx hir::Expr<'tcx>>,
+        hi_expr: Option<&'tcx hir::Expr<'tcx>>,
         end: RangeEnd,
+        ty: Ty<'tcx>,
         span: Span,
-        lo_expr: Option<&hir::Expr<'tcx>>,
-        hi_expr: Option<&hir::Expr<'tcx>>,
-    ) -> PatKind<'tcx> {
+    ) -> Result<PatKind<'tcx>, ErrorGuaranteed> {
+        if lo_expr.is_none() && hi_expr.is_none() {
+            let msg = format!("found twice-open range pattern (`..`) outside of error recovery");
+            return Err(self.tcx.sess.delay_span_bug(span, msg));
+        }
+
+        let (lo, lo_ascr) = self.lower_pattern_range_endpoint(lo_expr)?;
+        let (hi, hi_ascr) = self.lower_pattern_range_endpoint(hi_expr)?;
+
+        let lo = lo.unwrap_or_else(|| {
+            // Unwrap is ok because the type is known to be numeric.
+            let lo = ty.numeric_min_val(self.tcx).unwrap();
+            mir::Const::from_ty_const(lo, self.tcx)
+        });
+        let hi = hi.unwrap_or_else(|| {
+            // Unwrap is ok because the type is known to be numeric.
+            let hi = ty.numeric_max_val(self.tcx).unwrap();
+            mir::Const::from_ty_const(hi, self.tcx)
+        });
         assert_eq!(lo.ty(), ty);
         assert_eq!(hi.ty(), ty);
+
         let cmp = compare_const_vals(self.tcx, lo, hi, self.param_env);
-        let max = || {
-            self.tcx
-                .layout_of(self.param_env.with_reveal_all_normalized(self.tcx).and(ty))
-                .ok()
-                .unwrap()
-                .size
-                .unsigned_int_max()
-        };
-        match (end, cmp) {
+        let mut kind = match (end, cmp) {
             // `x..y` where `x < y`.
             // Non-empty because the range includes at least `x`.
             (RangeEnd::Excluded, Some(Ordering::Less)) => {
                 PatKind::Range(Box::new(PatRange { lo, hi, end }))
             }
-            // `x..y` where `x >= y`. The range is empty => error.
-            (RangeEnd::Excluded, _) => {
-                let mut lower_overflow = false;
-                let mut higher_overflow = false;
-                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 {
-                        lower_overflow = true;
-                        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 {
-                        higher_overflow = true;
-                        self.tcx.sess.emit_err(LiteralOutOfRange { span: lit.span, ty, max: max() });
-                    }
-                }
-                if !lower_overflow && !higher_overflow {
-                    self.tcx.sess.emit_err(LowerRangeBoundMustBeLessThanUpper { span });
-                }
-                PatKind::Wild
-            }
             // `x..=y` where `x == y`.
             (RangeEnd::Included, Some(Ordering::Equal)) => PatKind::Constant { value: lo },
             // `x..=y` where `x < y`.
             (RangeEnd::Included, Some(Ordering::Less)) => {
                 PatKind::Range(Box::new(PatRange { lo, hi, end }))
             }
-            // `x..=y` where `x > y` hence the range is empty => error.
-            (RangeEnd::Included, _) => {
-                let mut lower_overflow = false;
-                let mut higher_overflow = false;
-                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 {
-                        lower_overflow = true;
-                        self.tcx.sess.emit_err(LiteralOutOfRange { span: lit.span, ty, max: max() });
+            // `x..y` where `x >= y`, or `x..=y` where `x > y`. The range is empty => error.
+            _ => {
+                // 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 {
+                            span,
+                            teach: self.tcx.sess.teach(&error_code!(E0030)).then_some(()),
+                        })
                     }
-                }
-                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 {
-                        higher_overflow = true;
-                        self.tcx.sess.emit_err(LiteralOutOfRange { span: lit.span, ty, max: max() });
+                    RangeEnd::Excluded => {
+                        self.tcx.sess.emit_err(LowerRangeBoundMustBeLessThanUpper { span })
                     }
-                }
-                if !lower_overflow && !higher_overflow {
-                    self.tcx.sess.emit_err(LowerRangeBoundMustBeLessThanOrEqualToUpper {
-                        span,
-                        teach: self.tcx.sess.teach(&error_code!(E0030)).then_some(()),
-                    });
-                }
-                PatKind::Wild
+                };
+                return Err(e);
             }
-        }
-    }
+        };
 
-    fn normalize_range_pattern_ends(
-        &self,
-        ty: Ty<'tcx>,
-        lo: Option<&PatKind<'tcx>>,
-        hi: Option<&PatKind<'tcx>>,
-    ) -> Option<(mir::Const<'tcx>, mir::Const<'tcx>)> {
-        match (lo, hi) {
-            (Some(PatKind::Constant { value: lo }), Some(PatKind::Constant { value: hi })) => {
-                Some((*lo, *hi))
-            }
-            (Some(PatKind::Constant { value: lo }), None) => {
-                let hi = ty.numeric_max_val(self.tcx)?;
-                Some((*lo, mir::Const::from_ty_const(hi, self.tcx)))
-            }
-            (None, Some(PatKind::Constant { value: hi })) => {
-                let lo = ty.numeric_min_val(self.tcx)?;
-                Some((mir::Const::from_ty_const(lo, self.tcx), *hi))
+        // If we are handling a range with associated constants (e.g.
+        // `Foo::<'a>::A..=Foo::B`), we need to put the ascriptions for the associated
+        // constants somewhere. Have them on the range pattern.
+        for ascr in [lo_ascr, hi_ascr] {
+            if let Some(ascription) = ascr {
+                kind = PatKind::AscribeUserType {
+                    ascription,
+                    subpattern: Box::new(Pat { span, ty, kind }),
+                };
             }
-            _ => None,
         }
+        Ok(kind)
     }
 
     #[instrument(skip(self), level = "debug")]
@@ -220,37 +252,10 @@ 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 lo_span = lo_expr.map_or(pat.span, |e| e.span);
-                let lo = lo_expr.map(|e| self.lower_range_expr(e));
-                let hi = hi_expr.map(|e| self.lower_range_expr(e));
-
-                let (lp, hp) = (lo.as_ref().map(|(x, _)| x), hi.as_ref().map(|(x, _)| x));
-                let mut kind = match self.normalize_range_pattern_ends(ty, lp, hp) {
-                    Some((lc, hc)) => {
-                        self.lower_pattern_range(ty, lc, hc, end, lo_span, lo_expr, hi_expr)
-                    }
-                    None => {
-                        let msg = format!(
-                            "found bad range pattern `{:?}` outside of error recovery",
-                            (&lo, &hi),
-                        );
-                        self.tcx.sess.delay_span_bug(pat.span, msg);
-                        PatKind::Wild
-                    }
-                };
-
-                // If we are handling a range with associated constants (e.g.
-                // `Foo::<'a>::A..=Foo::B`), we need to put the ascriptions for the associated
-                // constants somewhere. Have them on the range pattern.
-                for end in &[lo, hi] {
-                    if let Some((_, Some(ascription))) = end {
-                        let subpattern = Box::new(Pat { span: pat.span, ty, kind });
-                        kind =
-                            PatKind::AscribeUserType { ascription: ascription.clone(), subpattern };
-                    }
-                }
-
-                kind
+                // FIXME?: returning `_` can cause inaccurate "unreachable" warnings. This can be
+                // fixed by returning `PatKind::Const(ConstKind::Error(...))` if #115937 gets
+                // merged.
+                self.lower_pattern_range(lo_expr, hi_expr, end, ty, span).unwrap_or(PatKind::Wild)
             }
 
             hir::PatKind::Path(ref qpath) => {

tests/ui/error-codes/E0030-teach.stderr

@@ -2,7 +2,7 @@ error[E0030]: lower range bound must be less than or equal to upper
   --> $DIR/E0030-teach.rs:5:9
    |
 LL |         1000 ..= 5 => {}
-   |         ^^^^ lower bound larger than upper bound
+   |         ^^^^^^^^^^ lower bound larger than upper bound
    |
    = note: When matching against a range, the compiler verifies that the range is non-empty. Range patterns include both end-points, so this is equivalent to requiring the start of the range to be less than or equal to the end of the range.