From 300f24009a9450002aa721c2547999fbf0a7ad85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Crozet?= Date: Sat, 22 Jun 2024 12:12:34 +0200 Subject: [PATCH] =?UTF-8?q?chore:=E2=80=AFclippy=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/simba-ci-build.yml | 18 ++++++++++++++++-- src/lib.rs | 14 ++++++++------ src/scalar/complex.rs | 8 ++++---- src/scalar/fixed_impl.rs | 4 ++-- src/scalar/subset.rs | 16 ++++++++-------- src/simd/auto_simd_impl.rs | 22 ++++++++++------------ src/simd/portable_simd_impl.rs | 6 +++--- src/simd/simd_value.rs | 10 ++++++++-- src/simd/wide_simd_impl.rs | 10 +++++----- 9 files changed, 64 insertions(+), 44 deletions(-) diff --git a/.github/workflows/simba-ci-build.yml b/.github/workflows/simba-ci-build.yml index bc4f721..57ca15b 100644 --- a/.github/workflows/simba-ci-build.yml +++ b/.github/workflows/simba-ci-build.yml @@ -10,14 +10,26 @@ env: CARGO_TERM_COLOR: always jobs: - check-fmt: + fmt: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - name: Check formatting run: cargo fmt -- --check + clippy: + runs-on: ubuntu-latest + env: + RUSTFLAGS: -D warnings + steps: + - uses: actions/checkout@v2 + - name: Check formatting + run: | + rustup install nightly + cargo +nightly clippy --all-features build-native: runs-on: ubuntu-latest + env: + RUSTFLAGS: -D warnings steps: - uses: actions/checkout@v2 - name: Build --no-default-feature @@ -29,7 +41,9 @@ jobs: - name: Build all features except libm run: cargo build --features wide,rkyv-serialize,serde_serialize,partial_fixed_point_support; - name: Build all features - run: cargo +nightly build --all-features + run: | + rustup install nightly + cargo +nightly build --all-features build-wasm: runs-on: ubuntu-latest steps: diff --git a/src/lib.rs b/src/lib.rs index 6d421a5..d014c00 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,24 +13,26 @@ with less code duplication. ## Cargo features Two cargo features can be optionally enabled: -- With the __`packed_simd`__ feature enabled, the `simba::simd` module will export several SIMD types like `f32x2`, - `f64x4`, `i32i8`, `u16i16`, etc. There types are wrappers around the SIMD types from the [__packed_simd__ - crate](https://docs.rs/packed_simd). This requires a nightly compiler. +- With the __`portable_simd`__ feature enabled, the `simba::simd` module will export several SIMD types like `f32x2`, + `f64x4`, `i32i8`, `u16i16`, etc. There types are wrappers around the SIMD types from the experimental [std::simd](https://doc.rust-lang.org/std/simd/index.html) + implementation. This requires a nightly compiler and might break after updating the compiler nightly version. - With the __`wide`__ feature enabled, the `simba::simd` module will export the `WideF32x4` and `WideBoolF32x4` types. The types are wrappers around the `wide::f32x4` type from the [__wide__ crate](https://docs.rs/wide). This will work with both a stable or nightly compiler. If none of those features are enabled, __simba__ will still define all the scalar and SIMD traits. However, the SIMD traits won't be implemented for any SIMD types. Therefore it is recommended to: -- Use the `packed_simd` feature if you want more features, and can afford to use a nightly compiler. +- Use the `portable_simd` feature if you want more features, and can afford to use a nightly compiler. - Use the `wide` feature if you only need 4-lanes 32-bits floats, and can't afford to use a nightly compiler. -*/ + */ #![deny(non_camel_case_types)] #![deny(unused_parens)] #![deny(non_upper_case_globals)] #![deny(unused_results)] -#![deny(missing_docs)] // FIXME: should be denied +#![deny(missing_docs)] +#![allow(clippy::just_underscores_and_digits)] +#![allow(clippy::too_many_arguments)] #![cfg_attr(not(feature = "std"), no_std)] #![cfg_attr(feature = "portable_simd", feature(portable_simd))] diff --git a/src/scalar/complex.rs b/src/scalar/complex.rs index b62269a..292752e 100644 --- a/src/scalar/complex.rs +++ b/src/scalar/complex.rs @@ -156,7 +156,7 @@ macro_rules! complex_trait_methods ( /// /// Complex numbers are equipped with functions that are commonly used on complex numbers and reals. /// The results of those functions only have to be approximately equal to the actual theoretical values. -// FIXME: SubsetOf should be removed when specialization will be supported by rustc. This will +// TODO: SubsetOf should be removed when specialization will be supported by rustc. This will // allow a blanket impl: impl SubsetOf for T { ... } #[allow(missing_docs)] pub trait ComplexField: @@ -305,7 +305,7 @@ macro_rules! impl_complex ( #[cfg(not(feature = "std"))] #[inline] fn powi(self, n: i32) -> Self { - // FIXME: is there a more accurate solution? + // TODO: is there a more accurate solution? $libm::powf(self, n as $T) } @@ -1162,7 +1162,7 @@ impl ComplexField for num_complex::Complex { #[inline] fn powi(self, n: i32) -> Self { - // FIXME: is there a more accurate solution? + // TODO: is there a more accurate solution? let n = N::from_subset(&(n as f64)); self.powf(n) } @@ -1437,7 +1437,7 @@ impl ComplexField for num_complex::Complex { let two = one.clone() + one.clone(); two.clone() * (((self.clone() + one.clone()) / two.clone()).sqrt() + ((self - one) / two).sqrt()) - .ln() + .ln() } /// Computes the principal value of inverse hyperbolic tangent of `self`. diff --git a/src/scalar/fixed_impl.rs b/src/scalar/fixed_impl.rs index b7709e8..ed3ceaf 100644 --- a/src/scalar/fixed_impl.rs +++ b/src/scalar/fixed_impl.rs @@ -17,7 +17,7 @@ use std::ops::{ Add, AddAssign, Div, DivAssign, Mul, MulAssign, Neg, Rem, RemAssign, Sub, SubAssign, }; -macro_rules! impl_fixed_type( +macro_rules! impl_fixed_type ( ($($FixedI: ident, $Int: ident, $LeEqDim: ident, $LeEqDim1: ident, $LeEqDim2: ident, $LeEqDim3: ident, $LeEqDim4: ident;)*) => {$( #[derive(Copy, Clone)] #[repr(transparent)] @@ -58,7 +58,7 @@ macro_rules! impl_fixed_type( impl PartialOrd for $FixedI { #[inline(always)] fn partial_cmp(&self, other: &Self) -> Option { - self.0.partial_cmp(&other.0) + Some(self.cmp(other)) } } diff --git a/src/scalar/subset.rs b/src/scalar/subset.rs index e8ee947..e499414 100644 --- a/src/scalar/subset.rs +++ b/src/scalar/subset.rs @@ -12,11 +12,11 @@ use num_complex::Complex; /// represent_, independently from their actual implementation details and limitations. For /// example: /// * f32 and f64 are both supposed to represent reals and are thus considered equal (even if in -/// practice f64 has more elements). +/// practice f64 has more elements). /// * u32 and i8 are respectively supposed to represent natural and relative numbers. Thus, u32 is -/// a subset of i8. +/// a subset of i8. /// * A quaternion and a 3x3 orthogonal matrix with unit determinant are both sets of rotations. -/// They can thus be considered equal. +/// They can thus be considered equal. /// /// In other words, implementation details due to machine limitations are ignored (otherwise we /// could not even, e.g., convert a u64 to an i64). If considering those limitations are @@ -52,11 +52,11 @@ pub trait SubsetOf: Sized { /// represent_, independently from their actual implementation details and limitations. For /// example: /// * f32 and f64 are both supposed to represent reals and are thus considered equal (even if in -/// practice f64 has more elements). +/// practice f64 has more elements). /// * u32 and i8 are respectively supposed to represent natural and relative numbers. Thus, i8 is -/// a superset of u32. +/// a superset of u32. /// * A quaternion and a 3x3 orthogonal matrix with unit determinant are both sets of rotations. -/// They can thus be considered equal. +/// They can thus be considered equal. /// /// In other words, implementation details due to machine limitations are ignored (otherwise we /// could not even, e.g., convert a u64 to an i64). If considering those limitations are @@ -106,7 +106,7 @@ impl, SP> SupersetOf for SP { } } -macro_rules! impl_subset( +macro_rules! impl_subset ( ($($subset: ty as $( $superset: ty),+ );* $(;)*) => { $($( impl SubsetOf<$superset> for $subset { @@ -189,7 +189,7 @@ impl> SubsetOf> for Complex { } } -macro_rules! impl_scalar_subset_of_complex( +macro_rules! impl_scalar_subset_of_complex ( ($($t: ident),*) => {$( impl> SubsetOf> for $t { #[inline] diff --git a/src/simd/auto_simd_impl.rs b/src/simd/auto_simd_impl.rs index d5c35f3..659530b 100644 --- a/src/simd/auto_simd_impl.rs +++ b/src/simd/auto_simd_impl.rs @@ -23,7 +23,7 @@ use std::{ // This is a hack to allow use to reuse `_0` as integers or as identifier, // depending on whether or not `ident_to_value` has been called in scope. // This helps writing macros that define both `::new` and `From([T; lanes()])`. -macro_rules! ident_to_value( +macro_rules! ident_to_value ( () => { const _0: usize = 0; const _1: usize = 1; const _2: usize = 2; const _3: usize = 3; const _4: usize = 4; const _5: usize = 5; const _6: usize = 6; const _7: usize = 7; const _8: usize = 8; const _9: usize = 9; const _10: usize = 10; const _11: usize = 11; const _12: usize = 12; const _13: usize = 13; const _14: usize = 14; const _15: usize = 15; @@ -60,7 +60,7 @@ pub struct AutoSimd(pub N); )] pub struct AutoBoolSimd(pub N); -macro_rules! impl_bool_simd( +macro_rules! impl_bool_simd ( ($($t: ty, $lanes: expr, $($i: ident),*;)*) => {$( impl_simd_value!($t, bool, $lanes, AutoSimd<$t> $(, $i)*;); @@ -200,7 +200,7 @@ macro_rules! impl_bool_simd( )*} ); -macro_rules! impl_scalar_subset_of_simd( +macro_rules! impl_scalar_subset_of_simd ( ($($t: ty),*) => {$( impl SubsetOf> for $t where AutoSimd: SimdValue + Copy, @@ -229,7 +229,7 @@ impl_scalar_subset_of_simd!(u8, u16, u32, u64, usize, i8, i16, i32, i64, isize, #[cfg(feature = "decimal")] impl_scalar_subset_of_simd!(d128); -macro_rules! impl_simd_value( +macro_rules! impl_simd_value ( ($($t: ty, $elt: ty, $lanes: expr, $bool: ty, $($i: ident),*;)*) => ($( impl ArrTransform for AutoSimd<$t> { #[inline(always)] @@ -332,7 +332,7 @@ macro_rules! impl_simd_value( )*) ); -macro_rules! impl_uint_simd( +macro_rules! impl_uint_simd ( ($($t: ty, $elt: ty, $lanes: expr, $bool: ty, $($i: ident),*;)*) => ($( impl_simd_value!($t, $elt, $lanes, $bool $(, $i)*;); @@ -617,7 +617,7 @@ macro_rules! impl_uint_simd( )*) ); -macro_rules! impl_int_simd( +macro_rules! impl_int_simd ( ($($t: ty, $elt: ty, $lanes: expr, $bool: ty, $($i: ident),*;)*) => ($( impl_uint_simd!($t, $elt, $lanes, $bool $(, $i)*;); @@ -632,13 +632,11 @@ macro_rules! impl_int_simd( )*) ); -macro_rules! impl_float_simd( +macro_rules! impl_float_simd ( ($($t: ty, $elt: ty, $lanes: expr, $int: ty, $bool: ty, $($i: ident),*;)*) => ($( impl_int_simd!($t, $elt, $lanes, $bool $(, $i)*;); - // FIXME: this should be part of impl_int_simd - // but those methods do not seem to be implemented - // by packed_simd for integers. + // TODO: this should be part of impl_int_simd impl SimdSigned for AutoSimd<$t> { #[inline(always)] fn simd_abs(&self) -> Self { @@ -1041,7 +1039,7 @@ macro_rules! impl_float_simd( fn simd_horizontal_product(self) -> Self::Element { let mut prod = self.extract(0); for ii in 1..$lanes { - prod = prod * self.extract(ii) + prod *= self.extract(ii) } prod } @@ -1172,7 +1170,7 @@ macro_rules! impl_float_simd( #[inline] fn simd_powi(self, n: i32) -> Self { - // FIXME: is there a more accurate solution? + // TODO: is there a more accurate solution? let n = AutoSimd::<$t>::from_subset(&(n as f64)); self.simd_powf(n) } diff --git a/src/simd/portable_simd_impl.rs b/src/simd/portable_simd_impl.rs index 9fc0bdc..e529cc6 100644 --- a/src/simd/portable_simd_impl.rs +++ b/src/simd/portable_simd_impl.rs @@ -656,7 +656,7 @@ macro_rules! impl_float_simd ( ($($t: ty, $elt: ident, $int: ty, $bool: ty, $($i: ident),*;)*) => ($( impl_int_simd!($t, $elt, $bool $(, $i)*;); - // FIXME: this should be part of impl_int_simd + // TODO: this should be part of impl_int_simd // but those methods do not seem to be implemented // by portable_simd for integers. impl SimdSigned for Simd<$t> { @@ -1076,7 +1076,7 @@ macro_rules! impl_float_simd ( fn simd_horizontal_product(self) -> Self::Element { let mut prod = self.extract(0); for ii in 1..Self::lanes() { - prod = prod * self.extract(ii) + prod *= self.extract(ii) } prod } @@ -1207,7 +1207,7 @@ macro_rules! impl_float_simd ( #[inline] fn simd_powi(self, n: i32) -> Self { - // FIXME: is there a more accurate solution? + // TODO: is there a more accurate solution? let n = Simd::<$t>::from_subset(&(n as f64)); self.simd_powf(n) } diff --git a/src/simd/simd_value.rs b/src/simd/simd_value.rs index 10968e0..5101ac0 100644 --- a/src/simd/simd_value.rs +++ b/src/simd/simd_value.rs @@ -3,7 +3,7 @@ use crate::simd::SimdBool; /// Base trait for every SIMD types. pub trait SimdValue: Sized { /// The type of the elements of each lane of this SIMD value. - type Element: SimdValue; + type Element: SimdValue; /// Type of the result of comparing two SIMD values like `self`. type SimdBool: SimdBool; @@ -16,12 +16,18 @@ pub trait SimdValue: Sized { /// Panics if `i >= Self::lanes()`. fn extract(&self, i: usize) -> Self::Element; /// Extracts the i-th lane of `self` without bound-checking. + /// + /// # Safety + /// Undefined behavior if `i >= Self::lanes()`. unsafe fn extract_unchecked(&self, i: usize) -> Self::Element; /// Replaces the i-th lane of `self` by `val`. /// /// Panics if `i >= Self::lanes()`. fn replace(&mut self, i: usize, val: Self::Element); /// Replaces the i-th lane of `self` by `val` without bound-checking. + /// + /// # Safety + /// Undefined behavior if `i >= Self::lanes()`. unsafe fn replace_unchecked(&mut self, i: usize, val: Self::Element); /// Merges `self` and `other` depending on the lanes of `cond`. @@ -141,7 +147,7 @@ impl SimdValue for num_complex::Complex { impl PrimitiveSimdValue for num_complex::Complex {} -macro_rules! impl_primitive_simd_value_for_scalar( +macro_rules! impl_primitive_simd_value_for_scalar ( ($($t: ty),*) => {$( impl PrimitiveSimdValue for $t {} impl SimdValue for $t { diff --git a/src/simd/wide_simd_impl.rs b/src/simd/wide_simd_impl.rs index 659e58c..7c201a6 100644 --- a/src/simd/wide_simd_impl.rs +++ b/src/simd/wide_simd_impl.rs @@ -109,7 +109,7 @@ pub struct WideBoolF64x4(pub wide::f64x4); #[cfg(feature = "rkyv")] impl_rkyv!(WideBoolF64x4, [f64; 4]); -macro_rules! impl_wide_f32( +macro_rules! impl_wide_f32 ( ($f32: ident, $f32xX: ident, $WideF32xX: ident, $WideBoolF32xX: ident, $lanes: expr; $($ii: expr),+) => { impl PrimitiveSimdValue for $WideF32xX {} impl PrimitiveSimdValue for $WideBoolF32xX {} @@ -705,7 +705,7 @@ macro_rules! impl_wide_f32( #[inline(always)] fn simd_signum(&self) -> Self { - // FIXME: is there a more efficient way? + // TODO: is there a more efficient way? self.map(|x| x.signum()) } @@ -1091,7 +1091,7 @@ macro_rules! impl_wide_f32( fn simd_horizontal_product(self) -> Self::Element { let mut prod = self.extract(0); for ii in 1..Self::lanes() { - prod = prod * self.extract(ii) + prod *= self.extract(ii) } prod } @@ -1222,7 +1222,7 @@ macro_rules! impl_wide_f32( #[inline] fn simd_powi(self, n: i32) -> Self { - // FIXME: is there a more accurate solution? + // TODO: is there a more accurate solution? let n = <$WideF32xX>::from_subset(&(n as f64)); self.simd_powf(n) } @@ -1514,7 +1514,7 @@ macro_rules! impl_wide_f32( } ); -macro_rules! impl_scalar_subset_of_simd( +macro_rules! impl_scalar_subset_of_simd ( ($WideF32xX: ty, $f32: ty, $lanes: expr; $($t: ty),*) => {$( impl SubsetOf<$WideF32xX> for $t { #[inline(always)]