From 79419b99bd2e8d524a63f3ba0f4f0e46bfc9b23a Mon Sep 17 00:00:00 2001 From: agra Date: Mon, 18 May 2026 18:32:55 +0300 Subject: [PATCH] issue-0028: ?Protocol = null sentinel-shaped optional protocols MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protocol structs registered via registerProtocolDecl carry a new is_protocol flag; the ?T paths in sizeOf/typeSizeBytes/toLLVMType recognise it and lay out ?Protocol as the protocol struct itself (ctx == null IS the "none" state), matching how ?Closure / ?*T are sentinel-shaped — no extra storage. Method dispatch on ?Protocol auto-unwraps in lowerCall's field-access path; the unwrap is structurally a no-op so we just rebind obj_ty to the payload type. resolveCallParamTypes extended for optional-protocol receivers so enum-literal args (gpu.create_texture(.r8, ...)) get the right target_type and don't silently collapse to tag=0 : s32 — same issue-0031-class bug closed in Session 66, one type-system layer deeper. Library: UIRenderer / UIPipeline / GlyphCache migrated from the verbose gpu: GPU = ---; has_gpu: bool pattern to gpu: ?GPU = null. set_gpu no longer maintains a parallel bool flag. Bundled: dock.sx threads delta_time as a struct field rather than via a global pointer (cleanup unrelated to issue-0028, committed alongside). Verified: 85/85 regression tests pass; iOS-sim chess + macOS chess both render correctly post-migration. --- examples/98-optional-protocol.sx | 34 +++++++++++++++ examples/issue-0028.sx | 53 ------------------------ library/modules/ui/dock.sx | 10 ++--- library/modules/ui/glyph_cache.sx | 17 ++++---- library/modules/ui/pipeline.sx | 20 ++++----- library/modules/ui/renderer.sx | 19 ++++----- src/ir/emit_llvm.zig | 4 ++ src/ir/lower.zig | 31 +++++++++++++- src/ir/types.zig | 19 ++++++++- tests/expected/98-optional-protocol.exit | 1 + tests/expected/98-optional-protocol.txt | 2 + 11 files changed, 117 insertions(+), 93 deletions(-) create mode 100644 examples/98-optional-protocol.sx delete mode 100644 examples/issue-0028.sx create mode 100644 tests/expected/98-optional-protocol.exit create mode 100644 tests/expected/98-optional-protocol.txt diff --git a/examples/98-optional-protocol.sx b/examples/98-optional-protocol.sx new file mode 100644 index 0000000..85e5253 --- /dev/null +++ b/examples/98-optional-protocol.sx @@ -0,0 +1,34 @@ +// `?Protocol = null` — optional protocol boxes use sentinel-shape +// (ctx == null is the "none" state), so they cost no extra storage +// beyond the protocol's standard 2-pointer layout. Method calls on +// a non-null optional protocol auto-unwrap and dispatch through the +// vtable / inline fn-ptrs as usual. + +#import "modules/std.sx"; + +GPU :: protocol { + ping :: () -> s64; +} + +Impl :: struct {} +impl GPU for Impl { + ping :: (self: *Impl) -> s64 { 42; } +} + +main :: () -> s32 { + g : ?GPU = null; + if g != null { + print("BAD: g not null at start\n"); + } else { + print("g initially null\n"); + } + + g = xx @Impl.{}; + if g != null { + n := g.ping(); + print("after assign: g.ping() = {}\n", n); + } else { + print("BAD: g still null after assign\n"); + } + 0; +} diff --git a/examples/issue-0028.sx b/examples/issue-0028.sx deleted file mode 100644 index 1fe8bc4..0000000 --- a/examples/issue-0028.sx +++ /dev/null @@ -1,53 +0,0 @@ -// issue-0028: Feature — make protocol boxes assignable to an optional -// type so callers can spell "no GPU bound" as `?GPU = null` instead of -// the verbose `T = ---; has_T: bool` pattern. -// -// ── Current pattern (verbose) ───────────────────────────────────────────── -// -// gpu: GPU = ---; -// has_gpu: bool = false; -// ... -// if self.has_gpu { self.gpu.create_shader(...); } -// -// ── Proposed pattern ────────────────────────────────────────────────────── -// -// gpu: ?GPU = null; -// ... -// if self.gpu != null { self.gpu.create_shader(...); } -// -// ── Where the verbose pattern lives today ───────────────────────────────── -// -// library/modules/ui/renderer.sx — UIRenderer.gpu + has_gpu -// library/modules/ui/glyph_cache.sx — GlyphCache.gpu + has_gpu -// library/modules/ui/pipeline.sx — UIPipeline.gpu + has_gpu (+ set_gpu) -// library/modules/platform/uikit.sx — UIKitPlatform.frame_closure + -// has_frame_closure (Closure type, -// same pattern but on a closure) -// -// ── Implementation sketch ───────────────────────────────────────────────── -// -// Protocol boxes are 2-pointer structs ({vtable, ctx} or {ctx, fn_ptrs...} -// depending on the inline-vs-vtable shape — see src/ir/lower.zig -// `buildProtocolValue` ~7800-7869). `?T` for these can use `vtable_ptr == -// null` (or `ctx == null`, depending on layout choice) as the "none" -// sentinel — no extra storage needed. This matches the existing -// optional-closure handling at src/ir/emit_llvm.zig where `?Closure` uses -// `fn_ptr == null` as none. -// -// Approach: -// 1. Extend `?T` type construction to accept T being a protocol type. -// Files: src/ir/types.zig + src/ir/lower.zig (type-resolution). -// 2. Implement `optional_wrap` / `optional_unwrap` / -// `optional_has_value` for protocol-typed payloads in -// src/ir/emit_llvm.zig — model after the closure-optional path. -// 3. Keep the existing `T = ---; has_T: bool` pattern working — the -// new `?T` is additive, not a replacement. Don't churn existing -// files (uikit.sx's frame_closure pattern stays). -// -// ── Syntax constraint ───────────────────────────────────────────────────── -// -// `?T` syntax already exists for primitives + pointers. Extending to -// protocols is a type-system change; no new surface syntax needed. - -#import "modules/std.sx"; -main :: () -> s32 { 0; } diff --git a/library/modules/ui/dock.sx b/library/modules/ui/dock.sx index 6c55ee8..4438b68 100755 --- a/library/modules/ui/dock.sx +++ b/library/modules/ui/dock.sx @@ -457,13 +457,11 @@ impl View for DockPanel { // Dock — dockable container with drag-and-drop zones // ============================================================================= -// Global delta_time pointer — set by main.sx -g_dock_delta_time : *f32 = null; - Dock :: struct { children: List(ViewChild); alignments: List(Alignment); interaction: *DockInteraction; // heap-allocated, shared with DockPanels + delta_time: *f32; // Config background: ?Color; @@ -475,11 +473,12 @@ Dock :: struct { enable_corners: bool; on_dock: ?Closure(s64, DockZone); - make :: (interaction: *DockInteraction) -> Dock { + make :: (interaction: *DockInteraction, delta_time: *f32) -> Dock { d : Dock = ---; d.children = List(ViewChild).{}; d.alignments = List(Alignment).{}; d.interaction = interaction; + d.delta_time = delta_time; d.background = null; d.corner_radius = 0.0; d.hint_size = 40.0; @@ -519,8 +518,7 @@ impl View for Dock { interaction := self.interaction; interaction.ensure_capacity(self.children.len); - // Tick animations (g_dock_delta_time is always set before main loop) - dt : f32 = g_dock_delta_time.*; + dt : f32 = self.delta_time.*; interaction.tick_animations(dt); i : s64 = 0; diff --git a/library/modules/ui/glyph_cache.sx b/library/modules/ui/glyph_cache.sx index feae197..37b7e5a 100755 --- a/library/modules/ui/glyph_cache.sx +++ b/library/modules/ui/glyph_cache.sx @@ -179,20 +179,17 @@ GlyphCache :: struct { last_shape_len: s64; last_shape_size_q: u16; - // GPU protocol backend. When `has_gpu`, atlas creation + dirty uploads - // route through `gpu` instead of raw GL. - gpu: GPU = ---; - has_gpu: bool = false; + // GPU protocol backend. When set, atlas creation + dirty uploads route + // through `gpu` instead of raw GL. + gpu: ?GPU = null; init :: (self: *GlyphCache, path: [:0]u8, default_size: f32) { // Preserve any pre-set GPU dispatch across the zero-out — the // surrounding struct memset would otherwise wipe it. saved_gpu := self.gpu; - saved_has_gpu := self.has_gpu; // Zero out the entire struct first (parent may be uninitialized with = ---) memset(self, 0, size_of(GlyphCache)); self.gpu = saved_gpu; - self.has_gpu = saved_has_gpu; // Load font file file_size : s32 = 0; @@ -264,7 +261,7 @@ GlyphCache :: struct { // update_texture_region — same result as the GL path's glTexImage2D // with the zeroed bitmap, but works whether or not the backend // accepts CPU pixel pointers at create time. - if self.has_gpu { + if self.gpu != null { self.texture_id = self.gpu.create_texture( self.atlas_width, self.atlas_height, .r8, null); self.dirty = true; @@ -435,7 +432,7 @@ GlyphCache :: struct { // the frame's render pass. flush :: (self: *GlyphCache) { if self.dirty == false { return; } - if self.has_gpu { return; } + if self.gpu != null { return; } glBindTexture(GL_TEXTURE_2D, self.texture_id); glPixelStorei(GL_UNPACK_ALIGNMENT, 1); glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, self.atlas_width, self.atlas_height, GL_RED, GL_UNSIGNED_BYTE, self.bitmap); @@ -443,7 +440,7 @@ GlyphCache :: struct { } upload_atlas_to_gpu :: (self: *GlyphCache) { - if self.has_gpu == false { return; } + if self.gpu == null { return; } if self.dirty == false { return; } self.gpu.update_texture_region(self.texture_id, 0, 0, self.atlas_width, self.atlas_height, xx self.bitmap); @@ -503,7 +500,7 @@ GlyphCache :: struct { self.atlas_height = new_h; // Recreate atlas at the new size. - if self.has_gpu { + if self.gpu != null { // No destroy_texture in the GPU protocol yet — old atlas // leaks in the backend table until process exit. Atlas grow // is rare so this is acceptable for now. diff --git a/library/modules/ui/pipeline.sx b/library/modules/ui/pipeline.sx index fdea550..88fc5e8 100755 --- a/library/modules/ui/pipeline.sx +++ b/library/modules/ui/pipeline.sx @@ -25,21 +25,17 @@ UIPipeline :: struct { has_body: bool; parent_allocator: Allocator; - // GPU protocol backend. When `has_gpu`, the pipeline propagates this - // to its renderer + font, and skips the per-frame GL state setup in + // GPU protocol backend. When set, the pipeline propagates this to its + // renderer + font, and skips the per-frame GL state setup in // commit_gpu (Metal bakes blend mode into the pipeline state). - gpu: GPU = ---; - has_gpu: bool = false; + gpu: ?GPU = null; // Set the GPU dispatch BEFORE calling init() / init_font() so the // shaders + atlas land on the right backend. set_gpu :: (self: *UIPipeline, gpu: GPU) { - self.gpu = gpu; - self.has_gpu = true; - self.renderer.gpu = gpu; - self.renderer.has_gpu = true; - self.font.gpu = gpu; - self.font.has_gpu = true; + self.gpu = xx gpu; + self.renderer.gpu = xx gpu; + self.font.gpu = xx gpu; } init :: (self: *UIPipeline, width: f32, height: f32) { @@ -167,7 +163,7 @@ UIPipeline :: struct { } commit_gpu :: (self: *UIPipeline) { - if !self.has_gpu { + if self.gpu == null { glEnable(GL_BLEND); glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); glDisable(GL_DEPTH_TEST); @@ -182,7 +178,7 @@ UIPipeline :: struct { self.font.upload_atlas_to_gpu(); self.renderer.flush(); - if !self.has_gpu { + if self.gpu == null { glDisable(GL_BLEND); } } diff --git a/library/modules/ui/renderer.sx b/library/modules/ui/renderer.sx index f098e80..5ee1921 100755 --- a/library/modules/ui/renderer.sx +++ b/library/modules/ui/renderer.sx @@ -32,11 +32,10 @@ UIRenderer :: struct { current_texture: u32; draw_calls: s64; - // GPU protocol backend. When `has_gpu`, the renderer routes shader / - // buffer / texture / draw calls through `gpu` instead of raw GL. The - // chess game sets this on iOS to a boxed `*MetalGPU`. - gpu: GPU = ---; - has_gpu: bool = false; + // GPU protocol backend. When set, the renderer routes shader / buffer / + // texture / draw calls through `gpu` instead of raw GL. The chess game + // sets this on iOS to a boxed `*MetalGPU`. + gpu: ?GPU = null; mtl_shader: ShaderHandle = 0; mtl_vbuf: BufferHandle = 0; // Per-frame byte offset into the Metal vertex buffer. Each flush writes @@ -56,7 +55,7 @@ UIRenderer :: struct { self.vertex_count = 0; self.dpi_scale = 1.0; - if self.has_gpu { + if self.gpu != null { // ── Metal backend (via GPU protocol) ─────────────────────── // Oversize the GPU buffer enough to hold many sub-batches per // frame without wrapping. With per-flush offset advance, each @@ -115,7 +114,7 @@ UIRenderer :: struct { proj := Mat4.ortho(0.0, width, height, 0.0, -1.0, 1.0); - if self.has_gpu { + if self.gpu != null { // Reset the per-frame ring offset; this frame's flushes start at 0. self.mtl_buf_offset = 0; // Pipeline state + vertex buffer + projection + initial texture. @@ -251,7 +250,7 @@ UIRenderer :: struct { case .clip_push: { self.flush(); dpi := self.dpi_scale; - if self.has_gpu { + if self.gpu != null { // Metal: pixel coords, top-left origin (no Y flip). self.gpu.set_scissor( xx (node.frame.origin.x * dpi), @@ -272,7 +271,7 @@ UIRenderer :: struct { } case .clip_pop: { self.flush(); - if self.has_gpu { + if self.gpu != null { self.gpu.disable_scissor(); } else { glDisable(GL_SCISSOR_TEST); @@ -290,7 +289,7 @@ UIRenderer :: struct { upload_size : s64 = self.vertex_count * UI_VERTEX_BYTES; - if self.has_gpu { + if self.gpu != null { // Mirror the GL path: bind current texture before drawing. // current_texture may have changed since the last flush. self.gpu.set_texture(0, self.current_texture); diff --git a/src/ir/emit_llvm.zig b/src/ir/emit_llvm.zig index 6933f65..b6111b2 100644 --- a/src/ir/emit_llvm.zig +++ b/src/ir/emit_llvm.zig @@ -2705,6 +2705,10 @@ pub const LLVMEmitter = struct { if (child_info == .closure) { return self.getClosureStructType(); } + // ?Protocol → protocol struct (ctx ptr = field 0 is null when none). + if (child_info == .@"struct" and child_info.@"struct".is_protocol) { + return self.toLLVMType(opt.child); + } // ?T → { T, i1 } var field_types: [2]c.LLVMTypeRef = .{ self.toLLVMType(opt.child), diff --git a/src/ir/lower.zig b/src/ir/lower.zig index 0a0047c..3f1d876 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -4216,6 +4216,23 @@ pub const Lowering = struct { return self.emitProtocolDispatch(obj, proto_info, fa.field, args.items, obj_ty); } + // Check if receiver is `?Protocol` — for sentinel-shaped + // optionals (Protocol has ctx as first ptr field, and a + // null ctx is the "none" state) the unwrap is a no-op + // structurally. Treat the optional value as the protocol + // value and dispatch. Calling a method on a null protocol + // is undefined (same as derefing a null pointer); user + // guards with `if x != null` first. + if (!obj_ty.isBuiltin()) { + const opt_info = self.module.types.get(obj_ty); + if (opt_info == .optional) { + const pay_ty = opt_info.optional.child; + if (self.getProtocolInfo(pay_ty)) |proto_info| { + return self.emitProtocolDispatch(obj, proto_info, fa.field, args.items, pay_ty); + } + } + } + var method_args = std.ArrayList(Ref).empty; defer method_args.deinit(self.alloc); method_args.append(self.alloc, obj) catch unreachable; @@ -6680,6 +6697,18 @@ pub const Lowering = struct { if (std.mem.eql(u8, m.name, fa.field)) return m.param_types; } } + // Optional-protocol receiver (`?GPU`): same as above but the + // protocol type sits inside the optional's payload. + if (!obj_ty.isBuiltin()) { + const opt_info = self.module.types.get(obj_ty); + if (opt_info == .optional) { + if (self.getProtocolInfo(opt_info.optional.child)) |proto_info| { + for (proto_info.methods) |m| { + if (std.mem.eql(u8, m.name, fa.field)) return m.param_types; + } + } + } + } // Closure-typed struct field: `c.on(args)` lowers to call_closure on // the field value. Pick up the callee's param types from the closure // type so each arg gets the right target_type during lowering. @@ -7646,7 +7675,7 @@ pub const Lowering = struct { }) catch unreachable; } - const struct_info: types.TypeInfo = .{ .@"struct" = .{ .name = name_id, .fields = fields.items } }; + const struct_info: types.TypeInfo = .{ .@"struct" = .{ .name = name_id, .fields = fields.items, .is_protocol = true } }; const id = if (table.findByName(name_id)) |existing| existing else table.intern(struct_info); table.update(id, struct_info); diff --git a/src/ir/types.zig b/src/ir/types.zig index d8a56c9..c696905 100644 --- a/src/ir/types.zig +++ b/src/ir/types.zig @@ -76,6 +76,12 @@ pub const TypeInfo = union(enum) { pub const StructInfo = struct { name: StringId, fields: []const Field, + // True iff this struct backs a protocol value (registered via + // `registerProtocolDecl`). Used by the optional code path: a + // `?Protocol` is sentinel-shaped (the protocol struct itself, + // null ctx == none) rather than the standard `{T, i1}` discriminated + // layout — matching how `?Closure` works. + is_protocol: bool = false, pub const Field = struct { name: StringId, @@ -370,7 +376,16 @@ pub const TypeTable = struct { .string => 16, // {ptr, len} .pointer, .many_pointer, .function => 8, .closure => 16, // {fn_ptr, env} - .optional => |opt| self.sizeOf(opt.child) + 8, // child + has_value flag (aligned) + .optional => |opt| blk: { + // Sentinel-shaped optionals (pointer/closure/protocol) cost + // no extra storage — null reuses the payload's null state. + const child_info = self.get(opt.child); + if (child_info == .pointer or child_info == .many_pointer or child_info == .function) break :blk 8; + if (child_info == .closure) break :blk 16; + if (child_info == .@"struct" and child_info.@"struct".is_protocol) break :blk self.sizeOf(opt.child); + // Discriminated form: payload + has_value flag (8-aligned). + break :blk self.sizeOf(opt.child) + 8; + }, .slice => 16, // {ptr, len} .array => |arr| arr.length * self.sizeOf(arr.element), .vector => |vec| vec.length * self.sizeOf(vec.element), @@ -445,6 +460,8 @@ pub const TypeTable = struct { break :blk ptr_size; if (child_info == .closure) break :blk 2 * ptr_size; + if (child_info == .@"struct" and child_info.@"struct".is_protocol) + break :blk self.typeSizeBytes(o.child); const cs = self.typeSizeBytes(o.child); const ca = self.typeAlignBytes(o.child); // { T, i1 } — i1 goes right after T, then pad to struct alignment diff --git a/tests/expected/98-optional-protocol.exit b/tests/expected/98-optional-protocol.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/expected/98-optional-protocol.exit @@ -0,0 +1 @@ +0 diff --git a/tests/expected/98-optional-protocol.txt b/tests/expected/98-optional-protocol.txt new file mode 100644 index 0000000..6fcd5c9 --- /dev/null +++ b/tests/expected/98-optional-protocol.txt @@ -0,0 +1,2 @@ +g initially null +after assign: g.ping() = 42