From 83a14300388ebddd21581346cc584aaeb7e6d781 Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Thu, 15 Jul 2021 07:34:46 -0700 Subject: [PATCH] Fix some warnings (#94) --- src/bin/bench_gmimc.rs | 8 +++----- src/bin/bench_rescue.rs | 6 ++---- src/field/fft.rs | 14 +++++++------- src/field/field.rs | 2 +- src/gadgets/insert.rs | 1 - src/gates/mod.rs | 3 +++ src/gmimc.rs | 4 ++-- src/hash.rs | 10 ++-------- src/permutation_argument.rs | 2 +- src/polynomial/commitment.rs | 1 + src/prover.rs | 2 +- src/rescue.rs | 4 ++-- src/util/mod.rs | 4 ++-- 13 files changed, 27 insertions(+), 34 deletions(-) diff --git a/src/bin/bench_gmimc.rs b/src/bin/bench_gmimc.rs index 2f81aac0..9611d48e 100644 --- a/src/bin/bench_gmimc.rs +++ b/src/bin/bench_gmimc.rs @@ -1,3 +1,4 @@ +use std::convert::TryInto; use std::thread; use std::time::Instant; @@ -15,15 +16,12 @@ fn main() { const THREADS: usize = 12; const LDE_BITS: i32 = 3; const W: usize = 12; - const HASHES_PER_POLY: usize = 1 << (13 + LDE_BITS) / 6; + const HASHES_PER_POLY: usize = 1 << ((13 + LDE_BITS) / 6); let threads = (0..THREADS) .map(|_i| { thread::spawn(move || { - let mut x = [F::ZERO; W]; - for i in 0..W { - x[i] = F::from_canonical_u64((i as u64) * 123456 + 789); - } + let mut x: [F; W] = F::rand_vec(W).try_into().unwrap(); let hashes_per_thread = HASHES_PER_POLY * PROVER_POLYS / THREADS; let start = Instant::now(); diff --git a/src/bin/bench_rescue.rs b/src/bin/bench_rescue.rs index 96334689..1b45c31c 100644 --- a/src/bin/bench_rescue.rs +++ b/src/bin/bench_rescue.rs @@ -1,3 +1,4 @@ +use std::convert::TryInto; use std::thread; use std::time::Instant; @@ -19,10 +20,7 @@ fn main() { let threads = (0..THREADS) .map(|_i| { thread::spawn(move || { - let mut x = [F::ZERO; W]; - for i in 0..W { - x[i] = F::from_canonical_u64((i as u64) * 123456 + 789); - } + let mut x: [F; W] = F::rand_vec(W).try_into().unwrap(); let hashes_per_thread = HASHES_PER_POLY * PROVER_POLYS / THREADS; let start = Instant::now(); diff --git a/src/field/fft.rs b/src/field/fft.rs index fa65a5ea..35b11852 100644 --- a/src/field/fft.rs +++ b/src/field/fft.rs @@ -250,9 +250,9 @@ fn fft_unrolled(input: Vec, r_orig: usize, root_table: FftRootTable // NB: Grouping statements as is done in the main loop below // does not seem to help here (worse by a few millis). let omega_0 = root_table[0][0]; - let tmp_0 = omega_0 * values[k + 2 + 0]; - values[k + 2 + 0] = values[k + 0] - tmp_0; - values[k + 0] += tmp_0; + let tmp_0 = omega_0 * values[k + 2]; + values[k + 2] = values[k] - tmp_0; + values[k] += tmp_0; let omega_1 = root_table[0][1]; let tmp_1 = omega_1 * values[k + 2 + 1]; @@ -281,21 +281,21 @@ fn fft_unrolled(input: Vec, r_orig: usize, root_table: FftRootTable let off1 = k + j; let off2 = k + m + j; - let omega_0 = root_table[lg_m - 1][j + 0]; + let omega_0 = root_table[lg_m - 1][j]; let omega_1 = root_table[lg_m - 1][j + 1]; let omega_2 = root_table[lg_m - 1][j + 2]; let omega_3 = root_table[lg_m - 1][j + 3]; - let tmp_0 = omega_0 * values[off2 + 0]; + let tmp_0 = omega_0 * values[off2]; let tmp_1 = omega_1 * values[off2 + 1]; let tmp_2 = omega_2 * values[off2 + 2]; let tmp_3 = omega_3 * values[off2 + 3]; - values[off2 + 0] = values[off1 + 0] - tmp_0; + values[off2] = values[off1] - tmp_0; values[off2 + 1] = values[off1 + 1] - tmp_1; values[off2 + 2] = values[off1 + 2] - tmp_2; values[off2 + 3] = values[off1 + 3] - tmp_3; - values[off1 + 0] += tmp_0; + values[off1] += tmp_0; values[off1 + 1] += tmp_1; values[off1 + 2] += tmp_2; values[off1 + 3] += tmp_3; diff --git a/src/field/field.rs b/src/field/field.rs index f4ef5990..9c733d04 100644 --- a/src/field/field.rs +++ b/src/field/field.rs @@ -131,7 +131,7 @@ pub trait Field: fn primitive_root_of_unity(n_log: usize) -> Self { assert!(n_log <= Self::TWO_ADICITY); - let mut base = Self::POWER_OF_TWO_GENERATOR; + let base = Self::POWER_OF_TWO_GENERATOR; base.exp_power_of_2(Self::TWO_ADICITY - n_log) } diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index a017ff02..734b6c03 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -68,7 +68,6 @@ impl, const D: usize> CircuitBuilder { new_list.push(new_item); } - new_list } } diff --git a/src/gates/mod.rs b/src/gates/mod.rs index fa23b273..298c9138 100644 --- a/src/gates/mod.rs +++ b/src/gates/mod.rs @@ -1,3 +1,6 @@ +// Gates have `new` methods that return `GateRef`s. +#![allow(clippy::new_ret_no_self)] + pub mod arithmetic; pub mod base_sum; pub mod constant; diff --git a/src/gmimc.rs b/src/gmimc.rs index 9a65d49d..db80f19a 100644 --- a/src/gmimc.rs +++ b/src/gmimc.rs @@ -56,8 +56,8 @@ pub fn gmimc_permute_array( xs[active] -= f; } - for i in 0..W { - xs[i] += addition_buffer; + for x_i in xs.iter_mut() { + *x_i += addition_buffer; } xs diff --git a/src/hash.rs b/src/hash.rs index 47d703cd..f1a30c6d 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -117,12 +117,6 @@ pub const GMIMC_CONSTANTS: [u64; GMIMC_ROUNDS] = [ 8651171085167737860, ]; -/// Controls the granularity of parallelization when building Merkle trees. I.e., we will try to -/// split up the task into units of work, such that each unit involves hashing roughly this many -/// elements. If this is too small, there may be too much synchronization overhead; if it's too -/// large, some threads may spend significant time idle. -const ELEMS_PER_CHUNK: usize = 1 << 8; - /// Hash the vector if necessary to reduce its length to ~256 bits. If it already fits, this is a /// no-op. pub fn hash_or_noop(inputs: Vec) -> Hash { @@ -226,8 +220,8 @@ pub fn hash_n_to_m(mut inputs: Vec, num_outputs: usize, pad: bool) // Squeeze until we have the desired number of outputs. let mut outputs = Vec::new(); loop { - for i in 0..SPONGE_RATE { - outputs.push(state[i]); + for &item in state.iter().take(SPONGE_RATE) { + outputs.push(item); if outputs.len() == num_outputs { return outputs; } diff --git a/src/permutation_argument.rs b/src/permutation_argument.rs index 37ecf8d2..c2312cd1 100644 --- a/src/permutation_argument.rs +++ b/src/permutation_argument.rs @@ -46,7 +46,7 @@ impl usize> TargetPartition } /// Path compression method, see https://en.wikipedia.org/wiki/Disjoint-set_data_structure#Finding_set_representatives. - pub fn find(&mut self, mut x: ForestNode) -> ForestNode { + pub fn find(&mut self, x: ForestNode) -> ForestNode { if x.parent != x.index { let root = self.find(self.forest[x.parent]); self.forest[x.index].parent = root.index; diff --git a/src/polynomial/commitment.rs b/src/polynomial/commitment.rs index 7698c39f..b6ff44cc 100644 --- a/src/polynomial/commitment.rs +++ b/src/polynomial/commitment.rs @@ -287,6 +287,7 @@ mod tests { use super::*; use crate::circuit_data::CircuitConfig; + use crate::fri::FriConfig; use crate::plonk_common::PlonkPolynomials; fn gen_random_test_case, const D: usize>( diff --git a/src/prover.rs b/src/prover.rs index 63c32995..f54c0719 100644 --- a/src/prover.rs +++ b/src/prover.rs @@ -80,7 +80,7 @@ pub(crate) fn prove, const D: usize>( let gammas = challenger.get_n_challenges(num_challenges); assert!( - common_data.quotient_degree_factor + 1 <=common_data.config.num_routed_wires, + common_data.quotient_degree_factor < common_data.config.num_routed_wires, "When the number of routed wires is smaller that the degree, we should change the logic to avoid computing partial products." ); let mut partial_products = timed!( diff --git a/src/rescue.rs b/src/rescue.rs index f088fdb7..9aa12bc5 100644 --- a/src/rescue.rs +++ b/src/rescue.rs @@ -179,7 +179,7 @@ const MDS: [[u64; W]; W] = [ ], ]; -const RESCUE_CONSTANTS: [[u64; W]; 16] = [ +const RESCUE_CONSTANTS: [[u64; W]; ROUNDS * 2] = [ [ 12050887499329086906, 1748247961703512657, @@ -442,7 +442,7 @@ fn mds_layer(x: [F; W]) -> [F; W] { let mut result = [F::ZERO; W]; for r in 0..W { for c in 0..W { - result[r] = result[r] + F::from_canonical_u64(MDS[r][c]) * x[c]; + result[r] += F::from_canonical_u64(MDS[r][c]) * x[c]; } } result diff --git a/src/util/mod.rs b/src/util/mod.rs index 85440d70..ce6bdb27 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -42,8 +42,8 @@ pub(crate) fn transpose(matrix: &[Vec]) -> Vec> { let old_cols = matrix[0].len(); let mut transposed = vec![Vec::with_capacity(old_rows); old_cols]; for new_r in 0..old_cols { - for new_c in 0..old_rows { - transposed[new_r].push(matrix[new_c][new_r].clone()); + for old_row in matrix.iter() { + transposed[new_r].push(old_row[new_r].clone()); } } transposed