Auto merge of #33214 - oli-obk:const_err_var_exprs, r=eddyb

report `const_err` on all expressions that can fail

also a drive-by fix for reporting an "overflow in shift *left*" when shifting an `i64` *right*

This increases the warning noise for shifting by more than the bitwidth and for `-T::MIN`. I can silence the bitwidth warnings explicitly and fix the const evaluator to make sure `--$expr` is treated exactly like `$expr` (which is kinda wrong, but mathematically right).

r? @eddyb

Author: bors

src/librustc_const_eval/eval.rs

@@ -562,44 +562,51 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &TyCtxt<'tcx>,
     let result = match e.node {
       hir::ExprUnary(hir::UnNeg, ref inner) => {
         // unary neg literals already got their sign during creation
-        if let hir::ExprLit(ref lit) = inner.node {
-            use syntax::ast::*;
-            use syntax::ast::LitIntType::*;
-            const I8_OVERFLOW: u64 = ::std::i8::MAX as u64 + 1;
-            const I16_OVERFLOW: u64 = ::std::i16::MAX as u64 + 1;
-            const I32_OVERFLOW: u64 = ::std::i32::MAX as u64 + 1;
-            const I64_OVERFLOW: u64 = ::std::i64::MAX as u64 + 1;
-            match (&lit.node, ety.map(|t| &t.sty)) {
-                (&LitKind::Int(I8_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I8))) |
-                (&LitKind::Int(I8_OVERFLOW, Signed(IntTy::I8)), _) => {
-                    return Ok(Integral(I8(::std::i8::MIN)))
-                },
-                (&LitKind::Int(I16_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I16))) |
-                (&LitKind::Int(I16_OVERFLOW, Signed(IntTy::I16)), _) => {
-                    return Ok(Integral(I16(::std::i16::MIN)))
-                },
-                (&LitKind::Int(I32_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I32))) |
-                (&LitKind::Int(I32_OVERFLOW, Signed(IntTy::I32)), _) => {
-                    return Ok(Integral(I32(::std::i32::MIN)))
-                },
-                (&LitKind::Int(I64_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I64))) |
-                (&LitKind::Int(I64_OVERFLOW, Signed(IntTy::I64)), _) => {
-                    return Ok(Integral(I64(::std::i64::MIN)))
-                },
-                (&LitKind::Int(n, Unsuffixed), Some(&ty::TyInt(IntTy::Is))) |
-                (&LitKind::Int(n, Signed(IntTy::Is)), _) => {
-                    match tcx.sess.target.int_type {
-                        IntTy::I32 => if n == I32_OVERFLOW {
-                            return Ok(Integral(Isize(Is32(::std::i32::MIN))));
-                        },
-                        IntTy::I64 => if n == I64_OVERFLOW {
-                            return Ok(Integral(Isize(Is64(::std::i64::MIN))));
-                        },
-                        _ => bug!(),
-                    }
-                },
-                _ => {},
-            }
+        match inner.node {
+            hir::ExprLit(ref lit) => {
+                use syntax::ast::*;
+                use syntax::ast::LitIntType::*;
+                const I8_OVERFLOW: u64 = ::std::i8::MAX as u64 + 1;
+                const I16_OVERFLOW: u64 = ::std::i16::MAX as u64 + 1;
+                const I32_OVERFLOW: u64 = ::std::i32::MAX as u64 + 1;
+                const I64_OVERFLOW: u64 = ::std::i64::MAX as u64 + 1;
+                match (&lit.node, ety.map(|t| &t.sty)) {
+                    (&LitKind::Int(I8_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I8))) |
+                    (&LitKind::Int(I8_OVERFLOW, Signed(IntTy::I8)), _) => {
+                        return Ok(Integral(I8(::std::i8::MIN)))
+                    },
+                    (&LitKind::Int(I16_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I16))) |
+                    (&LitKind::Int(I16_OVERFLOW, Signed(IntTy::I16)), _) => {
+                        return Ok(Integral(I16(::std::i16::MIN)))
+                    },
+                    (&LitKind::Int(I32_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I32))) |
+                    (&LitKind::Int(I32_OVERFLOW, Signed(IntTy::I32)), _) => {
+                        return Ok(Integral(I32(::std::i32::MIN)))
+                    },
+                    (&LitKind::Int(I64_OVERFLOW, Unsuffixed), Some(&ty::TyInt(IntTy::I64))) |
+                    (&LitKind::Int(I64_OVERFLOW, Signed(IntTy::I64)), _) => {
+                        return Ok(Integral(I64(::std::i64::MIN)))
+                    },
+                    (&LitKind::Int(n, Unsuffixed), Some(&ty::TyInt(IntTy::Is))) |
+                    (&LitKind::Int(n, Signed(IntTy::Is)), _) => {
+                        match tcx.sess.target.int_type {
+                            IntTy::I32 => if n == I32_OVERFLOW {
+                                return Ok(Integral(Isize(Is32(::std::i32::MIN))));
+                            },
+                            IntTy::I64 => if n == I64_OVERFLOW {
+                                return Ok(Integral(Isize(Is64(::std::i64::MIN))));
+                            },
+                            _ => bug!(),
+                        }
+                    },
+                    _ => {},
+                }
+            },
+            hir::ExprUnary(hir::UnNeg, ref inner) => {
+                // skip `--$expr`
+                return eval_const_expr_partial(tcx, inner, ty_hint, fn_args);
+            },
+            _ => {},
         }
         match eval_const_expr_partial(tcx, &inner, ty_hint, fn_args)? {
           Float(f) => Float(-f),

src/librustc_const_math/int.rs

@@ -503,7 +503,7 @@ impl ::std::ops::Shr<ConstInt> for ConstInt {
             I8(a) => Ok(I8(overflowing!(a.overflowing_shr(b), Op::Shr))),
             I16(a) => Ok(I16(overflowing!(a.overflowing_shr(b), Op::Shr))),
             I32(a) => Ok(I32(overflowing!(a.overflowing_shr(b), Op::Shr))),
-            I64(a) => Ok(I64(overflowing!(a.overflowing_shr(b), Op::Shl))),
+            I64(a) => Ok(I64(overflowing!(a.overflowing_shr(b), Op::Shr))),
             Isize(Is32(a)) => Ok(Isize(Is32(overflowing!(a.overflowing_shr(b), Op::Shr)))),
             Isize(Is64(a)) => Ok(Isize(Is64(overflowing!(a.overflowing_shr(b), Op::Shr)))),
             U8(a) => Ok(U8(overflowing!(a.overflowing_shr(b), Op::Shr))),

src/librustc_const_math/lib.rs

@@ -40,4 +40,4 @@ mod err;
 pub use int::*;
 pub use us::*;
 pub use is::*;
-pub use err::ConstMathErr;
+pub use err::{ConstMathErr, Op};

src/librustc_passes/Cargo.toml

@@ -12,4 +12,5 @@ crate-type = ["dylib"]
 log = { path = "../liblog" }
 rustc = { path = "../librustc" }
 rustc_const_eval = { path = "../librustc_const_eval" }
+rustc_const_math = { path = "../librustc_const_math" }
 syntax = { path = "../libsyntax" }

src/librustc_passes/consts.rs

@@ -28,9 +28,11 @@ use rustc::dep_graph::DepNode;
 use rustc::ty::cast::{CastKind};
 use rustc_const_eval::{ConstEvalErr, lookup_const_fn_by_id, compare_lit_exprs};
 use rustc_const_eval::{eval_const_expr_partial, lookup_const_by_id};
-use rustc_const_eval::ErrKind::{IndexOpFeatureGated, UnimplementedConstVal};
-use rustc_const_eval::ErrKind::ErroneousReferencedConstant;
+use rustc_const_eval::ErrKind::{IndexOpFeatureGated, UnimplementedConstVal, MiscCatchAll, Math};
+use rustc_const_eval::ErrKind::{ErroneousReferencedConstant, MiscBinaryOp, NonConstPath};
+use rustc_const_eval::ErrKind::UnresolvedPath;
 use rustc_const_eval::EvalHint::ExprTypeChecked;
+use rustc_const_math::{ConstMathErr, Op};
 use rustc::hir::def::Def;
 use rustc::hir::def_id::DefId;
 use rustc::middle::expr_use_visitor as euv;
@@ -437,29 +439,6 @@ impl<'a, 'tcx, 'v> Visitor<'v> for CheckCrateVisitor<'a, 'tcx> {
                 }
                 intravisit::walk_expr(self, ex);
             }
-            // Division by zero and overflow checking.
-            hir::ExprBinary(op, _, _) => {
-                intravisit::walk_expr(self, ex);
-                let div_or_rem = op.node == hir::BiDiv || op.node == hir::BiRem;
-                match node_ty.sty {
-                    ty::TyUint(_) | ty::TyInt(_) if div_or_rem => {
-                        if !self.qualif.intersects(ConstQualif::NOT_CONST) {
-                            match eval_const_expr_partial(
-                                    self.tcx, ex, ExprTypeChecked, None) {
-                                Ok(_) => {}
-                                Err(ConstEvalErr { kind: UnimplementedConstVal(_), ..}) |
-                                Err(ConstEvalErr { kind: IndexOpFeatureGated, ..}) => {},
-                                Err(msg) => {
-                                    self.tcx.sess.add_lint(CONST_ERR, ex.id,
-                                                           msg.span,
-                                                           msg.description().into_owned())
-                                }
-                            }
-                        }
-                    }
-                    _ => {}
-                }
-            }
             _ => intravisit::walk_expr(self, ex)
         }
 
