Merge pull request #361 from mulimoen/fix/clippy

Compile fixes

.github/workflows/ci.yml

@@ -66,7 +66,7 @@ jobs:
         include:
         - build: msrv
           os: ubuntu-20.04
-          rust: 1.64.0
+          rust: 1.77.0
         - build: stable
           os: ubuntu-20.04
           rust: stable

changelog.rst

@@ -4,7 +4,7 @@ Changelog
 
 - Unreleased
   - Fixed a compilation regression in csmat_binop
-  - Bump MSRV (1.64)
+  - Bump MSRV (1.77)
   - add support for reading/writing Complex{32,64} matrices in matrixmarket format.
     Also change semantics so that files of kind  {integer,real,complex} are only readable
     into matrices of the same kind (so integer can be read into [iu]{8,16,32,64,size}),

sprs-ldl/src/lib.rs

@@ -497,7 +497,7 @@ pub fn ldl_symbolic<N, I, PStorage>(
 
 /// Perform numeric LDLT decomposition
 ///
-/// `pattern_workspace` is a [`DStack`](DStack) of capacity n
+/// `pattern_workspace` is a [`DStack`] of capacity n
 #[allow(clippy::too_many_arguments)]
 pub fn ldl_numeric<N, I, PStorage>(
     mat: CsMatViewI<N, I>,

sprs/src/sparse/binop.rs

@@ -162,6 +162,8 @@ sparse_scalar_mul!(usize);
 sparse_scalar_mul!(f32);
 sparse_scalar_mul!(f64);
 
+/// Apply binary operation to two sparse matrices
+///
 /// Applies a binary operation to matching non-zero elements
 /// of two sparse matrices. When e.g. only the `lhs` has a non-zero at a
 /// given location, `0` is inferred for the non-zero value of the other matrix.

sprs/src/sparse/compressed.rs

@@ -30,22 +30,3 @@ where
         self.transpose_view()
     }
 }
-
-/// The `SpVecView` trait describes types that can be seen as a view into
-/// a `CsVec`
-pub trait SpVecView<N, I: SpIndex> {
-    /// Return a view into the current vector
-    fn view(&self) -> CsVecViewI<N, I>;
-}
-
-impl<N, I, IndStorage, DataStorage> SpVecView<N, I>
-    for CsVecBase<IndStorage, DataStorage, N, I>
-where
-    IndStorage: Deref<Target = [I]>,
-    DataStorage: Deref<Target = [N]>,
-    I: SpIndex,
-{
-    fn view(&self) -> CsVecViewI<N, I> {
-        self.view()
-    }
-}

sprs/src/sparse/csmat.rs

@@ -841,14 +841,14 @@ where
         nnz / (rows * cols)
     }
 
-    /// Number of outer dimensions, that ie equal to self.rows() for a CSR
-    /// matrix, and equal to self.cols() for a CSC matrix
+    /// Number of outer dimensions, that ie equal to `self.rows()` for a CSR
+    /// matrix, and equal to `self.cols()` for a CSC matrix
     pub fn outer_dims(&self) -> usize {
         outer_dimension(self.storage, self.nrows, self.ncols)
     }
 
-    /// Number of inner dimensions, that ie equal to self.cols() for a CSR
-    /// matrix, and equal to self.rows() for a CSC matrix
+    /// Number of inner dimensions, that ie equal to `self.cols()` for a CSR
+    /// matrix, and equal to `self.rows()` for a CSC matrix
     pub fn inner_dims(&self) -> usize {
         match self.storage {
             CSC => self.nrows,
@@ -870,10 +870,10 @@ where
         }
     }
 
-    /// The array of offsets in the indices() and data() slices.
+    /// The array of offsets in the `indices()` `and data()` slices.
     /// The elements of the slice at outer dimension i
-    /// are available between the elements indptr\[i\] and indptr\[i+1\]
-    /// in the indices() and data() slices.
+    /// are available between the elements `indptr\[i\]` and `indptr\[i+1\]`
+    /// in the `indices()` and `data()` slices.
     ///
     /// # Example
     ///
@@ -921,12 +921,12 @@ where
     }
 
     /// The inner dimension location for each non-zero value. See
-    /// the documentation of indptr() for more explanations.
+    /// the documentation of `indptr()` for more explanations.
     pub fn indices(&self) -> &[I] {
         &self.indices[..]
     }
 
