From 72593db95382508cf65cbcc972ba83e924689b44 Mon Sep 17 00:00:00 2001 From: agra Date: Mon, 25 May 2026 14:41:17 +0300 Subject: [PATCH] mem: List(T) mutations gain optional `alloc: Allocator = context.allocator` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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`. --- CLAUDE.md | 73 +++++++++++++++++++++++++++++++ current/CHECKPOINT-MEM.md | 69 +++++++++++++++++++---------- library/modules/gpu/gles3.sx | 13 ++++-- library/modules/gpu/metal.sx | 12 +++-- library/modules/std.sx | 28 ++++++++++-- library/modules/ui/dock.sx | 25 +++++------ library/modules/ui/glyph_cache.sx | 34 ++++++++------ library/modules/ui/pipeline.sx | 2 +- library/modules/ui/state.sx | 6 ++- specs.md | 41 ++++++++++++++++- src/ir/lower.zig | 18 ++++++++ 11 files changed, 258 insertions(+), 63 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 32bfafc..39bd31c 100644 --- a/CLAUDE.md +++ b/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), diff --git a/current/CHECKPOINT-MEM.md b/current/CHECKPOINT-MEM.md index 31a08c1..00911d9 100644 --- a/current/CHECKPOINT-MEM.md +++ b/current/CHECKPOINT-MEM.md @@ -5,14 +5,18 @@ Tracking checkpoint for the mem.sx Zig-aligned implementation ## Last completed step -- **Interp silent-arm sweep + typed raw-pointer stores.** Every - `else =>` arm in the interp now bails with a `bailDetail("...")` - reason that surfaces through the host diagnostic as - `op=X/X: `. `inst.Store` carries `val_ty: TypeId` so - comptime raw-pointer stores honour the declared destination width - (no more 8-byte-everywhere assumption). New CLAUDE.md REJECTED - PATTERN forbids silent unimplemented arms going forward. - 154/154 example tests + chess on macOS / iOS sim / Android green. +- **`List(T)` mutations gain an optional `alloc: Allocator = + context.allocator` argument** (plan: + `~/.claude/plans/lets-see-options-for-merry-dijkstra.md`). One + language-level change (function-param defaults already supported) + closes a whole bug class where long-lived containers' growth + silently landed in a per-frame arena. The chess panel-text + regression that triggered this work is fixed end-to-end on macOS. + GlyphCache, DockInteraction, StateStore, Gles3Gpu, and MetalGPU + all carry `parent_allocator` and pass it explicitly to internal + list growth. 155/155 example tests pass — zero-diff against + snapshots since every existing callsite still resolves to + `context.allocator`. ## Current state @@ -146,21 +150,23 @@ chain at comptime in the interp. No remaining shortcut. ## Next step -Phase 1.3 (closure env allocation through context) and Phase 1.4 -(codegen serializer for all interp Value variants) are unblocked. -Phase 1.2 (free / malloc through context) was considered and -**skipped** — `context.allocator.alloc/dealloc` already works -directly; wrapper-only `malloc`/`free` would be lossy renames. +Phase 1.3 (closure env allocation through context) shipped in commit +`8e21cc5`. Phase 1.4 (codegen serializer for all interp Value +variants) remains open. Phase 1.2 (free / malloc through context) was +considered and **skipped** — `context.allocator.alloc/dealloc` +already works directly; wrapper-only `malloc`/`free` would be lossy +renames. -Suggested next move: **Phase 1.3**. Closure trampolines in -[lower.zig:lowerLambda](../src/ir/lower.zig#L5549) call -`.heap_alloc` directly for the env pointer; routing through -`context.allocator.alloc` means closures respect -`push Context.{ allocator = ... }` and get leak-tracked by -`TrackingAllocator`. Contained change. Regression test pattern: -mirror `examples/130-xx-value-routes-through-context-allocator.sx` -with a closure that captures a variable, install a tracker via -`push`, verify the tracker's counter incremented. +The `List(T)` allocator-arg work documented above is **outside the +original MEM plan** but lives in the same problem space (long-lived +container growth silently capturing the wrong allocator). It +generalises the `parent_allocator` capture pattern that +`ChessGameState` / `UIPipeline` already used. + +Suggested next move: verify on iOS sim + Android via +`tools/verify-step.sh` to confirm the GlyphCache fix + Metal/Gles3 +sweep behave on those platforms, then either commit the verify-step +goldens or move to Phase 1.4. ## Phase 0.3 audit findings — chess allocator usage (closed) @@ -186,6 +192,25 @@ Allocator value naturally. ## Log +- **2026-05-25 (latest)** — `List(T)` mutation API gained an optional + trailing `alloc: Allocator = context.allocator` argument + (`library/modules/std.sx`). Default-arg substitution previously + only fired for identifier callees; extended to the generic-method + dispatch path via new `appendDefaultArgs` helper at + `lower.zig:7974-7991`, wired in at `lower.zig:5332`. Long-lived + owners that grew internal Lists during render — `GlyphCache`, + `DockInteraction`, `StateStore`, `Gles3Gpu`, `MetalGPU` — now + capture `parent_allocator: Allocator` at init and forward it to + every internal `.append` / `.alloc` / `.dealloc`. Chess panel-text + regression (text vanished after the first move because GlyphCache + hash + entries grew into the per-frame arena and died on reset) + fixed end-to-end on macOS. specs.md §11 gains a "Default Parameter + Values" subsection documenting the existing capability. Operator- + precedence fix kept in `pipeline.sx` (`(self.frame_index & 1) == 0` + instead of `self.frame_index & 1 == 0`, which was parsing as + `self.frame_index & (1 == 0)` = always 0). All diagnostic logging + added during the bug hunt has been stripped. 155/155 example tests + green. - **2026-05-25 (late)** — Interp silent-arm sweep (`e9df33a`). Every `else =>` arm has a `bailDetail` reason; `.deref` / `.unbox_any` previously silently passed through arbitrary Value diff --git a/library/modules/gpu/gles3.sx b/library/modules/gpu/gles3.sx index ed2a1ea..0863f7b 100644 --- a/library/modules/gpu/gles3.sx +++ b/library/modules/gpu/gles3.sx @@ -65,6 +65,12 @@ Gles3Gpu :: struct { shaders: List(Gles3ShaderSlot) = .{}; buffers: List(u32) = .{}; textures: List(Gles3TextureSlot) = .{}; + + // Captured at init() time so resource creation always grows the cache + // lists through the long-lived allocator, even when a caller (e.g. + // glyph_cache atlas-grow during render) is currently inside a transient + // arena context. + parent_allocator: Allocator = .{}; } // ── GPU impl ─────────────────────────────────────────────────────────── @@ -79,6 +85,7 @@ impl GPU for Gles3Gpu { inline if OS != .android { return false; } self.pixel_w = pixel_w; self.pixel_h = pixel_h; + self.parent_allocator = context.allocator; if !self.gl_loaded { load_gl(@eglGetProcAddress); @@ -133,7 +140,7 @@ impl GPU for Gles3Gpu { proj_loc = glGetUniformLocation(prog, "uProj".ptr), tex_loc = glGetUniformLocation(prog, "uTex".ptr), }; - self.shaders.append(slot); + self.shaders.append(slot, self.parent_allocator); xx self.shaders.len; } @@ -144,7 +151,7 @@ impl GPU for Gles3Gpu { glGenBuffers(1, @b); glBindBuffer(GL_ARRAY_BUFFER, b); glBufferData(GL_ARRAY_BUFFER, xx size_bytes, null, GL_DYNAMIC_DRAW); - self.buffers.append(b); + self.buffers.append(b, self.parent_allocator); xx self.buffers.len; } @@ -198,7 +205,7 @@ impl GPU for Gles3Gpu { glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, xx GL_CLAMP_TO_EDGE); slot : Gles3TextureSlot = .{ tex = t, bytes_per_pixel = bpp }; - self.textures.append(slot); + self.textures.append(slot, self.parent_allocator); xx self.textures.len; } diff --git a/library/modules/gpu/metal.sx b/library/modules/gpu/metal.sx index f3d7304..222a190 100644 --- a/library/modules/gpu/metal.sx +++ b/library/modules/gpu/metal.sx @@ -86,6 +86,11 @@ MetalGPU :: struct { shaders: List(*void) = .{}; // MTLRenderPipelineState* buffers: List(*void) = .{}; // MTLBuffer* textures: List(TextureSlot) = .{}; + + // Captured at init() so resource creation always grows the cache lists + // through the long-lived allocator, even when the caller is currently + // inside a transient arena context (e.g. glyph atlas grow during render). + parent_allocator: Allocator = .{}; } impl GPU for MetalGPU { @@ -101,6 +106,7 @@ impl GPU for MetalGPU { self.pixel_w = pixel_w; self.pixel_h = pixel_h; } + self.parent_allocator = context.allocator; metal_init_ios(self); } @@ -432,7 +438,7 @@ metal_create_shader_ios :: (self: *MetalGPU, src: string) -> u32 { return 0; } - self.shaders.append(state); + self.shaders.append(state, self.parent_allocator); xx self.shaders.len; } @@ -452,7 +458,7 @@ metal_create_buffer_ios :: (self: *MetalGPU, size_bytes: s64) -> u32 { xx size_bytes, 0); if buf == null { return 0; } - self.buffers.append(buf); + self.buffers.append(buf, self.parent_allocator); xx self.buffers.len; } @@ -541,7 +547,7 @@ metal_create_texture_ios :: (self: *MetalGPU, w: s32, h: s32, format: TextureFor if tex == null { return 0; } slot : TextureSlot = .{ tex = tex, bytes_per_pixel = bytes_per_pixel }; - self.textures.append(slot); + self.textures.append(slot, self.parent_allocator); if pixels != null { handle : u32 = xx self.textures.len; diff --git a/library/modules/std.sx b/library/modules/std.sx index 0a9fe65..a172de0 100644 --- a/library/modules/std.sx +++ b/library/modules/std.sx @@ -408,13 +408,13 @@ List :: struct ($T: Type) { len: s64 = 0; cap: s64 = 0; - append :: (list: *List(T), item: T) { + append :: (list: *List(T), item: T, alloc: Allocator = context.allocator) { if list.len >= list.cap { new_cap := if list.cap == 0 then 4 else list.cap * 2; - new_items : [*]T = xx context.allocator.alloc(new_cap * size_of(T)); + new_items : [*]T = xx alloc.alloc(new_cap * size_of(T)); if list.len > 0 { memcpy(new_items, list.items, list.len * size_of(T)); - context.allocator.dealloc(list.items); + alloc.dealloc(list.items); } list.items = new_items; list.cap = new_cap; @@ -422,4 +422,26 @@ List :: struct ($T: Type) { list.items[list.len] = item; list.len += 1; } + + ensure_capacity :: (list: *List(T), n: s64, alloc: Allocator = context.allocator) { + if list.cap >= n { return; } + new_cap := if list.cap == 0 then 4 else list.cap; + while new_cap < n { new_cap = new_cap * 2; } + new_items : [*]T = xx alloc.alloc(new_cap * size_of(T)); + if list.len > 0 { + memcpy(new_items, list.items, list.len * size_of(T)); + alloc.dealloc(list.items); + } + list.items = new_items; + list.cap = new_cap; + } + + deinit :: (list: *List(T), alloc: Allocator = context.allocator) { + if list.items != null { + alloc.dealloc(list.items); + } + list.items = null; + list.len = 0; + list.cap = 0; + } } \ No newline at end of file diff --git a/library/modules/ui/dock.sx b/library/modules/ui/dock.sx index 4438b68..388a0d3 100755 --- a/library/modules/ui/dock.sx +++ b/library/modules/ui/dock.sx @@ -134,22 +134,19 @@ DockInteraction :: struct { self.header_pressed = List(bool).{}; } - // BLOCKED on issue-0009: should use push instead of manual save/restore ensure_capacity :: (self: *DockInteraction, count: s64) { if self.child_count >= count { return; } - push Context.{ allocator = self.parent_allocator, data = context.data } { - while self.child_count < count { - self.natural_sizes.append(Size.zero()); - self.alignment_overrides.append(ALIGN_CENTER); - self.has_alignment_override.append(false); - self.is_floating.append(false); - self.is_fill.append(false); - self.floating_positions.append(Point.zero()); - self.child_bounds.append(Frame.zero()); - self.anim_sizes.append(Animated(Size).make(Size.zero())); - self.header_pressed.append(false); - self.child_count += 1; - } + while self.child_count < count { + self.natural_sizes.append(Size.zero(), self.parent_allocator); + self.alignment_overrides.append(ALIGN_CENTER, self.parent_allocator); + self.has_alignment_override.append(false, self.parent_allocator); + self.is_floating.append(false, self.parent_allocator); + self.is_fill.append(false, self.parent_allocator); + self.floating_positions.append(Point.zero(), self.parent_allocator); + self.child_bounds.append(Frame.zero(), self.parent_allocator); + self.anim_sizes.append(Animated(Size).make(Size.zero()), self.parent_allocator); + self.header_pressed.append(false, self.parent_allocator); + self.child_count += 1; } } diff --git a/library/modules/ui/glyph_cache.sx b/library/modules/ui/glyph_cache.sx index ef9bf8d..2432b5d 100755 --- a/library/modules/ui/glyph_cache.sx +++ b/library/modules/ui/glyph_cache.sx @@ -179,6 +179,11 @@ GlyphCache :: struct { last_shape_len: s64; last_shape_size_q: u16; + // Allocator that owns every dynamically-grown buffer on this cache — + // entries list, hash table, shaped_buf. Captured at init time so growth + // never accidentally lands in a transient per-frame arena. + parent_allocator: Allocator; + // GPU protocol backend. When set, atlas creation + dirty uploads route // through `gpu` instead of raw GL. gpu: ?GPU = null; @@ -190,6 +195,7 @@ GlyphCache :: struct { // Zero out the entire struct first (parent may be uninitialized with = ---) memset(self, 0, size_of(GlyphCache)); self.gpu = saved_gpu; + self.parent_allocator = context.allocator; // Load font file file_size : s32 = 0; @@ -204,7 +210,7 @@ GlyphCache :: struct { self.font_data_size = file_size; // Init stbtt_fontinfo - self.font_info = context.allocator.alloc(FONTINFO_SIZE); + self.font_info = self.parent_allocator.alloc(FONTINFO_SIZE); memset(self.font_info, 0, FONTINFO_SIZE); stbtt_InitFont(self.font_info, font_data, 0); @@ -235,7 +241,7 @@ GlyphCache :: struct { self.atlas_width = GLYPH_ATLAS_W; self.atlas_height = GLYPH_ATLAS_H; bitmap_size : s64 = xx self.atlas_width * xx self.atlas_height; - self.bitmap = xx context.allocator.alloc(bitmap_size); + self.bitmap = xx self.parent_allocator.alloc(bitmap_size); memset(self.bitmap, 0, bitmap_size); // Shelf packer init @@ -251,10 +257,10 @@ GlyphCache :: struct { // Init hash table (256 slots) self.hash_cap = 256; hash_bytes : s64 = self.hash_cap * 4; // u32 per slot - self.hash_keys = xx context.allocator.alloc(hash_bytes); + self.hash_keys = xx self.parent_allocator.alloc(hash_bytes); memset(self.hash_keys, 0, hash_bytes); val_bytes : s64 = self.hash_cap * 8; // s64 per slot (s32 would suffice but alignment) - self.hash_vals = xx context.allocator.alloc(val_bytes); + self.hash_vals = xx self.parent_allocator.alloc(val_bytes); // Create the atlas texture. In GPU-protocol mode we create empty and // let the first `flush()` push the (zero-initialized) bitmap via @@ -328,7 +334,7 @@ GlyphCache :: struct { advance = advance } }; - self.entries.append(entry); + self.entries.append(entry, self.parent_allocator); self.hash_insert(key, self.entries.len - 1); return @self.entries.items[self.entries.len - 1].glyph; } @@ -371,7 +377,7 @@ GlyphCache :: struct { advance = advance } }; - self.entries.append(entry); + self.entries.append(entry, self.parent_allocator); self.hash_insert(key, self.entries.len - 1); return @self.entries.items[self.entries.len - 1].glyph; } @@ -399,10 +405,10 @@ GlyphCache :: struct { self.hash_cap = old_cap * 2; hash_bytes : s64 = self.hash_cap * 4; - self.hash_keys = xx context.allocator.alloc(hash_bytes); + self.hash_keys = xx self.parent_allocator.alloc(hash_bytes); memset(self.hash_keys, 0, hash_bytes); val_bytes : s64 = self.hash_cap * 8; - self.hash_vals = xx context.allocator.alloc(val_bytes); + self.hash_vals = xx self.parent_allocator.alloc(val_bytes); // Rehash mask := self.hash_cap - 1; @@ -420,8 +426,8 @@ GlyphCache :: struct { i += 1; } - context.allocator.dealloc(old_keys); - context.allocator.dealloc(old_vals); + self.parent_allocator.dealloc(old_keys); + self.parent_allocator.dealloc(old_vals); } // Upload dirty atlas to GPU. On the Metal path, defer the upload to @@ -482,7 +488,7 @@ GlyphCache :: struct { new_w := self.atlas_width * 2; new_h := self.atlas_height * 2; new_size : s64 = xx new_w * xx new_h; - new_bitmap : [*]u8 = xx context.allocator.alloc(new_size); + new_bitmap : [*]u8 = xx self.parent_allocator.alloc(new_size); memset(new_bitmap, 0, new_size); // Copy old rows into new bitmap @@ -494,7 +500,7 @@ GlyphCache :: struct { y += 1; } - context.allocator.dealloc(self.bitmap); + self.parent_allocator.dealloc(self.bitmap); self.bitmap = new_bitmap; self.atlas_width = new_w; self.atlas_height = new_h; @@ -599,7 +605,7 @@ GlyphCache :: struct { x = total, y = 0.0, advance = adv - }); + }, self.parent_allocator); total += adv; i += 1; } @@ -632,7 +638,7 @@ GlyphCache :: struct { x = gx, y = gy, advance = adv - }); + }, self.parent_allocator); total += adv; } } diff --git a/library/modules/ui/pipeline.sx b/library/modules/ui/pipeline.sx index be9cc91..4d9414a 100755 --- a/library/modules/ui/pipeline.sx +++ b/library/modules/ui/pipeline.sx @@ -132,7 +132,7 @@ UIPipeline :: struct { } tick_with_body :: (self: *UIPipeline) { - build_arena : *Arena = if self.frame_index & 1 == 0 then self.arena_a else self.arena_b; + build_arena : *Arena = if (self.frame_index & 1) == 0 then self.arena_a else self.arena_b; build_arena.reset(); // Reset render_tree nodes (backing is stale after arena reset) diff --git a/library/modules/ui/state.sx b/library/modules/ui/state.sx index 943e49c..1ba381e 100755 --- a/library/modules/ui/state.sx +++ b/library/modules/ui/state.sx @@ -24,10 +24,12 @@ StateEntry :: struct { StateStore :: struct { entries: List(StateEntry); current_generation: s64; + parent_allocator: Allocator; init :: (self: *StateStore) { self.entries = List(StateEntry).{}; self.current_generation = 0; + self.parent_allocator = context.allocator; } get_or_create :: (self: *StateStore, id: s64, $T: Type, default: T) -> State(T) { @@ -42,14 +44,14 @@ StateStore :: struct { } // Create new entry - data : [*]u8 = xx context.allocator.alloc(size_of(T)); + data : [*]u8 = xx self.parent_allocator.alloc(size_of(T)); memcpy(data, @default, size_of(T)); self.entries.append(.{ id = id, data = data, size = size_of(T), generation = self.current_generation - }); + }, self.parent_allocator); State(T).{ ptr = xx data }; } diff --git a/specs.md b/specs.md index a82fd5c..312a525 100644 --- a/specs.md +++ b/specs.md @@ -1061,6 +1061,45 @@ main :: () { } ``` +#### Default Parameter Values + +A parameter can declare a default value with `name: type = expr`. When a +caller omits the trailing positional argument, the compiler substitutes +the default expression at the call site: + +```sx +greet :: (name: string, prefix: string = "Hello") { + print("{} {}!\n", prefix, name); +} + +greet("world"); // prints "Hello world!" +greet("world", "Good morning"); // prints "Good morning world!" +``` + +The default expression is captured as an AST node at parse time and +re-lowered fresh at each call site, so runtime expressions like +`context.allocator` resolve in the **caller's** scope, not the callee's +definition site. This is the mechanism that lets stdlib containers like +`List(T)` expose an optional allocator argument that defaults to +`context.allocator` without requiring callers to thread one through: + +```sx +// In std.sx: +List :: struct ($T: Type) { + append :: (list: *List(T), item: T, alloc: Allocator = context.allocator) { + // ... grows via `alloc.alloc(...)` ... + } +} + +// Call sites: +list.append(42); // alloc = current context.allocator +list.append(42, self.parent_allocator); // alloc = the named long-lived owner +``` + +Defaults are only consulted for **trailing** missing positional args; once +a position is provided, all earlier positions must also be provided. There +is no named-argument syntax for skipping middle defaults. + ### Enum Definition ```sx @@ -2041,7 +2080,7 @@ struct_decl = IDENT '::' 'struct' '{' struct_member* '}' struct_member = field_group | '#using' IDENT ';' field_group = IDENT (',' IDENT)* ':' type ('=' expr)? ';' params = param (',' param)* ','? -param = IDENT ':' type +param = IDENT ':' type ('=' expr)? block = '{' stmt* '}' stmt = decl | assignment ';' | multi_assign ';' | return_stmt | defer_stmt | insert_stmt | push_stmt | break_stmt | continue_stmt | expr ';' diff --git a/src/ir/lower.zig b/src/ir/lower.zig index ccfd32d..380f3bf 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -5329,6 +5329,7 @@ pub const Lowering = struct { const ret_ty = func.ret; const params = func.params; self.fixupMethodReceiver(&method_args, func, effective_obj_node, obj_ty); + self.appendDefaultArgs(fd, &method_args); const final_args = self.prependCtxIfNeeded(func, method_args.items); self.coerceCallArgs(final_args, params); return self.builder.call(fid, final_args, ret_ty); @@ -7971,6 +7972,23 @@ pub const Lowering = struct { return false; } + /// After args have been lowered, append the lowered values of any + /// `param: T = default_expr` defaults for positions past `args.items.len`. + /// Stops at the first param without a default. Used at method-dispatch + /// sites whose callee is a field_access (so `expandCallDefaults` can't + /// handle them up front). The default expression is lowered in the + /// caller's current scope, so identifiers like `context.allocator` + /// resolve to the caller's runtime context. + fn appendDefaultArgs(self: *Lowering, fd: *const ast.FnDecl, args: *std.ArrayList(Ref)) void { + if (args.items.len >= fd.params.len) return; + var i: usize = args.items.len; + while (i < fd.params.len) : (i += 1) { + const dflt = fd.params[i].default_expr orelse break; + const v = self.lowerExpr(dflt); + args.append(self.alloc, v) catch unreachable; + } + } + /// When a bare-identifier call omits trailing positional args and the /// callee's signature provides defaults for them, return a fresh Call /// node with the defaults filled in. Returns null when no expansion is