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

use MIN/MAX constant names in integer pattern coverage messages #57073

Closed
wants to merge 1 commit into from
Closed
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
15 changes: 13 additions & 2 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2624,7 +2624,13 @@ pub fn fmt_const_val(f: &mut impl Write, const_val: &ty::Const<'_>) -> fmt::Resu
Bool if bits == 1 => return write!(f, "true"),
Float(ast::FloatTy::F32) => return write!(f, "{}f32", Single::from_bits(bits)),
Float(ast::FloatTy::F64) => return write!(f, "{}f64", Double::from_bits(bits)),
Uint(ui) => return write!(f, "{:?}{}", bits, ui),
Uint(ui) => {
return match bits {
// writing 0 as uX::MIN wouldn't clarify
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// writing 0 as uX::MIN wouldn't clarify
// Writing `0` as `uX::MIN` wouldn't make the value any clearer,
// so we only special-case the maximum value.

n if n == ui.max_as_u128() => write!(f, "::std::{}::MAX", ui),
_ => write!(f, "{:?}{}", bits, ui)
}
}
Int(i) => {
let bit_width = ty::tls::with(|tcx| {
let ty = tcx.lift_to_global(&ty).unwrap();
Expand All @@ -2634,7 +2640,12 @@ pub fn fmt_const_val(f: &mut impl Write, const_val: &ty::Const<'_>) -> fmt::Resu
.bits()
});
let shift = 128 - bit_width;
return write!(f, "{:?}{}", ((bits as i128) << shift) >> shift, i);
let n = ((bits as i128) << shift) >> shift;
return match n {
m if m == i.min_as_i128() => write!(f, "::std::{}::MIN", i),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
m if m == i.min_as_i128() => write!(f, "::std::{}::MIN", i),
_ if n == i.min_as_i128() => write!(f, "::std::{}::MIN", i),

This is just stylistic, but I think it's clearer what value you're testing if you don't mind n to a different variable name (here and above).

m if m == i.max_as_i128() => write!(f, "::std::{}::MAX", i),
_ => write!(f, "{:?}{}", n, i)
}
}
Char => return write!(f, "{:?}", ::std::char::from_u32(bits as u32).unwrap()),
_ => {}
Expand Down
34 changes: 34 additions & 0 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,29 @@ impl IntTy {
IntTy::I128 => 128,
})
}

pub fn min_as_i128(&self) -> i128 {
Copy link
Member

Choose a reason for hiding this comment

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

You can avoid enumerating the integer types using the bit manipulations that are already used for range pattern matching, e.g.:
https://github.com/rust-lang/rust/blob/96295ad2957a389c7b108b769532ecd7cada9918/src/librustc_mir/hair/pattern/_match.rs#L686-L689

Copy link
Member

Choose a reason for hiding this comment

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

Although these should be refactored as part of #49937 anyway, it probably makes more sense to have these in mir/mod.rs if that's the only place they're used.

match *self {
IntTy::Isize => ::std::isize::MIN as i128,
Copy link
Member

@varkor varkor Dec 23, 2018

Choose a reason for hiding this comment

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

This is going to use the host isize size, rather than the target size. (Using the snippet above will address that.)

IntTy::I8 => ::std::i8::MIN as i128,
IntTy::I16 => ::std::i16::MIN as i128,
IntTy::I32 => ::std::i32::MIN as i128,
IntTy::I64 => ::std::i64::MIN as i128,
IntTy::I128 => ::std::i128::MIN,
}
}

pub fn max_as_i128(&self) -> i128 {
match *self {
IntTy::Isize => ::std::isize::MAX as i128,
IntTy::I8 => ::std::i8::MAX as i128,
IntTy::I16 => ::std::i16::MAX as i128,
IntTy::I32 => ::std::i32::MAX as i128,
IntTy::I64 => ::std::i64::MAX as i128,
IntTy::I128 => ::std::i128::MAX,
}
}

}

