From 43ee92c40f57caed18838ebb5d6bfc9b67578eb1 Mon Sep 17 00:00:00 2001 From: nicoo Date: Tue, 23 Jun 2020 12:09:07 +0200 Subject: [PATCH 01/28] factor::numeric: Generalise modular inverse computation --- Cargo.lock | 3 ++ src/uu/factor/Cargo.toml | 4 +++ src/uu/factor/build.rs | 4 +-- src/uu/factor/src/numeric.rs | 57 ++++++++++++++++++++++++++---------- 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 25997280c..c6dc3bbfa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,3 +1,5 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. [[package]] name = "advapi32-sys" version = "0.2.0" @@ -1313,6 +1315,7 @@ dependencies = [ name = "uu_factor" version = "0.0.1" dependencies = [ + "num-traits 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)", "uucore 0.0.4 (git+https://github.com/uutils/uucore.git?branch=canary)", "uucore_procs 0.0.4 (git+https://github.com/uutils/uucore.git?branch=canary)", diff --git a/src/uu/factor/Cargo.toml b/src/uu/factor/Cargo.toml index 7589fe1bb..5a3cc6077 100644 --- a/src/uu/factor/Cargo.toml +++ b/src/uu/factor/Cargo.toml @@ -14,7 +14,11 @@ edition = "2018" [lib] path = "src/factor.rs" +[build-dependencies] +num-traits="0.2" + [dependencies] +num-traits = "0.2" rand = "0.5" uucore = { version="0.0.4", package="uucore", git="https://github.com/uutils/uucore.git", branch="canary" } uucore_procs = { version="0.0.4", package="uucore_procs", git="https://github.com/uutils/uucore.git", branch="canary" } diff --git a/src/uu/factor/build.rs b/src/uu/factor/build.rs index 719ca63bb..642b646c6 100644 --- a/src/uu/factor/build.rs +++ b/src/uu/factor/build.rs @@ -29,7 +29,7 @@ use miller_rabin::is_prime; #[path = "src/numeric.rs"] mod numeric; -use numeric::inv_mod_u64; +use numeric::modular_inverse; mod sieve; @@ -57,7 +57,7 @@ fn main() { let mut x = primes.next().unwrap(); for next in primes { // format the table - let outstr = format!("({}, {}, {}),", x, inv_mod_u64(x), std::u64::MAX / x); + let outstr = format!("({}, {}, {}),", x, modular_inverse(x), std::u64::MAX / x); if cols + outstr.len() > MAX_WIDTH { write!(file, "\n {}", outstr).unwrap(); cols = 4 + outstr.len(); diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index 29babb281..f1b447583 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -6,6 +6,11 @@ // * For the full copyright and license information, please view the LICENSE file // * that was distributed with this source code. +use num_traits::{ + int::PrimInt, + ops::wrapping::{WrappingMul, WrappingSub}, +}; +use std::fmt::Debug; use std::mem::swap; // This is incorrectly reported as dead code, @@ -100,7 +105,7 @@ impl Arithmetic for Montgomery { type I = u64; fn new(n: u64) -> Self { - let a = inv_mod_u64(n).wrapping_neg(); + let a = modular_inverse(n).wrapping_neg(); debug_assert_eq!(n.wrapping_mul(a), 1_u64.wrapping_neg()); Montgomery { a, n } } @@ -172,35 +177,42 @@ impl Arithmetic for Montgomery { } } +pub(crate) trait Int: Debug + PrimInt + WrappingSub + WrappingMul {} +impl Int for u64 {} +impl Int for u32 {} + // extended Euclid algorithm // precondition: a is odd -pub(crate) fn inv_mod_u64(a: u64) -> u64 { - assert!(a % 2 == 1, "{} is not odd", a); - let mut t = 0u64; - let mut newt = 1u64; - let mut r = 0u64; +pub(crate) fn modular_inverse(a: T) -> T { + let zero = T::zero(); + let one = T::one(); + assert!(a % (one + one) == one, "{:?} is not odd", a); + + let mut t = zero; + let mut newt = one; + let mut r = zero; let mut newr = a; - while newr != 0 { - let quot = if r == 0 { + while newr != zero { + let quot = if r == zero { // special case when we're just starting out // This works because we know that // a does not divide 2^64, so floor(2^64 / a) == floor((2^64-1) / a); - std::u64::MAX + T::max_value() } else { r } / newr; - let newtp = t.wrapping_sub(quot.wrapping_mul(newt)); + let newtp = t.wrapping_sub(".wrapping_mul(&newt)); t = newt; newt = newtp; - let newrp = r.wrapping_sub(quot.wrapping_mul(newr)); + let newrp = r.wrapping_sub(".wrapping_mul(&newr)); r = newr; newr = newrp; } - assert_eq!(r, 1); + assert_eq!(r, one); t } @@ -208,12 +220,25 @@ pub(crate) fn inv_mod_u64(a: u64) -> u64 { mod tests { use super::*; - #[test] - fn test_inverter() { + fn test_inverter() { // All odd integers from 1 to 20 000 - let mut test_values = (0..10_000u64).map(|i| 2 * i + 1); + let one = T::from(1).unwrap(); + let two = T::from(2).unwrap(); + let mut test_values = (0..10_000) + .map(|i| T::from(i).unwrap()) + .map(|i| two * i + one); - assert!(test_values.all(|x| x.wrapping_mul(inv_mod_u64(x)) == 1)); + assert!(test_values.all(|x| x.wrapping_mul(&modular_inverse(x)) == one)); + } + + #[test] + fn test_inverter_u32() { + test_inverter::() + } + + #[test] + fn test_inverter_u64() { + test_inverter::() } #[test] From e68bb192f2f44d0bbd4b5ac10cd343bd057251dd Mon Sep 17 00:00:00 2001 From: nicoo Date: Sun, 21 Jun 2020 16:50:11 +0200 Subject: [PATCH 02/28] factor::numeric: Add a 32b Montgomery variant [WiP] ~32% faster --- src/uu/factor/src/numeric.rs | 108 +++++++++++++++++++++++++++++++++++ src/uu/factor/src/rho.rs | 13 ++++- 2 files changed, 118 insertions(+), 3 deletions(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index f1b447583..118b4d9fa 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -216,6 +216,114 @@ pub(crate) fn modular_inverse(a: T) -> T { t } +#[derive(Clone, Copy, Debug)] +pub(crate) struct Montgomery32 { + a: u32, + n: u32, +} + +impl Montgomery32 { + /// computes x/R mod n efficiently + fn reduce(&self, x: u64) -> u32 { + debug_assert!(x < (self.n as u64) << 32); + // TODO: optimiiiiiiise + let Montgomery32 { a, n } = self; + let m = (x as u32).wrapping_mul(*a); + let nm = (*n as u64) * (m as u64); + let (xnm, overflow) = x.overflowing_add(nm); // x + n*m + debug_assert_eq!(xnm % (1 << 32), 0); + + // (x + n*m) / R + // in case of overflow, this is (2¹²⁸ + xnm)/2⁶⁴ - n = xnm/2⁶⁴ + (2⁶⁴ - n) + let y = (xnm >> 32) as u32 + if !overflow { 0 } else { n.wrapping_neg() }; + + if y >= *n { + y - n + } else { + y + } + } +} + +impl Arithmetic for Montgomery32 { + // Montgomery transform, R=2⁶⁴ + // Provides fast arithmetic mod n (n odd, u64) + type I = u32; + + fn new(n: u64) -> Self { + debug_assert!(n < (1 << 32)); + let n = n as u32; + let a = modular_inverse(n).wrapping_neg(); + debug_assert_eq!(n.wrapping_mul(a), 1_u32.wrapping_neg()); + Montgomery32 { a, n } + } + + fn modulus(&self) -> u64 { + self.n as u64 + } + + fn from_u64(&self, x: u64) -> Self::I { + assert!(x < self.n as u64); + let r = ((x << 32) % self.n as u64) as u32; + debug_assert_eq!(x, self.to_u64(r)); + r + } + + fn to_u64(&self, n: Self::I) -> u64 { + self.reduce(n as u64) as u64 + } + + fn add(&self, a: Self::I, b: Self::I) -> Self::I { + let (r, overflow) = a.overflowing_add(b); + + // In case of overflow, a+b = 2⁶⁴ + r = (2⁶⁴ - n) + r (working mod n) + let r = if !overflow { + r + } else { + r + self.n.wrapping_neg() + }; + + // Normalise to [0; n[ + let r = if r < self.n { r } else { r - self.n }; + + // Check that r (reduced back to the usual representation) equals + // a+b % n + #[cfg(debug_assertions)] + { + let a_r = self.to_u64(a); + let b_r = self.to_u64(b); + let r_r = self.to_u64(r); + let r_2 = (((a_r as u128) + (b_r as u128)) % (self.n as u128)) as u64; + debug_assert_eq!( + r_r, r_2, + "[{}] = {} ≠ {} = {} + {} = [{}] + [{}] mod {}; a = {}", + r, r_r, r_2, a_r, b_r, a, b, self.n, self.a + ); + } + r + } + + fn mul(&self, a: Self::I, b: Self::I) -> Self::I { + let r = self.reduce((a as u64) * (b as u64)); + + // Check that r (reduced back to the usual representation) equals + // a*b % n + #[cfg(debug_assertions)] + { + let a_r = self.to_u64(a); + let b_r = self.to_u64(b); + let r_r = self.to_u64(r); + let r_2 = (a_r * b_r) % (self.n as u64); + debug_assert_eq!( + r_r, r_2, + "[{}] = {} ≠ {} = {} * {} = [{}] * [{}] mod {}; a = {}", + r, r_r, r_2, a_r, b_r, a, b, self.n, self.a + ); + } + r + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/uu/factor/src/rho.rs b/src/uu/factor/src/rho.rs index 9a53a40f4..d2023f70f 100644 --- a/src/uu/factor/src/rho.rs +++ b/src/uu/factor/src/rho.rs @@ -51,8 +51,11 @@ fn find_divisor(n: A) -> u64 { fn _factor(num: u64) -> Factors { // Shadow the name, so the recursion automatically goes from “Big” arithmetic to small. let _factor = |n| { - // TODO: Optimise with 32 and 64b versions - _factor::(n) + if n < (1 << 32) { + _factor::(n) + } else { + _factor::(n) + } }; if num == 1 { @@ -75,5 +78,9 @@ fn _factor(num: u64) -> Factors { } pub(crate) fn factor(n: u64) -> Factors { - _factor::(n) + if n < (1 << 32) { + _factor::(n) + } else { + _factor::(n) + } } From a440807e6c134b6f8baecea0e5641fa6784ed84f Mon Sep 17 00:00:00 2001 From: nicoo Date: Sun, 21 Jun 2020 17:00:46 +0200 Subject: [PATCH 03/28] factor::miller_rabin: Use a specialized basis for 32b integers ~3% faster --- src/uu/factor/src/miller_rabin.rs | 29 +++++++++++++++++++++++------ src/uu/factor/src/rho.rs | 4 ++-- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index d133bc674..45f4243ba 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -2,10 +2,27 @@ use crate::numeric::*; -// Small set of bases for the Miller-Rabin prime test, valid for all 64b integers; -// discovered by Jim Sinclair on 2011-04-20, see miller-rabin.appspot.com -#[allow(clippy::unreadable_literal)] -const BASIS: [u64; 7] = [2, 325, 9375, 28178, 450775, 9780504, 1795265022]; +pub(crate) trait Basis { + const BASIS: &'static [u64]; +} + +impl Basis for Montgomery { + // Small set of bases for the Miller-Rabin prime test, valid for all 64b integers; + // discovered by Jim Sinclair on 2011-04-20, see miller-rabin.appspot.com + #[allow(clippy::unreadable_literal)] + const BASIS: &'static [u64] = &[2, 325, 9375, 28178, 450775, 9780504, 1795265022]; +} + +impl Basis for Montgomery32 { + // Small set of bases for the Miller-Rabin prime test, valid for all 32b integers; + // discovered by Steve Worley on 2013-05-27, see miller-rabin.appspot.com + #[allow(clippy::unreadable_literal)] + const BASIS: &'static [u64] = &[ + 4230279247111683200, + 14694767155120705706, + 16641139526367750375, + ]; +} #[derive(Eq, PartialEq)] pub(crate) enum Result { @@ -23,7 +40,7 @@ impl Result { // Deterministic Miller-Rabin primality-checking algorithm, adapted to extract // (some) dividers; it will fail to factor strong pseudoprimes. #[allow(clippy::many_single_char_names)] -pub(crate) fn test(m: A) -> Result { +pub(crate) fn test(m: A) -> Result { use self::Result::*; let n = m.modulus(); @@ -41,7 +58,7 @@ pub(crate) fn test(m: A) -> Result { let one = m.one(); let minus_one = m.minus_one(); - for _a in BASIS.iter() { + for _a in A::BASIS.iter() { let _a = _a % n; if _a == 0 { break; diff --git a/src/uu/factor/src/rho.rs b/src/uu/factor/src/rho.rs index d2023f70f..29c58feb8 100644 --- a/src/uu/factor/src/rho.rs +++ b/src/uu/factor/src/rho.rs @@ -15,7 +15,7 @@ use crate::miller_rabin::Result::*; use crate::numeric::*; use crate::{miller_rabin, Factors}; -fn find_divisor(n: A) -> u64 { +fn find_divisor(n: A) -> u64 { #![allow(clippy::many_single_char_names)] let mut rand = { let range = Uniform::new(1, n.modulus()); @@ -48,7 +48,7 @@ fn find_divisor(n: A) -> u64 { } } -fn _factor(num: u64) -> Factors { +fn _factor(num: u64) -> Factors { // Shadow the name, so the recursion automatically goes from “Big” arithmetic to small. let _factor = |n| { if n < (1 << 32) { From 4d28f48ad94d2e4d9f8aeefef0d41f56ff0698b3 Mon Sep 17 00:00:00 2001 From: Alex Lyon Date: Wed, 24 Jun 2020 03:20:49 -0500 Subject: [PATCH 04/28] factor: combine Montgomery and Montgomery32 --- src/uu/factor/src/miller_rabin.rs | 6 +- src/uu/factor/src/numeric.rs | 251 ++++++++++++++---------------- src/uu/factor/src/rho.rs | 6 +- 3 files changed, 121 insertions(+), 142 deletions(-) diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index 45f4243ba..330c8f127 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -6,14 +6,14 @@ pub(crate) trait Basis { const BASIS: &'static [u64]; } -impl Basis for Montgomery { +impl Basis for Montgomery { // Small set of bases for the Miller-Rabin prime test, valid for all 64b integers; // discovered by Jim Sinclair on 2011-04-20, see miller-rabin.appspot.com #[allow(clippy::unreadable_literal)] const BASIS: &'static [u64] = &[2, 325, 9375, 28178, 450775, 9780504, 1795265022]; } -impl Basis for Montgomery32 { +impl Basis for Montgomery { // Small set of bases for the Miller-Rabin prime test, valid for all 32b integers; // discovered by Steve Worley on 2013-05-27, see miller-rabin.appspot.com #[allow(clippy::unreadable_literal)] @@ -104,7 +104,7 @@ pub(crate) fn test(m: A) -> Result { // Used by build.rs' tests #[allow(dead_code)] pub(crate) fn is_prime(n: u64) -> bool { - test::(Montgomery::new(n)).is_prime() + test::>(Montgomery::new(n)).is_prime() } #[cfg(test)] diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index 118b4d9fa..5934ab749 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -7,10 +7,12 @@ // * that was distributed with this source code. use num_traits::{ + cast::AsPrimitive, + identities::{One, Zero}, int::PrimInt, - ops::wrapping::{WrappingMul, WrappingSub}, + ops::wrapping::{WrappingMul, WrappingNeg, WrappingSub}, }; -use std::fmt::Debug; +use std::fmt::{Debug, Display}; use std::mem::swap; // This is incorrectly reported as dead code, @@ -71,63 +73,78 @@ pub(crate) trait Arithmetic: Copy + Sized { } #[derive(Clone, Copy, Debug)] -pub(crate) struct Montgomery { - a: u64, - n: u64, +pub(crate) struct Montgomery { + a: T, + n: T, } -impl Montgomery { +impl Montgomery { /// computes x/R mod n efficiently - fn reduce(&self, x: u128) -> u64 { - debug_assert!(x < (self.n as u128) << 64); + fn reduce(&self, x: T::Intermediate) -> T { + let t_bits = T::zero().count_zeros() as usize; + + debug_assert!(x < (self.n.as_intermediate()) << t_bits); // TODO: optimiiiiiiise let Montgomery { a, n } = self; - let m = (x as u64).wrapping_mul(*a); - let nm = (*n as u128) * (m as u128); - let (xnm, overflow) = (x as u128).overflowing_add(nm); // x + n*m - debug_assert_eq!(xnm % (1 << 64), 0); + let m = T::from_intermediate(x).wrapping_mul(a); + let nm = (n.as_intermediate()) * (m.as_intermediate()); + let (xnm, overflow) = x.overflowing_add_(nm); // x + n*m + debug_assert_eq!( + xnm % (T::Intermediate::one() << T::zero().count_zeros() as usize), + T::Intermediate::zero() + ); // (x + n*m) / R // in case of overflow, this is (2¹²⁸ + xnm)/2⁶⁴ - n = xnm/2⁶⁴ + (2⁶⁴ - n) - let y = (xnm >> 64) as u64 + if !overflow { 0 } else { n.wrapping_neg() }; + let y = T::from_intermediate(xnm >> t_bits) + + if !overflow { + T::zero() + } else { + n.wrapping_neg() + }; if y >= *n { - y - n + y - *n } else { y } } } -impl Arithmetic for Montgomery { +impl Arithmetic for Montgomery { // Montgomery transform, R=2⁶⁴ // Provides fast arithmetic mod n (n odd, u64) - type I = u64; + type I = T; fn new(n: u64) -> Self { + debug_assert!(T::zero().count_zeros() >= 64 || n < (1 << T::zero().count_zeros() as usize)); + let n = T::from_u64(n); let a = modular_inverse(n).wrapping_neg(); - debug_assert_eq!(n.wrapping_mul(a), 1_u64.wrapping_neg()); + debug_assert_eq!(n.wrapping_mul(&a), T::one().wrapping_neg()); Montgomery { a, n } } fn modulus(&self) -> u64 { - self.n + self.n.as_() } fn from_u64(&self, x: u64) -> Self::I { // TODO: optimise! - assert!(x < self.n); - let r = (((x as u128) << 64) % self.n as u128) as u64; + assert!(x < self.n.as_()); + let r = T::from_intermediate( + ((T::Intermediate::from(x)) << T::zero().count_zeros() as usize) + % self.n.as_intermediate(), + ); debug_assert_eq!(x, self.to_u64(r)); r } fn to_u64(&self, n: Self::I) -> u64 { - self.reduce(n as u128) + self.reduce(n.as_intermediate()).as_() } fn add(&self, a: Self::I, b: Self::I) -> Self::I { - let (r, overflow) = a.overflowing_add(b); + let (r, overflow) = a.overflowing_add_(b); // In case of overflow, a+b = 2⁶⁴ + r = (2⁶⁴ - n) + r (working mod n) let r = if !overflow { @@ -146,7 +163,8 @@ impl Arithmetic for Montgomery { let a_r = self.to_u64(a); let b_r = self.to_u64(b); let r_r = self.to_u64(r); - let r_2 = (((a_r as u128) + (b_r as u128)) % (self.n as u128)) as u64; + let r_2 = + (((a_r as u128) + (b_r as u128)) % >::as_(self.n)) as u64; debug_assert_eq!( r_r, r_2, "[{}] = {} ≠ {} = {} + {} = [{}] + [{}] mod {}; a = {}", @@ -157,7 +175,7 @@ impl Arithmetic for Montgomery { } fn mul(&self, a: Self::I, b: Self::I) -> Self::I { - let r = self.reduce((a as u128) * (b as u128)); + let r = self.reduce(a.as_intermediate() * b.as_intermediate()); // Check that r (reduced back to the usual representation) equals // a*b % n @@ -166,7 +184,9 @@ impl Arithmetic for Montgomery { let a_r = self.to_u64(a); let b_r = self.to_u64(b); let r_r = self.to_u64(r); - let r_2 = (((a_r as u128) * (b_r as u128)) % (self.n as u128)) as u64; + let r_2: u64 = ((T::Intermediate::from(a_r) * T::Intermediate::from(b_r)) + % self.n.as_intermediate()) + .as_(); debug_assert_eq!( r_r, r_2, "[{}] = {} ≠ {} = {} * {} = [{}] * [{}] mod {}; a = {}", @@ -177,9 +197,76 @@ impl Arithmetic for Montgomery { } } -pub(crate) trait Int: Debug + PrimInt + WrappingSub + WrappingMul {} -impl Int for u64 {} -impl Int for u32 {} +pub(crate) trait OverflowingAdd: Sized { + fn overflowing_add_(self, n: Self) -> (Self, bool); +} +impl OverflowingAdd for u32 { + fn overflowing_add_(self, n: Self) -> (Self, bool) { + self.overflowing_add(n) + } +} +impl OverflowingAdd for u64 { + fn overflowing_add_(self, n: Self) -> (Self, bool) { + self.overflowing_add(n) + } +} +impl OverflowingAdd for u128 { + fn overflowing_add_(self, n: Self) -> (Self, bool) { + self.overflowing_add(n) + } +} + +pub(crate) trait Int: + AsPrimitive + + AsPrimitive + + Display + + Debug + + PrimInt + + OverflowingAdd + + WrappingNeg + + WrappingSub + + WrappingMul +{ + type Intermediate: From + + AsPrimitive + + Display + + Debug + + PrimInt + + OverflowingAdd + + WrappingNeg + + WrappingSub + + WrappingMul; + + fn as_intermediate(self) -> Self::Intermediate; + fn from_intermediate(n: Self::Intermediate) -> Self; + fn from_u64(n: u64) -> Self; +} +impl Int for u64 { + type Intermediate = u128; + + fn as_intermediate(self) -> u128 { + self as _ + } + fn from_intermediate(n: u128) -> u64 { + n as _ + } + fn from_u64(n: u64) -> u64 { + n + } +} +impl Int for u32 { + type Intermediate = u64; + + fn as_intermediate(self) -> u64 { + self as _ + } + fn from_intermediate(n: u64) -> u32 { + n as _ + } + fn from_u64(n: u64) -> u32 { + n as _ + } +} // extended Euclid algorithm // precondition: a is odd @@ -216,114 +303,6 @@ pub(crate) fn modular_inverse(a: T) -> T { t } -#[derive(Clone, Copy, Debug)] -pub(crate) struct Montgomery32 { - a: u32, - n: u32, -} - -impl Montgomery32 { - /// computes x/R mod n efficiently - fn reduce(&self, x: u64) -> u32 { - debug_assert!(x < (self.n as u64) << 32); - // TODO: optimiiiiiiise - let Montgomery32 { a, n } = self; - let m = (x as u32).wrapping_mul(*a); - let nm = (*n as u64) * (m as u64); - let (xnm, overflow) = x.overflowing_add(nm); // x + n*m - debug_assert_eq!(xnm % (1 << 32), 0); - - // (x + n*m) / R - // in case of overflow, this is (2¹²⁸ + xnm)/2⁶⁴ - n = xnm/2⁶⁴ + (2⁶⁴ - n) - let y = (xnm >> 32) as u32 + if !overflow { 0 } else { n.wrapping_neg() }; - - if y >= *n { - y - n - } else { - y - } - } -} - -impl Arithmetic for Montgomery32 { - // Montgomery transform, R=2⁶⁴ - // Provides fast arithmetic mod n (n odd, u64) - type I = u32; - - fn new(n: u64) -> Self { - debug_assert!(n < (1 << 32)); - let n = n as u32; - let a = modular_inverse(n).wrapping_neg(); - debug_assert_eq!(n.wrapping_mul(a), 1_u32.wrapping_neg()); - Montgomery32 { a, n } - } - - fn modulus(&self) -> u64 { - self.n as u64 - } - - fn from_u64(&self, x: u64) -> Self::I { - assert!(x < self.n as u64); - let r = ((x << 32) % self.n as u64) as u32; - debug_assert_eq!(x, self.to_u64(r)); - r - } - - fn to_u64(&self, n: Self::I) -> u64 { - self.reduce(n as u64) as u64 - } - - fn add(&self, a: Self::I, b: Self::I) -> Self::I { - let (r, overflow) = a.overflowing_add(b); - - // In case of overflow, a+b = 2⁶⁴ + r = (2⁶⁴ - n) + r (working mod n) - let r = if !overflow { - r - } else { - r + self.n.wrapping_neg() - }; - - // Normalise to [0; n[ - let r = if r < self.n { r } else { r - self.n }; - - // Check that r (reduced back to the usual representation) equals - // a+b % n - #[cfg(debug_assertions)] - { - let a_r = self.to_u64(a); - let b_r = self.to_u64(b); - let r_r = self.to_u64(r); - let r_2 = (((a_r as u128) + (b_r as u128)) % (self.n as u128)) as u64; - debug_assert_eq!( - r_r, r_2, - "[{}] = {} ≠ {} = {} + {} = [{}] + [{}] mod {}; a = {}", - r, r_r, r_2, a_r, b_r, a, b, self.n, self.a - ); - } - r - } - - fn mul(&self, a: Self::I, b: Self::I) -> Self::I { - let r = self.reduce((a as u64) * (b as u64)); - - // Check that r (reduced back to the usual representation) equals - // a*b % n - #[cfg(debug_assertions)] - { - let a_r = self.to_u64(a); - let b_r = self.to_u64(b); - let r_r = self.to_u64(r); - let r_2 = (a_r * b_r) % (self.n as u64); - debug_assert_eq!( - r_r, r_2, - "[{}] = {} ≠ {} = {} * {} = [{}] * [{}] mod {}; a = {}", - r, r_r, r_2, a_r, b_r, a, b, self.n, self.a - ); - } - r - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/uu/factor/src/rho.rs b/src/uu/factor/src/rho.rs index 29c58feb8..37df5169d 100644 --- a/src/uu/factor/src/rho.rs +++ b/src/uu/factor/src/rho.rs @@ -52,7 +52,7 @@ fn _factor(num: u64) -> Factors { // Shadow the name, so the recursion automatically goes from “Big” arithmetic to small. let _factor = |n| { if n < (1 << 32) { - _factor::(n) + _factor::>(n) } else { _factor::(n) } @@ -79,8 +79,8 @@ fn _factor(num: u64) -> Factors { pub(crate) fn factor(n: u64) -> Factors { if n < (1 << 32) { - _factor::(n) + _factor::>(n) } else { - _factor::(n) + _factor::>(n) } } From 774feb0a40bfe118c55285e536c0f41459f0e5da Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 24 Jun 2020 17:50:38 +0200 Subject: [PATCH 05/28] factor::numeric: Generalise tests for Arithmetic trait --- src/uu/factor/src/numeric.rs | 41 ++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index 5934ab749..c6b40aa9b 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -328,11 +328,10 @@ mod tests { test_inverter::() } - #[test] - fn test_montgomery_add() { + fn test_add() { for n in 0..100 { let n = 2 * n + 1; - let m = Montgomery::new(n); + let m = A::new(n); for x in 0..n { let m_x = m.from_u64(x); for y in 0..=x { @@ -345,10 +344,19 @@ mod tests { } #[test] - fn test_montgomery_mult() { + fn test_add_m32() { + test_add::>() + } + + #[test] + fn test_add_m64() { + test_add::>() + } + + fn test_mult() { for n in 0..100 { let n = 2 * n + 1; - let m = Montgomery::new(n); + let m = A::new(n); for x in 0..n { let m_x = m.from_u64(x); for y in 0..=x { @@ -360,14 +368,33 @@ mod tests { } #[test] - fn test_montgomery_roundtrip() { + fn test_mult_m32() { + test_mult::>() + } + + #[test] + fn test_mult_m64() { + test_mult::>() + } + + fn test_roundtrip() { for n in 0..100 { let n = 2 * n + 1; - let m = Montgomery::new(n); + let m = A::new(n); for x in 0..n { let x_ = m.from_u64(x); assert_eq!(x, m.to_u64(x_)); } } } + + #[test] + fn test_roundtrip_m32() { + test_roundtrip::>() + } + + #[test] + fn test_roundtrip_m64() { + test_roundtrip::>() + } } From 3f79be0219285b99d33cff97fe5e92fdd3591d6b Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 24 Jun 2020 19:18:43 +0200 Subject: [PATCH 06/28] factor::numeric: Use debug_assert! for runtime assertions. --- src/uu/factor/src/numeric.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index c6b40aa9b..5f15142ef 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -130,7 +130,7 @@ impl Arithmetic for Montgomery { fn from_u64(&self, x: u64) -> Self::I { // TODO: optimise! - assert!(x < self.n.as_()); + debug_assert!(x < self.n.as_()); let r = T::from_intermediate( ((T::Intermediate::from(x)) << T::zero().count_zeros() as usize) % self.n.as_intermediate(), @@ -273,7 +273,7 @@ impl Int for u32 { pub(crate) fn modular_inverse(a: T) -> T { let zero = T::zero(); let one = T::one(); - assert!(a % (one + one) == one, "{:?} is not odd", a); + debug_assert!(a % (one + one) == one, "{:?} is not odd", a); let mut t = zero; let mut newt = one; @@ -299,7 +299,7 @@ pub(crate) fn modular_inverse(a: T) -> T { newr = newrp; } - assert_eq!(r, one); + debug_assert_eq!(r, one); t } From 28244413d1463ebde179a1adbb791e7f566d8408 Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 1 Jul 2020 12:55:54 +0200 Subject: [PATCH 07/28] factor::numeric: Document when to remove OverflowingAdd trait --- src/uu/factor/src/numeric.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index 5f15142ef..7d9c00c4d 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -197,6 +197,8 @@ impl Arithmetic for Montgomery { } } +// NOTE: Trait can be removed once num-traits adds a similar one; +// see https://github.com/rust-num/num-traits/issues/168 pub(crate) trait OverflowingAdd: Sized { fn overflowing_add_(self, n: Self) -> (Self, bool); } From caa79a1261d286faabd0883ffa54fe03f36a95d1 Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 1 Jul 2020 13:06:25 +0200 Subject: [PATCH 08/28] factor::numeric: Split Int and DoubleInt traits --- src/uu/factor/src/numeric.rs | 75 ++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index 7d9c00c4d..a640d8162 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -7,7 +7,7 @@ // * that was distributed with this source code. use num_traits::{ - cast::AsPrimitive, + cast::{AsPrimitive, ToPrimitive}, identities::{One, Zero}, int::PrimInt, ops::wrapping::{WrappingMul, WrappingNeg, WrappingSub}, @@ -73,30 +73,30 @@ pub(crate) trait Arithmetic: Copy + Sized { } #[derive(Clone, Copy, Debug)] -pub(crate) struct Montgomery { +pub(crate) struct Montgomery { a: T, n: T, } -impl Montgomery { +impl Montgomery { /// computes x/R mod n efficiently - fn reduce(&self, x: T::Intermediate) -> T { + fn reduce(&self, x: T::Double) -> T { let t_bits = T::zero().count_zeros() as usize; - debug_assert!(x < (self.n.as_intermediate()) << t_bits); + debug_assert!(x < (self.n.as_double()) << t_bits); // TODO: optimiiiiiiise let Montgomery { a, n } = self; - let m = T::from_intermediate(x).wrapping_mul(a); - let nm = (n.as_intermediate()) * (m.as_intermediate()); + let m = T::from_double(x).wrapping_mul(a); + let nm = (n.as_double()) * (m.as_double()); let (xnm, overflow) = x.overflowing_add_(nm); // x + n*m debug_assert_eq!( - xnm % (T::Intermediate::one() << T::zero().count_zeros() as usize), - T::Intermediate::zero() + xnm % (T::Double::one() << T::zero().count_zeros() as usize), + T::Double::zero() ); // (x + n*m) / R // in case of overflow, this is (2¹²⁸ + xnm)/2⁶⁴ - n = xnm/2⁶⁴ + (2⁶⁴ - n) - let y = T::from_intermediate(xnm >> t_bits) + let y = T::from_double(xnm >> t_bits) + if !overflow { T::zero() } else { @@ -111,7 +111,7 @@ impl Montgomery { } } -impl Arithmetic for Montgomery { +impl Arithmetic for Montgomery { // Montgomery transform, R=2⁶⁴ // Provides fast arithmetic mod n (n odd, u64) type I = T; @@ -131,16 +131,15 @@ impl Arithmetic for Montgomery { fn from_u64(&self, x: u64) -> Self::I { // TODO: optimise! debug_assert!(x < self.n.as_()); - let r = T::from_intermediate( - ((T::Intermediate::from(x)) << T::zero().count_zeros() as usize) - % self.n.as_intermediate(), + let r = T::from_double( + ((T::Double::from(x)) << T::zero().count_zeros() as usize) % self.n.as_double(), ); debug_assert_eq!(x, self.to_u64(r)); r } fn to_u64(&self, n: Self::I) -> u64 { - self.reduce(n.as_intermediate()).as_() + self.reduce(n.as_double()).as_() } fn add(&self, a: Self::I, b: Self::I) -> Self::I { @@ -175,7 +174,7 @@ impl Arithmetic for Montgomery { } fn mul(&self, a: Self::I, b: Self::I) -> Self::I { - let r = self.reduce(a.as_intermediate() * b.as_intermediate()); + let r = self.reduce(a.as_double() * b.as_double()); // Check that r (reduced back to the usual representation) equals // a*b % n @@ -184,9 +183,9 @@ impl Arithmetic for Montgomery { let a_r = self.to_u64(a); let b_r = self.to_u64(b); let r_r = self.to_u64(r); - let r_2: u64 = ((T::Intermediate::from(a_r) * T::Intermediate::from(b_r)) - % self.n.as_intermediate()) - .as_(); + let r_2: u64 = ((T::Double::from(a_r) * T::Double::from(b_r)) % self.n.as_double()) + .to_u64() + .unwrap(); debug_assert_eq!( r_r, r_2, "[{}] = {} ≠ {} = {} * {} = [{}] * [{}] mod {}; a = {}", @@ -229,40 +228,40 @@ pub(crate) trait Int: + WrappingSub + WrappingMul { - type Intermediate: From - + AsPrimitive - + Display - + Debug - + PrimInt - + OverflowingAdd - + WrappingNeg - + WrappingSub - + WrappingMul; +} - fn as_intermediate(self) -> Self::Intermediate; - fn from_intermediate(n: Self::Intermediate) -> Self; +pub(crate) trait DoubleInt: Int { + type Double: Int + From + ToPrimitive; + + fn as_double(self) -> Self::Double; + fn from_double(n: Self::Double) -> Self; fn from_u64(n: u64) -> Self; } -impl Int for u64 { - type Intermediate = u128; - fn as_intermediate(self) -> u128 { +impl Int for u32 {} +impl Int for u64 {} +impl Int for u128 {} + +impl DoubleInt for u64 { + type Double = u128; + + fn as_double(self) -> u128 { self as _ } - fn from_intermediate(n: u128) -> u64 { + fn from_double(n: u128) -> u64 { n as _ } fn from_u64(n: u64) -> u64 { n } } -impl Int for u32 { - type Intermediate = u64; +impl DoubleInt for u32 { + type Double = u64; - fn as_intermediate(self) -> u64 { + fn as_double(self) -> u64 { self as _ } - fn from_intermediate(n: u64) -> u32 { + fn from_double(n: u64) -> u32 { n as _ } fn from_u64(n: u64) -> u32 { From 19a8231fb27e2479a44471e2b48503bcc08b67eb Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 1 Jul 2020 13:15:50 +0200 Subject: [PATCH 09/28] factor::numeric::Arithmetic: Rename associated type I to ModInt --- src/uu/factor/src/numeric.rs | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index a640d8162..947b6bfef 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -27,16 +27,17 @@ pub(crate) fn gcd(mut a: u64, mut b: u64) -> u64 { } pub(crate) trait Arithmetic: Copy + Sized { - type I: Copy + Sized + Eq; + // The type of integers mod m, in some opaque representation + type ModInt: Copy + Sized + Eq; fn new(m: u64) -> Self; fn modulus(&self) -> u64; - fn from_u64(&self, n: u64) -> Self::I; - fn to_u64(&self, n: Self::I) -> u64; - fn add(&self, a: Self::I, b: Self::I) -> Self::I; - fn mul(&self, a: Self::I, b: Self::I) -> Self::I; + fn from_u64(&self, n: u64) -> Self::ModInt; + fn to_u64(&self, n: Self::ModInt) -> u64; + fn add(&self, a: Self::ModInt, b: Self::ModInt) -> Self::ModInt; + fn mul(&self, a: Self::ModInt, b: Self::ModInt) -> Self::ModInt; - fn pow(&self, mut a: Self::I, mut b: u64) -> Self::I { + fn pow(&self, mut a: Self::ModInt, mut b: u64) -> Self::ModInt { let (_a, _b) = (a, b); let mut result = self.one(); while b > 0 { @@ -61,13 +62,13 @@ pub(crate) trait Arithmetic: Copy + Sized { result } - fn one(&self) -> Self::I { + fn one(&self) -> Self::ModInt { self.from_u64(1) } - fn minus_one(&self) -> Self::I { + fn minus_one(&self) -> Self::ModInt { self.from_u64(self.modulus() - 1) } - fn zero(&self) -> Self::I { + fn zero(&self) -> Self::ModInt { self.from_u64(0) } } @@ -114,7 +115,7 @@ impl Montgomery { impl Arithmetic for Montgomery { // Montgomery transform, R=2⁶⁴ // Provides fast arithmetic mod n (n odd, u64) - type I = T; + type ModInt = T; fn new(n: u64) -> Self { debug_assert!(T::zero().count_zeros() >= 64 || n < (1 << T::zero().count_zeros() as usize)); @@ -128,7 +129,7 @@ impl Arithmetic for Montgomery { self.n.as_() } - fn from_u64(&self, x: u64) -> Self::I { + fn from_u64(&self, x: u64) -> Self::ModInt { // TODO: optimise! debug_assert!(x < self.n.as_()); let r = T::from_double( @@ -138,11 +139,11 @@ impl Arithmetic for Montgomery { r } - fn to_u64(&self, n: Self::I) -> u64 { + fn to_u64(&self, n: Self::ModInt) -> u64 { self.reduce(n.as_double()).as_() } - fn add(&self, a: Self::I, b: Self::I) -> Self::I { + fn add(&self, a: Self::ModInt, b: Self::ModInt) -> Self::ModInt { let (r, overflow) = a.overflowing_add_(b); // In case of overflow, a+b = 2⁶⁴ + r = (2⁶⁴ - n) + r (working mod n) @@ -173,7 +174,7 @@ impl Arithmetic for Montgomery { r } - fn mul(&self, a: Self::I, b: Self::I) -> Self::I { + fn mul(&self, a: Self::ModInt, b: Self::ModInt) -> Self::ModInt { let r = self.reduce(a.as_double() * b.as_double()); // Check that r (reduced back to the usual representation) equals From 53954badd7ae9d4d886648d82176129edfc719de Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 1 Jul 2020 15:03:56 +0200 Subject: [PATCH 10/28] factor::numeric: Refactor away the use of {To,From}Primitives --- src/uu/factor/src/numeric.rs | 100 +++++++++++++++++++++++------------ 1 file changed, 66 insertions(+), 34 deletions(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index 947b6bfef..7cb03a9e5 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -7,7 +7,6 @@ // * that was distributed with this source code. use num_traits::{ - cast::{AsPrimitive, ToPrimitive}, identities::{One, Zero}, int::PrimInt, ops::wrapping::{WrappingMul, WrappingNeg, WrappingSub}, @@ -126,21 +125,21 @@ impl Arithmetic for Montgomery { } fn modulus(&self) -> u64 { - self.n.as_() + self.n.as_u64() } fn from_u64(&self, x: u64) -> Self::ModInt { // TODO: optimise! - debug_assert!(x < self.n.as_()); + debug_assert!(x < self.n.as_u64()); let r = T::from_double( - ((T::Double::from(x)) << T::zero().count_zeros() as usize) % self.n.as_double(), + ((T::Double::from_u64(x)) << T::zero().count_zeros() as usize) % self.n.as_double(), ); debug_assert_eq!(x, self.to_u64(r)); r } fn to_u64(&self, n: Self::ModInt) -> u64 { - self.reduce(n.as_double()).as_() + self.reduce(n.as_double()).as_u64() } fn add(&self, a: Self::ModInt, b: Self::ModInt) -> Self::ModInt { @@ -160,11 +159,10 @@ impl Arithmetic for Montgomery { // a+b % n #[cfg(debug_assertions)] { - let a_r = self.to_u64(a); - let b_r = self.to_u64(b); + let a_r = self.to_u64(a) as u128; + let b_r = self.to_u64(b) as u128; let r_r = self.to_u64(r); - let r_2 = - (((a_r as u128) + (b_r as u128)) % >::as_(self.n)) as u64; + let r_2 = ((a_r + b_r) % self.n.as_u128()) as u64; debug_assert_eq!( r_r, r_2, "[{}] = {} ≠ {} = {} + {} = [{}] + [{}] mod {}; a = {}", @@ -181,12 +179,10 @@ impl Arithmetic for Montgomery { // a*b % n #[cfg(debug_assertions)] { - let a_r = self.to_u64(a); - let b_r = self.to_u64(b); + let a_r = self.to_u64(a) as u128; + let b_r = self.to_u64(b) as u128; let r_r = self.to_u64(r); - let r_2: u64 = ((T::Double::from(a_r) * T::Double::from(b_r)) % self.n.as_double()) - .to_u64() - .unwrap(); + let r_2: u64 = ((a_r * b_r) % self.n.as_u128()) as u64; debug_assert_eq!( r_r, r_2, "[{}] = {} ≠ {} = {} * {} = [{}] * [{}] mod {}; a = {}", @@ -219,29 +215,71 @@ impl OverflowingAdd for u128 { } pub(crate) trait Int: - AsPrimitive - + AsPrimitive - + Display - + Debug - + PrimInt - + OverflowingAdd - + WrappingNeg - + WrappingSub - + WrappingMul + Display + Debug + PrimInt + OverflowingAdd + WrappingNeg + WrappingSub + WrappingMul { + fn as_u64(&self) -> u64; + fn from_u64(n: u64) -> Self; + #[cfg(debug_assertions)] + fn as_u128(&self) -> u128; + #[cfg(debug_assertions)] + fn from_u128(n: u64) -> Self; } pub(crate) trait DoubleInt: Int { - type Double: Int + From + ToPrimitive; + type Double: Int; fn as_double(self) -> Self::Double; fn from_double(n: Self::Double) -> Self; - fn from_u64(n: u64) -> Self; } -impl Int for u32 {} -impl Int for u64 {} -impl Int for u128 {} +impl Int for u32 { + fn as_u64(&self) -> u64 { + *self as _ + } + fn from_u64(n: u64) -> Self { + n as _ + } + #[cfg(debug_assertions)] + fn as_u128(&self) -> u128 { + *self as _ + } + #[cfg(debug_assertions)] + fn from_u128(n: u64) -> Self { + n as _ + } +} +impl Int for u64 { + fn as_u64(&self) -> u64 { + *self as _ + } + fn from_u64(n: u64) -> Self { + n as _ + } + #[cfg(debug_assertions)] + fn as_u128(&self) -> u128 { + *self as _ + } + #[cfg(debug_assertions)] + fn from_u128(n: u64) -> Self { + n as _ + } +} +impl Int for u128 { + fn as_u64(&self) -> u64 { + *self as _ + } + fn from_u64(n: u64) -> Self { + n as _ + } + #[cfg(debug_assertions)] + fn as_u128(&self) -> u128 { + *self as _ + } + #[cfg(debug_assertions)] + fn from_u128(n: u64) -> Self { + n as _ + } +} impl DoubleInt for u64 { type Double = u128; @@ -252,9 +290,6 @@ impl DoubleInt for u64 { fn from_double(n: u128) -> u64 { n as _ } - fn from_u64(n: u64) -> u64 { - n - } } impl DoubleInt for u32 { type Double = u64; @@ -265,9 +300,6 @@ impl DoubleInt for u32 { fn from_double(n: u64) -> u32 { n as _ } - fn from_u64(n: u64) -> u32 { - n as _ - } } // extended Euclid algorithm From f95f977f98a68549901822861f37d71bf2b1919f Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 1 Jul 2020 15:43:02 +0200 Subject: [PATCH 11/28] factor::numeric: Generate implementations of Int with a macro --- src/uu/factor/src/numeric.rs | 70 ++++++++++++------------------------ 1 file changed, 23 insertions(+), 47 deletions(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index 7cb03a9e5..d2fc7604b 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -232,55 +232,31 @@ pub(crate) trait DoubleInt: Int { fn from_double(n: Self::Double) -> Self; } -impl Int for u32 { - fn as_u64(&self) -> u64 { - *self as _ - } - fn from_u64(n: u64) -> Self { - n as _ - } - #[cfg(debug_assertions)] - fn as_u128(&self) -> u128 { - *self as _ - } - #[cfg(debug_assertions)] - fn from_u128(n: u64) -> Self { - n as _ - } -} -impl Int for u64 { - fn as_u64(&self) -> u64 { - *self as _ - } - fn from_u64(n: u64) -> Self { - n as _ - } - #[cfg(debug_assertions)] - fn as_u128(&self) -> u128 { - *self as _ - } - #[cfg(debug_assertions)] - fn from_u128(n: u64) -> Self { - n as _ - } -} -impl Int for u128 { - fn as_u64(&self) -> u64 { - *self as _ - } - fn from_u64(n: u64) -> Self { - n as _ - } - #[cfg(debug_assertions)] - fn as_u128(&self) -> u128 { - *self as _ - } - #[cfg(debug_assertions)] - fn from_u128(n: u64) -> Self { - n as _ - } +macro_rules! int { + ( $x:ty ) => { + impl Int for $x { + fn as_u64(&self) -> u64 { + *self as u64 + } + fn from_u64(n: u64) -> Self { + n as _ + } + #[cfg(debug_assertions)] + fn as_u128(&self) -> u128 { + *self as u128 + } + #[cfg(debug_assertions)] + fn from_u128(n: u64) -> Self { + n as _ + } + } + }; } +int!(u32); +int!(u64); +int!(u128); + impl DoubleInt for u64 { type Double = u128; From b25c77c5f971c442ab2ca5fc4aec271270fc707f Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 1 Jul 2020 15:46:35 +0200 Subject: [PATCH 12/28] factor::numeric: Generate implementations of DoubleInt with a macro --- src/uu/factor/src/numeric.rs | 40 +++++++++++++++--------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index d2fc7604b..bbfce9f26 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -252,32 +252,26 @@ macro_rules! int { } }; } +macro_rules! double_int { + ( $x:ty, $y:ty ) => { + int!($x); + impl DoubleInt for $x { + type Double = $y; -int!(u32); -int!(u64); + fn as_double(self) -> $y { + self as _ + } + fn from_double(n: $y) -> $x { + n as _ + } + } + }; +} + +double_int!(u32, u64); +double_int!(u64, u128); int!(u128); -impl DoubleInt for u64 { - type Double = u128; - - fn as_double(self) -> u128 { - self as _ - } - fn from_double(n: u128) -> u64 { - n as _ - } -} -impl DoubleInt for u32 { - type Double = u64; - - fn as_double(self) -> u64 { - self as _ - } - fn from_double(n: u64) -> u32 { - n as _ - } -} - // extended Euclid algorithm // precondition: a is odd pub(crate) fn modular_inverse(a: T) -> T { From d2b43f49f95c1e34e068d1211a034c54a81694b2 Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 2 Jul 2020 11:34:05 +0200 Subject: [PATCH 13/28] factor::numeric::OverflowingAdd: Generate impls with a macro --- src/uu/factor/src/numeric.rs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index bbfce9f26..b1f929d4b 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -198,21 +198,18 @@ impl Arithmetic for Montgomery { pub(crate) trait OverflowingAdd: Sized { fn overflowing_add_(self, n: Self) -> (Self, bool); } -impl OverflowingAdd for u32 { - fn overflowing_add_(self, n: Self) -> (Self, bool) { - self.overflowing_add(n) - } -} -impl OverflowingAdd for u64 { - fn overflowing_add_(self, n: Self) -> (Self, bool) { - self.overflowing_add(n) - } -} -impl OverflowingAdd for u128 { - fn overflowing_add_(self, n: Self) -> (Self, bool) { - self.overflowing_add(n) - } +macro_rules! overflowing { + ($x:ty) => { + impl OverflowingAdd for $x { + fn overflowing_add_(self, n: Self) -> (Self, bool) { + self.overflowing_add(n) + } + } + }; } +overflowing!(u32); +overflowing!(u64); +overflowing!(u128); pub(crate) trait Int: Display + Debug + PrimInt + OverflowingAdd + WrappingNeg + WrappingSub + WrappingMul From 308290325a416539236d6c0d06234924d0dff10e Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 4 Jul 2020 12:58:09 +0200 Subject: [PATCH 14/28] factor::miller_rabin::is_prime: Fix bug Montgomery<_> only works for odd n, so attempting to construct an instance for an even number results in a panic! The most obvious solution is to special-case even numbers. --- src/uu/factor/src/miller_rabin.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index 330c8f127..f909f1ec1 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -101,10 +101,14 @@ pub(crate) fn test(m: A) -> Result { Prime } -// Used by build.rs' tests +// Used by build.rs' tests and debug assertions #[allow(dead_code)] pub(crate) fn is_prime(n: u64) -> bool { - test::>(Montgomery::new(n)).is_prime() + if n % 2 == 0 { + n == 2 + } else { + test::>(Montgomery::new(n)).is_prime() + } } #[cfg(test)] From 0a1200bdb8de7b9aff3c0f1bfc918f86f0e1e63c Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 4 Jul 2020 20:26:02 +0200 Subject: [PATCH 15/28] factor::miller_rabin: Add test for the largest 64b composite numbers --- src/uu/factor/src/miller_rabin.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index f909f1ec1..42de3b3c8 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -121,6 +121,13 @@ mod tests { assert!(is_prime(LARGEST_U64_PRIME)); } + #[test] + fn largest_composites() { + for i in LARGEST_U64_PRIME + 1..=u64::MAX { + assert!(!is_prime(i), "2⁶⁴ - {} reported prime", u64::MAX - i + 1); + } + } + #[test] fn first_primes() { use crate::table::{NEXT_PRIME, P_INVS_U64}; From 600268c6e4074a505c91cbceef4249ac744ef276 Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 4 Jul 2020 20:35:16 +0200 Subject: [PATCH 16/28] factor::miller_rabin::tests: Refactor --- src/uu/factor/src/miller_rabin.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index 42de3b3c8..e26f3be09 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -116,6 +116,14 @@ mod tests { use super::is_prime; const LARGEST_U64_PRIME: u64 = 0xFFFFFFFFFFFFFFC5; + fn primes() -> impl Iterator { + use crate::table::{NEXT_PRIME, P_INVS_U64}; + use std::iter::once; + once(2) + .chain(P_INVS_U64.iter().map(|(p, _, _)| *p)) + .chain(once(NEXT_PRIME)) + } + #[test] fn largest_prime() { assert!(is_prime(LARGEST_U64_PRIME)); @@ -130,11 +138,9 @@ mod tests { #[test] fn first_primes() { - use crate::table::{NEXT_PRIME, P_INVS_U64}; - for (p, _, _) in P_INVS_U64.iter() { - assert!(is_prime(*p), "{} reported composite", p); + for p in primes() { + assert!(is_prime(p), "{} reported composite", p); } - assert!(is_prime(NEXT_PRIME)); } #[test] @@ -145,11 +151,8 @@ mod tests { #[test] fn small_composites() { - use crate::table::P_INVS_U64; - - for i in 0..P_INVS_U64.len() { - let (p, _, _) = P_INVS_U64[i]; - for (q, _, _) in &P_INVS_U64[0..i] { + for p in primes() { + for q in primes().take_while(|q| *q <= p) { let n = p * q; assert!(!is_prime(n), "{} = {} × {} reported prime", n, p, q); } From 1e4d824829a8506f13558dd901d8e6b7d4f2204a Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 4 Jul 2020 20:38:58 +0200 Subject: [PATCH 17/28] factor::miller_rabin: Add negative test over all small composites --- src/uu/factor/src/miller_rabin.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index e26f3be09..b1e969cd2 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -143,6 +143,18 @@ mod tests { } } + #[test] + fn first_composites() { + assert!(!is_prime(0)); + assert!(!is_prime(1)); + + for (p, q) in primes().zip(primes().skip(1)) { + for i in p + 1..q { + assert!(!is_prime(i), "{} reported prime", i); + } + } + } + #[test] fn issue_1556() { // 10 425 511 = 2441 × 4271 From d2fa0fe63c73e76f44bb6c320bf3568dfa55e180 Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 4 Jul 2020 20:44:26 +0200 Subject: [PATCH 18/28] =?UTF-8?q?factor::miller=5Frabin::tests:=20small=5F?= =?UTF-8?q?composites=20=E2=86=92=20small=5Fsemiprimes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is more descriptive, as semiprime are the products of 2 primes; all semiprimes are composite, but not all composite numbers are semiprime. --- src/uu/factor/src/miller_rabin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index b1e969cd2..afbed5b69 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -162,7 +162,7 @@ mod tests { } #[test] - fn small_composites() { + fn small_semiprimes() { for p in primes() { for q in primes().take_while(|q| *q <= p) { let n = p * q; From 4f08e281674623c9d2afe1d95d1782196ab9a937 Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 4 Jul 2020 20:48:57 +0200 Subject: [PATCH 19/28] factor::miller_rabin: Add property-based test --- Cargo.lock | 23 +++++++++++++++++++++++ src/uu/factor/Cargo.toml | 3 +++ src/uu/factor/src/miller_rabin.rs | 7 +++++++ 3 files changed, 33 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index c6dc3bbfa..b059821bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -369,6 +369,15 @@ name = "either" version = "1.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "env_logger" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", + "regex 1.3.9 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "failure" version = "0.1.1" @@ -672,6 +681,17 @@ name = "quick-error" version = "1.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "quickcheck" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", + "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", + "rand 0.7.3 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "quote" version = "0.3.15" @@ -1316,6 +1336,7 @@ name = "uu_factor" version = "0.0.1" dependencies = [ "num-traits 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", + "quickcheck 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)", "uucore 0.0.4 (git+https://github.com/uutils/uucore.git?branch=canary)", "uucore_procs 0.0.4 (git+https://github.com/uutils/uucore.git?branch=canary)", @@ -2196,6 +2217,7 @@ dependencies = [ "checksum digest 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e5b29bf156f3f4b3c4f610a25ff69370616ae6e0657d416de22645483e72af0a" "checksum dunce 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b2641c4a7c0c4101df53ea572bffdc561c146f6c2eb09e4df02bc4811e3feeb4" "checksum either 1.5.3 (registry+https://github.com/rust-lang/crates.io-index)" = "bb1f6b1ce1c140482ea30ddd3335fc0024ac7ee112895426e0a629a6c20adfe3" +"checksum env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)" = "44533bbbb3bb3c1fa17d9f2e4e38bbbaf8396ba82193c4cb1b6445d711445d36" "checksum failure 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "934799b6c1de475a012a02dab0ace1ace43789ee4b99bcfbf1a2e3e8ced5de82" "checksum failure_derive 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "c7cdda555bb90c9bb67a3b670a0f42de8e73f5981524123ad8578aafec8ddb8b" "checksum fake-simd 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e88a8acf291dafb59c2d96e8f59828f3838bb1a70398823ade51a84de6a6deed" @@ -2237,6 +2259,7 @@ dependencies = [ "checksum ppv-lite86 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "237a5ed80e274dbc66f86bd59c1e25edc039660be53194b5fe0a482e0f2612ea" "checksum proc-macro2 1.0.18 (registry+https://github.com/rust-lang/crates.io-index)" = "beae6331a816b1f65d04c45b078fd8e6c93e8071771f41b8163255bbd8d7c8fa" "checksum quick-error 1.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" +"checksum quickcheck 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)" = "a44883e74aa97ad63db83c4bf8ca490f02b2fc02f92575e720c8551e843c945f" "checksum quote 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)" = "7a6e920b65c65f10b2ae65c831a81a073a89edd28c7cce89475bff467ab4167a" "checksum quote 1.0.7 (registry+https://github.com/rust-lang/crates.io-index)" = "aa563d17ecb180e500da1cfd2b028310ac758de548efdd203e18f283af693f37" "checksum rand 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)" = "c618c47cd3ebd209790115ab837de41425723956ad3ce2e6a7f09890947cacb9" diff --git a/src/uu/factor/Cargo.toml b/src/uu/factor/Cargo.toml index 5a3cc6077..8954ac2f4 100644 --- a/src/uu/factor/Cargo.toml +++ b/src/uu/factor/Cargo.toml @@ -23,6 +23,9 @@ rand = "0.5" uucore = { version="0.0.4", package="uucore", git="https://github.com/uutils/uucore.git", branch="canary" } uucore_procs = { version="0.0.4", package="uucore_procs", git="https://github.com/uutils/uucore.git", branch="canary" } +[dev-dependencies] +quickcheck = "0.9.2" + [[bin]] name = "factor" path = "src/main.rs" diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index afbed5b69..bed89795b 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -114,6 +114,7 @@ pub(crate) fn is_prime(n: u64) -> bool { #[cfg(test)] mod tests { use super::is_prime; + use quickcheck::quickcheck; const LARGEST_U64_PRIME: u64 = 0xFFFFFFFFFFFFFFC5; fn primes() -> impl Iterator { @@ -170,4 +171,10 @@ mod tests { } } } + + quickcheck! { + fn composites(i: u64, j: u64) -> bool { + i < 2 || j < 2 || !is_prime(i*j) + } + } } From 9b149a759bd9ea896e1819587f767a5a29c9a392 Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 4 Jul 2020 20:52:32 +0200 Subject: [PATCH 20/28] factor::miller_rabin: Hoist edge-cases (even, <2) out of test() test() takes a modulus that is known to not be even or <2 (otherwise the Montgomery value could not be constructed), so those checks can be hoisted into is_prime() and out of the critical path. --- src/uu/factor/src/miller_rabin.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index bed89795b..87efe24f9 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -44,12 +44,8 @@ pub(crate) fn test(m: A) -> Result { use self::Result::*; let n = m.modulus(); - if n < 2 { - return Pseudoprime; - } - if n % 2 == 0 { - return if n == 2 { Prime } else { Composite(2) }; - } + debug_assert!(n > 1); + debug_assert!(n % 2 != 0); // n-1 = r 2ⁱ let i = (n - 1).trailing_zeros(); @@ -104,7 +100,9 @@ pub(crate) fn test(m: A) -> Result { // Used by build.rs' tests and debug assertions #[allow(dead_code)] pub(crate) fn is_prime(n: u64) -> bool { - if n % 2 == 0 { + if n < 2 { + false + } else if n % 2 == 0 { n == 2 } else { test::>(Montgomery::new(n)).is_prime() From 3d6fdffe142f8f5a611b6697fc13086722d49831 Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 4 Jul 2020 21:56:03 +0200 Subject: [PATCH 21/28] factor::miller_rabin: Generalise tests to 32 and 64b Montgomery --- src/uu/factor/src/miller_rabin.rs | 79 ++++++++++++++++++++++++------- 1 file changed, 62 insertions(+), 17 deletions(-) diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index 87efe24f9..c98d343f4 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -111,16 +111,22 @@ pub(crate) fn is_prime(n: u64) -> bool { #[cfg(test)] mod tests { - use super::is_prime; + use super::*; + use crate::numeric::{Arithmetic, Montgomery}; use quickcheck::quickcheck; + use std::iter; const LARGEST_U64_PRIME: u64 = 0xFFFFFFFFFFFFFFC5; fn primes() -> impl Iterator { + iter::once(2).chain(odd_primes()) + } + + fn odd_primes() -> impl Iterator { use crate::table::{NEXT_PRIME, P_INVS_U64}; - use std::iter::once; - once(2) - .chain(P_INVS_U64.iter().map(|(p, _, _)| *p)) - .chain(once(NEXT_PRIME)) + P_INVS_U64 + .iter() + .map(|(p, _, _)| *p) + .chain(iter::once(NEXT_PRIME)) } #[test] @@ -136,40 +142,79 @@ mod tests { } #[test] - fn first_primes() { - for p in primes() { - assert!(is_prime(p), "{} reported composite", p); + fn two() { + assert!(is_prime(2)); + } + + fn first_primes() { + for p in odd_primes() { + assert!(test(A::new(p)).is_prime(), "{} reported composite", p); } } #[test] - fn first_composites() { - assert!(!is_prime(0)); - assert!(!is_prime(1)); + fn first_primes_32() { + first_primes::>() + } - for (p, q) in primes().zip(primes().skip(1)) { + #[test] + fn first_primes_64() { + first_primes::>() + } + + #[test] + fn one() { + assert!(!is_prime(1)); + } + #[test] + fn zero() { + assert!(!is_prime(0)); + } + + fn first_composites() { + for (p, q) in primes().zip(odd_primes()) { for i in p + 1..q { assert!(!is_prime(i), "{} reported prime", i); } } } + #[test] + fn first_composites_32() { + first_composites::>() + } + + #[test] + fn first_composites_64() { + first_composites::>() + } + #[test] fn issue_1556() { // 10 425 511 = 2441 × 4271 assert!(!is_prime(10_425_511)); } - #[test] - fn small_semiprimes() { - for p in primes() { - for q in primes().take_while(|q| *q <= p) { + fn small_semiprimes() { + for p in odd_primes() { + for q in odd_primes().take_while(|q| *q <= p) { let n = p * q; - assert!(!is_prime(n), "{} = {} × {} reported prime", n, p, q); + let m = A::new(n); + assert!(!test(m).is_prime(), "{} = {} × {} reported prime", n, p, q); } } } + #[test] + fn small_semiprimes_32() { + small_semiprimes::>() + } + + #[test] + fn small_semiprimes_64() { + small_semiprimes::>() + } + quickcheck! { fn composites(i: u64, j: u64) -> bool { i < 2 || j < 2 || !is_prime(i*j) From cbcc760f836e6ee6d27362c35d2018997aa01b62 Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 4 Jul 2020 23:51:57 +0200 Subject: [PATCH 22/28] factor::miller_rabin: Squash another bug! >:3 Detected by the testsuite improvement just prior. --- src/uu/factor/src/miller_rabin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index c98d343f4..04649fa72 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -57,7 +57,7 @@ pub(crate) fn test(m: A) -> Result { for _a in A::BASIS.iter() { let _a = _a % n; if _a == 0 { - break; + continue; } let a = m.from_u64(_a); From 7a1b86c9c2b2b9c27cf1a39ad0b734083eeb623e Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 4 Jul 2020 23:38:13 +0200 Subject: [PATCH 23/28] factor::numeric::tests: Use a macro to instantiate every test --- Cargo.lock | 26 +++++++++++++ src/uu/factor/Cargo.toml | 1 + src/uu/factor/src/numeric.rs | 71 +++++++++++++----------------------- 3 files changed, 52 insertions(+), 46 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b059821bd..991ac3a07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -649,6 +649,23 @@ dependencies = [ "pkg-config 0.3.17 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "paste" +version = "0.1.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "paste-impl 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)", + "proc-macro-hack 0.5.16 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "paste-impl" +version = "0.1.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "proc-macro-hack 0.5.16 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "pkg-config" version = "0.3.17" @@ -668,6 +685,11 @@ name = "ppv-lite86" version = "0.2.8" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "proc-macro-hack" +version = "0.5.16" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "proc-macro2" version = "1.0.18" @@ -1336,6 +1358,7 @@ name = "uu_factor" version = "0.0.1" dependencies = [ "num-traits 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", + "paste 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)", "quickcheck 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)", "uucore 0.0.4 (git+https://github.com/uutils/uucore.git?branch=canary)", @@ -2254,9 +2277,12 @@ dependencies = [ "checksum numtoa 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b8f8bdf33df195859076e54ab11ee78a1b208382d3a26ec40d142ffc1ecc49ef" "checksum onig 4.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "8518fcb2b1b8c2f45f0ad499df4fda6087fc3475ca69a185c173b8315d2fb383" "checksum onig_sys 69.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "388410bf5fa341f10e58e6db3975f4bea1ac30247dd79d37a9e5ced3cb4cc3b0" +"checksum paste 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)" = "45ca20c77d80be666aef2b45486da86238fabe33e38306bd3118fe4af33fa880" +"checksum paste-impl 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)" = "d95a7db200b97ef370c8e6de0088252f7e0dfff7d047a28528e47456c0fc98b6" "checksum pkg-config 0.3.17 (registry+https://github.com/rust-lang/crates.io-index)" = "05da548ad6865900e60eaba7f589cc0783590a92e940c26953ff81ddbab2d677" "checksum platform-info 0.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "f2fd076acdc7a98374de6e300bf3af675997225bef21aecac2219553f04dd7e8" "checksum ppv-lite86 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "237a5ed80e274dbc66f86bd59c1e25edc039660be53194b5fe0a482e0f2612ea" +"checksum proc-macro-hack 0.5.16 (registry+https://github.com/rust-lang/crates.io-index)" = "7e0456befd48169b9f13ef0f0ad46d492cf9d2dbb918bcf38e01eed4ce3ec5e4" "checksum proc-macro2 1.0.18 (registry+https://github.com/rust-lang/crates.io-index)" = "beae6331a816b1f65d04c45b078fd8e6c93e8071771f41b8163255bbd8d7c8fa" "checksum quick-error 1.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" "checksum quickcheck 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)" = "a44883e74aa97ad63db83c4bf8ca490f02b2fc02f92575e720c8551e843c945f" diff --git a/src/uu/factor/Cargo.toml b/src/uu/factor/Cargo.toml index 8954ac2f4..47104d5dd 100644 --- a/src/uu/factor/Cargo.toml +++ b/src/uu/factor/Cargo.toml @@ -24,6 +24,7 @@ uucore = { version="0.0.4", package="uucore", git="https://github.com/uutils/uuc uucore_procs = { version="0.0.4", package="uucore_procs", git="https://github.com/uutils/uucore.git", branch="canary" } [dev-dependencies] +paste = "0.1.18" quickcheck = "0.9.2" [[bin]] diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index b1f929d4b..7ecc8cf3a 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -308,6 +308,21 @@ pub(crate) fn modular_inverse(a: T) -> T { mod tests { use super::*; + macro_rules! parametrized_check { + ( $f:ident ) => { + paste::item! { + #[test] + fn [< $f _ u32 >]() { + $f::() + } + #[test] + fn [< $f _ u64 >]() { + $f::() + } + } + }; + } + fn test_inverter() { // All odd integers from 1 to 20 000 let one = T::from(1).unwrap(); @@ -318,21 +333,12 @@ mod tests { assert!(test_values.all(|x| x.wrapping_mul(&modular_inverse(x)) == one)); } + parametrized_check!(test_inverter); - #[test] - fn test_inverter_u32() { - test_inverter::() - } - - #[test] - fn test_inverter_u64() { - test_inverter::() - } - - fn test_add() { + fn test_add() { for n in 0..100 { let n = 2 * n + 1; - let m = A::new(n); + let m = Montgomery::::new(n); for x in 0..n { let m_x = m.from_u64(x); for y in 0..=x { @@ -343,21 +349,12 @@ mod tests { } } } + parametrized_check!(test_add); - #[test] - fn test_add_m32() { - test_add::>() - } - - #[test] - fn test_add_m64() { - test_add::>() - } - - fn test_mult() { + fn test_mult() { for n in 0..100 { let n = 2 * n + 1; - let m = A::new(n); + let m = Montgomery::::new(n); for x in 0..n { let m_x = m.from_u64(x); for y in 0..=x { @@ -367,35 +364,17 @@ mod tests { } } } + parametrized_check!(test_mult); - #[test] - fn test_mult_m32() { - test_mult::>() - } - - #[test] - fn test_mult_m64() { - test_mult::>() - } - - fn test_roundtrip() { + fn test_roundtrip() { for n in 0..100 { let n = 2 * n + 1; - let m = A::new(n); + let m = Montgomery::::new(n); for x in 0..n { let x_ = m.from_u64(x); assert_eq!(x, m.to_u64(x_)); } } } - - #[test] - fn test_roundtrip_m32() { - test_roundtrip::>() - } - - #[test] - fn test_roundtrip_m64() { - test_roundtrip::>() - } + parametrized_check!(test_roundtrip); } From 6256750376f476fe8b48800bcf9b7dee0642559a Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 4 Jul 2020 23:48:01 +0200 Subject: [PATCH 24/28] factor::miller_rabin: Use a macro to instantiate every test --- src/uu/factor/src/miller_rabin.rs | 49 ++++++++++++------------------- 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/src/uu/factor/src/miller_rabin.rs b/src/uu/factor/src/miller_rabin.rs index 04649fa72..762ee1b4c 100644 --- a/src/uu/factor/src/miller_rabin.rs +++ b/src/uu/factor/src/miller_rabin.rs @@ -146,21 +146,28 @@ mod tests { assert!(is_prime(2)); } + // TODO: Deduplicate with macro in numeric.rs + macro_rules! parametrized_check { + ( $f:ident ) => { + paste::item! { + #[test] + fn [< $f _ u32 >]() { + $f::>() + } + #[test] + fn [< $f _ u64 >]() { + $f::>() + } + } + }; + } + fn first_primes() { for p in odd_primes() { assert!(test(A::new(p)).is_prime(), "{} reported composite", p); } } - - #[test] - fn first_primes_32() { - first_primes::>() - } - - #[test] - fn first_primes_64() { - first_primes::>() - } + parametrized_check!(first_primes); #[test] fn one() { @@ -178,16 +185,7 @@ mod tests { } } } - - #[test] - fn first_composites_32() { - first_composites::>() - } - - #[test] - fn first_composites_64() { - first_composites::>() - } + parametrized_check!(first_composites); #[test] fn issue_1556() { @@ -204,16 +202,7 @@ mod tests { } } } - - #[test] - fn small_semiprimes_32() { - small_semiprimes::>() - } - - #[test] - fn small_semiprimes_64() { - small_semiprimes::>() - } + parametrized_check!(small_semiprimes); quickcheck! { fn composites(i: u64, j: u64) -> bool { From 86a4749e3a8fe7667eac757fc72f0112835702d1 Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 9 Jul 2020 11:41:45 +0200 Subject: [PATCH 25/28] factor::numeric: fix original "Generalise modular inverse computation" --- src/uu/factor/Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/uu/factor/Cargo.toml b/src/uu/factor/Cargo.toml index 61aeb59f1..b8e6fa384 100644 --- a/src/uu/factor/Cargo.toml +++ b/src/uu/factor/Cargo.toml @@ -12,7 +12,8 @@ categories = ["command-line-utilities"] edition = "2018" [build-dependencies] -num-traits="0.2" +num-traits = "0.2" # used in src/numerics.rs, which is included by build.rs + [dependencies] num-traits = "0.2" From 1172af09c0cd02f7d34a80af065b91fc0a73a8c4 Mon Sep 17 00:00:00 2001 From: nicoo Date: Tue, 21 Jul 2020 19:39:47 +0200 Subject: [PATCH 26/28] factor::numeric::DoubleInt: Clarify methods and associated types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `DoubleInt::Double` renamed to `DoubleWidth` - `{as,from}_double()` renamed to `{as,from}_double_width()`. This should hopefully clarify that this is not a “double precision” floating-point type, but an integer type with a larger range (used for storing intermediate results, typ. from a multiplication) --- src/uu/factor/src/numeric.rs | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index 7ecc8cf3a..e0edf06a5 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -80,23 +80,23 @@ pub(crate) struct Montgomery { impl Montgomery { /// computes x/R mod n efficiently - fn reduce(&self, x: T::Double) -> T { + fn reduce(&self, x: T::DoubleWidth) -> T { let t_bits = T::zero().count_zeros() as usize; - debug_assert!(x < (self.n.as_double()) << t_bits); + debug_assert!(x < (self.n.as_double_width()) << t_bits); // TODO: optimiiiiiiise let Montgomery { a, n } = self; - let m = T::from_double(x).wrapping_mul(a); - let nm = (n.as_double()) * (m.as_double()); + let m = T::from_double_width(x).wrapping_mul(a); + let nm = (n.as_double_width()) * (m.as_double_width()); let (xnm, overflow) = x.overflowing_add_(nm); // x + n*m debug_assert_eq!( - xnm % (T::Double::one() << T::zero().count_zeros() as usize), - T::Double::zero() + xnm % (T::DoubleWidth::one() << T::zero().count_zeros() as usize), + T::DoubleWidth::zero() ); // (x + n*m) / R // in case of overflow, this is (2¹²⁸ + xnm)/2⁶⁴ - n = xnm/2⁶⁴ + (2⁶⁴ - n) - let y = T::from_double(xnm >> t_bits) + let y = T::from_double_width(xnm >> t_bits) + if !overflow { T::zero() } else { @@ -131,15 +131,16 @@ impl Arithmetic for Montgomery { fn from_u64(&self, x: u64) -> Self::ModInt { // TODO: optimise! debug_assert!(x < self.n.as_u64()); - let r = T::from_double( - ((T::Double::from_u64(x)) << T::zero().count_zeros() as usize) % self.n.as_double(), + let r = T::from_double_width( + ((T::DoubleWidth::from_u64(x)) << T::zero().count_zeros() as usize) + % self.n.as_double_width(), ); debug_assert_eq!(x, self.to_u64(r)); r } fn to_u64(&self, n: Self::ModInt) -> u64 { - self.reduce(n.as_double()).as_u64() + self.reduce(n.as_double_width()).as_u64() } fn add(&self, a: Self::ModInt, b: Self::ModInt) -> Self::ModInt { @@ -173,7 +174,7 @@ impl Arithmetic for Montgomery { } fn mul(&self, a: Self::ModInt, b: Self::ModInt) -> Self::ModInt { - let r = self.reduce(a.as_double() * b.as_double()); + let r = self.reduce(a.as_double_width() * b.as_double_width()); // Check that r (reduced back to the usual representation) equals // a*b % n @@ -223,10 +224,10 @@ pub(crate) trait Int: } pub(crate) trait DoubleInt: Int { - type Double: Int; + type DoubleWidth: Int; - fn as_double(self) -> Self::Double; - fn from_double(n: Self::Double) -> Self; + fn as_double_width(self) -> Self::DoubleWidth; + fn from_double_width(n: Self::DoubleWidth) -> Self; } macro_rules! int { @@ -253,12 +254,12 @@ macro_rules! double_int { ( $x:ty, $y:ty ) => { int!($x); impl DoubleInt for $x { - type Double = $y; + type DoubleWidth = $y; - fn as_double(self) -> $y { + fn as_double_width(self) -> $y { self as _ } - fn from_double(n: $y) -> $x { + fn from_double_width(n: $y) -> $x { n as _ } } From 17c69674ebd2e05bed9769874162efefb033c77f Mon Sep 17 00:00:00 2001 From: nicoo Date: Tue, 21 Jul 2020 19:48:07 +0200 Subject: [PATCH 27/28] factor::numeric::Int: Remove `from_u128` method It was unused, the debug assertions only need `to_u128`. --- src/uu/factor/src/numeric.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index e0edf06a5..031674ce1 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -219,8 +219,6 @@ pub(crate) trait Int: fn from_u64(n: u64) -> Self; #[cfg(debug_assertions)] fn as_u128(&self) -> u128; - #[cfg(debug_assertions)] - fn from_u128(n: u64) -> Self; } pub(crate) trait DoubleInt: Int { @@ -243,10 +241,6 @@ macro_rules! int { fn as_u128(&self) -> u128 { *self as u128 } - #[cfg(debug_assertions)] - fn from_u128(n: u64) -> Self { - n as _ - } } }; } From 9a80ab7741f889f91853732b6351ee67b87364f7 Mon Sep 17 00:00:00 2001 From: nicoo Date: Tue, 21 Jul 2020 19:49:57 +0200 Subject: [PATCH 28/28] factor::numeric::DoubleInt: Document the DoubleWidth associated type --- src/uu/factor/src/numeric.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index 031674ce1..6383809f8 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -217,11 +217,15 @@ pub(crate) trait Int: { fn as_u64(&self) -> u64; fn from_u64(n: u64) -> Self; + #[cfg(debug_assertions)] fn as_u128(&self) -> u128; } pub(crate) trait DoubleInt: Int { + /// An integer type with twice the width of `Self`. + /// In particular, multiplications (of `Int` values) can be performed in + /// `Self::DoubleWidth` without possibility of overflow. type DoubleWidth: Int; fn as_double_width(self) -> Self::DoubleWidth;