mem: List(T) mutations gain optional alloc: Allocator = context.allocator
The chess panel-text regression (text vanished after the first move on macOS) had a single root cause: GlyphCache's entries List, hash table, and shaped_buf grew through `context.allocator` — which during render is the per-frame arena. On the next arena reset the backing died, and subsequent glyph lookups read garbage / wrote into freshly-allocated view-tree memory. Fix is shaped as the user proposed: `List(T)`'s mutations take an optional trailing `alloc: Allocator = context.allocator` argument. No allocator stored on the container, no init ceremony, every existing `list.append(item)` callsite keeps working unchanged. Long-lived owners now write `list.append(item, self.parent_allocator)` and the arena-leak bug becomes impossible to write accidentally. Default-arg substitution previously only fired for identifier callees (`expandCallDefaults` at lower.zig:7978). Extended to the generic struct-method dispatch path (`list.append(...)` lands here) via a new `appendDefaultArgs` helper that lowers fd.params[i].default_expr in the caller's scope and appends to the lowered args slice. Long-lived owners updated to capture `parent_allocator: Allocator` at init and use it for every internal growth: - GlyphCache (the chess bug) — entries, shaped_buf, hash_keys, hash_vals, atlas bitmap. - DockInteraction — drops the existing `push Context` workaround in `ensure_capacity` for the explicit-arg form. - StateStore — entries list + per-entry data buffer. - Gles3Gpu, MetalGPU — shaders, buffers, textures (atlas-grow during render would otherwise leak resources into the frame arena). Also kept: an operator-precedence fix in pipeline.sx (`(self.frame_index & 1) == 0` instead of `self.frame_index & 1 == 0`, which parses as `self.frame_index & (1 == 0)` = always 0). That was a stealth single-arena-only bug that masked the GlyphCache one for a long time. Docs: - specs.md §11 documents `param: T = expr` default parameter values. The parser already supported it — formalised in the spec now. - current/CHECKPOINT-MEM.md logs the change. - CLAUDE.md REJECTED PATTERNS gains a "Long-lived containers growing through context.allocator" section with the `parent_allocator` capture template and the list of existing examples to mirror. 155/155 example tests pass — zero-diff against snapshots since every existing callsite still resolves to `context.allocator`.
This commit is contained in:
73
CLAUDE.md
73
CLAUDE.md
@@ -218,6 +218,79 @@ history. If an existing allocator type still uses the old `create`
|
||||
pattern, migrate it OR ask the user — never propagate the pattern
|
||||
in new code, docstrings, examples, or tests.
|
||||
|
||||
### Long-lived containers growing through `context.allocator`
|
||||
|
||||
❌ **Forbidden:** a struct that **outlives any single
|
||||
`push Context { ... }` scope** (caches, persistent UI state, GPU
|
||||
resource tables, anything-accessed-across-frame-boundaries) appending
|
||||
to or growing an internal List/hash/buffer using whatever
|
||||
`context.allocator` happens to be at the call site:
|
||||
|
||||
```sx
|
||||
GlyphCache :: struct {
|
||||
entries: List(GlyphEntry);
|
||||
// ...
|
||||
rasterize :: (self: *GlyphCache, ...) {
|
||||
// BAD — during render, context.allocator is a per-frame arena,
|
||||
// so the entries List backing dies on the next arena reset.
|
||||
self.entries.append(entry);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
The chess panel-text bug (text vanished after the first move) was
|
||||
exactly this shape: `GlyphCache.entries`, `hash_keys`, `hash_vals`,
|
||||
and `shaped_buf` all grew through the per-frame arena.
|
||||
|
||||
✅ **Required:** capture the long-lived allocator at init time on a
|
||||
`parent_allocator: Allocator` field, and forward it explicitly to
|
||||
every internal growth point. `List(T)` mutations take an optional
|
||||
trailing `alloc: Allocator = context.allocator`, so the call site
|
||||
just names the owner:
|
||||
|
||||
```sx
|
||||
GlyphCache :: struct {
|
||||
entries: List(GlyphEntry);
|
||||
parent_allocator: Allocator;
|
||||
|
||||
init :: (self: *GlyphCache, ...) {
|
||||
self.parent_allocator = context.allocator; // libc / GPA at init
|
||||
// ...
|
||||
}
|
||||
|
||||
rasterize :: (self: *GlyphCache, ...) {
|
||||
// GOOD — entries always grows through the long-lived owner,
|
||||
// regardless of who's pushed what context above us.
|
||||
self.entries.append(entry, self.parent_allocator);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Heuristic for "is this struct long-lived?" — if its `init` is called
|
||||
once at startup (or once per logical instance) and its methods are
|
||||
called from a frame/event/render hot path, it's long-lived. Capture
|
||||
`parent_allocator` and use it for every internal growth call.
|
||||
|
||||
The same applies to direct `context.allocator.alloc(...)` /
|
||||
`.dealloc(...)` inside such structs — replace with
|
||||
`self.parent_allocator.alloc(...)` / `.dealloc(...)`.
|
||||
|
||||
Existing examples of this pattern (use as templates):
|
||||
|
||||
- `library/modules/ui/glyph_cache.sx` — atlas, hash table, entries,
|
||||
shaped_buf.
|
||||
- `library/modules/ui/dock.sx` — DockInteraction's nine per-child
|
||||
Lists.
|
||||
- `library/modules/ui/state.sx` — StateStore.entries.
|
||||
- `library/modules/gpu/gles3.sx`, `library/modules/gpu/metal.sx` —
|
||||
shaders / buffers / textures.
|
||||
- `library/modules/ui/pipeline.sx` — UIPipeline (used for arena
|
||||
parents).
|
||||
|
||||
`RenderTree.nodes` in `pipeline.sx` is the **opposite** case — it's
|
||||
*intentionally* per-frame arena-allocated and gets its `items` field
|
||||
zeroed at the top of `tick_with_body`. Don't migrate that one.
|
||||
|
||||
## On every session start
|
||||
|
||||
Three active workstreams run in parallel — **IR** (the language compiler),
|
||||
|
||||
Reference in New Issue
Block a user