#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable, Copy)]
Expand Down Expand Up @@ -1496,6 +1519,17 @@ impl UintTy {
UintTy::U128 => 128,
})
}

pub fn max_as_u128(&self) -> u128 {
Copy link
Member

Choose a reason for hiding this comment

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

match *self {
UintTy::Usize => ::std::usize::MAX as u128,
UintTy::U8 => ::std::u8::MAX as u128,
UintTy::U16 => ::std::u16::MAX as u128,
UintTy::U32 => ::std::u32::MAX as u128,
UintTy::U64 => ::std::u64::MAX as u128,
UintTy::U128 => ::std::u128::MAX,
}
}
}

impl fmt::Debug for UintTy {
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/consts/const-match-check.eval1.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0005]: refutable pattern in local binding: `-2147483648i32..=-1i32` not covered
error[E0005]: refutable pattern in local binding: `::std::i32::MIN..=-1i32` not covered
--> $DIR/const-match-check.rs:35:15
|
LL | A = { let 0 = 0; 0 },
| ^ pattern `-2147483648i32..=-1i32` not covered
| ^ pattern `::std::i32::MIN..=-1i32` not covered

error: aborting due to previous error

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/consts/const-match-check.eval2.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0005]: refutable pattern in local binding: `-2147483648i32..=-1i32` not covered
error[E0005]: refutable pattern in local binding: `::std::i32::MIN..=-1i32` not covered
--> $DIR/const-match-check.rs:41:24
|
LL | let x: [i32; { let 0 = 0; 0 }] = [];
| ^ pattern `-2147483648i32..=-1i32` not covered
| ^ pattern `::std::i32::MIN..=-1i32` not covered

error: aborting due to previous error

Expand Down
16 changes: 8 additions & 8 deletions src/test/ui/consts/const-match-check.matchck.stderr
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
error[E0005]: refutable pattern in local binding: `-2147483648i32..=-1i32` not covered
error[E0005]: refutable pattern in local binding: `::std::i32::MIN..=-1i32` not covered
--> $DIR/const-match-check.rs:14:22
|
LL | const X: i32 = { let 0 = 0; 0 };
| ^ pattern `-2147483648i32..=-1i32` not covered
| ^ pattern `::std::i32::MIN..=-1i32` not covered

error[E0005]: refutable pattern in local binding: `-2147483648i32..=-1i32` not covered
error[E0005]: refutable pattern in local binding: `::std::i32::MIN..=-1i32` not covered
--> $DIR/const-match-check.rs:18:23
|
LL | static Y: i32 = { let 0 = 0; 0 };
| ^ pattern `-2147483648i32..=-1i32` not covered
| ^ pattern `::std::i32::MIN..=-1i32` not covered

error[E0005]: refutable pattern in local binding: `-2147483648i32..=-1i32` not covered
error[E0005]: refutable pattern in local binding: `::std::i32::MIN..=-1i32` not covered
--> $DIR/const-match-check.rs:23:26
|
LL | const X: i32 = { let 0 = 0; 0 };
| ^ pattern `-2147483648i32..=-1i32` not covered
| ^ pattern `::std::i32::MIN..=-1i32` not covered

error[E0005]: refutable pattern in local binding: `-2147483648i32..=-1i32` not covered
error[E0005]: refutable pattern in local binding: `::std::i32::MIN..=-1i32` not covered
--> $DIR/const-match-check.rs:29:26
|
LL | const X: i32 = { let 0 = 0; 0 };
| ^ pattern `-2147483648i32..=-1i32` not covered
| ^ pattern `::std::i32::MIN..=-1i32` not covered

error: aborting due to 4 previous errors

Expand Down
28 changes: 14 additions & 14 deletions src/test/ui/exhaustive_integer_patterns.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ note: lint level defined here
LL | #![deny(unreachable_patterns)]
| ^^^^^^^^^^^^^^^^^^^^

