Skip to content

Commit 966cb0d

Browse files
authored
Resolve Clippy findings (#45)
* Applied the Clippy-suggested idioms across the utils crate so the reported lints now build cleanly. - `crates/utils/src/misc.rs:33` switched the even-length assertions in the left/right slice helpers to `.is_multiple_of(2)` so Clippy stops flagging the manual modulus checks. - `crates/utils/src/univariate.rs:9` introduced `CacheValue`/`SelectorsCache` type aliases and reused them in the `SELECTORS_CACHE` static to tame the complex type signature. - `crates/utils/src/multilinear.rs:24` replaced the raw `transmute` with a typed pointer cast, and in `:186` rewrote the recursion branch to drop needless returns while using `point.is_empty()`. Tests at `:280` and `:298` now iterate directly over mutable coefficients, removing the range-index loops. - `crates/utils/src/display.rs:8` now relies on `.is_multiple_of(3)` for clarity in the thousands separator logic. Checks: `cargo fmt` `cargo clippy --workspace --all-targets -- -Dwarnings` now continues past `crates/utils`, but fails later with pre-existing lints in `crates/packed_pcs` and `crates/sumcheck` (multiple bound locations, unused type parameter, `map`+`flatten`, needless borrows, function argument count). Those remain to be addressed separately. Next steps: 1. Tackle the newly surfaced Clippy findings in `crates/packed_pcs` and `crates/sumcheck`, or relax the lint levels for those items if appropriate. * - Updated `compute_chunks` and `num_packed_vars_for_dims` to drop the redundant `EF` parameter and return constraints, and reworked `packed_pcs_commit`’s generics so all trait bounds live in the `where` clause (`crates/packed_pcs/src/lib.rs:149`, `:181`, `:196`). - Replaced the `map(...).flatten()` pattern with `flat_map`, avoided needless reborrows when looking up chunks, and compared boolean prefixes directly rather than taking extra references (`crates/packed_pcs/src/lib.rs:299`, `:310`, `:352`, `:500`). - Adjusted verifier-side lookups to use the new chunk access pattern and the simplified comparisons (`crates/packed_pcs/src/lib.rs:465`, `:487`, `:500`). - Updated the two call sites that still referenced the old `num_packed_vars_for_dims::<EF, EF>` signature (`crates/lean_prover/src/prove_execution.rs:992`, `crates/lean_prover/src/verify_execution.rs:548`). Checks: `cargo fmt`. `cargo clippy --workspace --all-targets -- -Dwarnings` still stops at the existing `sumcheck` lints (too many arguments); no new warnings from `packed_pcs`. Next steps: 1. Resolve the `clippy::too_many_arguments` reports in `crates/sumcheck/src/mle.rs` or suppress them if that’s intentional. * - Introduced the `Poseidon2Config` workflow and split the prover/verifier logic into reusable helpers, which shrank the main entry point and dropped the lossless cast in `src/examples/prove_poseidon2.rs:18-275`; updated the CLI entrypoint to build that config once and pass it by reference (`src/main.rs:5-19`). - Cleaned up the PCS stack: `compute_chunks`/`num_packed_vars_for_dims` now avoid redundant generics, the commit/global-statement helpers use iterators instead of `map(...).flatten()`, and dictionary lookups no longer take needless borrows (`crates/packed_pcs/src/lib.rs:149-332`). - Refactored sumcheck execution to accept parameter structs instead of long argument lists and reused those structs for both packed and unpacked flows (`crates/sumcheck/src/mle.rs:309-517`, `crates/sumcheck/src/prove.rs:149-186`). - Converted numerous range-index loops to iterator-based updates (e.g. `crates/lean_vm/src/memory.rs:32-38`, `crates/lean_prover/witness_generation/src/execution_trace.rs:270-276`, `crates/lean_prover/src/prove_execution.rs:802-824`, `crates/lean_prover/src/verify_execution.rs:862-872`) and collapsed deep `if let` chains in the AIR/Lean compiler stack (`crates/air/src/prove.rs:118-182`, `crates/air/src/verify.rs:187-281`, `crates/lean_compiler/src/a_simplify_lang.rs:681-707`, `crates/lean_compiler/src/b_compile_intermediate.rs:50-79`, `crates/lean_compiler/src/c_compile_final.rs:152-175`). - Simplified and renamed helpers across the Lean prover pipeline: `get_base_dims` now groups its inputs into tuples (`crates/lean_prover/src/common.rs:16-39`); the runner uses a params struct instead of nine loose arguments and avoids redundant pointer casts (`crates/lean_vm/src/runner.rs:149-285`); bitfield generation no longer performs a modulo-by-one (`crates/rec_aggregation/src/xmss_aggregate.rs:215-231`); miscellaneous utilities use the standard `.is_multiple_of()` idiom (`crates/utils/src/misc.rs:32-53`, `crates/utils/src/display.rs:7-11`) and clarified cache aliases in `crates/utils/src/univariate.rs:9-13`. Clippy: `cargo clippy --workspace --all-targets -- -Dwarnings` * - Reworked the row-filling loops in `crates/air/src/test.rs:101-152` to eliminate range indexing: both structured and unstructured trace generators now walk column iterators row-by-row, keeping per-column state where needed and breaking cleanly when the iterators exhaust. This removes the needless index loop flagged by Clippy. Tests: `cargo clippy -p air --tests -- -Dwarnings`
1 parent 6144927 commit 966cb0d

File tree

24 files changed

+589
-362
lines changed

24 files changed

+589
-362
lines changed

crates/air/src/prove.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ impl<EF: ExtensionField<PF<EF>>, A: NormalAir<EF>, AP: PackedAir<EF>> AirTable<E
122122
univariate_skips: usize,
123123
witness: AirWitness<'a, PF<EF>>,
124124
) -> Vec<Evaluation<EF>> {
125-
prove_air::<PF<EF>, EF, A, AP>(prover_state, univariate_skips, &self, witness)
125+
prove_air::<PF<EF>, EF, A, AP>(prover_state, univariate_skips, self, witness)
126126
}
127127

