docs: rewrite the long-lived-container rule as a self-contained principle
Previous version leaned on chess-specific terminology (GlyphCache, render, frame arena) and made the rule read like a project memo. Replaced with a generic `LongLived` example, a two-question test for when to apply, and no incident-specific narrative. The "field name is by convention" line removes the implicit prescription of `parent_allocator` so projects can follow their own naming. Also drops the explicit cross-reference list of existing examples — those already drift with the code; the principle is enough to recognise the shape when it appears.
This commit is contained in:
100
CLAUDE.md
100
CLAUDE.md
@@ -220,76 +220,74 @@ 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:
|
||||
A struct's lifetime can outlast its caller's current `context.allocator`.
|
||||
When that happens, any internal allocation made via `context.allocator`
|
||||
(directly, or via a `List.append(item)` that uses the default) binds to
|
||||
a *transient* allocator, and dies the moment that transient scope is
|
||||
torn down — even though the owning struct is still alive and reachable.
|
||||
|
||||
❌ **Forbidden:** the implicit-context capture in any struct whose
|
||||
lifetime crosses a `push Context { ... }` boundary:
|
||||
|
||||
```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);
|
||||
LongLived :: struct {
|
||||
items: List(Entry);
|
||||
|
||||
add :: (self: *LongLived, e: Entry) {
|
||||
// BAD — `items` grows through whichever allocator happens to
|
||||
// be current at this call. If the caller is inside a transient
|
||||
// `push Context { allocator = ... }`, the new backing lives in
|
||||
// that transient allocator. When the push scope ends, the
|
||||
// backing is freed/reset, but `items.items` still points at it.
|
||||
self.items.append(e);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
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.
|
||||
The same trap applies to direct `context.allocator.alloc(...)` /
|
||||
`.dealloc(...)` calls inside such structs.
|
||||
|
||||
✅ **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:
|
||||
✅ **Required:** capture the owning allocator at construction time
|
||||
and forward it explicitly to every internal growth point. The
|
||||
container's API supports this directly — `List(T)`'s mutations take
|
||||
an optional trailing `alloc: Allocator = context.allocator`:
|
||||
|
||||
```sx
|
||||
GlyphCache :: struct {
|
||||
entries: List(GlyphEntry);
|
||||
parent_allocator: Allocator;
|
||||
LongLived :: struct {
|
||||
items: List(Entry);
|
||||
own_allocator: Allocator;
|
||||
|
||||
init :: (self: *GlyphCache, ...) {
|
||||
self.parent_allocator = context.allocator; // libc / GPA at init
|
||||
// ...
|
||||
init :: (self: *LongLived) {
|
||||
// Snapshot whatever allocator is in scope at construction.
|
||||
// That same allocator must outlive this struct.
|
||||
self.own_allocator = context.allocator;
|
||||
}
|
||||
|
||||
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);
|
||||
add :: (self: *LongLived, e: Entry) {
|
||||
self.items.append(e, self.own_allocator);
|
||||
}
|
||||
|
||||
// Direct allocs too:
|
||||
grow_buf :: (self: *LongLived, n: s64) {
|
||||
self.buf = self.own_allocator.alloc(n);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
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.
|
||||
Two-question test for whether a struct needs this pattern:
|
||||
|
||||
The same applies to direct `context.allocator.alloc(...)` /
|
||||
`.dealloc(...)` inside such structs — replace with
|
||||
`self.parent_allocator.alloc(...)` / `.dealloc(...)`.
|
||||
1. Can a caller's `push Context { ... }` wrap a method on this struct?
|
||||
2. Does any method allocate (directly, or by triggering List growth)?
|
||||
|
||||
Existing examples of this pattern (use as templates):
|
||||
If both yes, capture the owning allocator at init. Field name is by
|
||||
convention (`parent_allocator`, `owner`, `own_allocator` — pick the
|
||||
project's existing one and follow it).
|
||||
|
||||
- `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.
|
||||
Sibling case (do NOT migrate): a container whose backing is
|
||||
*intentionally* tied to the caller's scope — typically a per-scope
|
||||
scratch buffer that is reset/zeroed at the top of every scope. Those
|
||||
SHOULD use `context.allocator` so they live and die with the scope.
|
||||
A clear comment at the declaration site is mandatory.
|
||||
|
||||
## On every session start
|
||||
|
||||
|
||||
Reference in New Issue
Block a user