Skip to content

Replace scattered enum transmutes with an index_enum macro - #329

Merged
kirillbobyrev merged 1 commit into
mainfrom
claude/codebase-review-0hfehw
Jul 21, 2026
Merged

Replace scattered enum transmutes with an index_enum macro#329
kirillbobyrev merged 1 commit into
mainfrom
claude/codebase-review-0hfehw

Conversation

@kirillbobyrev

Copy link
Copy Markdown
Owner

Summary

Fifth and last in the sequence (after #325#328). The transmute safety cleanup.

Square, File, and Rank each hand-wrote unsafe { mem::transmute } to turn a u8 back into the enum — ~18 scattered call sites, each an independent chance to introduce UB if a bound was ever wrong.

Changes

An index_enum! macro generates, for a contiguous #[repr(u8)] enum:

  • ALL — the variants indexed by discriminant;
  • from_index — the safe, checked conversion (None out of range);
  • from_index_unchecked — one audited, debug_assert-guarded unsafe constructor wrapping the single remaining transmute, with a documented safety contract.

Cold and fallible sites — FEN/UCI parsing, all the TryFrom impls, Display/Debug, Square::next — now use the safe from_index / ALL and contain no unsafe at all. The few hot infallible conversions (Square::new/file/rank/flip_perspective, Bitboard::as_square, and the bitboard iterator) call from_index_unchecked with a // SAFETY note explaining why the index is in range by construction.

Net: 18 scattered transmutes → 1, inside the macro.

Performance

This is on the movegen hot path, so I benchmarked it. A first, fully-safe version (array indexing everywhere) regressed movegen ~3.4%; keeping the hot infallible conversions zero-cost via from_index_unchecked brings it back to 25.9 ms vs. the 27.4 ms baseline — no regression (within noise, if anything slightly faster).

Testing

  • 143 tests pass; the full expensive perft suite (incl. the shakmaty differential test) passes in release — move generation is exactly correct.
  • cargo bench movegen: no regression vs. baseline.
  • cargo clippy --all-targets --all-features zero warnings; cargo +nightly fmt --check clean.

Sequence complete

This finishes the five-PR plan you asked for: (1) unify Game + tablebase-in-search #325, (2) transposition table / tree reuse #326, (3) datagen self-play #327, (4) UCI completeness #328, and (5) this transmute cleanup.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GVrKvkMe4xDmkRAksacBzk


Generated by Claude Code

Square, File and Rank each hand-wrote unsafe { transmute } to convert a
u8 back into the enum (~18 call sites). Introduce an index_enum! macro
that generates, for a contiguous #[repr(u8)] enum:

- ALL: the variants indexed by discriminant,
- from_index: the safe checked conversion (None when out of range),
- from_index_unchecked: one audited, debug-asserted unsafe constructor
  wrapping the single remaining transmute.

Cold and fallible sites (FEN/UCI parsing, TryFrom, Display/Debug,
Square::next) now use the safe from_index / ALL and contain no unsafe at
all. The few hot infallible conversions (Square::new/file/rank/
flip_perspective, Bitboard::as_square and the bitboard iterator) call
from_index_unchecked with a SAFETY note, keeping them zero-cost.

This shrinks 18 scattered transmutes to a single one inside the macro.
Movegen benchmark is unchanged from baseline (no regression) and the full
perft suite still passes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GVrKvkMe4xDmkRAksacBzk
@kirillbobyrev
kirillbobyrev marked this pull request as ready for review July 21, 2026 21:53
@kirillbobyrev
kirillbobyrev merged commit 3efd3ce into main Jul 21, 2026
13 checks passed
@kirillbobyrev
kirillbobyrev deleted the claude/codebase-review-0hfehw branch July 21, 2026 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants