Skip to content

Commit bb2dee3

Browse files
committed
Document HashMap iteration-order invariant, improve SizeCache panic message
Review follow-up: - map_compute_size_stmt uses .values() when key size is constant, while map_write_to_stmt always uses for (k, v). The cache-slot order between these must match. For HashMap (both std and hashbrown) it does — both walk the table in slot order — but this is not obvious from a cold read. Added a comment at the optimization site. - next_size() previously panicked with a generic index-out-of-bounds message. Now reports cursor position vs slot count, with #[track_caller] so the panic location points at the caller. Helps manual Message implementers diagnose traversal-order mismatches. - Dropped model: opus from the rust-code-reviewer agent definition. That model was emitting text-based pseudo-tool-call syntax the harness does not parse (tool_uses: 0 on every run), so all findings were confabulated from nonexistent file content. Inherits the main loop model now.
1 parent b1ff00b commit bb2dee3

File tree

3 files changed

+15
-2
lines changed

3 files changed

+15
-2
lines changed

.claude/agents/rust-code-reviewer.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
name: rust-code-reviewer
33
description: Expert Rust code reviewer. Reviews Rust code for quality, safety, idioms, performance, maintainability, readability, and API ergonomics.
44
tools: Read, Glob, Grep
5-
model: opus
65
---
76

87
You are an expert Rust code reviewer with deep knowledge of:

buffa-codegen/src/impl_message.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2483,6 +2483,12 @@ fn map_compute_size_stmt(
24832483
let val_const = map_element_size_is_constant(val_ty);
24842484
// Iterate only the side(s) whose size varies: avoids unused-variable
24852485
// and clippy::for_kv_map lints for bool/fixed* map key or value types.
2486+
//
2487+
// The (true, false) arm uses `.values()` here while `map_write_to_stmt`
2488+
// uses `for (k, v) in &map`. Cache slot order must match between the two
2489+
// passes. For HashMap (both std and hashbrown), `.values()` and `.iter()`
2490+
// walk the same table slots in the same order — identical sequence for an
2491+
// unmodified instance, which `&self` guarantees across both passes.
24862492
Ok(match (key_const, val_const) {
24872493
(true, true) => quote! {
24882494
{

buffa/src/size_cache.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,16 @@ impl SizeCache {
9292
/// generated code this indicates a codegen bug; for manual `Message`
9393
/// implementations it indicates a traversal-order mismatch.
9494
#[inline]
95+
#[track_caller]
9596
pub fn next_size(&mut self) -> u32 {
96-
let size = self.sizes[self.cursor];
97+
let size = *self.sizes.get(self.cursor).unwrap_or_else(|| {
98+
panic!(
99+
"SizeCache cursor overrun: write_to consumed {} slots but \
100+
compute_size produced {} (traversal-order mismatch)",
101+
self.cursor + 1,
102+
self.sizes.len()
103+
)
104+
});
97105
self.cursor += 1;
98106
size
99107
}

0 commit comments

Comments
 (0)