Skip to content

Commit 5e6c262

Browse files
bors[bot]cuviper
andauthored
Merge #151
151: Revert PartialEq and PartialOrd with primitives r=cuviper a=cuviper This manually reverts the new implementations from pull request #136. As noted in issue #150, the mere existence of those impls can have a bad effect on type inference in other parts of a crate, even from afar. All comparisons of primitives with an unknown type become ambiguous whether that's meant to compare with itself or a bigint, even if `num-bigint` is not directly in scope at all. Since this can break unrelated code in surprising ways, I think it's not wise for us to have these implementations. Maybe we can explore other methods to compare with primitives in the future, though it won't be as convenient as using the operators. Co-authored-by: Josh Stone <cuviper@gmail.com>
2 parents 821007a + 7330b70 commit 5e6c262

File tree

9 files changed

+73
-436
lines changed

9 files changed

+73
-436
lines changed

ci/big_quickcheck/src/lib.rs

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -357,18 +357,3 @@ fn quickcheck_modpow() {
357357

358358
qc.quickcheck(test_modpow as fn(i128, u128, i128) -> TestResult);
359359
}
360-
361-
#[test]
362-
fn quickcheck_scalar_cmp() {
363-
let gen = StdThreadGen::new(usize::max_value());
364-
let mut qc = QuickCheck::with_gen(gen);
365-
366-
fn test_cmp(lhs: i64, rhs: i64) -> TestResult {
367-
let correct = lhs.partial_cmp(&rhs);
368-
let big_lhs = BigInt::from(lhs);
369-
let res = big_lhs.partial_cmp(&rhs);
370-
TestResult::from_bool(correct == res)
371-
}
372-
373-
qc.quickcheck(test_cmp as fn(i64, i64) -> TestResult);
374-
}

src/algorithms.rs

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -811,16 +811,6 @@ fn biguint_shr2(n: Cow<'_, BigUint>, digits: usize, shift: u8) -> BigUint {
811811
biguint_from_vec(data)
812812
}
813813

814-
pub(crate) fn cmp_zero_padded_slice(mut a: &[BigDigit], mut b: &[BigDigit]) -> Ordering {
815-
while let Some((&0, head)) = a.split_last() {
816-
a = head;
817-
}
818-
while let Some((&0, head)) = b.split_last() {
819-
b = head;
820-
}
821-
cmp_slice(a, b)
822-
}
823-
824814
pub(crate) fn cmp_slice(a: &[BigDigit], b: &[BigDigit]) -> Ordering {
825815
debug_assert!(a.last() != Some(&0));
826816
debug_assert!(b.last() != Some(&0));
@@ -854,29 +844,4 @@ mod algorithm_tests {
854844
assert_eq!(sub_sign_i(&a.data[..], &b.data[..]), &a_i - &b_i);
855845
assert_eq!(sub_sign_i(&b.data[..], &a.data[..]), &b_i - &a_i);
856846
}
857-
858-
#[test]
859-
fn test_cmp_zero_padded_slice() {
860-
use super::cmp_zero_padded_slice;
861-
use core::cmp::Ordering::*;
862-
863-
assert_eq!(cmp_zero_padded_slice(&[1, 0], &[1]), Equal);
864-
assert_eq!(cmp_zero_padded_slice(&[1], &[1, 0]), Equal);
865-
866-
assert_eq!(cmp_zero_padded_slice(&[0], &[0]), Equal);
867-
assert_eq!(cmp_zero_padded_slice(&[], &[0]), Equal);
868-
assert_eq!(cmp_zero_padded_slice(&[0], &[]), Equal);
869-
870-
assert_eq!(cmp_zero_padded_slice(&[1], &[0]), Greater);
871-
assert_eq!(cmp_zero_padded_slice(&[1000], &[0]), Greater);
872-
assert_eq!(cmp_zero_padded_slice(&[1000], &[0, 0, 0]), Greater);
873-
assert_eq!(cmp_zero_padded_slice(&[1000, 0], &[0, 0, 0]), Greater);
874-
assert_eq!(cmp_zero_padded_slice(&[0, 1000, 0], &[0, 1000]), Equal);
875-
876-
assert_eq!(cmp_zero_padded_slice(&[0], &[1]), Less);
877-
assert_eq!(cmp_zero_padded_slice(&[0], &[1000]), Less);
878-
assert_eq!(cmp_zero_padded_slice(&[0, 0, 0], &[1000]), Less);
879-
assert_eq!(cmp_zero_padded_slice(&[0, 0, 0], &[1000, 0]), Less);
880-
assert_eq!(cmp_zero_padded_slice(&[0, 1000], &[0, 1000, 0]), Equal);
881-
}
882847
}

