Skip to content

Commit 89a1156

Browse files
borsbors
committed
Auto merge of rust-lang#7847 - mikerite:fix-7829, r=flip1995
Fix false positive in `match_overlapping_arm` Fixes rust-lang#7829 changelog: Fix false positive in [`match_overlapping_arm`].
2 parents ed71add + 1ede540 commit 89a1156

File tree

4 files changed

+92
-102
lines changed

4 files changed

+92
-102
lines changed

‎clippy_lints/src/invalid_upcast_comparisons.rs

Lines changed: 3 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
1-
use std::cmp::Ordering;
2-
31
use rustc_hir::{Expr, ExprKind};
42
use rustc_lint::{LateContext, LateLintPass};
53
use rustc_middle::ty::layout::LayoutOf;
64
use rustc_middle::ty::{self, IntTy, UintTy};
75
use rustc_session::{declare_lint_pass, declare_tool_lint};
86
use rustc_span::Span;
97

8+
use clippy_utils::comparisons;
109
use clippy_utils::comparisons::Rel;
11-
use clippy_utils::consts::{constant, Constant};
10+
use clippy_utils::consts::{constant_full_int, FullInt};
1211
use clippy_utils::diagnostics::span_lint;
1312
use clippy_utils::source::snippet;
14-
use clippy_utils::{comparisons, sext};
1513

1614
declare_clippy_lint! {
1715
/// ### What it does
@@ -39,53 +37,6 @@ declare_clippy_lint! {
3937

4038
declare_lint_pass!(InvalidUpcastComparisons => [INVALID_UPCAST_COMPARISONS]);
4139

42-
#[derive(Copy, Clone, Debug, Eq)]
43-
enum FullInt {
44-
S(i128),
45-
U(u128),
46-
}
47-
48-
impl FullInt {
49-
#[allow(clippy::cast_sign_loss)]
50-
#[must_use]
51-
fn cmp_s_u(s: i128, u: u128) -> Ordering {
52-
if s < 0 {
53-
Ordering::Less
54-
} else if u > (i128::MAX as u128) {
55-
Ordering::Greater
56-
} else {
57-
(s as u128).cmp(&u)
58-
}
59-
}
60-
}
61-
62-
impl PartialEq for FullInt {
63-
#[must_use]
64-
fn eq(&self, other: &Self) -> bool {
65-
self.partial_cmp(other).expect("`partial_cmp` only returns `Some(_)`") == Ordering::Equal
66-
}
67-
}
68-
69-
impl PartialOrd for FullInt {
70-
#[must_use]
71-
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
72-
Some(match (self, other) {
73-
(&Self::S(s), &Self::S(o)) => s.cmp(&o),
74-
(&Self::U(s), &Self::U(o)) => s.cmp(&o),
75-
(&Self::S(s), &Self::U(o)) => Self::cmp_s_u(s, o),
76-
(&Self::U(s), &Self::S(o)) => Self::cmp_s_u(o, s).reverse(),
77-
})
78-
}
79-
}
80-
81-
impl Ord for FullInt {
82-
#[must_use]
83-
fn cmp(&self, other: &Self) -> Ordering {
84-
self.partial_cmp(other)
85-
.expect("`partial_cmp` for FullInt can never return `None`")
86-
}
87-
}
88-
8940
fn numeric_cast_precast_bounds<'a>(cx: &LateContext<'_>, expr: &'a Expr<'_>) -> Option<(FullInt, FullInt)> {
9041
if let ExprKind::Cast(cast_exp, _) = expr.kind {
9142
let pre_cast_ty = cx.typeck_results().expr_ty(cast_exp);
@@ -118,19 +69,6 @@ fn numeric_cast_precast_bounds<'a>(cx: &LateContext<'_>, expr: &'a Expr<'_>) ->
11869
}
11970
}
12071

