diff --git a/current/CHECKPOINT-FIBERS.md b/current/CHECKPOINT-FIBERS.md index 0f6d7ebd..f9d8ff0f 100644 --- a/current/CHECKPOINT-FIBERS.md +++ b/current/CHECKPOINT-FIBERS.md @@ -815,3 +815,16 @@ incomplete); a dedicated effort; lambda workers are the idiom meanwhile. runtime is feature-complete end-to-end (1800–1817), all adversarially reviewed. Suite GREEN **755/0**. Five compiler bugs fixed across the stream (0151/0152/0153/0154/0156-P1/0157 — 0151-3 in B1.2). Next carve: Stream B2 (channels / cancel / async stdlib). +- **Post-review hardening (this session) — 6 findings from an adversarial review of the B1 commits.** + Fixed: **P1-a** the UFCS generic PLANNER (`calls.zig`) used the last-wins `fn_ast_map` winner while + lowering reselected by receiver → plan/lowering could disagree and MISBOX the result; now both share + `selectUfcsGenericByReceiver`. **P1-b** the selection scanned `module_decls` globally, flagging a + transitively-hidden same-named overload as a FALSE ambiguity; now two-tier — directly-visible authors + first (ambiguity only among those), global fallback for receiver-reachable namespaced methods (e.g. + `Task.cancel`) that defers to `fd0` on a hidden tie. **P2-b** boolean specificity tied `*$T` with + `*Box($T)`; now peels pointer layers so the structurally-narrower receiver wins. **P1-c** a second + concurrent `Task.wait` overwrote the single waiter slot → silent deadlock; now one-awaiter-per-task + loud abort. **P2-c** `sleep(negative)` rewound the virtual clock; now rejected loudly. (**P2-a** + non-generic-winner-hides-generic did not reproduce — the non-generic arm already falls through.) + Regressions: `examples/generics/0218` (receiver specificity + plan/lowering agreement), + `examples/concurrency/1818` (negative-sleep abort), `1819` (double-wait abort). Suite GREEN **758/0**. diff --git a/examples/concurrency/1818-concurrency-fiber-sleep-negative.sx b/examples/concurrency/1818-concurrency-fiber-sleep-negative.sx new file mode 100644 index 00000000..7e5e89dc --- /dev/null +++ b/examples/concurrency/1818-concurrency-fiber-sleep-negative.sx @@ -0,0 +1,15 @@ +// `sleep(ms)` rejects a NEGATIVE duration loudly — the virtual clock is +// monotonic (advances only as timers fire), so a negative deadline would rewind +// it and break every ordering contract. Regression (B1.4b review, P2-c): the +// guard aborts instead of silently arming a past deadline. +// +// aborts (exit 134) after the diagnostic — aarch64-macOS-pinned. +#import "modules/std.sx"; +sched :: #import "modules/std/sched.sx"; +main :: () -> i64 { + s := sched.Scheduler.init(); ps := @s; + ps.spawn(() => { ps.sleep(10); ps.sleep(-5); }); // -5 → loud abort + s.run(); + print("unreachable\n"); + return 0; +} diff --git a/examples/concurrency/1819-concurrency-fiber-double-wait.sx b/examples/concurrency/1819-concurrency-fiber-double-wait.sx new file mode 100644 index 00000000..5de08867 --- /dev/null +++ b/examples/concurrency/1819-concurrency-fiber-double-wait.sx @@ -0,0 +1,18 @@ +// A `Task` allows ONE awaiter — a second concurrent `wait` on the same pending +// task would overwrite the single `waiter` slot, and completion would wake only +// the second, stranding the first forever. Regression (B1.4a review, P1-c): the +// guard aborts loudly instead of silently deadlocking. +// +// aborts (exit 134) after the diagnostic — aarch64-macOS-pinned. +#import "modules/std.sx"; +sched :: #import "modules/std/sched.sx"; +S :: struct { t: *sched.Task(i64); } +main :: () -> i64 { + st : S = ---; st.t = null; + s := sched.Scheduler.init(); ps := @s; pst := @st; + mkprod :: (ps: *sched.Scheduler, pst: *S) { pst.t = ps.go(() -> i64 => { ps.yield_now(); 42 }); } + mkw :: (ps: *sched.Scheduler, pst: *S) { ps.spawn(() => { x := pst.t.wait() or { -1 }; print("got {}\n", x); }); } + mkprod(ps, pst); mkw(ps, pst); mkw(ps, pst); // second waiter → loud abort + s.run(); + return 0; +} diff --git a/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.build b/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.build new file mode 100644 index 00000000..42e24dd2 --- /dev/null +++ b/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.build @@ -0,0 +1 @@ +{ "target": "macos" } diff --git a/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.exit b/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.exit new file mode 100644 index 00000000..405e2afe --- /dev/null +++ b/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.exit @@ -0,0 +1 @@ +134 diff --git a/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.stderr b/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.stderr new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.stderr @@ -0,0 +1 @@ + diff --git a/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.stdout b/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.stdout new file mode 100644 index 00000000..3476d4cc --- /dev/null +++ b/examples/concurrency/expected/1818-concurrency-fiber-sleep-negative.stdout @@ -0,0 +1 @@ +sched: sleep(-5) — negative duration would rewind the virtual clock diff --git a/examples/concurrency/expected/1819-concurrency-fiber-double-wait.build b/examples/concurrency/expected/1819-concurrency-fiber-double-wait.build new file mode 100644 index 00000000..42e24dd2 --- /dev/null +++ b/examples/concurrency/expected/1819-concurrency-fiber-double-wait.build @@ -0,0 +1 @@ +{ "target": "macos" } diff --git a/examples/concurrency/expected/1819-concurrency-fiber-double-wait.exit b/examples/concurrency/expected/1819-concurrency-fiber-double-wait.exit new file mode 100644 index 00000000..405e2afe --- /dev/null +++ b/examples/concurrency/expected/1819-concurrency-fiber-double-wait.exit @@ -0,0 +1 @@ +134 diff --git a/examples/concurrency/expected/1819-concurrency-fiber-double-wait.stderr b/examples/concurrency/expected/1819-concurrency-fiber-double-wait.stderr new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/concurrency/expected/1819-concurrency-fiber-double-wait.stderr @@ -0,0 +1 @@ + diff --git a/examples/concurrency/expected/1819-concurrency-fiber-double-wait.stdout b/examples/concurrency/expected/1819-concurrency-fiber-double-wait.stdout new file mode 100644 index 00000000..8fb9525a --- /dev/null +++ b/examples/concurrency/expected/1819-concurrency-fiber-double-wait.stdout @@ -0,0 +1 @@ +sched: wait() — task already has a waiter (one awaiter per task in the M:1 model) diff --git a/examples/generics/0218-generics-ufcs-receiver-specificity.sx b/examples/generics/0218-generics-ufcs-receiver-specificity.sx new file mode 100644 index 00000000..5a7fd05a --- /dev/null +++ b/examples/generics/0218-generics-ufcs-receiver-specificity.sx @@ -0,0 +1,28 @@ +// A more-specific UFCS receiver overload must win over a fully-generic one +// (`*Box($T)` beats `*$T`), AND call planning must agree with lowering on which +// overload is dispatched — else the result is typed by one function but produced +// by another, misboxing it. +// +// Regression (issue 0157 review, P1-a + P2-b): the planner (calls.zig) used the +// last-wins `fn_ast_map` winner while lowering reselected by receiver, so a +// `*$T->string` winner could type a call that lowering dispatched to +// `*Box($T)->i64` — boxing the i64 as a string pointer. And boolean specificity +// treated `*$T` and `*Box($T)` as equal (false ambiguity). Fixed by sharing one +// receiver-aware, pointer-peeling selection between planner and lowering. +#import "modules/std.sx"; +#import "0218-generics-ufcs-receiver-specificity/0218-shared.sx"; +#import "0218-generics-ufcs-receiver-specificity/anyref.sx"; + +// More-specific receiver than the imported `pick(x: *$T)`. +pick :: ufcs (b: *Box($T)) -> i64 { return 7; } + +// Generic passthrough — its result type is the PLANNED type of its argument, so +// a planner/lowering disagreement on `pick`'s return type surfaces here. +echo :: (x: $U) -> $U { return x; } + +main :: () -> i64 { + b : Box(i64) = ---; b.v = 0; + r := echo((@b).pick()); // *Box wins → i64 7; plan must agree (else misbox) + print("r: {}\n", r); + return 0; +} diff --git a/examples/generics/0218-generics-ufcs-receiver-specificity/0218-shared.sx b/examples/generics/0218-generics-ufcs-receiver-specificity/0218-shared.sx new file mode 100644 index 00000000..d2ce5542 --- /dev/null +++ b/examples/generics/0218-generics-ufcs-receiver-specificity/0218-shared.sx @@ -0,0 +1 @@ +Box :: struct ($T: Type) { v: T; } diff --git a/examples/generics/0218-generics-ufcs-receiver-specificity/anyref.sx b/examples/generics/0218-generics-ufcs-receiver-specificity/anyref.sx new file mode 100644 index 00000000..619c7f1c --- /dev/null +++ b/examples/generics/0218-generics-ufcs-receiver-specificity/anyref.sx @@ -0,0 +1,5 @@ +// A fully-generic-receiver overload of `pick` (matches ANY pointer receiver), +// returning a DIFFERENT type than the specific one — so a plan/lowering +// disagreement would misbox the result. +#import "0218-shared.sx"; +pick :: ufcs (x: *$T) -> string { return "generic"; } diff --git a/examples/generics/expected/0218-generics-ufcs-receiver-specificity.exit b/examples/generics/expected/0218-generics-ufcs-receiver-specificity.exit new file mode 100644 index 00000000..573541ac --- /dev/null +++ b/examples/generics/expected/0218-generics-ufcs-receiver-specificity.exit @@ -0,0 +1 @@ +0 diff --git a/examples/generics/expected/0218-generics-ufcs-receiver-specificity.stderr b/examples/generics/expected/0218-generics-ufcs-receiver-specificity.stderr new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/generics/expected/0218-generics-ufcs-receiver-specificity.stderr @@ -0,0 +1 @@ + diff --git a/examples/generics/expected/0218-generics-ufcs-receiver-specificity.stdout b/examples/generics/expected/0218-generics-ufcs-receiver-specificity.stdout new file mode 100644 index 00000000..058e0419 --- /dev/null +++ b/examples/generics/expected/0218-generics-ufcs-receiver-specificity.stdout @@ -0,0 +1 @@ +r: 7 diff --git a/library/modules/std/sched.sx b/library/modules/std/sched.sx index 33e6348a..c8780bab 100644 --- a/library/modules/std/sched.sx +++ b/library/modules/std/sched.sx @@ -267,6 +267,15 @@ Scheduler :: struct { print("sched: sleep() called outside a fiber (no running fiber)\n"); abort(); } + // The virtual clock is MONOTONIC — it only advances as timers fire. A + // negative duration would arm a deadline in the past, rewinding the + // clock when it fired and breaking every ordering contract. Reject it + // loudly rather than silently corrupting time. (`sleep(0)` is allowed: a + // same-tick yield to the timer wheel.) + if ms < 0 { + print("sched: sleep({}) — negative duration would rewind the virtual clock\n", ms); + abort(); + } t : Timer = .{ deadline_ms = self.clock_ms + ms, fiber = cur }; // Long-lived-container rule: a timer outlives this `sleep` call's scope // (it survives in `self.timers` until the scheduler fires it), so grow @@ -731,6 +740,16 @@ go :: ufcs (self: *Scheduler, work: Closure() -> $R) -> *Task($R) { wait :: ufcs (t: *Task($R)) -> ($R, !TaskErr) { if t.canceled != 0 { raise error.Canceled; } if t.state == .pending { + // ONE waiter per task (enforced). A `Task` holds a single `waiter` slot; + // a second concurrent `wait` on the same pending task would OVERWRITE the + // first, and completion would wake only the second — the first fiber + // would stay suspended forever (silent deadlock). The M:1 model is + // single-await per task; enforce it loudly (mirrors `block_on_fd`'s + // one-waiter-per-fd guard). A multi-waiter task would need a waiter list. + if t.waiter != null { + print("sched: wait() — task already has a waiter (one awaiter per task in the M:1 model)\n"); + abort(); + } t.waiter = xx t.sched.current; // register self as the waiter t.sched.suspend_self(); // park until the task's fiber wakes us } diff --git a/src/ir/calls.zig b/src/ir/calls.zig index 9a6b249f..3d61c639 100644 --- a/src/ir/calls.zig +++ b/src/ir/calls.zig @@ -316,12 +316,27 @@ pub const CallResolver = struct { // Generic ufcs target: infer the return type with the // RECEIVER prepended so binding positions align with // fd.params[0] (mirrors the lowering side's eff_args). - if (ufcs_fd) |fd| { - if (fd.type_params.len > 0) { + if (ufcs_fd) |fd0p| { + if (fd0p.type_params.len > 0) { const eff_call_args = self.l.alloc.alloc(*ast.Node, c.args.len + 1) catch return .{ .kind = .unresolved, .return_type = .unresolved }; eff_call_args[0] = cfa.object; @memcpy(eff_call_args[1..], c.args); + // RESELECT by receiver — the same selector + `fd0` + // lowering uses — so the PLANNED return type matches the + // function lowering actually dispatches. The last-wins + // `fd0p` can be a wrong-receiver overload (e.g. a + // `*Other($T)->string` winner over the `*Box($T)->i64` + // receiver-match); typing the call by `fd0p` while + // lowering calls the other one misboxes the result + // (issue 0157 review P1). A `*const Node` view of the + // args drives the receiver-aware selection. + const sel_args = self.l.alloc.alloc(*const ast.Node, c.args.len + 1) catch + return .{ .kind = .unresolved, .return_type = .unresolved }; + sel_args[0] = cfa.object; + for (c.args, 0..) |a, i| sel_args[i + 1] = a; + var amb = false; + const fd = self.l.selectUfcsGenericByReceiver(eff_field, sel_args, &amb, fd0p) orelse fd0p; var c2 = c.*; c2.args = eff_call_args; return .{ diff --git a/src/ir/lower/call.zig b/src/ir/lower/call.zig index 4947fd76..ba91e140 100644 --- a/src/ir/lower/call.zig +++ b/src/ir/lower/call.zig @@ -41,16 +41,21 @@ pub fn ufcsGenericBindsAll(self: *Lowering, fd: *const ast.FnDecl, args_ast: []c /// True if `fd`'s receiver param (`params[0]`) is a CONCRETE/structured type /// (`*Task($R)`, `Box($R)`, `*Foo`, `[]T`, …) rather than a BARE type-parameter -/// receiver (`$T` / `T`) that matches ANY receiver. Used to prefer the more -/// receiver-specific overload when several same-named generic ufcs bind. +/// receiver (`$T` / `T` / `*$T`) that matches ANY receiver. Used to prefer the +/// more receiver-specific overload when several same-named generic ufcs bind. +/// POINTER LAYERS ARE PEELED: a receiver's specificity is its core type, not +/// the `*` wrapper — `*Box($T)` (core `Box`, concrete) is strictly more specific +/// than `*$T` (core `$T`, bare), so the structurally-narrower overload wins +/// instead of tying. fn ufcsReceiverConcrete(fd: *const ast.FnDecl) bool { if (fd.params.len == 0) return false; - const te = fd.params[0].type_expr; + var te = fd.params[0].type_expr; + while (te.data == .pointer_type_expr) te = te.data.pointer_type_expr.pointee_type; const bare: ?[]const u8 = switch (te.data) { .comptime_pack_ref => |c| c.pack_name, .identifier => |id| id.name, .type_expr => |t| t.name, - else => null, // pointer / parameterized / array / slice → concrete + else => null, // parameterized (Box($T)) / array / slice → concrete }; if (bare) |nm| { for (fd.type_params) |tp| { @@ -60,26 +65,77 @@ fn ufcsReceiverConcrete(fd: *const ast.FnDecl) bool { return true; } -/// issue 0157: a bare-ufcs name resolves through a single last-wins -/// `fn_ast_map` winner, which may be a same-named generic ufcs whose receiver -/// does NOT match the call's receiver (e.g. a user `cancel :: ufcs (t: -/// *Task($R))` shadowed by the stdlib re-export `cancel :: ufcs (f: -/// *Future($R))`). UFCS dispatch is RECEIVER-driven, so the right candidate may -/// live in a namespaced-imported module that is not flat-visible from the -/// caller file — enumerate ALL module authors of `name` (via `module_decls`) -/// and pick the generic ufcs whose receiver binds ALL its type-params for this -/// call. Called for EVERY generic-ufcs dispatch (not only on bind-failure), so -/// a fully-generic `(x: $T)` last-wins winner can't silently shadow a specific -/// `*Task($R)`. To stay DETERMINISTIC despite the hashmap iteration order (two -/// candidates can both bind): prefer the more receiver-SPECIFIC candidate -/// (concrete > bare-`$T`); dedup re-exports by fd identity; and if two DISTINCT -/// equally-specific authors both bind, set `ambiguous.*` (the caller emits a -/// "qualify the call" diagnostic) rather than silently picking one. Returns null -/// when none bind (a genuine "cannot infer", or the author isn't in -/// `module_decls` — the caller then falls back to the last-wins `fd0` if it -/// binds, else diagnoses; never monomorphizes an `.unresolved` into LLVM). -pub fn selectUfcsGenericByReceiver(self: *Lowering, name: []const u8, args_ast: []const *const Node, ambiguous: *bool) ?*const ast.FnDecl { +/// Rank one candidate `maybe_fd` into the running (best, best_concrete, tie) +/// selection state: skip non-(generic ufcs) and non-binding candidates; a +/// strictly more receiver-specific candidate wins outright; two distinct +/// equally-specific binders set `tie`; a less-specific one is ignored. Re-export +/// aliases (same `*FnDecl` reached twice) are deduped by identity. +fn rankUfcsCand(self: *Lowering, maybe_fd: ?*const ast.FnDecl, args_ast: []const *const Node, best: *?*const ast.FnDecl, best_concrete: *bool, tie: *bool) void { + const fd = maybe_fd orelse return; + if (!(fd.type_params.len > 0 and fd.is_ufcs)) return; + if (!self.ufcsGenericBindsAll(fd, args_ast)) return; + const concrete = ufcsReceiverConcrete(fd); + if (best.*) |b| { + if (b == fd) return; // same decl via a re-export — dedup + if (concrete and !best_concrete.*) { + best.* = fd; + best_concrete.* = true; + tie.* = false; // strictly more specific wins outright + } else if (concrete == best_concrete.*) { + tie.* = true; // two distinct equally-specific binders + } + // else: strictly less specific → ignore + } else { + best.* = fd; + best_concrete.* = concrete; + } +} + +/// issue 0157 + review follow-ups: a bare-ufcs name resolves through a single +/// last-wins `fn_ast_map` winner (`fd0`), which may be a same-named generic ufcs +/// whose receiver does NOT match the call's receiver (e.g. user `cancel :: ufcs +/// (t: *Task($R))` shadowed by the stdlib `cancel :: ufcs (f: *Future($R))`). +/// Pick the most receiver-specific BINDING author, in two tiers so visibility is +/// respected (the non-transitive import model) without losing receiver-reachable +/// namespaced methods: +/// +/// Tier 1 — DIRECTLY-VISIBLE authors (own + one-hop flat, via +/// `collectVisibleAuthors`). A genuine user-facing ambiguity (two distinct +/// equally-specific VISIBLE binders) sets `ambiguous.*` here. +/// Tier 2 (only if no visible author binds) — receiver-reachable methods that +/// aren't flat-visible (a `*Task($R)` method reached through a `sched :: +/// #import` namespace). Scan all module authors for the unique most-specific +/// binder; on a tie among non-visible binders DON'T cry ambiguous — defer to +/// `fd0` (the global last-wins) so a transitively-hidden collision never +/// surfaces as a false error. +/// +/// MUST be called identically from call planning (`calls.zig`) and lowering so +/// the planned result type and the dispatched function never disagree (which +/// would misbox the result). Returns null when nothing binds (the caller falls +/// back to `fd0` if it binds, else diagnoses — never monomorphizes `.unresolved`). +pub fn selectUfcsGenericByReceiver(self: *Lowering, name: []const u8, args_ast: []const *const Node, ambiguous: *bool, fd0: ?*const ast.FnDecl) ?*const ast.FnDecl { ambiguous.* = false; + // Tier 1: directly-visible authors. Ambiguity is a user-facing error only here. + if (self.current_source_file) |caller_file| { + var res = self.resolver(); + const set = res.collectVisibleAuthors(name, caller_file, .user_bare_flat); + defer if (set.flat.len > 0) self.alloc.free(set.flat); + var best: ?*const ast.FnDecl = null; + var best_concrete = false; + var tie = false; + if (set.own) |own| rankUfcsCand(self, Lowering.fnDeclOfRaw(own.raw), args_ast, &best, &best_concrete, &tie); + for (set.flat) |fa| rankUfcsCand(self, Lowering.fnDeclOfRaw(fa.raw), args_ast, &best, &best_concrete, &tie); + if (best) |b| { + if (tie) { + ambiguous.* = true; + return null; + } + return b; + } + } + // Tier 2: receiver-reachable but not flat-visible (namespaced methods defined + // alongside the receiver type). Pick the unique most-specific binder; on a + // hidden tie defer to `fd0` rather than reporting a false ambiguity. const decls = self.program_index.module_decls orelse return null; var best: ?*const ast.FnDecl = null; var best_concrete = false; @@ -87,28 +143,13 @@ pub fn selectUfcsGenericByReceiver(self: *Lowering, name: []const u8, args_ast: var it = decls.iterator(); while (it.next()) |entry| { const ref = entry.value_ptr.names.get(name) orelse continue; - const fd = Lowering.fnDeclOfRaw(ref) orelse continue; - if (!(fd.type_params.len > 0 and fd.is_ufcs)) continue; - if (!self.ufcsGenericBindsAll(fd, args_ast)) continue; - const concrete = ufcsReceiverConcrete(fd); - if (best) |b| { - if (b == fd) continue; // same decl reached via a re-export — dedup - if (concrete and !best_concrete) { - best = fd; - best_concrete = true; - tie = false; // a strictly more specific candidate wins outright - } else if (concrete == best_concrete) { - tie = true; // two distinct equally-specific authors → ambiguous - } - // else: fd is strictly less specific than best → ignore - } else { - best = fd; - best_concrete = concrete; - } + rankUfcsCand(self, Lowering.fnDeclOfRaw(ref), args_ast, &best, &best_concrete, &tie); } if (best == null) return null; if (tie) { - ambiguous.* = true; + if (fd0) |w| { + if (self.ufcsGenericBindsAll(w, args_ast)) return w; + } return null; } return best; @@ -1167,7 +1208,7 @@ pub fn lowerCall(self: *Lowering, c_in: *const ast.Call) Ref { // (never monomorphize an `.unresolved` into LLVM). var fd = fd0; var amb = false; - if (self.selectUfcsGenericByReceiver(eff_field, eff_args.items, &amb)) |sel| { + if (self.selectUfcsGenericByReceiver(eff_field, eff_args.items, &amb, fd0)) |sel| { fd = sel; } else if (amb) { if (self.diagnostics) |d|