Traced both functions by hand against the runtime/codegen for shr, band, and string.concat; logic is correct and the documented behavior matches reality.

Correctness

int->bits-loop (imath.seq:159) — LSB-first loop, MSB-first accumulator via string.concat (bit-str ++ acc). Tail call is in tail position, same shape as gcd above it, so TCO applies.
int->bits (imath.seq:172) — handles 0 specially since the loop's terminator (n == 0) would otherwise return the empty string for 0. Good.
int->bits-padded-loop (imath.seq:183) — verified the swap / rot / rot shuffle leaves (n>>1) (width-1) result for the recursive call. Also tail-recursive.
int->bits-padded (imath.seq:196) — width <= 0 returns "", which also covers the negative-width case correctly.

Logical-shift claim is accurate

The header comment at imath.seq:33-34 says shr is logical/zero-fill. Confirmed against runtime/src/arithmetic/bitwise.rs:107 ((v as u64).checked_shr) and codegen/specialization/codegen_safe_math.rs:190 (lshr). So -1 int->bits correctly produces 64 ones, and int->bits-padded truncating to the low N bits works for negative inputs too.

(Aside, not this PR's problem: the builtin doc at crates/compiler/src/builtins/arith.rs:109 still describes shr as "arithmetic" — that line is stale relative to actual semantics.)

One suggestion

The "negative ints render as full 64-bit two's-complement pattern" behavior is documented but not exercised by any test. It's the most regression-prone bit because it depends on shr staying logical. Worth pinning down with e.g. -1 4 int->bits-padded "1111" test.assert-eq-str and/or a -1 int->bits length check.

## Review Traced both functions by hand against the runtime/codegen for `shr`, `band`, and `string.concat`; logic is correct and the documented behavior matches reality. **Correctness** - `int->bits-loop` (`imath.seq:159`) — LSB-first loop, MSB-first accumulator via `string.concat (bit-str ++ acc)`. Tail call is in tail position, same shape as `gcd` above it, so TCO applies. - `int->bits` (`imath.seq:172`) — handles `0` specially since the loop's terminator (`n == 0`) would otherwise return the empty string for `0`. Good. - `int->bits-padded-loop` (`imath.seq:183`) — verified the `swap / rot / rot` shuffle leaves `(n>>1) (width-1) result` for the recursive call. Also tail-recursive. - `int->bits-padded` (`imath.seq:196`) — `width <= 0` returns `""`, which also covers the negative-width case correctly. **Logical-shift claim is accurate** The header comment at `imath.seq:33-34` says `shr` is logical/zero-fill. Confirmed against `runtime/src/arithmetic/bitwise.rs:107` (`(v as u64).checked_shr`) and `codegen/specialization/codegen_safe_math.rs:190` (`lshr`). So `-1 int->bits` correctly produces 64 ones, and `int->bits-padded` truncating to the low N bits works for negative inputs too. (Aside, not this PR's problem: the builtin doc at `crates/compiler/src/builtins/arith.rs:109` still describes `shr` as "arithmetic" — that line is stale relative to actual semantics.) **One suggestion** The "negative ints render as full 64-bit two's-complement pattern" behavior is documented but not exercised by any test. It's the most regression-prone bit because it depends on `shr` staying logical. Worth pinning down with e.g. `-1 4 int->bits-padded "1111" test.assert-eq-str` and/or a `-1 int->bits` length check.

navicore added 1 commit

2026-05-02 17:59:04 +00:00

pr review response

CI - Linux / CI - Linux x86_64 (pull_request) Successful in 4m21s

Details

a8c9b58d79

navicore merged commit 7d45d077da into main

2026-05-02 18:19:00 +00:00

navicore deleted branch string-bits

2026-05-02 18:19:01 +00:00

navicore referenced this pull request from a commit

2026-05-02 18:19:01 +00:00