128128
#[instrument(name = "air: prove in extension", skip_all)]
@@ -132,7 +132,7 @@ impl<EF: ExtensionField<PF<EF>>, A: NormalAir<EF>, AP: PackedAir<EF>> AirTable<E
132132
univariate_skips: usize,
133133
witness: AirWitness<'a, EF>,
134134
) -> Vec<Evaluation<EF>> {
135-
prove_air::<EF, EF, A, AP>(prover_state, univariate_skips, &self, witness)
135+
prove_air::<EF, EF, A, AP>(prover_state, univariate_skips, self, witness)
136136
}
137137
}
138138

@@ -224,9 +224,13 @@ fn open_structured_columns<'a, EF: ExtensionField<PF<EF>> + ExtensionField<IF>,
224224
let mut column_scalars = vec![];
225225
let mut index = 0;
226226
for group in &witness.column_groups {
227-
for i in index..index + group.len() {
228-
column_scalars.push(poly_eq_batching_scalars[i]);
229-
}
227+
column_scalars.extend(
228+
poly_eq_batching_scalars
229+
.iter()
230+
.skip(index)
231+
.take(group.len())
232+
.copied(),
233+
);
230234
index += witness.max_columns_per_group().next_power_of_two();
231235
}
232236

crates/air/src/table.rs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -81,18 +81,18 @@ impl<EF: ExtensionField<PF<EF>>, A: NormalAir<EF>, AP: PackedAir<EF>> AirTable<E
8181
};
8282
if TypeId::of::<IF>() == TypeId::of::<EF>() {
8383
unsafe {
84-
self.air
85-
.eval(transmute::<_, &mut ConstraintChecker<'_, EF, EF>>(
86-
&mut constraints_checker,
87-
));
84+
self.air.eval(transmute::<
85+
&mut ConstraintChecker<'_, IF, EF>,
86+
&mut ConstraintChecker<'_, EF, EF>,
87+
>(&mut constraints_checker));
8888
}
8989
} else {
9090
assert_eq!(TypeId::of::<IF>(), TypeId::of::<PF<EF>>());
9191
unsafe {
92-
self.air
93-
.eval(transmute::<_, &mut ConstraintChecker<'_, PF<EF>, EF>>(
94-
&mut constraints_checker,
95-
));
92+
self.air.eval(transmute::<
93+
&mut ConstraintChecker<'_, IF, EF>,
94+
&mut ConstraintChecker<'_, PF<EF>, EF>,
95+
>(&mut constraints_checker));
9696
}
9797
}
9898
handle_errors(row, &mut constraints_checker)?;
@@ -110,18 +110,18 @@ impl<EF: ExtensionField<PF<EF>>, A: NormalAir<EF>, AP: PackedAir<EF>> AirTable<E
110110
};
111111
if TypeId::of::<IF>() == TypeId::of::<EF>() {
112112
unsafe {
113-
self.air
114-
.eval(transmute::<_, &mut ConstraintChecker<'_, EF, EF>>(
115-
&mut constraints_checker,
116-
));
113+
self.air.eval(transmute::<
114+
&mut ConstraintChecker<'_, IF, EF>,
115+
&mut ConstraintChecker<'_, EF, EF>,
116+
>(&mut constraints_checker));
117117
}
118118
} else {
119119
assert_eq!(TypeId::of::<IF>(), TypeId::of::<PF<EF>>());
120120
unsafe {
121-
self.air
122-
.eval(transmute::<_, &mut ConstraintChecker<'_, PF<EF>, EF>>(
123-
&mut constraints_checker,
124-
));
121+
self.air.eval(transmute::<
122+
&mut ConstraintChecker<'_, IF, EF>,
123+
&mut ConstraintChecker<'_, PF<EF>, EF>,
124+
>(&mut constraints_checker));
125125
}
126126
}
127127
handle_errors(row, &mut constraints_checker)?;