@@ -505,6 +484,27 @@ impl<'a, 'tcx, 'v> Visitor<'v> for CheckCrateVisitor<'a, 'tcx> {
             }
             None => {}
         }
+
+        if self.mode == Mode::Var && !self.qualif.intersects(ConstQualif::NOT_CONST) {
+            match eval_const_expr_partial(self.tcx, ex, ExprTypeChecked, None) {
+                Ok(_) => {}
+                Err(ConstEvalErr { kind: UnimplementedConstVal(_), ..}) |
+                Err(ConstEvalErr { kind: MiscCatchAll, ..}) |
+                Err(ConstEvalErr { kind: MiscBinaryOp, ..}) |
+                Err(ConstEvalErr { kind: NonConstPath, ..}) |
+                Err(ConstEvalErr { kind: UnresolvedPath, ..}) |
+                Err(ConstEvalErr { kind: ErroneousReferencedConstant(_), ..}) |
+                Err(ConstEvalErr { kind: Math(ConstMathErr::Overflow(Op::Shr)), ..}) |
+                Err(ConstEvalErr { kind: Math(ConstMathErr::Overflow(Op::Shl)), ..}) |
+                Err(ConstEvalErr { kind: IndexOpFeatureGated, ..}) => {},
+                Err(msg) => {
+                    self.tcx.sess.add_lint(CONST_ERR, ex.id,
+                                           msg.span,
+                                           msg.description().into_owned())
+                }
+            }
+        }
+
         self.tcx.const_qualif_map.borrow_mut().insert(ex.id, self.qualif);
         // Don't propagate certain flags.
         self.qualif = outer | (self.qualif - ConstQualif::HAS_STATIC_BORROWS);

