Skip to content

Remove the unimplemented pointer variant from BitVec - #34

Open
arthaud wants to merge 1 commit into
facebook:mainfrom
arthaud:remove-bitvec-pointer-variant
Open

Remove the unimplemented pointer variant from BitVec#34
arthaud wants to merge 1 commit into
facebook:mainfrom
arthaud:remove-bitvec-pointer-variant

Conversation

@arthaud

@arthaud arthaud commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

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.

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
@meta-cla meta-cla Bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant