From 665ff57fde7630474b59fc3f292f2b01a26a3493 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 30 Oct 2021 06:22:19 +0200 Subject: [PATCH 1/5] Remove expects from FullInt Partial{Ord,Eq} --- clippy_utils/src/consts.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 3e5d74a66f4e..3b718d64ce60 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -246,27 +246,26 @@ impl FullInt { impl PartialEq for FullInt { #[must_use] fn eq(&self, other: &Self) -> bool { - self.partial_cmp(other).expect("`partial_cmp` only returns `Some(_)`") == Ordering::Equal + self.cmp(other) == Ordering::Equal } } impl PartialOrd for FullInt { #[must_use] fn partial_cmp(&self, other: &Self) -> Option { - Some(match (self, other) { - (&Self::S(s), &Self::S(o)) => s.cmp(&o), - (&Self::U(s), &Self::U(o)) => s.cmp(&o), - (&Self::S(s), &Self::U(o)) => Self::cmp_s_u(s, o), - (&Self::U(s), &Self::S(o)) => Self::cmp_s_u(o, s).reverse(), - }) + Some(self.cmp(other)) } } impl Ord for FullInt { #[must_use] fn cmp(&self, other: &Self) -> Ordering { - self.partial_cmp(other) - .expect("`partial_cmp` for FullInt can never return `None`") + match (self, other) { + (&Self::S(s), &Self::S(o)) => s.cmp(&o), + (&Self::U(s), &Self::U(o)) => s.cmp(&o), + (&Self::S(s), &Self::U(o)) => Self::cmp_s_u(s, o), + (&Self::U(s), &Self::S(o)) => Self::cmp_s_u(o, s).reverse(), + } } } From c8edd9a16e2e9db1fa2f26a0164740ae426f4876 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 30 Oct 2021 06:22:19 +0200 Subject: [PATCH 2/5] Remove casts from FullInt impl --- clippy_utils/src/consts.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 3b718d64ce60..85dad1dcfb26 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -230,16 +230,9 @@ pub enum FullInt { } impl FullInt { - #[allow(clippy::cast_sign_loss)] #[must_use] fn cmp_s_u(s: i128, u: u128) -> Ordering { - if s < 0 { - Ordering::Less - } else if u > (i128::MAX as u128) { - Ordering::Greater - } else { - (s as u128).cmp(&u) - } + u128::try_from(s).map_or(Ordering::Less, |x| x.cmp(&u)) } } From c6dca68eca75a46af31d7175486909c720c2aebc Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 30 Oct 2021 06:22:19 +0200 Subject: [PATCH 3/5] Simplify FullInt Ord impl --- clippy_utils/src/consts.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 85dad1dcfb26..7ce9676ed05a 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -253,11 +253,11 @@ impl PartialOrd for FullInt { impl Ord for FullInt { #[must_use] fn cmp(&self, other: &Self) -> Ordering { - match (self, other) { - (&Self::S(s), &Self::S(o)) => s.cmp(&o), - (&Self::U(s), &Self::U(o)) => s.cmp(&o), - (&Self::S(s), &Self::U(o)) => Self::cmp_s_u(s, o), - (&Self::U(s), &Self::S(o)) => Self::cmp_s_u(o, s).reverse(), + match (*self, *other) { + (Self::S(s), Self::S(o)) => s.cmp(&o), + (Self::U(s), Self::U(o)) => s.cmp(&o), + (Self::S(s), Self::U(o)) => Self::cmp_s_u(s, o), + (Self::U(s), Self::S(o)) => Self::cmp_s_u(o, s).reverse(), } } } From 4a86156c66a8cb66da85ffe68b90bd0f643e441b Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 30 Oct 2021 06:22:19 +0200 Subject: [PATCH 4/5] Simplify FullInt Ord impl (2) --- clippy_utils/src/consts.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 7ce9676ed05a..e40a1d2f2798 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -253,11 +253,13 @@ impl PartialOrd for FullInt { impl Ord for FullInt { #[must_use] fn cmp(&self, other: &Self) -> Ordering { + use FullInt::{S, U}; + match (*self, *other) { - (Self::S(s), Self::S(o)) => s.cmp(&o), - (Self::U(s), Self::U(o)) => s.cmp(&o), - (Self::S(s), Self::U(o)) => Self::cmp_s_u(s, o), - (Self::U(s), Self::S(o)) => Self::cmp_s_u(o, s).reverse(), + (S(s), S(o)) => s.cmp(&o), + (U(s), U(o)) => s.cmp(&o), + (S(s), U(o)) => Self::cmp_s_u(s, o), + (U(s), S(o)) => Self::cmp_s_u(o, s).reverse(), } } } From e8c4046841b43e4b3ea3ff8f5c22858f34c8fe9f Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 30 Oct 2021 06:22:19 +0200 Subject: [PATCH 5/5] Simplify FullInt Ord impl `cmp_s_u` is a tiny helper function only used by `cmp` and isn't useful on it's own. Making it a nested function of `cmp` makes that clear and as a bonus it's easier to call and doesn't require a `#[must_use]` attribute. --- clippy_utils/src/consts.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index e40a1d2f2798..04347672e0fb 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -229,13 +229,6 @@ pub enum FullInt { U(u128), } -impl FullInt { - #[must_use] - fn cmp_s_u(s: i128, u: u128) -> Ordering { - u128::try_from(s).map_or(Ordering::Less, |x| x.cmp(&u)) - } -} - impl PartialEq for FullInt { #[must_use] fn eq(&self, other: &Self) -> bool { @@ -255,11 +248,15 @@ impl Ord for FullInt { fn cmp(&self, other: &Self) -> Ordering { use FullInt::{S, U}; + fn cmp_s_u(s: i128, u: u128) -> Ordering { + u128::try_from(s).map_or(Ordering::Less, |x| x.cmp(&u)) + } + match (*self, *other) { (S(s), S(o)) => s.cmp(&o), (U(s), U(o)) => s.cmp(&o), - (S(s), U(o)) => Self::cmp_s_u(s, o), - (U(s), S(o)) => Self::cmp_s_u(o, s).reverse(), + (S(s), U(o)) => cmp_s_u(s, o), + (U(s), S(o)) => cmp_s_u(o, s).reverse(), } } }