-    /// The non-zero values. See the documentation of indptr()
+    /// The non-zero values. See the documentation of `indptr()`
     /// for more explanations.
     pub fn data(&self) -> &[N] {
         &self.data[..]
@@ -1170,9 +1170,9 @@ where
     pub fn outer_iterator_papt<'a, 'perm: 'a>(
         &'a self,
         perm: PermViewI<'perm, I>,
-    ) -> impl std::iter::DoubleEndedIterator<Item = (usize, CsVecViewI<N, I>)>
-           + std::iter::ExactSizeIterator<Item = (usize, CsVecViewI<N, I>)>
-           + '_ {
+    ) -> impl std::iter::DoubleEndedIterator<Item = (usize, CsVecViewI<'a, N, I>)>
+           + std::iter::ExactSizeIterator<Item = (usize, CsVecViewI<'a, N, I>)>
+           + 'a {
         (0..self.outer_dims()).map(move |outer_ind| {
             let outer_ind_perm = perm.at(outer_ind);
             let range = self.indptr.outer_inds_sz(outer_ind_perm);

sprs/src/sparse/indptr.rs

@@ -54,7 +54,7 @@ where
             .last()
             .copied()
             .map(Iptr::index_unchecked)
-            .map_or(false, |i| i > usize::max_value() / 2)
+            .map_or(false, |i| i > usize::MAX / 2)
         {
             // We do not allow indptr values to be larger than half
             // the maximum value of an usize, as that would clearly exhaust

sprs/src/sparse/linalg/bicgstab.rs

@@ -74,18 +74,18 @@
 //!   * Hard restart to check true error before claiming convergence
 //! * Soft-restart logic uses a correct metric of perpendicularity instead of a magnitude heuristic
 //!   * The usual method, which compares a fixed value to `rho`, does not capture the fact that the
-//!   magnitude of `rho` will naturally decrease as the solver approaches convergence
+//!     magnitude of `rho` will naturally decrease as the solver approaches convergence
 //!   * This change eliminates the effect where the a soft restart is performed on every iteration for the last few
-//!   iterations of any solve with a reasonable error tolerance
+//!     iterations of any solve with a reasonable error tolerance
 //! * Hard-restart logic provides some real guarantee of correctness
 //!   * The usual implementations keep a cheap, but inaccurate, running estimate of the error
 //!     * That decreases the cost of iterations by about half by eliminating a matrix-vector multiplication,
-//!     but allows the estimate of error to drift numerically, which causes the solver to return claiming
-//!     convergence when the solved output does not, in fact, match the input system
+//!       but allows the estimate of error to drift numerically, which causes the solver to return claiming
+//!       convergence when the solved output does not, in fact, match the input system
 //!   * This change guarantees that the solver will not return claiming convergence unless the solution
-//!   actually matches the input system, and will refresh its estimate of the error and continue iterations
-//!   if it has reached a falsely-converged state, continuing until it either reaches true convergence or
-//!   reaches maximum iterations
+//!     actually matches the input system, and will refresh its estimate of the error and continue iterations
+//!     if it has reached a falsely-converged state, continuing until it either reaches true convergence or
+//!     reaches maximum iterations
 use crate::indexing::SpIndex;
 use crate::sparse::{CsMatViewI, CsVecI, CsVecViewI};
 use num_traits::One;

sprs/src/sparse/linalg/ordering.rs

@@ -272,6 +272,8 @@ pub mod order {
     use crate::indexing::SpIndex;
     use crate::sparse::permutation::PermOwnedI;
 
+    /// Internal trait for working with Cuthill-McKee
+    ///
     /// This trait is very deeply integrated with the inner workings of the
     /// Cuthill-McKee algorithm implemented here.  It is conceptually only an
     /// enum, specifying if the Cuthill-McKee ordering should be built in
@@ -434,7 +436,7 @@ pub mod order {
 /// - `starting_strategy` - The strategy to use for choosing a starting vertex.
 ///
 /// - `directed_ordering` - The order of the computed ordering, should either be
-/// `Forward` or `Reverse`.
+///   `Forward` or `Reverse`.
 pub fn cuthill_mckee_custom<N, I, Iptr, S, D>(
     mat: CsMatViewI<N, I, Iptr>,
     mut starting_strategy: S,

sprs/src/sparse/linalg/trisolve.rs

@@ -279,7 +279,7 @@ where
 ///
 /// # Panics
 ///
-/// * if dstack.capacity() is too small
+/// * if `dstack.capacity()` is too small
 /// * if dstack is not empty
 /// * if `w_workspace` is not of length n
 ///

sprs/src/sparse/smmp.rs

@@ -32,9 +32,10 @@ pub enum ThreadingStrategy {
 }
 
 #[cfg(feature = "multi_thread")]
-thread_local!(static THREADING_STRAT: RefCell<ThreadingStrategy> =
-    RefCell::new(ThreadingStrategy::Automatic)
-);
+thread_local! {
+    static THREADING_STRAT: RefCell<ThreadingStrategy> =
+    const { RefCell::new(ThreadingStrategy::Automatic) };
+}
 
 /// Set the threading strategy for matrix products in this thread.
 ///

sprs/src/sparse/vec.rs

@@ -296,8 +296,9 @@ where
         // context to a plain function, which enables specifying the type
         // Needless to say, this needs to go when it's no longer necessary
         #[inline(always)]
-        fn hack_instead_of_closure<N, V: ?Sized>(vi: (&V, usize)) -> (usize, &N)
+        fn hack_instead_of_closure<N, V>(vi: (&V, usize)) -> (usize, &N)
         where
+            V: ?Sized,
             V: DenseVector<Scalar = N>,
         {
             (vi.1, vi.0.index(vi.1))
@@ -688,7 +689,7 @@ where
 
     /// Check the sparse structure, namely that:
     /// - indices are sorted
-    /// - all indices are less than dims()
+    /// - all indices are less than `dims()`
     pub fn check_structure(&self) -> Result<(), StructureError> {
         // Make sure indices can be converted to usize
         for i in self.indices.iter() {
@@ -824,10 +825,11 @@ where
     /// assert_eq!(4., v1.dot(&v1));
     /// assert_eq!(16., v2.dot(&v2));
     /// ```
-    pub fn dot<'b, T: IntoSparseVecIter<'b, N>>(&'b self, rhs: T) -> N
+    pub fn dot<'b, T>(&'b self, rhs: T) -> N
     where
         N: 'b + crate::MulAcc + num_traits::Zero,
         I: 'b,
+        T: IntoSparseVecIter<'b, N>,
         <T as IntoSparseVecIter<'b, N>>::IterType:
             Iterator<Item = (usize, &'b N)>,
         T: Copy, // T is supposed to be a reference type
@@ -841,13 +843,11 @@ where
     /// that can be interpreted as a sparse vector (hence sparse vectors, std
     /// vectors and slices, and ndarray's dense vectors work).
     /// The output type can be any type supporting `MulAcc`.
-    pub fn dot_acc<'b, T: IntoSparseVecIter<'b, M>, M, Acc>(
-        &'b self,
-        rhs: T,
-    ) -> Acc
+    pub fn dot_acc<'b, T, M, Acc>(&'b self, rhs: T) -> Acc
     where
         Acc: 'b + crate::MulAcc<N, M> + num_traits::Zero,
         M: 'b,
+        T: IntoSparseVecIter<'b, M>,
         <T as IntoSparseVecIter<'b, M>>::IterType:
             Iterator<Item = (usize, &'b M)>,
         T: Copy, // T is supposed to be a reference type

suitesparse_bindings/sprs_suitesparse_camd/src/lib.rs

@@ -5,8 +5,10 @@ use suitesparse_camd_sys::*;
 // FIXME should be using SuiteSparseInt::MAX but this will not compile
 // in rust 1.42 as u32::MAX was introduced in 1.43. This can be changed if
 // the MSRV is bumped.
-const MAX_INT32: usize = std::u32::MAX as usize;
+const MAX_INT32: usize = u32::MAX as usize;
 
+/// Find permutation of `mat`
+///
 /// Find a permutation matrix P which reduces the fill-in of the square
 /// sparse matrix `mat` in Cholesky factorization (ie, the number of nonzeros
 /// of the Cholesky factorization of P A P^T is less than for the Cholesky
@@ -77,6 +79,8 @@ where
     Ok(PermOwnedI::new(perm))
 }
 
+/// Find permutation of `mat`
+///
 /// Find a permutation matrix P which reduces the fill-in of the square
 /// sparse matrix `mat` in Cholesky factorization (ie, the number of nonzeros
 /// of the Cholesky factorization of P A P^T is less than for the Cholesky

suitesparse_bindings/suitesparse-src/build.rs

@@ -44,26 +44,21 @@ fn main() {
     if std::env::var_os("CARGO_FEATURE_LDL").is_some() {
         // We first build ldl with LDL_LONG to make the bindings to
         // the long bits of the library
-        cc::Build::new()
+        let ldll_artifact = cc::Build::new()
             .include("SuiteSparse/SuiteSparse_config")
             .include("SuiteSparse/LDL/Include")
             .file("SuiteSparse/LDL/Source/ldl.c")
             .cargo_metadata(false)
             .define("LDL_LONG", None)
-            .compile("ldll");
-        // We must then copy this to another location since the next
-        // invocation is just a compile definition
-        let mut ldl_path = std::path::PathBuf::from(root.clone());
-        ldl_path.push("SuiteSparse/LDL/Source/ldl.o");
-        let mut ldll_path = ldl_path.clone();
-        ldll_path.set_file_name("ldll.o");
-        std::fs::copy(&ldl_path, &ldll_path).unwrap();
+            .compile_intermediates();
+        let (ldll_artifact, rest) = ldll_artifact.split_first().unwrap();
+        assert!(rest.is_empty());
         // And now we build ldl again (in int form), and link with the long bits
         cc::Build::new()
             .include("SuiteSparse/SuiteSparse_config")
             .include("SuiteSparse/LDL/Include")
             .file("SuiteSparse/LDL/Source/ldl.c")
-            .object(&ldll_path)
+            .object(ldll_artifact)
             .cargo_metadata(false)
             .compile("ldl");
     }