Auto merge of rust-lang#73513 - oli-obk:const_binop_overflow, r=estebank

Show the values and computation that would overflow a const evaluation or propagation

Fixes rust-lang#71134

In contrast to the example in the issue it doesn't use individual spans for each operand. The effort required to implement that is quite high compared to the little (if at all) benefit it would bring to diagnostics.

cc @shepmaster

The way this is implemented it is also fairly easy to do the same for overflow panics at runtime, but that should be done in a separate PR since it may have runtime performance implications.

Author: bors

src/librustc_codegen_ssa/mir/block.rs

@@ -380,7 +380,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         // checked operation, just a comparison with the minimum
         // value, so we have to check for the assert message.
         if !bx.check_overflow() {
-            if let AssertKind::OverflowNeg = *msg {
+            if let AssertKind::OverflowNeg(_) = *msg {
                 const_cond = Some(expected);
             }
         }

src/librustc_middle/mir/mod.rs

@@ -2,6 +2,8 @@
 //!
 //! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/mir/index.html
 
+// ignore-tidy-filelength
+
 use crate::mir::interpret::{GlobalAlloc, Scalar};
 use crate::mir::visit::MirVisitable;
 use crate::ty::adjustment::PointerCast;
@@ -1246,10 +1248,10 @@ pub enum TerminatorKind<'tcx> {
 #[derive(Clone, RustcEncodable, RustcDecodable, HashStable, PartialEq)]
 pub enum AssertKind<O> {
     BoundsCheck { len: O, index: O },
-    Overflow(BinOp),
-    OverflowNeg,
-    DivisionByZero,
-    RemainderByZero,
+    Overflow(BinOp, O, O),
+    OverflowNeg(O),
+    DivisionByZero(O),
+    RemainderByZero(O),
     ResumedAfterReturn(GeneratorKind),
     ResumedAfterPanic(GeneratorKind),
 }
@@ -1522,17 +1524,17 @@ impl<O> AssertKind<O> {
     pub fn description(&self) -> &'static str {
         use AssertKind::*;
         match self {
-            Overflow(BinOp::Add) => "attempt to add with overflow",
-            Overflow(BinOp::Sub) => "attempt to subtract with overflow",
-            Overflow(BinOp::Mul) => "attempt to multiply with overflow",
-            Overflow(BinOp::Div) => "attempt to divide with overflow",
-            Overflow(BinOp::Rem) => "attempt to calculate the remainder with overflow",
-            OverflowNeg => "attempt to negate with overflow",
-            Overflow(BinOp::Shr) => "attempt to shift right with overflow",
-            Overflow(BinOp::Shl) => "attempt to shift left with overflow",
-            Overflow(op) => bug!("{:?} cannot overflow", op),
-            DivisionByZero => "attempt to divide by zero",
-            RemainderByZero => "attempt to calculate the remainder with a divisor of zero",
+            Overflow(BinOp::Add, _, _) => "attempt to add with overflow",
+            Overflow(BinOp::Sub, _, _) => "attempt to subtract with overflow",
+            Overflow(BinOp::Mul, _, _) => "attempt to multiply with overflow",
+            Overflow(BinOp::Div, _, _) => "attempt to divide with overflow",
+            Overflow(BinOp::Rem, _, _) => "attempt to calculate the remainder with overflow",
+            OverflowNeg(_) => "attempt to negate with overflow",
+            Overflow(BinOp::Shr, _, _) => "attempt to shift right with overflow",
+            Overflow(BinOp::Shl, _, _) => "attempt to shift left with overflow",
+            Overflow(op, _, _) => bug!("{:?} cannot overflow", op),
+            DivisionByZero(_) => "attempt to divide by zero",
+            RemainderByZero(_) => "attempt to calculate the remainder with a divisor of zero",
             ResumedAfterReturn(GeneratorKind::Gen) => "generator resumed after completion",
             ResumedAfterReturn(GeneratorKind::Async(_)) => "`async fn` resumed after completion",
             ResumedAfterPanic(GeneratorKind::Gen) => "generator resumed after panicking",
@@ -1546,12 +1548,54 @@ impl<O> AssertKind<O> {
     where
         O: Debug,
     {
+        use AssertKind::*;
         match self {
-            AssertKind::BoundsCheck { ref len, ref index } => write!(
+            BoundsCheck { ref len, ref index } => write!(
                 f,
                 "\"index out of bounds: the len is {{}} but the index is {{}}\", {:?}, {:?}",
                 len, index
             ),
+
+            OverflowNeg(op) => {
+                write!(f, "\"attempt to negate {{}} which would overflow\", {:?}", op)
+            }
+            DivisionByZero(op) => write!(f, "\"attempt to divide {{}} by zero\", {:?}", op),
+            RemainderByZero(op) => write!(
+                f,
+                "\"attempt to calculate the remainder of {{}} with a divisor of zero\", {:?}",
+                op
+            ),
+            Overflow(BinOp::Add, l, r) => write!(
+                f,
+                "\"attempt to compute `{{}} + {{}}` which would overflow\", {:?}, {:?}",
+                l, r
+            ),
+            Overflow(BinOp::Sub, l, r) => write!(
+                f,
+                "\"attempt to compute `{{}} - {{}}` which would overflow\", {:?}, {:?}",
+                l, r
+            ),
+            Overflow(BinOp::Mul, l, r) => write!(
+                f,
+                "\"attempt to compute `{{}} * {{}}` which would overflow\", {:?}, {:?}",
+                l, r
+            ),
+            Overflow(BinOp::Div, l, r) => write!(
+                f,
+                "\"attempt to compute `{{}} / {{}}` which would overflow\", {:?}, {:?}",
+                l, r
+            ),
+            Overflow(BinOp::Rem, l, r) => write!(
+                f,
+                "\"attempt to compute the remainder of `{{}} % {{}}` which would overflow\", {:?}, {:?}",
+                l, r
+            ),
+            Overflow(BinOp::Shr, _, r) => {
+                write!(f, "\"attempt to shift right by {{}} which would overflow\", {:?}", r)
+            }
+            Overflow(BinOp::Shl, _, r) => {
+                write!(f, "\"attempt to shift left by {{}} which would overflow\", {:?}", r)
+            }
             _ => write!(f, "\"{}\"", self.description()),
         }
     }
@@ -1564,6 +1608,34 @@ impl<O: fmt::Debug> fmt::Debug for AssertKind<O> {
             BoundsCheck { ref len, ref index } => {
                 write!(f, "index out of bounds: the len is {:?} but the index is {:?}", len, index)
             }
+            OverflowNeg(op) => write!(f, "attempt to negate {:#?} which would overflow", op),
+            DivisionByZero(op) => write!(f, "attempt to divide {:#?} by zero", op),
+            RemainderByZero(op) => {
+                write!(f, "attempt to calculate the remainder of {:#?} with a divisor of zero", op)
+            }
+            Overflow(BinOp::Add, l, r) => {
+                write!(f, "attempt to compute `{:#?} + {:#?}` which would overflow", l, r)
+            }
+            Overflow(BinOp::Sub, l, r) => {
+                write!(f, "attempt to compute `{:#?} - {:#?}` which would overflow", l, r)
+            }
+            Overflow(BinOp::Mul, l, r) => {
+                write!(f, "attempt to compute `{:#?} * {:#?}` which would overflow", l, r)
+            }
+            Overflow(BinOp::Div, l, r) => {
+                write!(f, "attempt to compute `{:#?} / {:#?}` which would overflow", l, r)
+            }
+            Overflow(BinOp::Rem, l, r) => write!(
+                f,
+                "attempt to compute the remainder of `{:#?} % {:#?}` which would overflow",
+                l, r
+            ),
+            Overflow(BinOp::Shr, _, r) => {
+                write!(f, "attempt to shift right by {:#?} which would overflow", r)
+            }
+            Overflow(BinOp::Shl, _, r) => {
+                write!(f, "attempt to shift left by {:#?} which would overflow", r)
+            }
             _ => write!(f, "{}", self.description()),
         }
     }

src/librustc_middle/mir/type_foldable.rs

@@ -58,15 +58,14 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
             Assert { ref cond, expected, ref msg, target, cleanup } => {
                 use AssertKind::*;
                 let msg = match msg {
-                    BoundsCheck { ref len, ref index } => {
+                    BoundsCheck { len, index } => {
                         BoundsCheck { len: len.fold_with(folder), index: index.fold_with(folder) }
                     }
-                    Overflow(_)
-                    | OverflowNeg
-                    | DivisionByZero
-                    | RemainderByZero
-                    | ResumedAfterReturn(_)
-                    | ResumedAfterPanic(_) => msg.clone(),
+                    Overflow(op, l, r) => Overflow(*op, l.fold_with(folder), r.fold_with(folder)),
+                    OverflowNeg(op) => OverflowNeg(op.fold_with(folder)),
+                    DivisionByZero(op) => DivisionByZero(op.fold_with(folder)),
+                    RemainderByZero(op) => RemainderByZero(op.fold_with(folder)),
+                    ResumedAfterReturn(_) | ResumedAfterPanic(_) => msg.clone(),
                 };
                 Assert { cond: cond.fold_with(folder), expected, msg, target, cleanup }
             }
@@ -117,12 +116,11 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
                         BoundsCheck { ref len, ref index } => {
                             len.visit_with(visitor) || index.visit_with(visitor)
                         }
-                        Overflow(_)
-                        | OverflowNeg
-                        | DivisionByZero
-                        | RemainderByZero
-                        | ResumedAfterReturn(_)
-                        | ResumedAfterPanic(_) => false,
+                        Overflow(_, l, r) => l.visit_with(visitor) || r.visit_with(visitor),
+                        OverflowNeg(op) | DivisionByZero(op) | RemainderByZero(op) => {
+                            op.visit_with(visitor)
+                        }
+                        ResumedAfterReturn(_) | ResumedAfterPanic(_) => false,
                     }
                 } else {
                     false

src/librustc_middle/mir/visit.rs

@@ -571,7 +571,13 @@ macro_rules! make_mir_visitor {
                         self.visit_operand(len, location);
                         self.visit_operand(index, location);
                     }
-                    Overflow(_) | OverflowNeg | DivisionByZero | RemainderByZero |
+                    Overflow(_, l, r) => {
+                        self.visit_operand(l, location);
+                        self.visit_operand(r, location);
+                    }
+                    OverflowNeg(op) | DivisionByZero(op) | RemainderByZero(op) => {
+                        self.visit_operand(op, location);
+                    }
                     ResumedAfterReturn(_) | ResumedAfterPanic(_) => {
                         // Nothing to visit
                     }

src/librustc_middle/ty/consts.rs

@@ -0,0 +1,111 @@
+use crate::mir::interpret::truncate;
+use rustc_target::abi::Size;
+
+#[derive(Copy, Clone)]
+/// A type for representing any integer. Only used for printing.
+// FIXME: Use this for the integer-tree representation needed for type level ints and
+// const generics?
+pub struct ConstInt {
+    /// Number of bytes of the integer. Only 1, 2, 4, 8, 16 are legal values.
+    size: u8,
+    /// Whether the value is of a signed integer type.
+    signed: bool,
+    /// Whether the value is a `usize` or `isize` type.
+    is_ptr_sized_integral: bool,
+    /// Raw memory of the integer. All bytes beyond the `size` are unused and must be zero.
+    raw: u128,
+}
+
+impl ConstInt {
+    pub fn new(raw: u128, size: Size, signed: bool, is_ptr_sized_integral: bool) -> Self {
+        assert!(raw <= truncate(u128::MAX, size));
+        Self { raw, size: size.bytes() as u8, signed, is_ptr_sized_integral }
+    }
+}
+
+impl std::fmt::Debug for ConstInt {
+    fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let Self { size, signed, raw, is_ptr_sized_integral } = *self;
+        if signed {
+            let bit_size = size * 8;
+            let min = 1u128 << (bit_size - 1);
+            let max = min - 1;
+            if raw == min {
+                match (size, is_ptr_sized_integral) {
+                    (_, true) => write!(fmt, "isize::MIN"),
+                    (1, _) => write!(fmt, "i8::MIN"),
+                    (2, _) => write!(fmt, "i16::MIN"),
+                    (4, _) => write!(fmt, "i32::MIN"),
+                    (8, _) => write!(fmt, "i64::MIN"),
+                    (16, _) => write!(fmt, "i128::MIN"),
+                    _ => bug!("ConstInt 0x{:x} with size = {} and signed = {}", raw, size, signed),
+                }
+            } else if raw == max {
+                match (size, is_ptr_sized_integral) {
+                    (_, true) => write!(fmt, "isize::MAX"),
+                    (1, _) => write!(fmt, "i8::MAX"),
+                    (2, _) => write!(fmt, "i16::MAX"),
+                    (4, _) => write!(fmt, "i32::MAX"),
+                    (8, _) => write!(fmt, "i64::MAX"),
+                    (16, _) => write!(fmt, "i128::MAX"),
+                    _ => bug!("ConstInt 0x{:x} with size = {} and signed = {}", raw, size, signed),
+                }
+            } else {
+                match size {
+                    1 => write!(fmt, "{}", raw as i8)?,
+                    2 => write!(fmt, "{}", raw as i16)?,
+                    4 => write!(fmt, "{}", raw as i32)?,
+                    8 => write!(fmt, "{}", raw as i64)?,
+                    16 => write!(fmt, "{}", raw as i128)?,
+                    _ => bug!("ConstInt 0x{:x} with size = {} and signed = {}", raw, size, signed),
+                }
+                if fmt.alternate() {
+                    match (size, is_ptr_sized_integral) {
+                        (_, true) => write!(fmt, "_isize")?,
+                        (1, _) => write!(fmt, "_i8")?,
+                        (2, _) => write!(fmt, "_i16")?,
+                        (4, _) => write!(fmt, "_i32")?,
+                        (8, _) => write!(fmt, "_i64")?,
+                        (16, _) => write!(fmt, "_i128")?,
+                        _ => bug!(),
+                    }
+                }
+                Ok(())
+            }
+        } else {
+            let max = truncate(u128::MAX, Size::from_bytes(size));
+            if raw == max {
+                match (size, is_ptr_sized_integral) {
+                    (_, true) => write!(fmt, "usize::MAX"),
+                    (1, _) => write!(fmt, "u8::MAX"),
+                    (2, _) => write!(fmt, "u16::MAX"),
+                    (4, _) => write!(fmt, "u32::MAX"),
+                    (8, _) => write!(fmt, "u64::MAX"),
+                    (16, _) => write!(fmt, "u128::MAX"),
+                    _ => bug!("ConstInt 0x{:x} with size = {} and signed = {}", raw, size, signed),
+                }
+            } else {
+                match size {
+                    1 => write!(fmt, "{}", raw as u8)?,
+                    2 => write!(fmt, "{}", raw as u16)?,
+                    4 => write!(fmt, "{}", raw as u32)?,
+                    8 => write!(fmt, "{}", raw as u64)?,
+                    16 => write!(fmt, "{}", raw as u128)?,
+                    _ => bug!("ConstInt 0x{:x} with size = {} and signed = {}", raw, size, signed),
+                }
+                if fmt.alternate() {
+                    match (size, is_ptr_sized_integral) {
+                        (_, true) => write!(fmt, "_usize")?,
+                        (1, _) => write!(fmt, "_u8")?,
+                        (2, _) => write!(fmt, "_u16")?,
+                        (4, _) => write!(fmt, "_u32")?,
+                        (8, _) => write!(fmt, "_u64")?,
+                        (16, _) => write!(fmt, "_u128")?,
+                        _ => bug!(),
+                    }
+                }
+                Ok(())
+            }
+        }
+    }
+}

src/librustc_middle/ty/mod.rs

@@ -84,6 +84,8 @@ pub use self::trait_def::TraitDef;
 
 pub use self::query::queries;
 
+pub use self::consts::ConstInt;
+
 pub mod adjustment;
 pub mod binding;
 pub mod cast;
@@ -108,6 +110,7 @@ pub mod trait_def;
 pub mod util;
 pub mod walk;
 
+mod consts;
 mod context;
 mod diagnostics;
 mod instance;

src/librustc_middle/ty/print/pretty.rs

@@ -1,10 +1,8 @@
 use crate::middle::cstore::{ExternCrate, ExternCrateSource};
-use crate::mir::interpret::{
-    sign_extend, truncate, AllocId, ConstValue, GlobalAlloc, Pointer, Scalar,
-};
+use crate::mir::interpret::{AllocId, ConstValue, GlobalAlloc, Pointer, Scalar};
 use crate::ty::layout::IntegerExt;
 use crate::ty::subst::{GenericArg, GenericArgKind, Subst};
-use crate::ty::{self, DefIdTree, ParamConst, Ty, TyCtxt, TypeFoldable};
+use crate::ty::{self, ConstInt, DefIdTree, ParamConst, Ty, TyCtxt, TypeFoldable};
 use rustc_apfloat::ieee::{Double, Single};
 use rustc_apfloat::Float;
 use rustc_ast::ast;
@@ -981,35 +979,14 @@ pub trait PrettyPrinter<'tcx>:
             }
             // Int
             (Scalar::Raw { data, .. }, ty::Uint(ui)) => {
-                let bit_size = Integer::from_attr(&self.tcx(), UnsignedInt(*ui)).size();
-                let max = truncate(u128::MAX, bit_size);
-
-                let ui_str = ui.name_str();
-                if data == max {
-                    p!(write("{}::MAX", ui_str))
-                } else {
-                    if print_ty { p!(write("{}{}", data, ui_str)) } else { p!(write("{}", data)) }
-                };
+                let size = Integer::from_attr(&self.tcx(), UnsignedInt(*ui)).size();
+                let int = ConstInt::new(data, size, false, ty.is_ptr_sized_integral());
+                if print_ty { p!(write("{:#?}", int)) } else { p!(write("{:?}", int)) }
             }
             (Scalar::Raw { data, .. }, ty::Int(i)) => {
                 let size = Integer::from_attr(&self.tcx(), SignedInt(*i)).size();
-                let bit_size = size.bits() as u128;
-                let min = 1u128 << (bit_size - 1);
-                let max = min - 1;
-
-                let i_str = i.name_str();
-                match data {
-                    d if d == min => p!(write("{}::MIN", i_str)),
-                    d if d == max => p!(write("{}::MAX", i_str)),
-                    _ => {
-                        let data = sign_extend(data, size) as i128;
-                        if print_ty {
-                            p!(write("{}{}", data, i_str))
-                        } else {
-                            p!(write("{}", data))
-                        }
-                    }
-                }
+                let int = ConstInt::new(data, size, true, ty.is_ptr_sized_integral());
+                if print_ty { p!(write("{:#?}", int)) } else { p!(write("{:?}", int)) }
             }
             // Char
             (Scalar::Raw { data, .. }, ty::Char) if char::from_u32(data as u32).is_some() => {