-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Exhaustive integer matching #50912
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
Exhaustive integer matching #50912
Changes from 1 commit
e3357d9
b3d2baf
384db4f
ed5a4d5
b8702a0
121fa8d
7476ba4
9778a81
a20cb10
7f72030
c00fd8f
7695bd0
a553fa7
8389972
f4af3b0
a9f2c5a
effb3d0
c388c11
97a032e
be12b24
72cc4bd
1aa7494
732d638
6c21a03
d27c21c
07064de
25ba911
af366b0
bfc0807
4aa929c
5959a35
bfc8ce3
99754ad
400cb14
9e9e023
527cccb
e9c8361
1dbc781
0383539
798b9ff
87463c3
6e8a625
c421af9
61b6363
6a957e1
dec5563
6971c5d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -483,11 +483,11 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, | |
ty::TyUint(_) if exhaustive_integer_patterns => { | ||
// FIXME(49937): refactor these bit manipulations into interpret. | ||
let bits = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(pcx.ty)) | ||
.unwrap().size.bits() as u32; | ||
let max = (!0u128).wrapping_shr(128 - bits); | ||
.unwrap().size.bits() as u128; | ||
let max = !0u128 >> (128 - bits); | ||
value_constructors = true; | ||
vec![ConstantRange(ty::Const::from_bits(cx.tcx, 0u128, pcx.ty), | ||
ty::Const::from_bits(cx.tcx, max as u128, pcx.ty), | ||
vec![ConstantRange(ty::Const::from_bits(cx.tcx, 0, pcx.ty), | ||
ty::Const::from_bits(cx.tcx, max, pcx.ty), | ||
RangeEnd::Included)] | ||
} | ||
_ => { | ||
|
@@ -604,21 +604,21 @@ fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( | |
} | ||
|
||
/// An inclusive interval, used for precise integer exhaustiveness checking. | ||
/// `Interval`s always store a contiguous range of integers. This means that | ||
/// signed values are encoded by offsetting them such that `0` represents the | ||
/// minimum value for the integer, regardless of sign. | ||
/// For example, the range `-128...127` is encoded as `0...255`. | ||
/// `IntRange`s always store a contiguous range. This means that values are | ||
/// encoded such that `0` encodes the minimum value for the integer, | ||
/// regardless of the signedness. | ||
/// For example, the pattern `-128...127i8` is encoded as `0..=255`. | ||
/// This makes comparisons and arithmetic on interval endpoints much more | ||
/// straightforward. See `offset_sign` for the conversion technique. | ||
struct Interval<'tcx> { | ||
/// straightforward. See `encode` and `decode` for details. | ||
struct IntRange<'tcx> { | ||
pub range: RangeInclusive<u128>, | ||
pub ty: Ty<'tcx>, | ||
} | ||
|
||
impl<'tcx> Interval<'tcx> { | ||
impl<'tcx> IntRange<'tcx> { | ||
fn from_ctor(tcx: TyCtxt<'_, 'tcx, 'tcx>, | ||
ctor: &Constructor<'tcx>) | ||
-> Option<Interval<'tcx>> { | ||
-> Option<IntRange<'tcx>> { | ||
match ctor { | ||
ConstantRange(lo, hi, end) => { | ||
assert_eq!(lo.ty, hi.ty); | ||
|
@@ -627,13 +627,13 @@ impl<'tcx> Interval<'tcx> { | |
if let Some(hi) = hi.assert_bits(ty) { | ||
// Perform a shift if the underlying types are signed, | ||
// which makes the interval arithmetic simpler. | ||
let (lo, hi) = Self::offset_sign(tcx, ty, lo..=hi, true); | ||
let (lo, hi) = Self::encode(tcx, ty, lo..=hi); | ||
// Make sure the interval is well-formed. | ||
return if lo > hi || lo == hi && *end == RangeEnd::Excluded { | ||
None | ||
} else { | ||
let offset = (*end == RangeEnd::Excluded) as u128; | ||
Some(Interval { range: lo..=(hi - offset), ty }) | ||
Some(IntRange { range: lo..=(hi - offset), ty }) | ||
}; | ||
} | ||
} | ||
|
@@ -642,8 +642,8 @@ impl<'tcx> Interval<'tcx> { | |
ConstantValue(val) => { | ||
let ty = val.ty; | ||
if let Some(val) = val.assert_bits(ty) { | ||
let (lo, hi) = Self::offset_sign(tcx, ty, val..=val, true); | ||
Some(Interval { range: lo..=hi, ty }) | ||
let (lo, hi) = Self::encode(tcx, ty, val..=val); | ||
Some(IntRange { range: lo..=hi, ty }) | ||
} else { | ||
None | ||
} | ||
|
@@ -654,11 +654,11 @@ impl<'tcx> Interval<'tcx> { | |
} | ||
} | ||
|
||
fn offset_sign(tcx: TyCtxt<'_, 'tcx, 'tcx>, | ||
ty: Ty<'tcx>, | ||
range: RangeInclusive<u128>, | ||
encode: bool) | ||
-> (u128, u128) { | ||
fn convert(tcx: TyCtxt<'_, 'tcx, 'tcx>, | ||
ty: Ty<'tcx>, | ||
range: RangeInclusive<u128>, | ||
encode: bool) | ||
-> (u128, u128) { | ||
// We ensure that all integer values are contiguous: that is, that their | ||
// minimum value is represented by 0, so that comparisons and increments/ | ||
// decrements on interval endpoints work consistently whether the endpoints | ||
|
@@ -670,13 +670,14 @@ impl<'tcx> Interval<'tcx> { | |
let bits = tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) | ||
.unwrap().size.bits() as u128; | ||
let min = 1u128 << (bits - 1); | ||
let shift = 1u128.overflowing_shl(bits as u32); | ||
let mask = shift.0.wrapping_sub(1 + (shift.1 as u128)); | ||
let mask = !0u128 >> (128 - bits); | ||
if encode { | ||
let offset = |x: u128| x.wrapping_sub(min) & mask; | ||
(offset(lo), offset(hi)) | ||
} else { | ||
let offset = |x: u128| { | ||
// FIXME: this shouldn't be necessary once `print_miri_value` | ||
// sign-extends `TyInt`. | ||
interpret::sign_extend(tcx, x.wrapping_add(min) & mask, ty) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't want a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW this produces "wrong" values, that could result in weird behavior in miri. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I'm fixing it. |
||
.expect("layout error for TyInt") | ||
}; | ||
|
@@ -686,10 +687,24 @@ impl<'tcx> Interval<'tcx> { | |
ty::TyUint(_) | ty::TyChar => { | ||
(lo, hi) | ||
} | ||
_ => bug!("`Interval` should only contain integer types") | ||
_ => bug!("`IntRange` should only contain integer types") | ||
} | ||
} | ||
|
||
fn encode(tcx: TyCtxt<'_, 'tcx, 'tcx>, | ||
ty: Ty<'tcx>, | ||
range: RangeInclusive<u128>) | ||
-> (u128, u128) { | ||
Self::convert(tcx, ty, range, true) | ||
} | ||
|
||
fn decode(tcx: TyCtxt<'_, 'tcx, 'tcx>, | ||
ty: Ty<'tcx>, | ||
range: RangeInclusive<u128>) | ||
-> (u128, u128) { | ||
Self::convert(tcx, ty, range, false) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be nice to have two separate signatures (i.e. |
||
} | ||
|
||
fn into_inner(self) -> (u128, u128) { | ||
self.range.into_inner() | ||
} | ||
|
@@ -702,10 +717,10 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, | |
pat_ctor: &Constructor<'tcx>, | ||
ranges: Vec<Constructor<'tcx>>) | ||
-> Vec<Constructor<'tcx>> { | ||
if let Some(pat_interval) = Interval::from_ctor(cx.tcx, pat_ctor) { | ||
if let Some(pat_interval) = IntRange::from_ctor(cx.tcx, pat_ctor) { | ||
let mut remaining_ranges = vec![]; | ||
let mut ranges: Vec<_> = ranges.into_iter().filter_map(|r| { | ||
Interval::from_ctor(cx.tcx, &r).map(|i| i.into_inner()) | ||
IntRange::from_ctor(cx.tcx, &r).map(|i| i.into_inner()) | ||
}).collect(); | ||
let ty = pat_interval.ty; | ||
let (pat_interval_lo, pat_interval_hi) = pat_interval.into_inner(); | ||
|
@@ -729,7 +744,7 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, | |
} | ||
// Convert the remaining ranges from pairs to inclusive `ConstantRange`s. | ||
remaining_ranges.into_iter().map(|r| { | ||
let (lo, hi) = Interval::offset_sign(cx.tcx, ty, r, false); | ||
let (lo, hi) = IntRange::decode(cx.tcx, ty, r); | ||
ConstantRange(ty::Const::from_bits(cx.tcx, lo, ty), | ||
ty::Const::from_bits(cx.tcx, hi, ty), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should definitely be using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably. My TODO already says "ConstValue or smaller" ;). That should not block this PR though, since it's a preexisting issue. |
||
RangeEnd::Included) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use
ty::layout::Integer
here or similar.