From aca077d72074ab6cf093b78d3fec650921d687d2 Mon Sep 17 00:00:00 2001 From: agra Date: Wed, 3 Jun 2026 16:05:31 +0300 Subject: [PATCH] fix(reflection): replace silent `.s64` arg-type fallback with loud `.unresolved` (issue 0075) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `type_name` / `type_eq` reflection builtins resolved their Type arg's IR type via `getRefIRType(...) orelse TypeId.s64`, then gated `== .any`. A failed must-succeed lookup silently became `.s64` (`!= .any`), classifying a boxed `Any` arg as bare i64 and reading the wrong value with no diagnostic. Add the sibling classifier `LLVMEmitter.reflectArgRepr`, which routes the lookup through `argIRTypeOrFail` (the issue-0074 `.unresolved` resolver) and returns `{ boxed, bare, unresolved }`. The three emit sites in ops.zig (`type_name` + `type_eq` x2) now switch on it: `.boxed` extracts the Any value field, `.bare` uses the value directly, `.unresolved` hits a hard `@panic` tripwire — never silently treated as bare. Real args always resolve, so the happy path is byte-identical (suite stays 361/0, zero snapshot churn). Secondary `lower.zig` `null_literal`/`undef_literal => target_type orelse .void` confirmed intentional (typeless-literal default deliberately handled by emitConstNull/emitConstUndef as null-ptr / undef-i64) — left with an invariant comment, not the `.unresolved` tripwire. Regression test in emit_llvm.test.zig asserts the loud path: fail-before with `orelse .s64` yields `.bare`; pass-after yields `.unresolved`. --- ...75-reflection-builtin-s64-type-fallback.md | 20 ++++++++ src/backend/llvm/ops.zig | 28 +++++----- src/ir/emit_llvm.test.zig | 51 +++++++++++++++++++ src/ir/emit_llvm.zig | 16 ++++++ src/ir/lower.zig | 4 ++ 5 files changed, 106 insertions(+), 13 deletions(-) diff --git a/issues/0075-reflection-builtin-s64-type-fallback.md b/issues/0075-reflection-builtin-s64-type-fallback.md index 8e3c3b7..e8d9818 100644 --- a/issues/0075-reflection-builtin-s64-type-fallback.md +++ b/issues/0075-reflection-builtin-s64-type-fallback.md @@ -1,3 +1,23 @@ +> **RESOLVED** (2026-06-03) +> **Root cause:** the `type_name` / `type_eq` reflection builtins resolved their +> `Type` arg's IR type with `getRefIRType(...) orelse TypeId.s64`, then gated `== .any` +> — so a failed must-succeed lookup silently became "bare i64" (`.s64 != .any`), +> reading the wrong value with no diagnostic. +> **Fix:** added the sibling classifier `LLVMEmitter.reflectArgRepr` +> (`src/ir/emit_llvm.zig`) which routes the lookup through `argIRTypeOrFail` → +> `.unresolved` and returns `{ boxed, bare, unresolved }`. The three emit sites +> (`src/backend/llvm/ops.zig` `type_name` + `type_eq` ×2) now `switch` on it: `.boxed` +> extracts the `Any` value field, `.bare` uses the value directly, and `.unresolved` +> hits a hard `@panic` tripwire — never silently classified as bare. Happy path +> (real args always resolve) is byte-identical; suite stays 361/0. +> **Secondary (confirmed intentional):** `src/ir/lower.zig:2531/2532` +> (`null_literal` / `undef_literal` → `target_type orelse .void`) is a typeless-literal +> default, not a lookup-swallow — `emitConstNull`/`emitConstUndef` deliberately handle +> `.void` (null-ptr / undef-i64). Left in place with an invariant comment. +> **Regression test:** `src/ir/emit_llvm.test.zig` — "emit: reflectArgRepr surfaces +> .unresolved for an unresolvable reflection arg ref (issue 0075)" (fail-before with +> `orelse .s64` → `.bare`; pass-after → `.unresolved`). + # 0075 — silent `getRefIRType(...) orelse TypeId.s64` fallback in reflection builtins ## Symptom diff --git a/src/backend/llvm/ops.zig b/src/backend/llvm/ops.zig index e2f529e..6542735 100644 --- a/src/backend/llvm/ops.zig +++ b/src/backend/llvm/ops.zig @@ -1020,14 +1020,12 @@ pub const Ops = struct { // index directly. const arg_ref = bi.args[0]; const arg_val = self.e.resolveRef(arg_ref); - const arg_ir_ty = self.e.getRefIRType(arg_ref) orelse TypeId.s64; - const tid_idx = blk: { - if (arg_ir_ty == .any) { - // Boxed: extract value field. - break :blk c.LLVMBuildExtractValue(self.e.builder, arg_val, 1, "tn.tid"); - } + const tid_idx = switch (self.e.reflectArgRepr(arg_ref)) { + .unresolved => @panic("type_name: reflection arg IR-type unresolved — a type-resolution failure reached LLVM emission without a diagnostic"), + // Boxed: extract value field from the Any aggregate. + .boxed => c.LLVMBuildExtractValue(self.e.builder, arg_val, 1, "tn.tid"), // Bare i64 (TypeId index). - break :blk arg_val; + .bare => arg_val, }; const arr_global = self.e.reflection().getOrBuildTypeNameArray(); const arr_len = self.e.type_name_array_len; @@ -1046,15 +1044,19 @@ pub const Ops = struct { // icmp eq. const a = blk: { const v = self.e.resolveRef(bi.args[0]); - const ty = self.e.getRefIRType(bi.args[0]) orelse TypeId.s64; - if (ty == .any) break :blk c.LLVMBuildExtractValue(self.e.builder, v, 1, "te.a"); - break :blk v; + break :blk switch (self.e.reflectArgRepr(bi.args[0])) { + .unresolved => @panic("type_eq: first reflection arg IR-type unresolved — a type-resolution failure reached LLVM emission without a diagnostic"), + .boxed => c.LLVMBuildExtractValue(self.e.builder, v, 1, "te.a"), + .bare => v, + }; }; const b = blk: { const v = self.e.resolveRef(bi.args[1]); - const ty = self.e.getRefIRType(bi.args[1]) orelse TypeId.s64; - if (ty == .any) break :blk c.LLVMBuildExtractValue(self.e.builder, v, 1, "te.b"); - break :blk v; + break :blk switch (self.e.reflectArgRepr(bi.args[1])) { + .unresolved => @panic("type_eq: second reflection arg IR-type unresolved — a type-resolution failure reached LLVM emission without a diagnostic"), + .boxed => c.LLVMBuildExtractValue(self.e.builder, v, 1, "te.b"), + .bare => v, + }; }; const eq_res = c.LLVMBuildICmp(self.e.builder, c.LLVMIntEQ, a, b, "te.eq"); self.e.mapRef(eq_res); diff --git a/src/ir/emit_llvm.test.zig b/src/ir/emit_llvm.test.zig index ff20cda..99c4e30 100644 --- a/src/ir/emit_llvm.test.zig +++ b/src/ir/emit_llvm.test.zig @@ -1144,3 +1144,54 @@ test "emit: argIRTypeOrFail surfaces .unresolved for an unresolvable FFI arg ref try std.testing.expectEqual(TypeId.unresolved, emitter.argIRTypeOrFail(bogus)); try std.testing.expect(emitter.argIRTypeOrFail(bogus) != .void); } + +// ── issue 0075: reflection-builtin arg-type lookup must fail loudly, never `.s64` ── +// `reflectArgRepr` backs the `type_name` / `type_eq` reflection builtins, which read +// their `Type` arg as a boxed `Any` aggregate (`.any` → extract value field) or a bare +// i64 TypeId index. A ref it cannot resolve is a codegen invariant violation; it must +// surface `.unresolved` (which the emit site hard-panics on) instead of the old silent +// `getRefIRType(arg) orelse .s64` default that would mis-classify a boxed arg as bare +// and read the wrong value with no diagnostic. +test "emit: reflectArgRepr surfaces .unresolved for an unresolvable reflection arg ref (issue 0075)" { + const alloc = std.testing.allocator; + var module = Module.init(alloc); + defer module.deinit(); + + var b = Builder.init(&module); + + // func reflfn(boxed: any, bare: s64) -> void { } + const fid = b.beginFunction(str(&module, "reflfn"), &[_]Function.Param{ + .{ .name = str(&module, "boxed"), .ty = .any }, + .{ .name = str(&module, "bare"), .ty = .s64 }, + }, .void); + const entry = b.appendBlock(str(&module, "entry"), &.{}); + b.switchToBlock(entry); + b.retVoid(); + b.finalize(); + + var emitter = LLVMEmitter.init(alloc, &module, "test_refl_argty", .{}); + defer emitter.deinit(); + emitter.current_func_idx = fid.index(); + + // Happy path: a boxed `.any` Type arg classifies as `.boxed` (extract value + // field); a bare `.s64` TypeId arg classifies as `.bare` (use directly). + // These decisions are byte-identical to the pre-fix `== .any` gate. + try std.testing.expectEqual(LLVMEmitter.ReflectArgRepr.boxed, emitter.reflectArgRepr(Ref.fromIndex(0))); + try std.testing.expectEqual(LLVMEmitter.ReflectArgRepr.bare, emitter.reflectArgRepr(Ref.fromIndex(1))); + + // A ref past every param and instruction is unresolvable. + const bogus = Ref.fromIndex(100_000); + try std.testing.expectEqual(@as(?TypeId, null), emitter.getRefIRType(bogus)); + + // Fail-before: the old `getRefIRType(arg) orelse .s64` would silently yield + // `.s64` here — which `!= .any`, so the reflection arm would treat a failed + // lookup as a bare i64 and read the wrong value with no diagnostic. + try std.testing.expectEqual(TypeId.s64, emitter.getRefIRType(bogus) orelse TypeId.s64); + try std.testing.expect((emitter.getRefIRType(bogus) orelse TypeId.s64) != .any); + + // Pass-after: the classifier returns the dedicated `.unresolved` variant, + // never `.bare`, so the emit site trips its hard panic instead of silently + // reading the wrong value. + try std.testing.expectEqual(LLVMEmitter.ReflectArgRepr.unresolved, emitter.reflectArgRepr(bogus)); + try std.testing.expect(emitter.reflectArgRepr(bogus) != .bare); +} diff --git a/src/ir/emit_llvm.zig b/src/ir/emit_llvm.zig index 0506f10..183892c 100644 --- a/src/ir/emit_llvm.zig +++ b/src/ir/emit_llvm.zig @@ -2249,6 +2249,22 @@ pub const LLVMEmitter = struct { return self.getRefIRType(arg_ref) orelse .unresolved; } + /// How a reflection builtin (`type_name` / `type_eq`) must read its `Type` + /// argument: boxed inside an `Any` aggregate (extract the value field) vs a + /// bare i64 `TypeId` index. The IR-type lookup is must-succeed, so it routes + /// through `argIRTypeOrFail`; a failed lookup surfaces as `.unresolved` — + /// never a silent `.s64` that would mis-classify a boxed arg as bare and read + /// the wrong value. The caller turns `.unresolved` into a hard tripwire. + pub const ReflectArgRepr = enum { boxed, bare, unresolved }; + + pub fn reflectArgRepr(self: *LLVMEmitter, arg_ref: Ref) ReflectArgRepr { + return switch (self.argIRTypeOrFail(arg_ref)) { + .unresolved => .unresolved, + .any => .boxed, + else => .bare, + }; + } + /// Coerce both binary operands to match the instruction's result type. /// E.g. if result is i64 but one operand is i32, sext it. diff --git a/src/ir/lower.zig b/src/ir/lower.zig index 6a84203..6bd5d98 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -2524,6 +2524,10 @@ pub const Lowering = struct { const sid = self.module.types.internString(str); break :blk self.builder.constString(sid); }, + // A bare `null` / `---` with no surrounding type expectation is a + // legitimate typeless literal, not a failed lookup: `.void` is its + // intentional default (emitConstNull/emitConstUndef handle void as + // null-ptr / undef-i64). Not a candidate for the `.unresolved` tripwire. .null_literal => self.builder.constNull(self.target_type orelse .void), .undef_literal => self.builder.constUndef(self.target_type orelse .void),