error[E0004]: non-exhaustive patterns: `128u8..=255u8` not covered
error[E0004]: non-exhaustive patterns: `128u8..=::std::u8::MAX` not covered
Copy link
Contributor

Choose a reason for hiding this comment

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

There's quite a lot of syntax in here (..=::) making this a bit hard to read. Could we get away with not including the ::std:: prefix and just using 128u..=u8::MAX? I realize this may not work if the user decides to directly copy&paste this to create an extra range, but it would reduce the noise in the message.

Copy link
Member

@varkor varkor Dec 23, 2018

Choose a reason for hiding this comment

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

I agree that it's quite noisy, but considering that the compiler doesn't currently suggest importing std::u8 if u8::MAX is not found, it could be confusing that it's not possible to directly copy and paste. Maybe we could start with this more verbose version and then cut it down after implementing a suggestion for importing std::u8.

--> $DIR/exhaustive_integer_patterns.rs:38:11
|
LL | match x { //~ ERROR non-exhaustive patterns
| ^ pattern `128u8..=255u8` not covered
| ^ pattern `128u8..=::std::u8::MAX` not covered

error[E0004]: non-exhaustive patterns: `11u8..=19u8`, `31u8..=34u8`, `36u8..=69u8` and 1 more not covered
--> $DIR/exhaustive_integer_patterns.rs:43:11
Expand All @@ -28,53 +28,53 @@ error: unreachable pattern
LL | -2..=20 => {} //~ ERROR unreachable pattern
| ^^^^^^^

error[E0004]: non-exhaustive patterns: `-128i8..=-8i8`, `-6i8`, `121i8..=124i8` and 1 more not covered
error[E0004]: non-exhaustive patterns: `::std::i8::MIN..=-8i8`, `-6i8`, `121i8..=124i8` and 1 more not covered
--> $DIR/exhaustive_integer_patterns.rs:51:11
|
LL | match x { //~ ERROR non-exhaustive patterns
| ^ patterns `-128i8..=-8i8`, `-6i8`, `121i8..=124i8` and 1 more not covered
| ^ patterns `::std::i8::MIN..=-8i8`, `-6i8`, `121i8..=124i8` and 1 more not covered

error[E0004]: non-exhaustive patterns: `-128i8` not covered
error[E0004]: non-exhaustive patterns: `::std::i8::MIN` not covered
--> $DIR/exhaustive_integer_patterns.rs:92:11
|
LL | match 0i8 { //~ ERROR non-exhaustive patterns
| ^^^ pattern `-128i8` not covered
| ^^^ pattern `::std::i8::MIN` not covered

error[E0004]: non-exhaustive patterns: `0i16` not covered
--> $DIR/exhaustive_integer_patterns.rs:100:11
|
LL | match 0i16 { //~ ERROR non-exhaustive patterns
| ^^^^ pattern `0i16` not covered

error[E0004]: non-exhaustive patterns: `128u8..=255u8` not covered
error[E0004]: non-exhaustive patterns: `128u8..=::std::u8::MAX` not covered
--> $DIR/exhaustive_integer_patterns.rs:118:11
|
LL | match 0u8 { //~ ERROR non-exhaustive patterns
| ^^^ pattern `128u8..=255u8` not covered
| ^^^ pattern `128u8..=::std::u8::MAX` not covered

error[E0004]: non-exhaustive patterns: `(0u8, Some(_))` and `(2u8..=255u8, Some(_))` not covered
error[E0004]: non-exhaustive patterns: `(0u8, Some(_))` and `(2u8..=::std::u8::MAX, Some(_))` not covered
--> $DIR/exhaustive_integer_patterns.rs:130:11
|
LL | match (0u8, Some(())) { //~ ERROR non-exhaustive patterns
| ^^^^^^^^^^^^^^^ patterns `(0u8, Some(_))` and `(2u8..=255u8, Some(_))` not covered
| ^^^^^^^^^^^^^^^ patterns `(0u8, Some(_))` and `(2u8..=::std::u8::MAX, Some(_))` not covered

error[E0004]: non-exhaustive patterns: `(126u8..=127u8, false)` not covered
--> $DIR/exhaustive_integer_patterns.rs:135:11
|
LL | match (0u8, true) { //~ ERROR non-exhaustive patterns
| ^^^^^^^^^^^ pattern `(126u8..=127u8, false)` not covered

error[E0004]: non-exhaustive patterns: `340282366920938463463374607431768211455u128` not covered
error[E0004]: non-exhaustive patterns: `::std::u128::MAX` not covered
--> $DIR/exhaustive_integer_patterns.rs:155:11
|
LL | match 0u128 { //~ ERROR non-exhaustive patterns
| ^^^^^ pattern `340282366920938463463374607431768211455u128` not covered
| ^^^^^ pattern `::std::u128::MAX` not covered

error[E0004]: non-exhaustive patterns: `5u128..=340282366920938463463374607431768211455u128` not covered
error[E0004]: non-exhaustive patterns: `5u128..=::std::u128::MAX` not covered
--> $DIR/exhaustive_integer_patterns.rs:159:11
|
LL | match 0u128 { //~ ERROR non-exhaustive patterns
| ^^^^^ pattern `5u128..=340282366920938463463374607431768211455u128` not covered
| ^^^^^ pattern `5u128..=::std::u128::MAX` not covered

error[E0004]: non-exhaustive patterns: `0u128..=3u128` not covered
--> $DIR/exhaustive_integer_patterns.rs:163:11
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0005]: refutable pattern in `for` loop binding: `&-2147483648i32..=0i32` not covered
error[E0005]: refutable pattern in `for` loop binding: `&::std::i32::MIN..=0i32` not covered
--> $DIR/for-loop-refutable-pattern-error-message.rs:12:9
|
LL | for &1 in [1].iter() {} //~ ERROR refutable pattern in `for` loop binding
| ^^ pattern `&-2147483648i32..=0i32` not covered
| ^^ pattern `&::std::i32::MIN..=0i32` not covered

error: aborting due to previous error

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/match/match-non-exhaustive.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0004]: non-exhaustive patterns: `-2147483648i32..=0i32` and `2i32..=2147483647i32` not covered
error[E0004]: non-exhaustive patterns: `::std::i32::MIN..=0i32` and `2i32..=::std::i32::MAX` not covered
--> $DIR/match-non-exhaustive.rs:12:11
|
LL | match 0 { 1 => () } //~ ERROR non-exhaustive patterns
| ^ patterns `-2147483648i32..=0i32` and `2i32..=2147483647i32` not covered
| ^ patterns `::std::i32::MIN..=0i32` and `2i32..=::std::i32::MAX` not covered

error[E0004]: non-exhaustive patterns: `_` not covered
--> $DIR/match-non-exhaustive.rs:13:11
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/non-exhaustive/non-exhaustive-match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ fn main() {
match Some(10) { //~ ERROR non-exhaustive patterns: `Some(_)` not covered
None => {}
}
match (2, 3, 4) { //~ ERROR non-exhaustive patterns: `(_, _, -2147483648i32..=3i32)`
// and `(_, _, 5i32..=2147483647i32)` not covered
match (2, 3, 4) { //~ ERROR non-exhaustive patterns: `(_, _, ::std::i32::MIN..=3i32)`
// and `(_, _, 5i32..=::std::i32::MAX)` not covered
(_, _, 4) => {}
}
match (t::a, t::a) { //~ ERROR non-exhaustive patterns: `(a, a)` not covered
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/non-exhaustive/non-exhaustive-match.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ error[E0004]: non-exhaustive patterns: `Some(_)` not covered
LL | match Some(10) { //~ ERROR non-exhaustive patterns: `Some(_)` not covered
| ^^^^^^^^ pattern `Some(_)` not covered

error[E0004]: non-exhaustive patterns: `(_, _, -2147483648i32..=3i32)` and `(_, _, 5i32..=2147483647i32)` not covered
error[E0004]: non-exhaustive patterns: `(_, _, ::std::i32::MIN..=3i32)` and `(_, _, 5i32..=::std::i32::MAX)` not covered
--> $DIR/non-exhaustive-match.rs:25:11
|
LL | match (2, 3, 4) { //~ ERROR non-exhaustive patterns: `(_, _, -2147483648i32..=3i32)`
| ^^^^^^^^^ patterns `(_, _, -2147483648i32..=3i32)` and `(_, _, 5i32..=2147483647i32)` not covered
LL | match (2, 3, 4) { //~ ERROR non-exhaustive patterns: `(_, _, ::std::i32::MIN..=3i32)`
| ^^^^^^^^^ patterns `(_, _, ::std::i32::MIN..=3i32)` and `(_, _, 5i32..=::std::i32::MAX)` not covered

error[E0004]: non-exhaustive patterns: `(a, a)` not covered
--> $DIR/non-exhaustive-match.rs:29:11
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/precise_pointer_size_matching.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
error[E0004]: non-exhaustive patterns: `$ISIZE_MIN..=-6isize` and `21isize..=$ISIZE_MAX` not covered
error[E0004]: non-exhaustive patterns: `::std::isize::MIN..=-6isize` and `21isize..=::std::isize::MAX` not covered
--> $DIR/precise_pointer_size_matching.rs:24:11
|
LL | match 0isize { //~ ERROR non-exhaustive patterns
| ^^^^^^ patterns `$ISIZE_MIN..=-6isize` and `21isize..=$ISIZE_MAX` not covered
| ^^^^^^ patterns `::std::isize::MIN..=-6isize` and `21isize..=::std::isize::MAX` not covered

error[E0004]: non-exhaustive patterns: `0usize` and `21usize..=$USIZE_MAX` not covered
error[E0004]: non-exhaustive patterns: `0usize` and `21usize..=::std::usize::MAX` not covered
--> $DIR/precise_pointer_size_matching.rs:29:11
|
LL | match 0usize { //~ ERROR non-exhaustive patterns
| ^^^^^^ patterns `0usize` and `21usize..=$USIZE_MAX` not covered
| ^^^^^^ patterns `0usize` and `21usize..=::std::usize::MAX` not covered

error: aborting due to 2 previous errors

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/refutable-pattern-errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ fn func((1, (Some(1), 2..=3)): (isize, (Option<isize>, isize))) { }

fn main() {
let (1, (Some(1), 2..=3)) = (1, (None, 2));
//~^ ERROR refutable pattern in local binding: `(-2147483648i32..=0i32, _)` not covered
//~^ ERROR refutable pattern in local binding: `(::std::i32::MIN..=0i32, _)` not covered
}
4 changes: 2 additions & 2 deletions src/test/ui/refutable-pattern-errors.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ error[E0005]: refutable pattern in function argument: `(_, _)` not covered
LL | fn func((1, (Some(1), 2..=3)): (isize, (Option<isize>, isize))) { }
| ^^^^^^^^^^^^^^^^^^^^^ pattern `(_, _)` not covered

error[E0005]: refutable pattern in local binding: `(-2147483648i32..=0i32, _)` not covered
error[E0005]: refutable pattern in local binding: `(::std::i32::MIN..=0i32, _)` not covered
--> $DIR/refutable-pattern-errors.rs:16:9
|
LL | let (1, (Some(1), 2..=3)) = (1, (None, 2));
| ^^^^^^^^^^^^^^^^^^^^^ pattern `(-2147483648i32..=0i32, _)` not covered
| ^^^^^^^^^^^^^^^^^^^^^ pattern `(::std::i32::MIN..=0i32, _)` not covered

error: aborting due to 2 previous errors

Expand Down