Merge pull request filecoin-project#188 from filecoin-project/ci-impr…

…ove-clippy

.circleci/config.yml

@@ -117,8 +117,6 @@ jobs:
       - run: rustup default $(cat rust-toolchain)
       # A nightly build is needed for code coverage reporting
       - run: rustup toolchain install --profile minimal << pipeline.parameters.nightly-version >>
-      - run: rustup component add rustfmt-preview
-      - run: rustup component add clippy-preview
       - run: rustup component add --toolchain << pipeline.parameters.nightly-version >> llvm-tools-preview
       - run: rustc --version
       - run: rm -rf .git
@@ -182,34 +180,20 @@ jobs:
       - *restore-cache
       - set-env-path
       - run:
-          name: Run cargo clippy (blst)
-          command: cargo clippy --no-default-features --features blst
+          name: Run cargo clippy (default features (pairing))
+          command: cargo clippy --all-targets --features gpu -- -D warnings
       - run:
           name: Run cargo clippy (pairing)
-          command: cargo clippy --no-default-features --features pairing
+          command: cargo clippy --all-targets --no-default-features --features pairing -- -D warnings
       - run:
-          name: Run cargo clippy (gpu)
-          command: cargo clippy --features gpu
-
-  build_blst:
-    executor: default
-    steps:
-      - *restore-workspace
-      - *restore-cache
-      - set-env-path
+          name: Run cargo clippy (blst)
+          command: cargo clippy --all-targets --no-default-features --features blst -- -D warnings
       - run:
-          name: Run cargo release build
-          command: cargo build --release --no-default-features --features blst
-
-  build_pairing:
-    executor: default
-    steps:
-      - *restore-workspace
-      - *restore-cache
-      - set-env-path
+          name: Run cargo clippy (pairing,gpu)
+          command: cargo clippy --all-targets --no-default-features --features pairing,gpu -- -D warnings
       - run:
-          name: Run cargo release build
-          command: cargo build --release --no-default-features --features pairing
+          name: Run cargo clippy (blst,gpu)
+          command: cargo clippy --all-targets --no-default-features --features blst,gpu -- -D warnings
 
   coverage_run:
     executor: default
@@ -286,13 +270,6 @@ workflows:
       - test_blst_gpu_x86_64-unknown-linux-gnu:
           requires:
             - cargo_fetch
-
-      - build_blst:
-          requires:
-            - cargo_fetch
-      - build_pairing:
-          requires:
-            - cargo_fetch
 
       #- coverage_run:
       #    name: coverage_default_features

rust-toolchain

@@ -1 +1 @@
-1.46.0
+1.51.0

src/domain.rs

@@ -89,7 +89,7 @@ impl<E: Engine, G: Group<E>> EvaluationDomain<E, G> {
         worker: &Worker,
         kern: &mut Option<gpu::LockedFFTKernel<E>>,
     ) -> gpu::GPUResult<()> {
-        best_fft(kern, &mut self.coeffs, worker, &self.omega, self.exp)?;
+        best_fft(kern, &mut self.coeffs, worker, &self.omega, self.exp);
         Ok(())
     }
 
