Resolve Clippy findings #45
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…orted 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.
… 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.
…fier 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`
…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`
Collaborator
|
Thanks a lot. I would like ideally to avoid creating structs only used for the arguments of 1 function (that has too many arguments), to make clippy happy. The best would be to find a way to reduce the number of arguments without introducing a new struct. To make things easy, I am going to merge your PR and modify parts of it after. EDIT: for prove_poseidon2.rs I am going to let the structs, they look nice in this context, thanks! |
Contributor
Author
@TomWambsgans
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
crates/air/src/prove.rs:125andcrates/air/src/table.rs:101drop redundant borrows and tighten transmute signatures so Clippy no longer flags needless references or unsound casts.src/examples/prove_poseidon2.rs:62introduces Poseidon2Config plussetup/prover/verifier helpers, eliminating the huge argument list and duplicated logic that triggered too_many_arguments/clippy::needless_pass_by_value.crates/lean_vm/src/runner.rs:90wraps the sprawling helper parameters in ExecuteBytecodeParams and rewrites slice initialisation with iterator adapters, addressing too_many_arguments, ptr_arg, and manual loop lints.crates/sumcheck/src/mle.rs:309adds SumcheckComputeParams / SumcheckComputeNotPackedParams to replace long parameter lists and clarifies data flow for packed vs. unpacked cases, satisfying Clippy’s complexity warnings.crates/utils/src/multilinear.rs:21,crates/utils/src/univariate.rs:10, and related helpers swap raw transmute/index loops for safe casts, cached type aliases, and iterator-driven mutations, clearing transmute_ptr_to_ptr and manual_memcpy lint complaints.crates/air/src/test.rs:105, crates/packed_pcs/src/lib.rs:146,crates/lean_compiler/src/a_simplify_lang.rs:678) systematically convert nested for loops to iterator chains, tighten equality checks, and ensure option chaining, resolving the remaining Clippy warnings across the branch.