Skip to content

Don't panic if the lhs of a div by zero is not statically known #74221

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

Merged
merged 1 commit into from
Jul 16, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
26 changes: 21 additions & 5 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
lint: &'static lint::Lint,
source_info: SourceInfo,
message: &'static str,
panic: AssertKind<ConstInt>,
panic: AssertKind<impl std::fmt::Debug>,
) -> Option<()> {
let lint_root = self.lint_root(source_info)?;
self.tcx.struct_span_lint_hir(lint, lint_root, source_info.span, |lint| {
Expand Down Expand Up @@ -1004,11 +1004,27 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
let expected = ScalarMaybeUninit::from(Scalar::from_bool(*expected));
let value_const = self.ecx.read_scalar(value).unwrap();
if expected != value_const {
enum DbgVal<T> {
Copy link
Member

Choose a reason for hiding this comment

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

This looks like something more generally useful that shouldn't be 5 layers deep inside a private function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's very specifically tailored to this case. I'm sure we could come up with something more general and put it into rustc_data_structures

Val(T),
Underscore,
}
impl<T: std::fmt::Debug> std::fmt::Debug for DbgVal<T> {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Val(val) => val.fmt(fmt),
Self::Underscore => fmt.write_str("_"),
}
}
}
let mut eval_to_int = |op| {
let op = self
.eval_operand(op, source_info)
.expect("if we got here, it must be const");
self.ecx.read_immediate(op).unwrap().to_const_int()
// This can be `None` if the lhs wasn't const propagated and we just
// triggered the assert on the value of the rhs.
match self.eval_operand(op, source_info) {
Some(op) => {
DbgVal::Val(self.ecx.read_immediate(op).unwrap().to_const_int())
}
None => DbgVal::Underscore,
}
};
let msg = match msg {
AssertKind::DivisionByZero(op) => {
Expand Down
11 changes: 7 additions & 4 deletions src/test/ui/const_prop/ice-assert-fail-div-by-zero.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
// check-pass

// compile-flags: --crate-type lib

#![warn(unconditional_panic)]

pub struct Fixed64(i64);

pub fn div(f: Fixed64) {
f.0 / 0;
// HACK: this test passes only because this is a const fn that is written to metadata
pub const fn div(f: Fixed64) {
f.0 / 0; //~ WARN will panic at runtime
}

fn main() {}
14 changes: 14 additions & 0 deletions src/test/ui/const_prop/ice-assert-fail-div-by-zero.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
warning: this operation will panic at runtime
--> $DIR/ice-assert-fail-div-by-zero.rs:11:5
|
LL | f.0 / 0;
| ^^^^^^^ attempt to divide _ by zero
|
note: the lint level is defined here
--> $DIR/ice-assert-fail-div-by-zero.rs:5:9
|
LL | #![warn(unconditional_panic)]
| ^^^^^^^^^^^^^^^^^^^

warning: 1 warning emitted