121-
fn node_as_const_fullint<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<FullInt> {
122-
let val = constant(cx, cx.typeck_results(), expr)?.0;
123-
if let Constant::Int(const_int) = val {
124-
match *cx.typeck_results().expr_ty(expr).kind() {
125-
ty::Int(ity) => Some(FullInt::S(sext(cx.tcx, const_int, ity))),
126-
ty::Uint(_) => Some(FullInt::U(const_int)),
127-
_ => None,
128-
}
129-
} else {
130-
None
131-
}
132-
}
133-
13472
fn err_upcast_comparison(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, always: bool) {
13573
if let ExprKind::Cast(cast_val, _) = expr.kind {
13674
span_lint(
@@ -156,7 +94,7 @@ fn upcast_comparison_bounds_err<'tcx>(
15694
invert: bool,
15795
) {
15896
if let Some((lb, ub)) = lhs_bounds {
159-
if let Some(norm_rhs_val) = node_as_const_fullint(cx, rhs) {
97+
if let Some(norm_rhs_val) = constant_full_int(cx, cx.typeck_results(), rhs) {
16098
if rel == Rel::Eq || rel == Rel::Ne {
16199
if norm_rhs_val < lb || norm_rhs_val > ub {
162100
err_upcast_comparison(cx, span, lhs, rel == Rel::Ne);

‎clippy_lints/src/matches.rs

Lines changed: 14 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use clippy_utils::consts::{constant, miri_to_const, Constant};
1+
use clippy_utils::consts::{constant, constant_full_int, miri_to_const, FullInt};
22
use clippy_utils::diagnostics::{
33
multispan_sugg, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then,
44
};
@@ -930,9 +930,8 @@ fn check_match_bool(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr:
930930
fn check_overlapping_arms<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) {
931931
if arms.len() >= 2 && cx.typeck_results().expr_ty(ex).is_integral() {
932932
let ranges = all_ranges(cx, arms, cx.typeck_results().expr_ty(ex));
933-
let type_ranges = type_ranges(&ranges);
934-
if !type_ranges.is_empty() {
935-
if let Some((start, end)) = overlapping(&type_ranges) {
933+
if !ranges.is_empty() {
934+
if let Some((start, end)) = overlapping(&ranges) {
936935
span_lint_and_note(
937936
cx,
938937
MATCH_OVERLAPPING_ARM,
@@ -1601,7 +1600,7 @@ fn opt_parent_let<'a>(cx: &LateContext<'a>, ex: &Expr<'a>) -> Option<&'a Local<'
16011600
}
16021601

16031602
/// Gets all arms that are unbounded `PatRange`s.
1604-
fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>) -> Vec<SpannedRange<Constant>> {
1603+
fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>) -> Vec<SpannedRange<FullInt>> {
16051604
arms.iter()
16061605
.filter_map(|arm| {
16071606
if let Arm { pat, guard: None, .. } = *arm {
@@ -1614,21 +1613,25 @@ fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>)
16141613
Some(rhs) => constant(cx, cx.typeck_results(), rhs)?.0,
16151614
None => miri_to_const(ty.numeric_max_val(cx.tcx)?)?,
16161615
};
1617-
let rhs = match range_end {
1618-
RangeEnd::Included => Bound::Included(rhs),
1619-
RangeEnd::Excluded => Bound::Excluded(rhs),
1616+
1617+
let lhs_val = lhs.int_value(cx, ty)?;
1618+
let rhs_val = rhs.int_value(cx, ty)?;
1619+
1620+
let rhs_bound = match range_end {
1621+
RangeEnd::Included => Bound::Included(rhs_val),
1622+
RangeEnd::Excluded => Bound::Excluded(rhs_val),
16201623
};
16211624
return Some(SpannedRange {
16221625
span: pat.span,
1623-
node: (lhs, rhs),
1626+
node: (lhs_val, rhs_bound),
16241627
});
16251628
}
16261629

16271630
if let PatKind::Lit(value) = pat.kind {
1628-
let value = constant(cx, cx.typeck_results(), value)?.0;
1631+
let value = constant_full_int(cx, cx.typeck_results(), value)?;
16291632
return Some(SpannedRange {
16301633
span: pat.span,
1631-
node: (value.clone(), Bound::Included(value)),
1634+
node: (value, Bound::Included(value)),
16321635
});
16331636
}
16341637
}
@@ -1643,32 +1646,6 @@ pub struct SpannedRange<T> {
16431646
pub node: (T, Bound<T>),
16441647
}
16451648

1646-
type TypedRanges = Vec<SpannedRange<u128>>;
1647-
1648-
/// Gets all `Int` ranges or all `Uint` ranges. Mixed types are an error anyway
1649-
/// and other types than
1650-
/// `Uint` and `Int` probably don't make sense.
1651-
fn type_ranges(ranges: &[SpannedRange<Constant>]) -> TypedRanges {
1652-
ranges
1653-
.iter()
1654-
.filter_map(|range| match range.node {
1655-
(Constant::Int(start), Bound::Included(Constant::Int(end))) => Some(SpannedRange {
1656-
span: range.span,
1657-
node: (start, Bound::Included(end)),
1658-
}),
1659-
(Constant::Int(start), Bound::Excluded(Constant::Int(end))) => Some(SpannedRange {
1660-
span: range.span,
1661-
node: (start, Bound::Excluded(end)),
1662-
}),
1663-
(Constant::Int(start), Bound::Unbounded) => Some(SpannedRange {
1664-
span: range.span,
1665-
node: (start, Bound::Unbounded),
1666-
}),
1667-
_ => None,
1668-
})
1669-
.collect()
1670-
}
1671-
16721649
// Checks if arm has the form `None => None`
16731650
fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
16741651
matches!(arm.pat.kind, PatKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone))

‎clippy_utils/src/consts.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,19 @@ impl Constant {
155155
_ => None,
156156
}
157157
}
158+
159+
/// Returns the integer value or `None` if `self` or `val_type` is not integer type.
160+
pub fn int_value(&self, cx: &LateContext<'_>, val_type: Ty<'_>) -> Option<FullInt> {
161+
if let Constant::Int(const_int) = *self {
162+
match *val_type.kind() {
163+
ty::Int(ity) => Some(FullInt::S(sext(cx.tcx, const_int, ity))),
164+
ty::Uint(_) => Some(FullInt::U(const_int)),
165+
_ => None,
166+
}
167+
} else {
168+
None
169+
}
170+
}
158171
}
159172

160173
/// Parses a `LitKind` to a `Constant`.
@@ -202,6 +215,61 @@ pub fn constant_simple<'tcx>(
202215
constant(lcx, typeck_results, e).and_then(|(cst, res)| if res { None } else { Some(cst) })
203216
}
204217

218+
pub fn constant_full_int(
219+
lcx: &LateContext<'tcx>,
220+
typeck_results: &ty::TypeckResults<'tcx>,
221+
e: &Expr<'_>,
222+
) -> Option<FullInt> {
223+
constant_simple(lcx, typeck_results, e)?.int_value(lcx, typeck_results.expr_ty(e))
224+
}
225+
226+
#[derive(Copy, Clone, Debug, Eq)]
227+
pub enum FullInt {
228+
S(i128),
229+
U(u128),
230+
}
231+
232+
impl FullInt {
233+
#[allow(clippy::cast_sign_loss)]
234+
#[must_use]
235+
fn cmp_s_u(s: i128, u: u128) -> Ordering {
236+
if s < 0 {
237+
Ordering::Less
238+
} else if u > (i128::MAX as u128) {
239+
Ordering::Greater
240+
} else {
241+
(s as u128).cmp(&u)
242+
}
243+
}
244+
}
245+
246+
impl PartialEq for FullInt {
247+
#[must_use]
248+
fn eq(&self, other: &Self) -> bool {
249+
self.partial_cmp(other).expect("`partial_cmp` only returns `Some(_)`") == Ordering::Equal
250+
}
251+
}
252+
253+
impl PartialOrd for FullInt {
254+
#[must_use]
255+
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
256+
Some(match (self, other) {
257+
(&Self::S(s), &Self::S(o)) => s.cmp(&o),
258+
(&Self::U(s), &Self::U(o)) => s.cmp(&o),
259+
(&Self::S(s), &Self::U(o)) => Self::cmp_s_u(s, o),
260+
(&Self::U(s), &Self::S(o)) => Self::cmp_s_u(o, s).reverse(),
261+
})
262+
}
263+
}
264+
265+
impl Ord for FullInt {
266+
#[must_use]
267+
fn cmp(&self, other: &Self) -> Ordering {
268+
self.partial_cmp(other)
269+
.expect("`partial_cmp` for FullInt can never return `None`")
270+
}
271+
}
272+
205273
/// Creates a `ConstEvalLateContext` from the given `LateContext` and `TypeckResults`.
206274
pub fn constant_context<'a, 'tcx>(
207275
lcx: &'a LateContext<'tcx>,

‎tests/ui/match_overlapping_arm.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,13 @@ fn overlapping() {
100100
_ => (),
101101
}
102102

103+
// Issue #7829
104+
match 0 {
105+
-1..=1 => (),
106+
-2..=2 => (),
107+
_ => (),
108+
}
109+
103110
if let None = Some(42) {
104111
// nothing
105112
} else if let None = Some(42) {

0 commit comments

Comments
 (0)