Enable five more clippy lints - #10935
Merged
Merged
Conversation
github-actions
Bot
added
parquet
An `else` after a block that always diverges only adds a level of indentation. Removing it keeps the happy path at the outer level. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`assert_eq!`/`assert_ne!` print both operands on failure, while `assert!(a == b)` only prints the source text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`&mut x as *mut _` creates a real `&mut` first, which asserts unique access. In the FFI stream code the callee gets a pointer to the same stream, so `&raw mut x` is the right way to build these pointers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A leading underscore says a binding is unused; reading it anyway makes the name lie. The two conditionally-used parameters keep their names and get a feature-gated expect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Casting a byte pointer to a wider type is UB unless the access is unaligned or the buffer alignment is known. The five existing casts all hold, and now say why. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
emilk
force-pushed
the
emilk/high-value-clippy-lints
branch
from
September 1, 2026 12:17
ec0023c to
191998c
Compare
emilk
marked this pull request as ready for review
September 1, 2026 12:25
Rich-T-kid
approved these changes
Sep 1, 2026
Rich-T-kid
left a comment
Rich-T-kid
left a comment
Contributor
There was a problem hiding this comment.
thank you for splitting this up into one lint per commits. It makes it very easy to review in focused chunks.
This looks good to me except for the cast_ptr_alignment lint which unlike the others I dont think it improves the code.
| cast_possible_truncation = "allow" # ~890 violations in ~190 files | ||
| cast_possible_wrap = "allow" # ~570 violations in ~120 files | ||
| cast_precision_loss = "allow" # ~130 violations in ~33 files | ||
| cast_ptr_alignment = "allow" # ~3 violations in ~3 files |
Contributor
There was a problem hiding this comment.
Im not sure about this lint. we just added expect() blocks for each site instead of changing anything.
I don't see much difference between the current state of this commit and the current saftey comments that already exist
Contributor
Author
There was a problem hiding this comment.
It didn't fix anything in the current code no, but it will help protect us against future mis-alignment bugs 🤷
See https://rust-lang.github.io/rust-clippy/master/index.html#cast_ptr_alignment for more
Jefffrey
approved these changes
Sep 1, 2026
Contributor
|
thanks @emilk & @Rich-T-kid |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Which issue does this PR close?
No issue in particular
Rationale for this change
#10742 left the
pedanticlints we violate asallow, with a violation count each. These five are the ones worth paying for: two guard raw pointers, one guards bindings that claim to be unused, and two improve readability and test output.What changes are included in this PR?
One commit per new lint (easiest to review commit by commit!):
redundant_elsemanual_assert_eqborrow_as_ptr- the FFI stream code now builds its pointers with&raw mutinstead of going through a&mutused_underscore_binding- two conditionally-used parameters keep their names and get a feature-gatedexpectcast_ptr_alignment- the five byte-pointer casts now say why they are soundLet me know if you disagree with any of them.
Are these changes tested?
Covered by existing tests plus the clippy CI job.
Are there any user-facing changes?
No.