From 1f3663739930a3a619631ddd3d69dcb7b2114586 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 14:49:25 +0000 Subject: [PATCH] onebrc/lane-j: give the gridlake carrier a typed error (CodeRabbit #641) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the #641 review: as_gridlake_columns returned Result<_, ()> with a clippy allow, losing which grid failed alignment. Replace with a GridlakeCarrierError::UnalignedGrid { grid } that surfaces the offending grid size, drops the #[allow(clippy::result_unit_err)], and impls Display + std::error::Error. Kept a plain enum rather than a snafu type: onebrc-probe is a workspace-excluded standalone probe whose lanes A/C are dependency-free by design (its Cargo.toml), so a snafu type would drag a new dep into a crate that deliberately carries none — the enum gives the same diagnostic context without that cost. The unaligned-grid test now asserts the variant (grid=72). 2/2 gridlake tests green; lane_j.rs fmt + clippy clean. Not addressed: the STATUS_BOARD "append-only" comment is a false positive — the board's sections are newest-first and this prepended a NEW section without reordering or rewriting any prior entry. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01MLBnPuScZy6w9di2QEjsXM --- crates/onebrc-probe/src/lane_j.rs | 54 +++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/crates/onebrc-probe/src/lane_j.rs b/crates/onebrc-probe/src/lane_j.rs index 147de6cf..1e213d42 100644 --- a/crates/onebrc-probe/src/lane_j.rs +++ b/crates/onebrc-probe/src/lane_j.rs @@ -206,15 +206,46 @@ pub struct GridlakeColumns { pub counts: MultiLaneColumn, } +/// Failure rendering a [`GridBatch`] as gridlake [`MultiLaneColumn`]s. +/// +/// A plain enum rather than a `snafu` type on purpose: `onebrc-probe` is a +/// workspace-excluded standalone probe whose lanes A/C are dependency-free by +/// design (see its `Cargo.toml`) — it carries no `snafu`. This surfaces the +/// same alignment failure `MultiLaneColumn::new` reports, but with the +/// offending `grid` for diagnostics instead of a bare `()`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum GridlakeCarrierError { + /// A column buffer was not 64-byte aligned because `grid` is not a + /// multiple of 16 (i32·16 = i64·8 = u64·8 = 64 B), so + /// `MultiLaneColumn::new` rejected it. + UnalignedGrid { grid: usize }, +} + +impl std::fmt::Display for GridlakeCarrierError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::UnalignedGrid { grid } => write!( + f, + "gridlake carrier: grid {grid} is not a multiple of 16, so a \ + column buffer is not 64-byte aligned for MultiLaneColumn" + ), + } + } +} + +impl std::error::Error for GridlakeCarrierError {} + impl GridBatch { /// Render the four accumulator columns as [`MultiLaneColumn`] gridlake /// carriers (little-endian bytes, zero semantic change — a *reading*, not /// a re-layout). `count` is widened `u32 → u64` to ride the unsigned - /// 64-bit accumulator lane. Returns `Err(())` if a column buffer is not - /// 64-byte aligned (i.e. `grid % 16 != 0`), mirroring - /// `MultiLaneColumn::new`'s own contract. - #[allow(clippy::result_unit_err)] // pass-through of MultiLaneColumn::new's Result<_, ()> alignment contract - pub fn as_gridlake_columns(&self) -> Result { + /// 64-bit accumulator lane. Returns + /// [`GridlakeCarrierError::UnalignedGrid`] if a column buffer is not + /// 64-byte aligned (i.e. `grid % 16 != 0`), surfacing the alignment + /// contract `MultiLaneColumn::new` enforces with the offending grid size. + pub fn as_gridlake_columns(&self) -> Result { + let grid = self.mins.len(); + let unaligned = |_: ()| GridlakeCarrierError::UnalignedGrid { grid }; fn col_i32(v: &[i32]) -> Result { let mut b = Vec::with_capacity(v.len() * 4); for &x in v { @@ -237,10 +268,10 @@ impl GridBatch { MultiLaneColumn::new(Arc::from(b)) } Ok(GridlakeColumns { - mins: col_i32(&self.mins)?, - maxs: col_i32(&self.maxs)?, - sums: col_i64(&self.sums)?, - counts: col_u64_from_u32(&self.counts)?, + mins: col_i32(&self.mins).map_err(unaligned)?, + maxs: col_i32(&self.maxs).map_err(unaligned)?, + sums: col_i64(&self.sums).map_err(unaligned)?, + counts: col_u64_from_u32(&self.counts).map_err(unaligned)?, }) } } @@ -724,7 +755,10 @@ mod tests { #[test] fn gridlake_carrier_rejects_unaligned_grid() { let batch = GridBatch::new(72); // 72 % 16 != 0 → i32 col = 288 B, not 64-mult - assert!(batch.as_gridlake_columns().is_err()); + assert!(matches!( + batch.as_gridlake_columns(), + Err(GridlakeCarrierError::UnalignedGrid { grid: 72 }) + )); } /// Parity across the knob matrix corners: gridlake (4096) and full