Skip to content

Commit 1ede540

Browse files
author
Michael Wright
committed
Fix false positive in match_overlapping_arm
The bug was dues to the constant bytes being compared instead of their values. This meant that negative values were being treated as larger than some positive values. Fixes #7829
1 parent c4d5471 commit 1ede540

File tree

1 file changed

+14
-37
lines changed

1 file changed

+14
-37
lines changed

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))

0 commit comments

Comments
 (0)