Skip to content

Commit b776d1c

Browse files
committed
Auto merge of rust-lang#79523 - Nadrieril:fix-usize-ranges, r=varkor
Fix overlap detection of `usize`/`isize` range patterns `usize` and `isize` are a bit of a special case in the match usefulness algorithm, because the range of values they contain depends on the platform. Specifically, we don't want `0..usize::MAX` to count as an exhaustive match (see also [`precise_pointer_size_matching`](rust-lang#56354)). The way this was initially implemented is by treating those ranges like float ranges, i.e. with limited cleverness. This means we didn't catch the following as unreachable: ```rust match 0usize { 0..10 => {}, 10..20 => {}, 5..15 => {}, // oops, should be detected as unreachable _ => {}, } ``` This PRs fixes this oversight. Now the only difference between `usize` and `u64` range patterns is in what ranges count as exhaustive. r? `@varkor` `@rustbot` label +A-exhaustiveness-checking
2 parents 88b8197 + bdd2bdb commit b776d1c

File tree

4 files changed

+74
-95
lines changed

4 files changed

+74
-95
lines changed

compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs

+56-91
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,12 @@ use std::ops::RangeInclusive;
3737
///
3838
/// `IntRange` is never used to encode an empty range or a "range" that wraps
3939
/// around the (offset) space: i.e., `range.lo <= range.hi`.
40-
#[derive(Clone, Debug)]
41-
pub(super) struct IntRange<'tcx> {
40+
#[derive(Clone, Debug, PartialEq, Eq)]
41+
pub(super) struct IntRange {
4242
range: RangeInclusive<u128>,
43-
ty: Ty<'tcx>,
44-
span: Span,
4543
}
4644

