Remove the unimplemented pointer variant from BitVec - #34
Open
arthaud wants to merge 1 commit into
Open
Conversation
BitVec stored its bits in a `PointerOrBits` union whose `pointer` arm was
never written or read: every operation that would have needed it bailed
out with `todo!("Long bitvec not implemented")`, and there was no
allocation or `Drop` logic backing it.
Those branches were also unreachable. Nothing can build a BitVec longer
than `POINTER_SIZE_IN_BITS`: `new` is empty, `from_int` is exactly one
word, `from_int_with_len` asserts, `common_prefix` delegates to
`from_int_with_len`, and the `From` impls only cover integral types of at
most that width. The sole method that could have grown one past the limit
was the private `push`, which had no callers and survived only because of
the crate-level `#![allow(dead_code)]`.
Replace the union with a plain `bits: usize` field. This drops all six
`todo!`s and every `unsafe` block in the crate, and lets `Clone`,
`PartialEq` and `Eq` be derived rather than hand-written (the derives are
equivalent: `from_int_with_len` masks off the bits above `len`, so two
values of equal length always have equal representations).
Also removed:
- `push`, dead code that existed only to grow into the long representation.
- The redundant length assertion in `from_int_with_len`; `make_mask`
asserts the same bound on the following line.
Algorithms are unchanged; this is representation only. Supporting keys
wider than a word would need a real long variant here (an enum over
`Box<[usize]>` rather than a union), plus a decision about how iteration
hands such keys back.
Test plan: `cargo test` — 53 tests pass. `cargo clippy --all-targets`
goes from 18 warnings to 17; `cargo fmt --check` reports no diff for this
file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KRRfrza7QDBPhTT3yAS8NV
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.
BitVec stored its bits in a
PointerOrBitsunion whosepointerarm was never written or read: every operation that would have needed it bailed out withtodo!("Long bitvec not implemented"), and there was no allocation orDroplogic backing it.Those branches were also unreachable. Nothing can build a BitVec longer than
POINTER_SIZE_IN_BITS:newis empty,from_intis exactly one word,from_int_with_lenasserts,common_prefixdelegates tofrom_int_with_len, and theFromimpls only cover integral types of at most that width. The sole method that could have grown one past the limit was the privatepush, which had no callers and survived only because of the crate-level#![allow(dead_code)].Replace the union with a plain
bits: usizefield. This drops all sixtodo!s and everyunsafeblock in the crate, and letsClone,PartialEqandEqbe derived rather than hand-written (the derives are equivalent:from_int_with_lenmasks off the bits abovelen, so two values of equal length always have equal representations).Also removed:
push, dead code that existed only to grow into the long representation.from_int_with_len;make_maskasserts the same bound on the following line.Algorithms are unchanged; this is representation only. Supporting keys wider than a word would need a real long variant here (an enum over
Box<[usize]>rather than a union), plus a decision about how iteration hands such keys back.Test plan:
cargo test— 53 tests pass.cargo clippy --all-targetsgoes from 18 warnings to 17;cargo fmt --checkreports no diff for this file.