@@ -98,7 +98,7 @@ impl<E: Engine, G: Group<E>> EvaluationDomain<E, G> {
         worker: &Worker,
         kern: &mut Option<gpu::LockedFFTKernel<E>>,
     ) -> gpu::GPUResult<()> {
-        best_fft(kern, &mut self.coeffs, worker, &self.omegainv, self.exp)?;
+        best_fft(kern, &mut self.coeffs, worker, &self.omegainv, self.exp);
 
         worker.scope(self.coeffs.len(), |scope, chunk| {
             let minv = self.minv;
@@ -293,13 +293,13 @@ fn best_fft<E: Engine, T: Group<E>>(
     worker: &Worker,
     omega: &E::Fr,
     log_n: u32,
-) -> gpu::GPUResult<()> {
+) {
     if let Some(ref mut kern) = kern {
         if kern
             .with(|k: &mut gpu::FFTKernel<E>| gpu_fft(k, a, omega, log_n))
             .is_ok()
         {
-            return Ok(());
+            return;
         }
     }
 
@@ -309,8 +309,6 @@ fn best_fft<E: Engine, T: Group<E>>(
     } else {
         parallel_fft(a, worker, omega, log_n, log_cpus);
     }
-
-    Ok(())
 }
 
 pub fn gpu_fft<E: Engine, T: Group<E>>(
@@ -326,11 +324,12 @@ pub fn gpu_fft<E: Engine, T: Group<E>>(
     // size.
     // For compatibility/performance reasons we decided to transmute the array to the desired type
     // as it seems safe and needs less modifications in the current structure of Bellman library.
-    let a = unsafe { std::mem::transmute::<&mut [T], &mut [E::Fr]>(a) };
+    let a = unsafe { &mut *(a as *mut [T] as *mut [E::Fr]) };
     kern.radix_fft(a, omega, log_n)?;
     Ok(())
 }
 
+#[allow(clippy::many_single_char_names)]
 pub fn serial_fft<E: ScalarEngine, T: Group<E>>(a: &mut [T], omega: &E::Fr, log_n: u32) {
     fn bitreverse(mut n: u32, l: u32) -> u32 {
         let mut r = 0;
@@ -609,8 +608,7 @@ mod tests {
 
             let mut now = Instant::now();
             gpu_fft(&mut kern, &mut v1.coeffs, &v1.omega, log_d).expect("GPU FFT failed!");
-            let gpu_dur =
-                now.elapsed().as_secs() * 1000 as u64 + now.elapsed().subsec_millis() as u64;
+            let gpu_dur = now.elapsed().as_secs() * 1000 + now.elapsed().subsec_millis() as u64;
             println!("GPU took {}ms.", gpu_dur);
 
             now = Instant::now();
@@ -619,8 +617,7 @@ mod tests {
             } else {
                 parallel_fft(&mut v2.coeffs, &worker, &v2.omega, log_d, log_cpus);
             }
-            let cpu_dur =
-                now.elapsed().as_secs() * 1000 as u64 + now.elapsed().subsec_millis() as u64;
+            let cpu_dur = now.elapsed().as_secs() * 1000 + now.elapsed().subsec_millis() as u64;
             println!("CPU ({} cores) took {}ms.", 1 << log_cpus, cpu_dur);
 
             println!("Speedup: x{}", cpu_dur as f32 / gpu_dur as f32);

src/gadgets/blake2s.rs

@@ -2,6 +2,8 @@
 //!
 //! [BLAKE2s]: https://tools.ietf.org/html/rfc7693
 
+#![allow(clippy::many_single_char_names)]
+
 use super::{boolean::Boolean, multieq::MultiEq, uint32::UInt32};
 use crate::{ConstraintSystem, SynthesisError};
 use ff::ScalarEngine;
@@ -79,6 +81,7 @@ const SIGMA: [[usize; 16]; 10] = [
        END FUNCTION.
 */
 
+#[allow(clippy::too_many_arguments)]
 fn mixing_g<E: ScalarEngine, CS: ConstraintSystem<E>, M>(
     mut cs: M,
     v: &mut [UInt32],
@@ -347,21 +350,17 @@ pub fn blake2s<E: ScalarEngine, CS: ConstraintSystem<E>>(
     assert_eq!(personalization.len(), 8);
     assert!(input.len() % 8 == 0);
 
-    let mut h = Vec::with_capacity(8);
-    h.push(UInt32::constant(0x6A09_E667 ^ 0x0101_0000 ^ 32));
-    h.push(UInt32::constant(0xBB67_AE85));
-    h.push(UInt32::constant(0x3C6E_F372));
-    h.push(UInt32::constant(0xA54F_F53A));
-    h.push(UInt32::constant(0x510E_527F));
-    h.push(UInt32::constant(0x9B05_688C));
-
-    // Personalization is stored here
-    h.push(UInt32::constant(
-        0x1F83_D9AB ^ LittleEndian::read_u32(&personalization[0..4]),
-    ));
-    h.push(UInt32::constant(
-        0x5BE0_CD19 ^ LittleEndian::read_u32(&personalization[4..8]),
-    ));
+    let mut h = vec![
+        UInt32::constant(0x6A09_E667 ^ 0x0101_0000 ^ 32),
+        UInt32::constant(0xBB67_AE85),
+        UInt32::constant(0x3C6E_F372),
+        UInt32::constant(0xA54F_F53A),
+        UInt32::constant(0x510E_527F),
+        UInt32::constant(0x9B05_688C),
+        // Personalization is stored here
+        UInt32::constant(0x1F83_D9AB ^ LittleEndian::read_u32(&personalization[0..4])),
+        UInt32::constant(0x5BE0_CD19 ^ LittleEndian::read_u32(&personalization[4..8])),
+    ];
 
     let mut blocks: Vec<Vec<UInt32>> = vec![];
 
@@ -638,7 +637,7 @@ mod test {
             hex!("a1309e334376c8f36a736a4ab0e691ef931ee3ebdb9ea96187127136fea622a1"),
             hex!("82fefff60f265cea255252f7c194a7f93965dffee0609ef74eb67f0d76cd41c6"),
         ];
-        for i in 0..2 {
+        for expected in &expecteds {
             let mut h = Blake2sParams::new()
                 .hash_length(32)
                 .personal(b"12345678")
@@ -690,7 +689,7 @@ mod test {
                 }
             }
 
-            assert_eq!(expecteds[i], hash_result.as_bytes());
+            assert_eq!(expected, hash_result.as_bytes());
         }
     }
 }

src/gadgets/boolean.rs

@@ -360,10 +360,7 @@ pub enum Boolean {
 
 impl Boolean {
     pub fn is_constant(&self) -> bool {
-        match *self {
-            Boolean::Constant(_) => true,
-            _ => false,
-        }
+        matches!(*self, Boolean::Constant(_))
     }
 
     pub fn enforce_equal<E, CS>(mut cs: CS, a: &Self, b: &Self) -> Result<(), SynthesisError>
@@ -1513,6 +1510,7 @@ mod test {
         }
     }
 
+    #[allow(clippy::identity_op)]
     #[test]
     fn test_u64_into_boolean_vec_le() {
         let mut cs = TestConstraintSystem::<Bls12>::new();
@@ -1534,6 +1532,7 @@ mod test {
         assert_eq!(bits[63 - 22].get_value().unwrap(), false);
     }
 
+    #[allow(clippy::identity_op)]
     #[test]
     fn test_field_into_allocated_bits_le() {
         let mut cs = TestConstraintSystem::<Bls12>::new();

src/gadgets/lookup.rs

@@ -286,6 +286,7 @@ mod test {
         }
     }
 
+    #[allow(clippy::needless_range_loop)]
     #[test]
     fn test_synth() {
         let mut rng = XorShiftRng::from_seed([

src/gadgets/multipack.rs

@@ -86,8 +86,7 @@ where
     }
 
     let alloc_num = AllocatedNum::alloc(cs.namespace(|| "input"), || {
-        num.get_value()
-            .ok_or_else(|| SynthesisError::AssignmentMissing)
+        num.get_value().ok_or(SynthesisError::AssignmentMissing)
     })?;
 
     // num * 1 = input

src/gadgets/num.rs

@@ -121,7 +121,9 @@ impl<E: ScalarEngine> AllocatedNum<E> {
             found_one |= b;
             if !found_one {
                 // a_bit should also be false
-                a_bit.map(|e| assert!(!e));
+                if let Some(e) = a_bit {
+                    assert!(!e);
+                }
                 continue;
             }
 
@@ -409,6 +411,7 @@ impl<E: ScalarEngine> Num<E> {
         }
     }
 
+    #[allow(clippy::should_implement_trait)]
     pub fn add(self, other: &Self) -> Self {
         let lc = self.lc + &other.lc;
         let value = match (self.value, other.value) {
@@ -590,7 +593,7 @@ mod test {
                 .skip(1)
                 .zip(bits.iter().rev())
             {
-                if let &Boolean::Is(ref a) = a {
+                if let Boolean::Is(ref a) = *a {
                     assert_eq!(b, a.get_value().unwrap());
                 } else {
                     unreachable!()
@@ -630,20 +633,18 @@ mod test {
 
         let mut expected_sums = vec![Fr::zero(); n];
         let mut value = Fr::zero();
-        for i in 0..n {
+        for (i, expected_sum) in expected_sums.iter_mut().enumerate() {
             let coeff = Fr::random(&mut rng);
             lc = lc + (coeff, Variable::new_unchecked(Index::Aux(i)));
-            let mut tmp = expected_sums[i];
-            tmp.add_assign(&coeff);
-            expected_sums[i] = tmp;
+            expected_sum.add_assign(&coeff);
 
             value.add_assign(&coeff);
         }
 
         let scalar = Fr::random(&mut rng);
         let num = Num {
             value: Some(value),
-            lc: lc.clone(),
+            lc,
         };
 
         let scaled_num = num.clone().scale(scalar);

src/gadgets/sha256.rs

@@ -3,6 +3,8 @@
 //!
 //! [SHA-256]: https://tools.ietf.org/html/rfc6234
 
+#![allow(clippy::many_single_char_names)]
+
 use super::boolean::Boolean;
 use super::multieq::MultiEq;
 use super::uint32::UInt32;

src/gadgets/test/mod.rs

@@ -21,6 +21,7 @@ enum NamedObject {
 }
 
 /// Constraint system for testing purposes.
+#[allow(clippy::type_complexity)]
 pub struct TestConstraintSystem<E: ScalarEngine> {
     named_objects: HashMap<String, NamedObject>,
     current_namespace: Vec<String>,
@@ -75,7 +76,7 @@ fn proc_lc<E: ScalarEngine>(terms: &LinearCombination<E>) -> BTreeMap<OrderedVar
     let mut to_remove = vec![];
     for (var, coeff) in map.iter() {
         if coeff.is_zero() {
-            to_remove.push(var.clone())
+            to_remove.push(*var)
         }
     }
 
@@ -184,7 +185,7 @@ impl<E: ScalarEngine> TestConstraintSystem<E> {
         };
 
         for &(ref a, ref b, ref c, ref name) in &self.constraints {
-            write!(&mut s, "\n").unwrap();
+            writeln!(&mut s).unwrap();
 
             write!(&mut s, "{}: ", name).unwrap();
             pp(&mut s, a);
@@ -194,7 +195,7 @@ impl<E: ScalarEngine> TestConstraintSystem<E> {
             pp(&mut s, c);
         }
 
-        write!(&mut s, "\n").unwrap();
+        writeln!(&mut s).unwrap();
 
         s
     }
@@ -404,7 +405,7 @@ impl<E: ScalarEngine> ConstraintSystem<E> for TestConstraintSystem<E> {
     {
         let name = name_fn().into();
         let path = compute_path(&self.current_namespace, name.clone());
-        self.set_named_obj(path.clone(), NamedObject::Namespace);
+        self.set_named_obj(path, NamedObject::Namespace);
         self.current_namespace.push(name);
     }