47-
impl<'tcx> IntRange<'tcx> {
45+
impl IntRange {
4846
#[inline]
4947
fn is_integral(ty: Ty<'_>) -> bool {
5048
matches!(ty.kind(), ty::Char | ty::Int(_) | ty::Uint(_) | ty::Bool)
@@ -58,14 +56,8 @@ impl<'tcx> IntRange<'tcx> {
5856
(*self.range.start(), *self.range.end())
5957
}
6058

61-
/// Don't treat `usize`/`isize` exhaustively unless the `precise_pointer_size_matching` feature
62-
/// is enabled.
63-
fn treat_exhaustively(&self, tcx: TyCtxt<'tcx>) -> bool {
64-
!self.ty.is_ptr_sized_integral() || tcx.features().precise_pointer_size_matching
65-
}
66-
6759
#[inline]
68-
fn integral_size_and_signed_bias(tcx: TyCtxt<'tcx>, ty: Ty<'_>) -> Option<(Size, u128)> {
60+
fn integral_size_and_signed_bias(tcx: TyCtxt<'_>, ty: Ty<'_>) -> Option<(Size, u128)> {
6961
match *ty.kind() {
7062
ty::Bool => Some((Size::from_bytes(1), 0)),
7163
ty::Char => Some((Size::from_bytes(4), 0)),
@@ -79,12 +71,11 @@ impl<'tcx> IntRange<'tcx> {
7971
}
8072

8173
#[inline]
82-
fn from_const(
74+
fn from_const<'tcx>(
8375
tcx: TyCtxt<'tcx>,
8476
param_env: ty::ParamEnv<'tcx>,
8577
value: &Const<'tcx>,
86-
span: Span,
87-
) -> Option<IntRange<'tcx>> {
78+
) -> Option<IntRange> {
8879
if let Some((target_size, bias)) = Self::integral_size_and_signed_bias(tcx, value.ty) {
8980
let ty = value.ty;
9081
let val = (|| {
@@ -101,21 +92,20 @@ impl<'tcx> IntRange<'tcx> {
10192
value.try_eval_bits(tcx, param_env, ty)
10293
})()?;
10394
let val = val ^ bias;
104-
Some(IntRange { range: val..=val, ty, span })
95+
Some(IntRange { range: val..=val })
10596
} else {
10697
None
10798
}
10899
}
109100

110101
#[inline]
111-
fn from_range(
102+
fn from_range<'tcx>(
112103
tcx: TyCtxt<'tcx>,
113104
lo: u128,
114105
hi: u128,
115106
ty: Ty<'tcx>,
116107
end: &RangeEnd,
117-
span: Span,
118-
) -> Option<IntRange<'tcx>> {
108+
) -> Option<IntRange> {
119109
if Self::is_integral(ty) {
120110
// Perform a shift if the underlying types are signed,
121111
// which makes the interval arithmetic simpler.
@@ -126,14 +116,14 @@ impl<'tcx> IntRange<'tcx> {
126116
// This should have been caught earlier by E0030.
127117
bug!("malformed range pattern: {}..={}", lo, (hi - offset));
128118
}
129-
Some(IntRange { range: lo..=(hi - offset), ty, span })
119+
Some(IntRange { range: lo..=(hi - offset) })
130120
} else {
131121
None
132122
}
133123
}
134124

135125
// The return value of `signed_bias` should be XORed with an endpoint to encode/decode it.
136-
fn signed_bias(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> u128 {
126+
fn signed_bias(tcx: TyCtxt<'_>, ty: Ty<'_>) -> u128 {
137127
match *ty.kind() {
138128
ty::Int(ity) => {
139129
let bits = Integer::from_attr(&tcx, SignedInt(ity)).size().bits() as u128;
@@ -147,20 +137,13 @@ impl<'tcx> IntRange<'tcx> {
147137
other.range.start() <= self.range.start() && self.range.end() <= other.range.end()
148138
}
149139

150-
fn intersection(&self, tcx: TyCtxt<'tcx>, other: &Self) -> Option<Self> {
151-
let ty = self.ty;
140+
fn intersection(&self, other: &Self) -> Option<Self> {
152141
let (lo, hi) = self.boundaries();
153142
let (other_lo, other_hi) = other.boundaries();
154-
if self.treat_exhaustively(tcx) {
155-
if lo <= other_hi && other_lo <= hi {
156-
let span = other.span;
157-
Some(IntRange { range: max(lo, other_lo)..=min(hi, other_hi), ty, span })
158-
} else {
159-
None
160-
}
143+
if lo <= other_hi && other_lo <= hi {
144+
Some(IntRange { range: max(lo, other_lo)..=min(hi, other_hi) })
161145
} else {
162-
// If the range should not be treated exhaustively, fallback to checking for inclusion.
163-
if self.is_subrange(other) { Some(self.clone()) } else { None }
146+
None
164147
}
165148
}
166149

@@ -181,24 +164,23 @@ impl<'tcx> IntRange<'tcx> {
181164
lo == other_hi || hi == other_lo
182165
}
183166

184-
fn to_pat(&self, tcx: TyCtxt<'tcx>) -> Pat<'tcx> {
167+
fn to_pat<'tcx>(&self, tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Pat<'tcx> {
185168
let (lo, hi) = self.boundaries();
186169

187-
let bias = IntRange::signed_bias(tcx, self.ty);
170+
let bias = IntRange::signed_bias(tcx, ty);
188171
let (lo, hi) = (lo ^ bias, hi ^ bias);
189172

190-
let ty = ty::ParamEnv::empty().and(self.ty);
191-
let lo_const = ty::Const::from_bits(tcx, lo, ty);
192-
let hi_const = ty::Const::from_bits(tcx, hi, ty);
173+
let env = ty::ParamEnv::empty().and(ty);
174+
let lo_const = ty::Const::from_bits(tcx, lo, env);
175+
let hi_const = ty::Const::from_bits(tcx, hi, env);
193176

194177
let kind = if lo == hi {
195178
PatKind::Constant { value: lo_const }
196179
} else {
197180
PatKind::Range(PatRange { lo: lo_const, hi: hi_const, end: RangeEnd::Included })
198181
};
199182

200-
// This is a brand new pattern, so we don't reuse `self.span`.
201-
Pat { ty: self.ty, span: DUMMY_SP, kind: Box::new(kind) }
183+
Pat { ty, span: DUMMY_SP, kind: Box::new(kind) }
202184
}
203185

204186
/// For exhaustive integer matching, some constructors are grouped within other constructors
@@ -233,13 +215,11 @@ impl<'tcx> IntRange<'tcx> {
233215
/// boundaries for each interval range, sort them, then create constructors for each new interval
234216
/// between every pair of boundary points. (This essentially sums up to performing the intuitive
235217
/// merging operation depicted above.)
236-
fn split<'p>(
218+
fn split<'p, 'tcx>(
237219
&self,
238220
pcx: PatCtxt<'_, 'p, 'tcx>,
239221
hir_id: Option<HirId>,
240222
) -> SmallVec<[Constructor<'tcx>; 1]> {
241-
let ty = pcx.ty;
242-
243223
/// Represents a border between 2 integers. Because the intervals spanning borders
244224
/// must be able to cover every integer, we need to be able to represent
245225
/// 2^128 + 1 such borders.
@@ -250,7 +230,7 @@ impl<'tcx> IntRange<'tcx> {
250230
}
251231

252232
// A function for extracting the borders of an integer interval.
253-
fn range_borders(r: IntRange<'_>) -> impl Iterator<Item = Border> {
233+
fn range_borders(r: IntRange) -> impl Iterator<Item = Border> {
254234
let (lo, hi) = r.range.into_inner();
255235
let from = Border::JustBefore(lo);
256236
let to = match hi.checked_add(1) {
@@ -268,21 +248,23 @@ impl<'tcx> IntRange<'tcx> {
268248
// class lies between 2 borders.
269249
let row_borders = pcx
270250
.matrix
271-
.head_ctors(pcx.cx)
272-
.filter_map(|ctor| ctor.as_int_range())
273-
.filter_map(|range| {
274-
let intersection = self.intersection(pcx.cx.tcx, &range);
251+
.head_ctors_and_spans(pcx.cx)
252+
.filter_map(|(ctor, span)| Some((ctor.as_int_range()?, span)))
253+
.filter_map(|(range, span)| {
254+
let intersection = self.intersection(&range);
275255
let should_lint = self.suspicious_intersection(&range);
276256
if let (Some(range), 1, true) = (&intersection, row_len, should_lint) {
277257
// FIXME: for now, only check for overlapping ranges on simple range
278258
// patterns. Otherwise with the current logic the following is detected
279259
// as overlapping:
280-
// match (10u8, true) {
281-
// (0 ..= 125, false) => {}
282-
// (126 ..= 255, false) => {}
283-
// (0 ..= 255, true) => {}
284-
// }
285-
overlaps.push(range.clone());
260+
// ```
261+
// match (0u8, true) {
262+
// (0 ..= 125, false) => {}
263+
// (125 ..= 255, true) => {}
264+
// _ => {}
265+
// }
266+
// ```
267+
overlaps.push((range.clone(), span));
286268
}
287269
intersection
288270
})
@@ -291,7 +273,7 @@ impl<'tcx> IntRange<'tcx> {
291273
let mut borders: Vec<_> = row_borders.chain(self_borders).collect();
292274
borders.sort_unstable();
293275

294-
self.lint_overlapping_patterns(pcx.cx.tcx, hir_id, ty, overlaps);
276+
self.lint_overlapping_patterns(pcx, hir_id, overlaps);
295277

296278
// We're going to iterate through every adjacent pair of borders, making sure that
297279
// each represents an interval of nonnegative length, and convert each such
@@ -309,33 +291,32 @@ impl<'tcx> IntRange<'tcx> {
309291
[Border::JustBefore(n), Border::AfterMax] => Some(n..=u128::MAX),
310292
[Border::AfterMax, _] => None,
311293
})
312-
.map(|range| IntRange { range, ty, span: pcx.span })
294+
.map(|range| IntRange { range })
313295
.map(IntRange)
314296
.collect()
315297
}
316298

317299
fn lint_overlapping_patterns(
318300
&self,
319-
tcx: TyCtxt<'tcx>,
301+
pcx: PatCtxt<'_, '_, '_>,
320302
hir_id: Option<HirId>,
321-
ty: Ty<'tcx>,
322-
overlaps: Vec<IntRange<'tcx>>,
303+
overlaps: Vec<(IntRange, Span)>,
323304
) {
324305
if let (true, Some(hir_id)) = (!overlaps.is_empty(), hir_id) {
325-
tcx.struct_span_lint_hir(
306+
pcx.cx.tcx.struct_span_lint_hir(
326307
lint::builtin::OVERLAPPING_PATTERNS,
327308
hir_id,
328-
self.span,
309+
pcx.span,
329310
|lint| {
330311
let mut err = lint.build("multiple patterns covering the same range");
331-
err.span_label(self.span, "overlapping patterns");
332-
for int_range in overlaps {
312+
err.span_label(pcx.span, "overlapping patterns");
313+
for (int_range, span) in overlaps {
333314
// Use the real type for user display of the ranges:
334315
err.span_label(
335-
int_range.span,
316+
span,
336317
&format!(
337318
"this range overlaps on `{}`",
338-
IntRange { range: int_range.range, ty, span: DUMMY_SP }.to_pat(tcx),
319+
int_range.to_pat(pcx.cx.tcx, pcx.ty),
339320
),
340321
);
341322
}
@@ -346,8 +327,8 @@ impl<'tcx> IntRange<'tcx> {
346327
}
347328

348329
/// See `Constructor::is_covered_by`
349-
fn is_covered_by<'p>(&self, pcx: PatCtxt<'_, 'p, 'tcx>, other: &Self) -> bool {
350-
if self.intersection(pcx.cx.tcx, other).is_some() {
330+
fn is_covered_by(&self, other: &Self) -> bool {
331+
if self.intersection(other).is_some() {
351332
// Constructor splitting should ensure that all intersections we encounter are actually
352333
// inclusions.
353334
assert!(self.is_subrange(other));
@@ -358,13 +339,6 @@ impl<'tcx> IntRange<'tcx> {
358339
}
359340
}
360341

361-
/// Ignore spans when comparing, they don't carry semantic information as they are only for lints.
362-
impl<'tcx> std::cmp::PartialEq for IntRange<'tcx> {
363-
fn eq(&self, other: &Self) -> bool {
364-
self.range == other.range && self.ty == other.ty
365-
}
366-
}
367-
368342
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
369343
enum SliceKind {
370344
/// Patterns of length `n` (`[x, y]`).
@@ -558,7 +532,7 @@ pub(super) enum Constructor<'tcx> {
558532
/// Enum variants.
559533
Variant(DefId),
560534
/// Ranges of integer literal values (`2`, `2..=5` or `2..5`).
561-
IntRange(IntRange<'tcx>),
535+
IntRange(IntRange),
562536
/// Ranges of floating-point literal values (`2.0..=5.2`).
563537
FloatRange(&'tcx ty::Const<'tcx>, &'tcx ty::Const<'tcx>, RangeEnd),
564538
/// String literals. Strings are not quite the same as `&[u8]` so we treat them separately.
@@ -581,7 +555,7 @@ impl<'tcx> Constructor<'tcx> {
581555
matches!(self, Wildcard)
582556
}
583557

584-
fn as_int_range(&self) -> Option<&IntRange<'tcx>> {
558+
fn as_int_range(&self) -> Option<&IntRange> {
585559
match self {
586560
IntRange(range) => Some(range),
587561
_ => None,
@@ -616,8 +590,7 @@ impl<'tcx> Constructor<'tcx> {
616590
Variant(adt_def.variants[variant_index].def_id)
617591
}
618592
PatKind::Constant { value } => {
619-
if let Some(int_range) = IntRange::from_const(cx.tcx, cx.param_env, value, pat.span)
620-
{
593+
if let Some(int_range) = IntRange::from_const(cx.tcx, cx.param_env, value) {
621594
IntRange(int_range)
622595
} else {
623596
match pat.ty.kind() {
@@ -641,7 +614,6 @@ impl<'tcx> Constructor<'tcx> {
641614
hi.eval_bits(cx.tcx, cx.param_env, hi.ty),
642615
ty,
643616
&end,
644-
pat.span,
645617
) {
646618
IntRange(int_range)
647619
} else {
@@ -694,11 +666,7 @@ impl<'tcx> Constructor<'tcx> {
694666
Wildcard => Constructor::split_wildcard(pcx),
695667
// Fast-track if the range is trivial. In particular, we don't do the overlapping
696668
// ranges check.
697-
IntRange(ctor_range)
698-
if ctor_range.treat_exhaustively(pcx.cx.tcx) && !ctor_range.is_singleton() =>
699-
{
700-
ctor_range.split(pcx, hir_id)
701-
}
669+
IntRange(ctor_range) if !ctor_range.is_singleton() => ctor_range.split(pcx, hir_id),
702670
Slice(slice @ Slice { kind: VarLen(..), .. }) => slice.split(pcx),
703671
// Any other constructor can be used unchanged.
704672
_ => smallvec![self.clone()],
@@ -740,9 +708,7 @@ impl<'tcx> Constructor<'tcx> {
740708
(Single, Single) => true,
741709
(Variant(self_id), Variant(other_id)) => self_id == other_id,
742710

743-
(IntRange(self_range), IntRange(other_range)) => {
744-
self_range.is_covered_by(pcx, other_range)
745-
}
711+
(IntRange(self_range), IntRange(other_range)) => self_range.is_covered_by(other_range),
746712
(
747713
FloatRange(self_from, self_to, self_end),
748714
FloatRange(other_from, other_to, other_end),
@@ -803,15 +769,15 @@ impl<'tcx> Constructor<'tcx> {
803769
IntRange(range) => used_ctors
804770
.iter()
805771
.filter_map(|c| c.as_int_range())
806-
.any(|other| range.is_covered_by(pcx, other)),
772+
.any(|other| range.is_covered_by(other)),
807773
Slice(slice) => used_ctors
808774
.iter()
809775
.filter_map(|c| c.as_slice())
810776
.any(|other| slice.is_covered_by(other)),
811777
// This constructor is never covered by anything else
812778
NonExhaustive => false,
813779
Str(..) | FloatRange(..) | Opaque | Wildcard => {
814-
bug!("found unexpected ctor in all_ctors: {:?}", self)
780+
span_bug!(pcx.span, "found unexpected ctor in all_ctors: {:?}", self)
815781
}
816782
}
817783
}
@@ -832,8 +798,7 @@ fn all_constructors<'p, 'tcx>(pcx: PatCtxt<'_, 'p, 'tcx>) -> Vec<Constructor<'tc
832798
let make_range = |start, end| {
833799
IntRange(
834800
// `unwrap()` is ok because we know the type is an integer.
835-
IntRange::from_range(cx.tcx, start, end, pcx.ty, &RangeEnd::Included, pcx.span)
836-
.unwrap(),
801+
IntRange::from_range(cx.tcx, start, end, pcx.ty, &RangeEnd::Included).unwrap(),
837802
)
838803
};
839804
match pcx.ty.kind() {
@@ -1238,7 +1203,7 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
12381203
},
12391204
&Str(value) => PatKind::Constant { value },
12401205
&FloatRange(lo, hi, end) => PatKind::Range(PatRange { lo, hi, end }),
1241-
IntRange(range) => return range.to_pat(pcx.cx.tcx),
1206+
IntRange(range) => return range.to_pat(pcx.cx.tcx, pcx.ty),
12421207
NonExhaustive => PatKind::Wild,
12431208
Opaque => bug!("we should not try to apply an opaque constructor"),
12441209
Wildcard => bug!(

0 commit comments

Comments
 (0)