crates/air/src/test.rs

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -105,16 +105,41 @@ fn generate_structured_trace<const N_COLUMNS: usize, const N_PREPROCESSED_COLUMN
105105
trace.push((0..n_rows).map(|_| rng.random()).collect::<Vec<F>>());
106106
}
107107
let mut witness_cols = vec![vec![F::ZERO]; N_COLUMNS - N_PREPROCESSED_COLUMNS];
108-
for i in 1..n_rows {
109-
for j in 0..N_COLUMNS - N_PREPROCESSED_COLUMNS {
110-
let witness_cols_j_i_min_1 = witness_cols[j][i - 1];
111-
witness_cols[j].push(
112-
witness_cols_j_i_min_1
113-
+ F::from_usize(j + N_PREPROCESSED_COLUMNS)
114-
+ (0..N_PREPROCESSED_COLUMNS)
115-
.map(|k| trace[k][i])
116-
.product::<F>(),
117-
);
108+
let mut prev_values = vec![F::ZERO; N_COLUMNS - N_PREPROCESSED_COLUMNS];
109+
let mut column_iters = trace[..N_PREPROCESSED_COLUMNS]
110+
.iter()
111+
.map(|col| col.iter())
112+
.collect::<Vec<_>>();
113+
if column_iters.is_empty() {
114+
trace.extend(witness_cols);
115+
return trace;
116+
}
117+
for iter in &mut column_iters {
118+
iter.next(); // skip first row, already initialised
119+
}
120+
loop {
121+
let mut row_product = F::ONE;
122+
let mut progressed = true;
123+
for iter in &mut column_iters {
124+
match iter.next() {
125+
Some(value) => row_product *= *value,
126+
None => {
127+
progressed = false;
128+
break;
129+
}
130+
}
131+
}
132+
if !progressed {
133+
break;
134+
}
135+
for (j, (witness_col, prev)) in witness_cols
136+
.iter_mut()
137+
.zip(prev_values.iter_mut())
138+
.enumerate()
139+
{
140+
let next_val = *prev + F::from_usize(j + N_PREPROCESSED_COLUMNS) + row_product;
141+
witness_col.push(next_val);
142+
*prev = next_val;
118143
}
119144
}
120145
trace.extend(witness_cols);
@@ -131,14 +156,31 @@ fn generate_unstructured_trace<const N_COLUMNS: usize, const N_PREPROCESSED_COLU
131156
trace.push((0..n_rows).map(|_| rng.random()).collect::<Vec<F>>());
132157
}
133158
let mut witness_cols = vec![vec![]; N_COLUMNS - N_PREPROCESSED_COLUMNS];
134-
for i in 0..n_rows {
135-
for j in 0..N_COLUMNS - N_PREPROCESSED_COLUMNS {
136-
witness_cols[j].push(
137-
F::from_usize(j + N_PREPROCESSED_COLUMNS)
138-
+ (0..N_PREPROCESSED_COLUMNS)
139-
.map(|k| trace[k][i])
140-
.product::<F>(),
141-
);
159+
let mut column_iters = trace[..N_PREPROCESSED_COLUMNS]
160+
.iter()
161+
.map(|col| col.iter())
162+
.collect::<Vec<_>>();
163+
if column_iters.is_empty() {
164+
trace.extend(witness_cols);
165+
return trace;
166+
}
167+
loop {
168+
let mut row_product = F::ONE;
169+
let mut progressed = true;
170+
for iter in &mut column_iters {
171+
match iter.next() {
172+
Some(value) => row_product *= *value,
173+
None => {
174+
progressed = false;
175+
break;
176+
}
177+
}
178+
}
179+
if !progressed {
180+
break;
181+
}
182+
for (j, witness_col) in witness_cols.iter_mut().enumerate() {
183+
witness_col.push(F::from_usize(j + N_PREPROCESSED_COLUMNS) + row_product);
142184
}
143185
}
144186
trace.extend(witness_cols);

crates/air/src/verify.rs

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -84,16 +84,18 @@ fn verify_air<EF: ExtensionField<PF<EF>>, A: NormalAir<EF>, AP: PackedAir<EF>>(
8484
if structured_air {
8585
verify_structured_columns(
8686
verifier_state,
87-
table.n_columns(),
88-
univariate_skips,
89-
&inner_sums,
90-
&column_groups,
91-
&Evaluation::new(
92-
outer_statement.point[1..log_length - univariate_skips + 1].to_vec(),
93-
outer_statement.value,
94-
),
95-
&outer_selector_evals,
96-
log_length,
87+
StructuredColumnsArgs {
88+
n_columns: table.n_columns(),
89+
univariate_skips,
90+
all_inner_sums: &inner_sums,
91+
column_groups,
92+
outer_sumcheck_challenge: &Evaluation::new(
93+
outer_statement.point[1..log_length - univariate_skips + 1].to_vec(),
94+
outer_statement.value,
95+
),
96+
outer_selector_evals: &outer_selector_evals,
97+
log_n_rows: log_length,
98+
},
9799
)
98100
} else {
99101
verify_many_unstructured_columns(
@@ -181,16 +183,30 @@ fn verify_many_unstructured_columns<EF: ExtensionField<PF<EF>>>(
181183
Ok(evaluations_remaining_to_verify)
182184
}
183185

184-
fn verify_structured_columns<EF: ExtensionField<PF<EF>>>(
185-
verifier_state: &mut FSVerifier<EF, impl FSChallenger<EF>>,
186+
#[derive(Debug)]
187+
struct StructuredColumnsArgs<'a, EF> {
186188
n_columns: usize,
187189
univariate_skips: usize,
188-
all_inner_sums: &[EF],
189-
column_groups: &[Range<usize>],
190-
outer_sumcheck_challenge: &Evaluation<EF>,
191-
outer_selector_evals: &[EF],
190+
all_inner_sums: &'a [EF],
191+
column_groups: &'a [Range<usize>],
192+
outer_sumcheck_challenge: &'a Evaluation<EF>,
193+
outer_selector_evals: &'a [EF],
192194
log_n_rows: usize,
195+
}
196+
197+
fn verify_structured_columns<EF: ExtensionField<PF<EF>>>(
198+
verifier_state: &mut FSVerifier<EF, impl FSChallenger<EF>>,
199+
args: StructuredColumnsArgs<'_, EF>,
193200
) -> Result<Vec<Evaluation<EF>>, ProofError> {
201+
let StructuredColumnsArgs {
202+
n_columns,
203+
univariate_skips,
204+
all_inner_sums,
205+
column_groups,
206+
outer_sumcheck_challenge,
207+
outer_selector_evals,
208+
log_n_rows,
209+
} = args;
194210
let log_n_groups = log2_ceil_usize(column_groups.len());
195211
let max_columns_per_group = Iterator::max(column_groups.iter().map(|g| g.len())).unwrap();
196212
let log_max_columns_per_group = log2_ceil_usize(max_columns_per_group);
@@ -201,9 +217,13 @@ fn verify_structured_columns<EF: ExtensionField<PF<EF>>>(
201217
let mut column_scalars = vec![];
202218
let mut index = 0;
203219
for group in column_groups {
204-
for i in index..index + group.len() {
205-
column_scalars.push(poly_eq_batching_scalars[i]);
206-
}
220+
column_scalars.extend(
221+
poly_eq_batching_scalars
222+
.iter()
223+
.skip(index)
224+
.take(group.len())
225+
.copied(),
226+
);
207227
index += max_columns_per_group.next_power_of_two();
208228
}
209229

crates/lean_compiler/src/a_simplify_lang.rs

Lines changed: 34 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,10 @@ fn simplify_lines(
460460
unimplemented!("Reverse for non-unrolled loops are not implemented yet");
461461
}
462462

463-
let mut loop_const_malloc = ConstMalloc::default();
464-
loop_const_malloc.counter = const_malloc.counter;
463+
let mut loop_const_malloc = ConstMalloc {
464+
counter: const_malloc.counter,
465+
..ConstMalloc::default()
466+
};
465467
let valid_aux_vars_in_array_manager_before = array_manager.valid.clone();
466468
array_manager.valid.clear();
467469
let simplified_body = simplify_lines(
@@ -678,16 +680,15 @@ fn simplify_expr(
678680
match expr {
679681
Expression::Value(value) => value.simplify_if_const(),
680682
Expression::ArrayAccess { array, index } => {
681-
if let SimpleExpr::Var(array_var) = array {
682-
if let Some(label) = const_malloc.map.get(array_var) {
683-
if let Ok(mut offset) = ConstExpression::try_from(*index.clone()) {
684-
offset = offset.try_naive_simplification();
685-
return SimpleExpr::ConstMallocAccess {
686-
malloc_label: *label,
687-
offset,
688-
};
689-
}
690-
}
683+
if let SimpleExpr::Var(array_var) = array
684+
&& let Some(label) = const_malloc.map.get(array_var)
685+
&& let Ok(mut offset) = ConstExpression::try_from(*index.clone())
686+
{
687+
offset = offset.try_naive_simplification();
688+
return SimpleExpr::ConstMallocAccess {
689+
malloc_label: *label,
690+
offset,
691+
};
691692
}
692693

693694
let aux_arr = array_manager.get_aux_var(array, index); // auxiliary var to store m[array + index]
@@ -1082,30 +1083,27 @@ fn handle_array_assignment(
10821083
) {
10831084
let simplified_index = simplify_expr(index, res, counters, array_manager, const_malloc);
10841085

1085-
if let SimpleExpr::Constant(offset) = simplified_index.clone() {
1086-
if let SimpleExpr::Var(array_var) = &array {
1087-
if let Some(label) = const_malloc.map.get(array_var) {
1088-
if let ArrayAccessType::ArrayIsAssigned(Expression::Binary {
1089-
left,
1090-
operation,
1091-
right,
1092-
}) = access_type
1093-
{
1094-
let arg0 = simplify_expr(&left, res, counters, array_manager, const_malloc);
1095-
let arg1 = simplify_expr(&right, res, counters, array_manager, const_malloc);
1096-
res.push(SimpleLine::Assignment {
1097-
var: VarOrConstMallocAccess::ConstMallocAccess {
1098-
malloc_label: *label,
1099-
offset,
1100-
},
1101-
operation,
1102-
arg0,
1103-
arg1,
1104-
});
1105-
return;
1106-
}
1107-
}
1108-
}
1086+
if let SimpleExpr::Constant(offset) = simplified_index.clone()
1087+
&& let SimpleExpr::Var(array_var) = &array
1088+
&& let Some(label) = const_malloc.map.get(array_var)
1089+
&& let ArrayAccessType::ArrayIsAssigned(Expression::Binary {
1090+
left,
1091+
operation,
1092+
right,
1093+
}) = &access_type
1094+
{
1095+
let arg0 = simplify_expr(left, res, counters, array_manager, const_malloc);
1096+
let arg1 = simplify_expr(right, res, counters, array_manager, const_malloc);
1097+
res.push(SimpleLine::Assignment {
1098+
var: VarOrConstMallocAccess::ConstMallocAccess {
1099+
malloc_label: *label,
1100+
offset,
1101+
},
1102+
operation: *operation,
1103+
arg0,
1104+
arg1,
1105+
});
1106+
return;
11091107
}
11101108

11111109
let value_simplified = match access_type {

0 commit comments

Comments
 (0)