src/bigint.rs

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -199,26 +199,6 @@ impl PartialOrd for BigInt {
199199
}
200200
}
201201

202-
fn sign<T>(num: T) -> Sign
203-
where
204-
T: Ord + Zero,
205-
{
206-
if num < T::zero() {
207-
Sign::Minus
208-
} else if num > T::zero() {
209-
Sign::Plus
210-
} else {
211-
Sign::NoSign
212-
}
213-
}
214-
215-
impl_partialord_partialeq_for_bigint!(i8, u8);
216-
impl_partialord_partialeq_for_bigint!(i16, u16);
217-
impl_partialord_partialeq_for_bigint!(i32, u32);
218-
impl_partialord_partialeq_for_bigint!(i64, u64);
219-
impl_partialord_partialeq_for_bigint!(i128, u128);
220-
impl_partialord_partialeq_for_bigint!(isize, usize);
221-
222202
impl Ord for BigInt {
223203
#[inline]
224204
fn cmp(&self, other: &BigInt) -> Ordering {

src/biguint.rs

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ mod monty;
3737

3838
use self::algorithms::{__add2, __sub2rev, add2, sub2, sub2rev};
3939
use self::algorithms::{biguint_shl, biguint_shr};
40-
use self::algorithms::{cmp_slice, cmp_zero_padded_slice, fls, ilog2};
40+
use self::algorithms::{cmp_slice, fls, ilog2};
4141
use self::algorithms::{div_rem, div_rem_digit, div_rem_ref, rem_digit};
4242
use self::algorithms::{mac_with_carry, mul3, scalar_mul};
4343
use self::monty::monty_modpow;
@@ -118,54 +118,6 @@ impl Ord for BigUint {
118118
}
119119
}
120120

121-
impl_partialord_partialeq_for_biguint_below_digit!(u8);
122-
impl_partialord_partialeq_for_biguint_below_digit!(u16);
123-
impl_partialord_partialeq_for_biguint_below_digit!(u32);
124-
125-
impl_scalar_partialeq!(impl PartialEq<u64> for BigUint);
126-
impl_partialord_rev!(impl PartialOrd<u64> for BigUint);
127-
impl PartialOrd<u64> for BigUint {
128-
#[cfg(u64_digit)]
129-
#[inline]
130-
fn partial_cmp(&self, other: &u64) -> Option<Ordering> {
131-
Some(cmp_zero_padded_slice(&self.data[..], &[*other]))
132-
}
133-
134-
#[cfg(not(u64_digit))]
135-
#[inline]
136-
fn partial_cmp(&self, other: &u64) -> Option<Ordering> {
137-
let (hi, lo) = big_digit::from_doublebigdigit(*other);
138-
Some(cmp_zero_padded_slice(&self.data[..], &[lo, hi]))
139-
}
140-
}
141-
142-
impl_scalar_partialeq!(impl PartialEq<u128> for BigUint);
143-
impl_partialord_rev!(impl PartialOrd<u128> for BigUint);
144-
impl PartialOrd<u128> for BigUint {
145-
#[cfg(u64_digit)]
146-
#[inline]
147-
fn partial_cmp(&self, other: &u128) -> Option<Ordering> {
148-
let (hi, lo) = big_digit::from_doublebigdigit(*other);
149-
Some(cmp_zero_padded_slice(&self.data[..], &[lo, hi]))
150-
}
151-
152-
#[cfg(not(u64_digit))]
153-
#[inline]
154-
fn partial_cmp(&self, other: &u128) -> Option<Ordering> {
155-
let (a, b, c, d) = u32_from_u128(*other);
156-
Some(cmp_zero_padded_slice(&self.data[..], &[d, c, b, a]))
157-
}
158-
}
159-
160-
impl_scalar_partialeq!(impl PartialEq<usize> for BigUint);
161-
impl_partialord_rev!(impl PartialOrd<usize> for BigUint);
162-
impl PartialOrd<usize> for BigUint {
163-
#[inline]
164-
fn partial_cmp(&self, other: &usize) -> Option<Ordering> {
165-
self.partial_cmp(&(*other as UsizePromotion))
166-
}
167-
}
168-
169121
impl Default for BigUint {
170122
#[inline]
171123
fn default() -> BigUint {

src/macros.rs

Lines changed: 0 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -439,86 +439,3 @@ macro_rules! impl_product_iter_type {
439439
}
440440
};
441441
}
442-
443-
/// Implements PartialEq based on PartialOrd
444-
macro_rules! impl_scalar_partialeq {
445-
(impl PartialEq < $scalar:ty > for $res:ty) => {
446-
impl PartialEq<$scalar> for $res {
447-
#[inline]
448-
fn eq(&self, other: &$scalar) -> bool {
449-
match self.partial_cmp(other) {
450-
Some(Equal) => true,
451-
_ => false,
452-
}
453-
}
454-
}
455-
impl PartialEq<$res> for $scalar {
456-
#[inline]
457-
fn eq(&self, other: &$res) -> bool {
458-
match self.partial_cmp(other) {
459-
Some(Equal) => true,
460-
_ => false,
461-
}
462-
}
463-
}
464-
};
465-
}
466-
467-
macro_rules! impl_partialord_rev {
468-
(impl PartialOrd < $scalar:ty > for $typ:ty) => {
469-
impl PartialOrd<$typ> for $scalar {
470-
#[inline]
471-
fn partial_cmp(&self, other: &$typ) -> Option<Ordering> {
472-
other.partial_cmp(self).map(Ordering::reverse)
473-
}
474-
}
475-
};
476-
}
477-
478-
macro_rules! impl_partialord_partialeq_for_biguint_below_digit {
479-
($typ_unsigned:ty) => {
480-
impl_scalar_partialeq!(impl PartialEq<$typ_unsigned> for BigUint);
481-
impl_partialord_rev!(impl PartialOrd<$typ_unsigned> for BigUint);
482-
impl PartialOrd<$typ_unsigned> for BigUint {
483-
#[inline]
484-
fn partial_cmp(&self, other: &$typ_unsigned) -> Option<Ordering> {
485-
Some(cmp_zero_padded_slice(&self.data[..], &[BigDigit::from(*other)]))
486-
}
487-
}
488-
};
489-
}
490-
491-
macro_rules! impl_partialord_partialeq_for_bigint {
492-
($typ_signed:ty, $typ_unsigned:ty) => {
493-
impl_scalar_partialeq!(impl PartialEq<$typ_signed> for BigInt);
494-
impl_partialord_rev!(impl PartialOrd<$typ_signed> for BigInt);
495-
impl PartialOrd<$typ_signed> for BigInt {
496-
#[inline]
497-
fn partial_cmp(&self, other: &$typ_signed) -> Option<Ordering> {
498-
let scmp = self.sign().cmp(&sign(*other));
499-
if scmp != Equal {
500-
return Some(scmp);
501-
}
502-
503-
let abs: $typ_unsigned = other.unsigned_abs();
504-
match self.sign {
505-
NoSign => Some(Equal),
506-
Plus => self.data.partial_cmp(&abs),
507-
Minus => abs.partial_cmp(&self.data),
508-
}
509-
}
510-
}
511-
512-
impl_scalar_partialeq!(impl PartialEq<$typ_unsigned> for BigInt);
513-
impl_partialord_rev!(impl PartialOrd<$typ_unsigned> for BigInt);
514-
impl PartialOrd<$typ_unsigned> for BigInt {
515-
#[inline]
516-
fn partial_cmp(&self, other: &$typ_unsigned) -> Option<Ordering> {
517-
match self.sign {
518-
Minus => Some(Less),
519-
_ => self.data.partial_cmp(other),
520-
}
521-
}
522-
}
523-
};
524-
}

tests/bigint.rs

Lines changed: 64 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,7 @@ fn test_from_bytes_be() {
3434
check("AB", "16706");
3535
check("Hello world!", "22405534230753963835153736737");
3636
assert_eq!(BigInt::from_bytes_be(Plus, &[]), BigInt::zero());
37-
assert_eq!(BigInt::from_bytes_be(Plus, &[]), 0);
3837
assert_eq!(BigInt::from_bytes_be(Minus, &[]), BigInt::zero());
39-
assert_eq!(BigInt::from_bytes_be(Minus, &[]), 0);
4038
}
4139

4240
#[test]
@@ -1022,26 +1020,74 @@ fn test_lcm() {
10221020

10231021
#[test]
10241022
fn test_next_multiple_of() {
1025-
assert_eq!(BigInt::from(16).next_multiple_of(&BigInt::from(8)), 16);
1026-
assert_eq!(BigInt::from(23).next_multiple_of(&BigInt::from(8)), 24);
1027-
assert_eq!(BigInt::from(16).next_multiple_of(&BigInt::from(-8)), 16);
1028-
assert_eq!(BigInt::from(23).next_multiple_of(&BigInt::from(-8)), 16);
1029-
assert_eq!(BigInt::from(-16).next_multiple_of(&BigInt::from(8)), -16);
1030-
assert_eq!(BigInt::from(-23).next_multiple_of(&BigInt::from(8)), -16);
1031-
assert_eq!(BigInt::from(-16).next_multiple_of(&BigInt::from(-8)), -16);
1032-
assert_eq!(BigInt::from(-23).next_multiple_of(&BigInt::from(-8)), -24);
1023+
assert_eq!(
1024+
BigInt::from(16).next_multiple_of(&BigInt::from(8)),
1025+
BigInt::from(16)
1026+
);
1027+
assert_eq!(
1028+
BigInt::from(23).next_multiple_of(&BigInt::from(8)),
1029+
BigInt::from(24)
1030+
);
1031+
assert_eq!(
1032+
BigInt::from(16).next_multiple_of(&BigInt::from(-8)),
1033+
BigInt::from(16)
1034+
);
1035+
assert_eq!(
1036+
BigInt::from(23).next_multiple_of(&BigInt::from(-8)),
1037+
BigInt::from(16)
1038+
);
1039+
assert_eq!(
1040+
BigInt::from(-16).next_multiple_of(&BigInt::from(8)),
1041+
BigInt::from(-16)
1042+
);
1043+
assert_eq!(
1044+
BigInt::from(-23).next_multiple_of(&BigInt::from(8)),
1045+
BigInt::from(-16)
1046+
);
1047+
assert_eq!(
1048+
BigInt::from(-16).next_multiple_of(&BigInt::from(-8)),
1049+
BigInt::from(-16)
1050+
);
1051+
assert_eq!(
1052+
BigInt::from(-23).next_multiple_of(&BigInt::from(-8)),
1053+
BigInt::from(-24)
1054+
);
10331055
}
10341056

10351057
#[test]
10361058
fn test_prev_multiple_of() {
1037-
assert_eq!(BigInt::from(16).prev_multiple_of(&BigInt::from(8)), 16);
1038-
assert_eq!(BigInt::from(23).prev_multiple_of(&BigInt::from(8)), 16);
1039-
assert_eq!(BigInt::from(16).prev_multiple_of(&BigInt::from(-8)), 16);
1040-
assert_eq!(BigInt::from(23).prev_multiple_of(&BigInt::from(-8)), 24);
1041-
assert_eq!(BigInt::from(-16).prev_multiple_of(&BigInt::from(8)), -16);
1042-
assert_eq!(BigInt::from(-23).prev_multiple_of(&BigInt::from(8)), -24);
1043-
assert_eq!(BigInt::from(-16).prev_multiple_of(&BigInt::from(-8)), -16);
1044-
assert_eq!(BigInt::from(-23).prev_multiple_of(&BigInt::from(-8)), -16);
1059+
assert_eq!(
1060+
BigInt::from(16).prev_multiple_of(&BigInt::from(8)),
1061+
BigInt::from(16)
1062+
);
1063+
assert_eq!(
1064+
BigInt::from(23).prev_multiple_of(&BigInt::from(8)),
1065+
BigInt::from(16)
1066+
);
1067+
assert_eq!(
1068+
BigInt::from(16).prev_multiple_of(&BigInt::from(-8)),
1069+
BigInt::from(16)
1070+
);
1071+
assert_eq!(
1072+
BigInt::from(23).prev_multiple_of(&BigInt::from(-8)),
1073+
BigInt::from(24)
1074+
);
1075+
assert_eq!(
1076+
BigInt::from(-16).prev_multiple_of(&BigInt::from(8)),
1077+
BigInt::from(-16)
1078+
);
1079+
assert_eq!(
1080+
BigInt::from(-23).prev_multiple_of(&BigInt::from(8)),
1081+
BigInt::from(-24)
1082+
);
1083+
assert_eq!(
1084+
BigInt::from(-16).prev_multiple_of(&BigInt::from(-8)),
1085+
BigInt::from(-16)
1086+
);
1087+
assert_eq!(
1088+
BigInt::from(-23).prev_multiple_of(&BigInt::from(-8)),
1089+
BigInt::from(-16)
1090+
);
10451091
}
10461092

10471093
#[test]

0 commit comments

Comments
 (0)