● refactor core #516

Merged

navicore merged 1 commit from refactor-core into main

2026-05-25 00:24:25 +00:00

navicore commented

2026-05-25 00:02:06 +00:00

Owner

crates/core/src session recap: all 4 HIGH and both MEDIUM items audited.

Refactored (5): stack.rs (is_inline helper), value.rs
(fmt_variant/fmt_map — depth 6→3), memory_stats.rs (atomic_max helper),
son.rs (child_indent — collapsed the pretty/compact duplication),
tagged_stack.rs (stack_layout — unified the 5 layout sites for
alloc/dealloc soundness).
Reviewed clean, no changes (1): seqstring.rs.

Every step verified green (clippy clean, 80 seq-core / 1140 workspace tests
passing). The checklist at tmp/rust-audit-crates-core-src.md reflects all
of it, with deferred cross-file notes recorded (the
stack/{convert,shuffle}.rs split, pop_two's stale param,
is_tagged_heap/MemorySlot visibility, sharing atomic_max with
scheduler.rs).

Remaining whenever you pick it back up: the 2 LOW files (arena.rs,
error.rs) in core, plus the MEDIUM/LOW/test buckets still open in the
compiler checklist. Cheers!

crates/core/src session recap: all 4 HIGH and both MEDIUM items audited. - Refactored (5): stack.rs (is_inline helper), value.rs (fmt_variant/fmt_map — depth 6→3), memory_stats.rs (atomic_max helper), son.rs (child_indent — collapsed the pretty/compact duplication), tagged_stack.rs (stack_layout — unified the 5 layout sites for alloc/dealloc soundness). - Reviewed clean, no changes (1): seqstring.rs. Every step verified green (clippy clean, 80 seq-core / 1140 workspace tests passing). The checklist at tmp/rust-audit-crates-core-src.md reflects all of it, with deferred cross-file notes recorded (the stack/{convert,shuffle}.rs split, pop_two's stale param, is_tagged_heap/MemorySlot visibility, sharing atomic_max with scheduler.rs). Remaining whenever you pick it back up: the 2 LOW files (arena.rs, error.rs) in core, plus the MEDIUM/LOW/test buckets still open in the compiler checklist. Cheers!

navicore added 1 commit

2026-05-25 00:02:06 +00:00

● refactor core

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

Details

a3610e5b2d

crates/core/src session recap: all 4 HIGH and both MEDIUM items audited.
  - Refactored (5): stack.rs (is_inline helper), value.rs
  (fmt_variant/fmt_map — depth 6→3), memory_stats.rs (atomic_max helper),
  son.rs (child_indent — collapsed the pretty/compact duplication),
  tagged_stack.rs (stack_layout — unified the 5 layout sites for
  alloc/dealloc soundness).
  - Reviewed clean, no changes (1): seqstring.rs.

  Every step verified green (clippy clean, 80 seq-core / 1140 workspace tests
   passing). The checklist at tmp/rust-audit-crates-core-src.md reflects all
  of it, with deferred cross-file notes recorded (the
  stack/{convert,shuffle}.rs split, pop_two's stale param,
  is_tagged_heap/MemorySlot visibility, sharing atomic_max with
  scheduler.rs).

  Remaining whenever you pick it back up: the 2 LOW files (arena.rs,
  error.rs) in core, plus the MEDIUM/LOW/test buckets still open in the
  compiler checklist. Cheers!

navicore commented

2026-05-25 00:24:01 +00:00

Author

Owner

Review — approve, no blockers

Small, tight refactor (+92/−89, 5 files in crates/core/src). Reviewed each diff behavior-by-behavior; all are behavior-preserving and two improve memory-safety hygiene.

tagged_stack.rs — the standout. The 5 independent Layout::array::<StackValue>(cap) sites (alloc in new/clone_stack, realloc old+new in grow, dealloc in Drop) now route through one stack_layout(). This eliminates the class of bug where one site's layout computation drifts from another's → alloc/dealloc UB. Soundness-positive; only the panic message changes (now uniform).
stack.rs — is_inline(sv) consolidates the memory-safety-critical "tagged int or bool, else Arc pointer" predicate used at 3 decode sites (clone_stack_value, drop_stack_value, heap_value_mut). Same upside: the three guards can no longer diverge.
memory_stats.rs — atomic_max extracts the CAS-max loop; identical logic, and the note about sharing it with scheduler.rs's PEAK_STRANDS is correctly left as a deferred cross-crate follow-up.
son.rs — child_indent collapses the pretty/compact duplication across format_variant / format_list / format_map. Traced all four cases per function (pretty/compact × empty/non-empty); the dropped early returns for empty collections are equivalent because the loops don't iterate, and the trailing newline_at_indent(depth) is correctly re-guarded by pretty && !empty. Output byte-identical.
value.rs — fmt_variant / fmt_map extracted from the Display impl (nesting 6→3). fmt_variant trades the two if !empty paren guards for an early return Ok(()) on empty then unconditional parens — same output.

Deferred cross-file notes in the description (stack/{convert,shuffle}.rs split, pop_two's stale param, is_tagged_heap/MemorySlot visibility, sharing atomic_max with scheduler.rs) confirmed not addressed here — correctly out of scope.

Note on process: reviewed by diff inspection against main; did not re-run just ci on this branch locally. The PR's clippy-clean + 80 seq-core / 1140 workspace passing claim is consistent with these provably behavior-preserving changes.

## Review — approve, no blockers Small, tight refactor (+92/−89, 5 files in `crates/core/src`). Reviewed each diff behavior-by-behavior; all are behavior-preserving and two improve memory-safety hygiene. - **`tagged_stack.rs`** — the standout. The 5 independent `Layout::array::<StackValue>(cap)` sites (alloc in `new`/`clone_stack`, realloc old+new in `grow`, dealloc in `Drop`) now route through one `stack_layout()`. This eliminates the class of bug where one site's layout computation drifts from another's → alloc/dealloc UB. Soundness-positive; only the panic message changes (now uniform). - **`stack.rs`** — `is_inline(sv)` consolidates the memory-safety-critical "tagged int or bool, else `Arc` pointer" predicate used at 3 decode sites (`clone_stack_value`, `drop_stack_value`, `heap_value_mut`). Same upside: the three guards can no longer diverge. - **`memory_stats.rs`** — `atomic_max` extracts the CAS-max loop; identical logic, and the note about sharing it with `scheduler.rs`'s `PEAK_STRANDS` is correctly left as a deferred cross-crate follow-up. - **`son.rs`** — `child_indent` collapses the pretty/compact duplication across `format_variant` / `format_list` / `format_map`. Traced all four cases per function (pretty/compact × empty/non-empty); the dropped early `return`s for empty collections are equivalent because the loops don't iterate, and the trailing `newline_at_indent(depth)` is correctly re-guarded by `pretty && !empty`. Output byte-identical. - **`value.rs`** — `fmt_variant` / `fmt_map` extracted from the `Display` impl (nesting 6→3). `fmt_variant` trades the two `if !empty` paren guards for an early `return Ok(())` on empty then unconditional parens — same output. Deferred cross-file notes in the description (`stack/{convert,shuffle}.rs` split, `pop_two`'s stale param, `is_tagged_heap`/`MemorySlot` visibility, sharing `atomic_max` with `scheduler.rs`) confirmed not addressed here — correctly out of scope. Note on process: reviewed by diff inspection against `main`; did not re-run `just ci` on this branch locally. The PR's clippy-clean + 80 seq-core / 1140 workspace passing claim is consistent with these provably behavior-preserving changes.

navicore merged commit ee59451b84 into main

2026-05-25 00:24:25 +00:00

navicore deleted branch refactor-core

2026-05-25 00:24:26 +00:00

navicore referenced this pull request from a commit

2026-05-25 00:24:26 +00:00

Merge pull request '● refactor core' (#516) from refactor-core into main