Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix const_err with -(-0.0) #64063

Merged
merged 7 commits into from
Sep 5, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,13 +405,16 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}

let arg = self.eval_operand(arg, source_info)?;
let oflo_check = self.tcx.sess.overflow_checks();
let val = self.use_ecx(source_info, |this| {
let prim = this.ecx.read_immediate(arg)?;
match op {
UnOp::Neg => {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
JohnTitor marked this conversation as resolved.
Show resolved Hide resolved
// Need to do overflow check here: For actual CTFE, MIR
// generation emits code that does this before calling the op.
if prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) {
// We check overflow in debug mode already
// so should only check in release mode.
if !oflo_check
&& prim.layout.ty.is_signed()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

essentially this condition the only needed thing for the fix. The rest is what's causing the CI failure

&& prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) {
throw_panic!(OverflowNeg)
}
}
Expand Down Expand Up @@ -485,7 +488,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
Scalar::from_bool(overflow).into(),
)
} else {
if overflow {
// We check overflow in debug mode already
// so should only check in release mode.
if !self.tcx.sess.overflow_checks() && overflow {
let err = err_panic!(Overflow(op)).into();
let _: Option<()> = self.use_ecx(source_info, |_| Err(err));
return None;
Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/consts/const-err2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#![feature(rustc_attrs)]
#![allow(exceeding_bitshifts)]

#![deny(const_err)]

fn black_box<T>(_: T) {
Expand All @@ -21,7 +22,7 @@ fn main() {
let d = 42u8 - (42u8 + 1);
//~^ ERROR const_err
let _e = [5u8][1];
//~^ ERROR const_err
//~^ ERROR index out of bounds
black_box(a);
black_box(b);
black_box(c);
Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/consts/const-err2.stderr
Original file line number Diff line number Diff line change
@@ -1,35 +1,35 @@
error: this expression will panic at runtime
--> $DIR/const-err2.rs:15:13
--> $DIR/const-err2.rs:16:13
|
LL | let a = -std::i8::MIN;
| ^^^^^^^^^^^^^ attempt to negate with overflow
|
note: lint level defined here
--> $DIR/const-err2.rs:8:9
--> $DIR/const-err2.rs:9:9
|
LL | #![deny(const_err)]
| ^^^^^^^^^

error: this expression will panic at runtime
--> $DIR/const-err2.rs:17:13
--> $DIR/const-err2.rs:18:13
|
LL | let b = 200u8 + 200u8 + 200u8;
| ^^^^^^^^^^^^^ attempt to add with overflow

error: this expression will panic at runtime
--> $DIR/const-err2.rs:19:13
--> $DIR/const-err2.rs:20:13
|
LL | let c = 200u8 * 4;
| ^^^^^^^^^ attempt to multiply with overflow

error: this expression will panic at runtime
--> $DIR/const-err2.rs:21:13
--> $DIR/const-err2.rs:22:13
|
LL | let d = 42u8 - (42u8 + 1);
| ^^^^^^^^^^^^^^^^^ attempt to subtract with overflow

error: index out of bounds: the len is 1 but the index is 1
--> $DIR/const-err2.rs:23:14
--> $DIR/const-err2.rs:24:14
|
LL | let _e = [5u8][1];
| ^^^^^^^^
Expand Down
30 changes: 30 additions & 0 deletions src/test/ui/consts/const-err3.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// needed because negating int::MIN will behave differently between
// optimized compilation and unoptimized compilation and thus would
// lead to different lints being emitted
// compile-flags: -C overflow-checks=on -O

#![feature(rustc_attrs)]
#![allow(exceeding_bitshifts)]

#![deny(const_err)]

fn black_box<T>(_: T) {
unimplemented!()
}

fn main() {
let a = -std::i8::MIN;
//~^ ERROR const_err
let b = 200u8 + 200u8 + 200u8;
//~^ ERROR const_err
let c = 200u8 * 4;
//~^ ERROR const_err
let d = 42u8 - (42u8 + 1);
//~^ ERROR const_err
let _e = [5u8][1];
//~^ ERROR const_err
black_box(a);
black_box(b);
black_box(c);
black_box(d);
}
38 changes: 38 additions & 0 deletions src/test/ui/consts/const-err3.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
error: attempt to negate with overflow
--> $DIR/const-err3.rs:16:13
|
LL | let a = -std::i8::MIN;
| ^^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/const-err3.rs:9:9
|
LL | #![deny(const_err)]
| ^^^^^^^^^

error: attempt to add with overflow
--> $DIR/const-err3.rs:18:13
|
LL | let b = 200u8 + 200u8 + 200u8;
| ^^^^^^^^^^^^^

error: attempt to multiply with overflow
--> $DIR/const-err3.rs:20:13
|
LL | let c = 200u8 * 4;
| ^^^^^^^^^

error: attempt to subtract with overflow
--> $DIR/const-err3.rs:22:13
|
LL | let d = 42u8 - (42u8 + 1);
| ^^^^^^^^^^^^^^^^^

error: index out of bounds: the len is 1 but the index is 1
--> $DIR/const-err3.rs:24:14
|
LL | let _e = [5u8][1];
| ^^^^^^^^

error: aborting due to 5 previous errors

2 changes: 1 addition & 1 deletion src/test/ui/consts/const-eval/promoted_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
fn main() {
println!("{}", 0u32 - 1);
let _x = 0u32 - 1;
//~^ ERROR this expression will panic at runtime [const_err]
//~^ ERROR const_err
println!("{}", 1/(1-1));
//~^ ERROR attempt to divide by zero [const_err]
//~| ERROR reaching this expression at runtime will panic or abort [const_err]
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/consts/const-eval/promoted_errors2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// compile-flags: -C overflow-checks=on -O

#![deny(const_err)]

fn main() {
println!("{}", 0u32 - 1);
//~^ ERROR attempt to subtract with overflow
let _x = 0u32 - 1;
//~^ ERROR attempt to subtract with overflow
println!("{}", 1/(1-1));
//~^ ERROR attempt to divide by zero [const_err]
//~| ERROR reaching this expression at runtime will panic or abort [const_err]
let _x = 1/(1-1);
//~^ ERROR const_err
//~| ERROR const_err
println!("{}", 1/(false as u32));
//~^ ERROR attempt to divide by zero [const_err]
//~| ERROR reaching this expression at runtime will panic or abort [const_err]
let _x = 1/(false as u32);
//~^ ERROR const_err
//~| ERROR const_err
}
68 changes: 68 additions & 0 deletions src/test/ui/consts/const-eval/promoted_errors2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
error: attempt to subtract with overflow
--> $DIR/promoted_errors2.rs:6:20
|
LL | println!("{}", 0u32 - 1);
| ^^^^^^^^
|
note: lint level defined here
--> $DIR/promoted_errors2.rs:3:9
|
LL | #![deny(const_err)]
| ^^^^^^^^^

error: attempt to subtract with overflow
--> $DIR/promoted_errors2.rs:8:14
|
LL | let _x = 0u32 - 1;
| ^^^^^^^^

error: attempt to divide by zero
--> $DIR/promoted_errors2.rs:10:20
|
LL | println!("{}", 1/(1-1));
| ^^^^^^^

error: reaching this expression at runtime will panic or abort
--> $DIR/promoted_errors2.rs:10:20
|
LL | println!("{}", 1/(1-1));
| ^^^^^^^ attempt to divide by zero

error: attempt to divide by zero
--> $DIR/promoted_errors2.rs:13:14
|
LL | let _x = 1/(1-1);
| ^^^^^^^

error: this expression will panic at runtime
--> $DIR/promoted_errors2.rs:13:14
|
LL | let _x = 1/(1-1);
| ^^^^^^^ attempt to divide by zero

error: attempt to divide by zero
--> $DIR/promoted_errors2.rs:16:20
|
LL | println!("{}", 1/(false as u32));
| ^^^^^^^^^^^^^^^^

error: reaching this expression at runtime will panic or abort
--> $DIR/promoted_errors2.rs:16:20
|
LL | println!("{}", 1/(false as u32));
| ^^^^^^^^^^^^^^^^ attempt to divide by zero

error: attempt to divide by zero
--> $DIR/promoted_errors2.rs:19:14
|
LL | let _x = 1/(false as u32);
| ^^^^^^^^^^^^^^^^

error: this expression will panic at runtime
--> $DIR/promoted_errors2.rs:19:14
|
LL | let _x = 1/(false as u32);
| ^^^^^^^^^^^^^^^^ attempt to divide by zero

error: aborting due to 10 previous errors

6 changes: 6 additions & 0 deletions src/test/ui/consts/issue-64059-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// compile-flags: -C overflow-checks=on -O
// run-pass

fn main() {
let _ = -(-0.0);
}
5 changes: 5 additions & 0 deletions src/test/ui/consts/issue-64059.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// run-pass

fn main() {
let _ = -(-0.0);
}
59 changes: 59 additions & 0 deletions src/test/ui/issues/issue-8460-const2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// compile-flags: -C overflow-checks=on -O
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe noopt overwrites this -O? That would be weird. Also weird that noopt causes fewer diagnostics... I'm not sure what's going on. You can undo the changes unrelated to -(-0.0) if you want so we can get the PR merged and open an issue about not being able to test noopt properly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The weird thing is that it happened in issue-8460-const.rs not issue-8460-const2.rs. issue-8460-const.rs has no compile flags.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. That may explain it... not sure what the best thing to do here is. Maybe instead of copying the files we should use the scheme where one test is built in multiple modes? I forget what the appropriate flags are, but incremental has loads of them

Copy link
Member Author

@JohnTitor JohnTitor Sep 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or could we add the -O flag to issue-8460-const.rs? The other tests we touch have this flag but that doesn't.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that solves it, sure

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the flag to the test, could we run bors try?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can test this locally by setting

#optimize-tests = true
to false in your config.toml

Unfortunately bors try won't run the noopt tests, so that's the only way to test

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't know that.
I created config.toml with optimize-tests = false and ran x.py test src/test/ui --stage=1 --bless --config config.toml, the test seems passed.


#![deny(const_err)]

use std::{isize, i8, i16, i32, i64};
use std::thread;

fn main() {
assert!(thread::spawn(move|| { isize::MIN / -1; }).join().is_err());
//~^ ERROR attempt to divide with overflow
assert!(thread::spawn(move|| { i8::MIN / -1; }).join().is_err());
//~^ ERROR attempt to divide with overflow
assert!(thread::spawn(move|| { i16::MIN / -1; }).join().is_err());
//~^ ERROR attempt to divide with overflow
assert!(thread::spawn(move|| { i32::MIN / -1; }).join().is_err());
//~^ ERROR attempt to divide with overflow
assert!(thread::spawn(move|| { i64::MIN / -1; }).join().is_err());
//~^ ERROR attempt to divide with overflow
assert!(thread::spawn(move|| { 1isize / 0; }).join().is_err());
//~^ ERROR attempt to divide by zero
//~| ERROR this expression will panic at runtime
assert!(thread::spawn(move|| { 1i8 / 0; }).join().is_err());
//~^ ERROR attempt to divide by zero
//~| ERROR this expression will panic at runtime
assert!(thread::spawn(move|| { 1i16 / 0; }).join().is_err());
//~^ ERROR attempt to divide by zero
//~| ERROR this expression will panic at runtime
assert!(thread::spawn(move|| { 1i32 / 0; }).join().is_err());
//~^ ERROR attempt to divide by zero
//~| ERROR this expression will panic at runtime
assert!(thread::spawn(move|| { 1i64 / 0; }).join().is_err());
//~^ ERROR attempt to divide by zero
//~| ERROR this expression will panic at runtime
assert!(thread::spawn(move|| { isize::MIN % -1; }).join().is_err());
//~^ ERROR attempt to calculate the remainder with overflow
assert!(thread::spawn(move|| { i8::MIN % -1; }).join().is_err());
//~^ ERROR attempt to calculate the remainder with overflow
assert!(thread::spawn(move|| { i16::MIN % -1; }).join().is_err());
//~^ ERROR attempt to calculate the remainder with overflow
assert!(thread::spawn(move|| { i32::MIN % -1; }).join().is_err());
//~^ ERROR attempt to calculate the remainder with overflow
assert!(thread::spawn(move|| { i64::MIN % -1; }).join().is_err());
//~^ ERROR attempt to calculate the remainder with overflow
assert!(thread::spawn(move|| { 1isize % 0; }).join().is_err());
//~^ ERROR attempt to calculate the remainder with a divisor of zero
//~| ERROR this expression will panic at runtime
assert!(thread::spawn(move|| { 1i8 % 0; }).join().is_err());
//~^ ERROR attempt to calculate the remainder with a divisor of zero
//~| ERROR this expression will panic at runtime
assert!(thread::spawn(move|| { 1i16 % 0; }).join().is_err());
//~^ ERROR attempt to calculate the remainder with a divisor of zero
//~| ERROR this expression will panic at runtime
assert!(thread::spawn(move|| { 1i32 % 0; }).join().is_err());
//~^ ERROR attempt to calculate the remainder with a divisor of zero
//~| ERROR this expression will panic at runtime
assert!(thread::spawn(move|| { 1i64 % 0; }).join().is_err());
//~^ ERROR attempt to calculate the remainder with a divisor of zero
//~| ERROR this expression will panic at runtime
}
Loading