Clippy: allow or fix clippy::unnecessary_cast lint in src/#1238
Clippy: allow or fix clippy::unnecessary_cast lint in src/#1238DanielEScherzer wants to merge 2 commits into
clippy::unnecessary_cast lint in src/#1238Conversation
In a lot of places, the lint is triggered when casting a constant from the libgit2 bindings that is created (in libgit2-sys) via `git_enum!`. That macro will use i32 when `#[cfg(target_env = "msvc")]`, and u32 otherwise, meaning that a number of the casts that clippy flags as "unnecessary" are indeed necessary. In those cases, use `#[allow]` with a reason. Similarly, clippy also lints against casting of `c_int` and `c_uint` to `i32` and `u32` - while those are usually the same types, the libc documentation makes it clear that they are not always the same. In those cases too, use `#[allow]` with a reason. The only libc-provided type with a consistent rust type is that `size_t` is always the same as `usize` - fix the places that clippy identified where that conversion is unneeded. Other fixes include casts when the underlying type is explicitly set in the libgit2 bindings to be a specific rust type, or when converting mutable pointers to the same type they already are. After these changes, running the command `cargo clippy -- -A clippy::all -W clippy::unnecessary_cast` no longer reports any hits.
|
This is the first of a few PRs to get clippy passing, and then I'll send a PR to add it to CI |
There was a problem hiding this comment.
A couple of thoughts:
- What is our MSRV? Edition 2021 is 1.56 I remembered, and lint reason was introduced along with
#[expect]which is 1.81. - This adds lots of visual noise. Not sure if there are other good way to improve this, like, should we just suppress for all?
- We don't have clippy in CI yet. Should probably add one in a separate PR.
There was a problem hiding this comment.
What is our MSRV? Edition 2021 is 1.56 I remembered, and lint reason was introduced along with #[expect] which is 1.81.
I guess I had assumed that CI tested against the MSRV
This adds lots of visual noise. Not sure if there are other good way to improve this, like, should we just suppress for all?
Some of the places that it caught things were indeed unneeded casts that I removed, so I think it is useful to have
We don't have clippy in CI yet. Should probably add one in a separate PR.
I wanted to get it passing first, see my comment above that
This is the first of a few PRs to get clippy passing, and then I'll send a PR to add it to CI
3 participants
In a lot of places, the lint is triggered when casting a constant from the libgit2 bindings that is created (in libgit2-sys) via
git_enum!. That macro will use i32 when#[cfg(target_env = "msvc")], and u32 otherwise, meaning that a number of the casts that clippy flags as "unnecessary" are indeed necessary. In those cases, use#[allow]with a reason.Similarly, clippy also lints against casting of
c_intandc_uinttoi32andu32- while those are usually the same types, the libc documentation makes it clear that they are not always the same. In those cases too, use#[allow]with a reason.The only libc-provided type with a consistent rust type is that
size_tis always the same asusize- fix the places that clippy identified where that conversion is unneeded. Other fixes include casts when the underlying type is explicitly set in the libgit2 bindings to be a specific rust type, or when converting mutable pointers to the same type they already are.After these changes, running the command
cargo clippy -- -A clippy::all -W clippy::unnecessary_castno longer reports any hits.