Repository navigation
Fix the u8 shift overflow in the buffer_bit_ops benchmark - #11175
Merged
Jefffrey merged 1 commit intoSep 23, 2026
Merged
Conversation
`0b01010101 << i` infers u8, so for every i >= 8 the shift overflows. `cargo bench` masks the shift amount and carries on; `cargo test --benches` (debug, overflow checks on) panics at line 31 before the first benchmark runs. `wrapping_shl` masks the same way, so the buffers are byte-identical to what the release build produces today. Part of apache#10952
Jefffrey
approved these changes
Sep 23, 2026
Jefffrey
added
the
development-process
Contributor
|
thanks @amitvijapur |
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
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?
Part of #10952.
Rationale for this change
Running the benches once under
cargo test --benches(the check #10952 asks for) turns up one bench that cannot even start:buffer_bit_opspanics at line 31 withattempt to shift left with overflow.0b01010101 << iis assigned into au8slot, so the literal infersu8and the shift overflows for everyi >= 8.cargo benchbuilds in release, where the shift amount is masked and the bench runs, which is why it has never shown up.What changes are included in this PR?
The two shifts become
wrapping_shl, which masks the shift amount the same way release mode does, so the fixture bytes are identical to what the benchmark has been measuring all along (checked foriin0..64). rustfmt split the expression over three lines.Are these changes tested?
cargo test -p arrow --features test_utils --bench buffer_bit_opspanics onmainand passes here. The byte-identity check was a small standalone program comparing the new expression against the masked semantics of the old one.Are there any user-facing changes?
No.