src/librustc_passes/lib.rs

@@ -30,6 +30,7 @@
 extern crate core;
 #[macro_use] extern crate rustc;
 extern crate rustc_const_eval;
+extern crate rustc_const_math;
 
 #[macro_use] extern crate log;
 #[macro_use] extern crate syntax;

src/test/compile-fail/const-err-early.rs

@@ -18,5 +18,5 @@ pub const D: u8 = 42u8 - (42u8 + 1); //~ ERROR attempted to subtract with overfl
 pub const E: u8 = [5u8][1]; //~ ERROR index out of bounds
 
 fn main() {
-    let _e = [6u8][1];
+    let _e = [6u8][1]; //~ ERROR: array index out of bounds
 }

src/test/compile-fail/const-err.rs

@@ -8,8 +8,12 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// these errors are not actually "const_err", they occur in trans/consts
+// and are unconditional warnings that can't be denied or allowed
+
 #![feature(rustc_attrs)]
 #![allow(exceeding_bitshifts)]
+#![allow(const_err)]
 
 fn black_box<T>(_: T) {
     unimplemented!()
@@ -21,7 +25,7 @@ fn main() {
     //~^ WARN attempted to negate with overflow
     let b = 200u8 + 200u8 + 200u8;
     //~^ WARN attempted to add with overflow
-    //~^^ WARN attempted to add with overflow
+    //~| WARN attempted to add with overflow
     let c = 200u8 * 4;
     //~^ WARN attempted to multiply with overflow
     let d = 42u8 - (42u8 + 1);

src/test/compile-fail/const-err2.rs

@@ -0,0 +1,34 @@
+// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(rustc_attrs)]
+#![allow(exceeding_bitshifts)]
+#![deny(const_err)]
+
+fn black_box<T>(_: T) {
+    unimplemented!()
+}
+
+fn main() {
+    let a = -std::i8::MIN;
+    //~^ ERROR attempted to negate with overflow
+    let b = 200u8 + 200u8 + 200u8;
+    //~^ ERROR attempted to add with overflow
+    //~| ERROR attempted to add with overflow
+    let c = 200u8 * 4;
+    //~^ ERROR attempted to multiply with overflow
+    let d = 42u8 - (42u8 + 1);
+    //~^ ERROR attempted to subtract with overflow
+    let _e = [5u8][1];
+    black_box(a);
+    black_box(b);
+    black_box(c);
+    black_box(d);
+}

src/test/compile-fail/lint-exceeding-bitshifts.rs

@@ -53,6 +53,7 @@ fn main() {
       let n = n << 8; //~ ERROR: bitshift exceeds the type's number of bits
 
       let n = 1u8 << -8; //~ ERROR: bitshift exceeds the type's number of bits
+      //~^ WARN: attempted to shift by a negative amount
 
       let n = 1u8 << (4+3);
       let n = 1u8 << (4+4); //~ ERROR: bitshift exceeds the type's number of bits

src/test/compile-fail/lint-type-overflow2.rs

@@ -10,6 +10,7 @@
 //
 
 #![deny(overflowing_literals)]
+#![deny(const_err)]
 
 #[allow(unused_variables)]
 fn main() {