You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "emilk (via GitHub)" <gi...@apache.org> on 2026/09/01 11:41:13 UTC
[PR] Enable five more clippy lints [arrow-rs]
emilk opened a new pull request, #10935:
URL: https://github.com/apache/arrow-rs/pull/10935
# Rationale for this change
Five lints from the `pedantic` allow-list in the root `Cargo.toml` are worth
turning on: two guard raw-pointer correctness, one guards against reading
bindings that claim to be unused, and two improve readability and test output.
All five have few enough violations to fix in one pass.
# What changes are included in this PR?
One commit per lint, each with the lint plus its fixes:
* `borrow_as_ptr` — `&mut x as *mut _` materializes a `&mut` first; the FFI
stream code now uses `&raw mut x`.
* `cast_ptr_alignment` — the five byte-pointer-to-`u64`/`i64` casts now
document why they are sound (unaligned access, or a 64-byte-aligned buffer).
* `used_underscore_binding` — dropped underscores from bindings that are read.
The two conditionally-used parameters keep theirs and get a feature-gated
`expect`.
* `manual_assert_eq` — `assert_eq!`/`assert_ne!` print both operands on failure.
* `redundant_else` — dropped `else` after diverging blocks.
No behavior changes.
# Are these changes tested?
Covered by existing tests. `cargo clippy --workspace --all-targets` is clean
with and without `--all-features`.
# Are there any user-facing changes?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Enable five more clippy lints [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey merged PR #10935:
URL: https://github.com/apache/arrow-rs/pull/10935
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Enable five more clippy lints [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #10935:
URL: https://github.com/apache/arrow-rs/pull/10935#issuecomment-5496068996
thanks @emilk & @Rich-T-kid
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Enable five more clippy lints [arrow-rs]
Posted by "emilk (via GitHub)" <gi...@apache.org>.
emilk commented on code in PR #10935:
URL: https://github.com/apache/arrow-rs/pull/10935#discussion_r3905115143
##########
Cargo.toml:
##########
@@ -224,7 +224,6 @@ cast_lossless = "allow" # ~640 violations in ~100 files
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
Review 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Enable five more clippy lints [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10935:
URL: https://github.com/apache/arrow-rs/pull/10935#discussion_r3905498491
##########
Cargo.toml:
##########
@@ -224,7 +224,6 @@ cast_lossless = "allow" # ~640 violations in ~100 files
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
Review Comment:
sounds reasonable 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Enable five more clippy lints [arrow-rs]
Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on code in PR #10935:
URL: https://github.com/apache/arrow-rs/pull/10935#discussion_r3905057531
##########
Cargo.toml:
##########
@@ -224,7 +224,6 @@ cast_lossless = "allow" # ~640 violations in ~100 files
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
Review 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org