Skip to content

Fix clippy - #264

Merged
RobinLinus merged 3 commits into
BitVM:mainfrom
uncomputable:2025-03-clippy
Mar 17, 2025
Merged

RobinLinus merged 3 commits into
BitVM:mainfrom
uncomputable:2025-03-clippy

Conversation

@uncomputable

@uncomputable uncomputable commented Mar 14, 2025 •

Copy link
Copy Markdown
Contributor

Progress towards #259. There are still many warnings outside of bitvm/bitvm.

There were a lot of clippy lints. Especially when running cargo clippy --all-targets. This PR resolves all of them.

I tried to be as conservative as possible, keeping the general function structure the same, not deleting any types, etc. At some places, I allowed the lint instead of resolving it. These lines are searchable via git grep and can be fixed in follow-up PRs.

The manual_div_ceil lint explanation might help reviewers.

There is also the usage of cfg_chunks_mut! from ark_std that caused problems due to the missing parallel feature. I replaced that line with the non-parallel function call, as by the macro.

warning: unexpected `cfg` condition value: `parallel`
   --> bitvm/src/bn254/ell_coeffs.rs:329:21
    |
329 |         let mut f = cfg_chunks_mut!(pairs, 4)
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: expected values for `feature` are: `fuzzing`
    = note: using a cfg inside a macro will use the cfgs from the destination crate and not the ones from the defining crate
    = help: try referring to `cfg_chunks_mut` crate for guidance on how handle this unexpected cfg
    = help: the macro `cfg_chunks_mut` may come from an old version of the `ark_std` crate, try updating your dependency with `cargo update -p ark_std`
    = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
    = note: `#[warn(unexpected_cfgs)]` on by default
    = note: this warning originates in the macro `cfg_chunks_mut` (in Nightly builds, run with -Z macro-backtrace for more info)

@AaronFeickert AaronFeickert left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pending a rebase since #257 has been merged.

@manishbista28

manishbista28 commented Mar 15, 2025 •

Copy link
Copy Markdown
Contributor

looks good to me,
thanks for fixing these.

Comment thread bitvm/src/bn254/ell_coeffs.rs Outdated
Comment thread bitvm/src/chunk/g16_runner_core.rs Outdated
@AaronFeickert

Copy link
Copy Markdown
Contributor

We should update CI to fail on linter warnings (and enforce passing CI for merges).

@uncomputable

uncomputable commented Mar 15, 2025 •

Copy link
Copy Markdown
Contributor Author

I rebased onto main. I removed the unused code and updated CI to deny clippy warnings.

@AaronFeickert

Copy link
Copy Markdown
Contributor

Many more lints from bridge.

@uncomputable

Copy link
Copy Markdown
Contributor Author

I reverted the CI change because clippy finds a lot of warnings in the bridge and in other places. Fixing these warnings is out of scope for this PR.

@uncomputable uncomputable mentioned this pull request Mar 15, 2025

@AaronFeickert AaronFeickert left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@uncomputable

Copy link
Copy Markdown
Contributor Author

Rebased onto main

@RobinLinus
RobinLinus merged commit 17c2215 into BitVM:main Mar 17, 2025
@uncomputable
uncomputable deleted the 2025-03-clippy branch April 28, 2025 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants