From b72d49073e5718951198cfcc78865dd2cb7860ec Mon Sep 17 00:00:00 2001 From: agra Date: Tue, 2 Jun 2026 17:57:17 +0300 Subject: [PATCH] fix(ir): diagnose non-constant global initializers loudly (issue 0072) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit globalInitValue's issue-0071 .identifier arm closed the bare-identifier hole, but .field_access (and every other non-literal expression shape) still fell through to `else => null`, so a global like `g : s32 = K.x;` was emitted with no payload and silently zero-initialized (g=0). Make the `else` emit a diagnostic — "global '' must be initialized by a compile-time constant" — instead of a null payload, so no unsupported shape can silently zero. Two arms added alongside: - `.null_literal => .null_val`: a `*void = null` global was previously a no-payload zero-init; this preserves the exact LLVMConstNull emission (fixes 3 ffi examples that regressed on the first cut). - explicit `.enum_literal => null` carve-out: the stdlib's `OS : OperatingSystem = .unknown;` zero-init is load-bearing for compile-time `inline if OS == .X`; documented, not folded into a silent fallthrough. Field-access constant *evaluation* (materializing K.x -> 9) is intentionally not implemented: a typed struct const like K is not registered in module_const_map, so it would require new plumbing whose writes are read at runtime — out of scope. The diagnostic is the issue-sanctioned outcome. Regression: examples/1118-diagnostics-global-non-const-initializer-rejected.sx (exit 1). Gate: zig build, zig build test, run_examples.sh -> 356/0. --- ...s-global-non-const-initializer-rejected.sx | 19 +++ ...global-non-const-initializer-rejected.exit | 1 + ...obal-non-const-initializer-rejected.stderr | 5 + ...obal-non-const-initializer-rejected.stdout | 0 ...ld-access-const-initializer-silent-zero.md | 113 ++++++++++++++++++ src/ir/lower.zig | 19 ++- 6 files changed, 153 insertions(+), 4 deletions(-) create mode 100644 examples/1118-diagnostics-global-non-const-initializer-rejected.sx create mode 100644 examples/expected/1118-diagnostics-global-non-const-initializer-rejected.exit create mode 100644 examples/expected/1118-diagnostics-global-non-const-initializer-rejected.stderr create mode 100644 examples/expected/1118-diagnostics-global-non-const-initializer-rejected.stdout create mode 100644 issues/0072-global-field-access-const-initializer-silent-zero.md diff --git a/examples/1118-diagnostics-global-non-const-initializer-rejected.sx b/examples/1118-diagnostics-global-non-const-initializer-rejected.sx new file mode 100644 index 0000000..a6a240c --- /dev/null +++ b/examples/1118-diagnostics-global-non-const-initializer-rejected.sx @@ -0,0 +1,19 @@ +// A top-level global initialized from a non-constant expression (here a field +// access on a module constant, `K.x`) is rejected with a diagnostic. Without +// the fix `registerTopLevelGlobal`'s init_val serializer handled only literals +// / array / struct literals / identifiers and let every other shape fall through +// to a null payload, so the global silently zero-initialized (`g=0`) — a wrong +// value with no error. +// Regression (issue 0072). +// Expected: "global 'g' must be initialized by a compile-time constant"; exit 1. + +#import "modules/std.sx"; + +Point :: struct { x: s32; y: s32; } +K : Point : Point.{ x = 9, y = 4 }; +g : s32 = K.x; + +main :: () -> s32 { + print("g={}\n", g); + return g; +} diff --git a/examples/expected/1118-diagnostics-global-non-const-initializer-rejected.exit b/examples/expected/1118-diagnostics-global-non-const-initializer-rejected.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/examples/expected/1118-diagnostics-global-non-const-initializer-rejected.exit @@ -0,0 +1 @@ +1 diff --git a/examples/expected/1118-diagnostics-global-non-const-initializer-rejected.stderr b/examples/expected/1118-diagnostics-global-non-const-initializer-rejected.stderr new file mode 100644 index 0000000..e5eed34 --- /dev/null +++ b/examples/expected/1118-diagnostics-global-non-const-initializer-rejected.stderr @@ -0,0 +1,5 @@ +error: global 'g' must be initialized by a compile-time constant + --> examples/1118-diagnostics-global-non-const-initializer-rejected.sx:14:11 + | +14 | g : s32 = K.x; + | ^^^ diff --git a/examples/expected/1118-diagnostics-global-non-const-initializer-rejected.stdout b/examples/expected/1118-diagnostics-global-non-const-initializer-rejected.stdout new file mode 100644 index 0000000..e69de29 diff --git a/issues/0072-global-field-access-const-initializer-silent-zero.md b/issues/0072-global-field-access-const-initializer-silent-zero.md new file mode 100644 index 0000000..9c2af07 --- /dev/null +++ b/issues/0072-global-field-access-const-initializer-silent-zero.md @@ -0,0 +1,113 @@ +# 0072 — global field-access constant initializer silently zero-initializes + +> **RESOLVED.** Root cause: `Lowering.globalInitValue`'s issue-0071 `.identifier` +> arm closed the bare-identifier hole, but `.field_access` (and every other +> non-literal expression shape) still fell through to `else => null`, so the +> global was emitted with no payload and silently zero-initialized (`g=0`). +> Fix: the `else` now emits a diagnostic — "global '' must be initialized +> by a compile-time constant" — instead of returning a null payload, so an +> unsupported initializer shape can never silently zero. Two arms were added +> alongside it: `.null_literal => .null_val` (a `*void = null` global was +> previously a no-payload zero-init; this preserves that exact emission — +> `LLVMConstNull`), and an explicit `.enum_literal => null` carve-out (the +> stdlib's `OS : OperatingSystem = .unknown;` zero-init is load-bearing for +> compile-time `inline if OS == .X`; documented, not folded into a silent +> fallthrough). Field-access constant *evaluation* (materializing `K.x` → 9) was +> intentionally not implemented: a typed struct const like `K` is not registered +> in `module_const_map`, so it would require new plumbing whose `module_const_map` +> writes are read at runtime — out of scope; the diagnostic is the chosen, +> issue-sanctioned outcome. Regression +> `examples/1118-diagnostics-global-non-const-initializer-rejected.sx` (exit 1). + +## Symptom + +A top-level global initialized from a field access on a module constant compiles +but is zero-initialized. + +Observed: `g=0` + +Expected: `g` should be initialized to `9`, or the compiler should reject the +initializer loudly if field-access global constants are not supported yet. + +## Reproduction + +```sx +#import "modules/std.sx"; + +Point :: struct { + x: s32; + y: s32; +} + +K : Point : Point.{ x = 9, y = 4 }; +g : s32 = K.x; + +main :: () -> s32 { + print("g={}\n", g); + return g; +} +``` + +Run: + +```sh +./zig-out/bin/sx run .sx-tmp/review-0071-field-access-const.sx +``` + +The repro is standalone; the inline source above is sufficient to recreate the +scratch file under `.sx-tmp/`. + +## Investigation prompt + +Fix issue 0072: a top-level global initialized from a field access on a module +constant must not silently become zero. + +Context: +- This surfaced during Codex re-review of commit `ad7200c`, the issue-0071 fix. +- `ad7200c` correctly handles identifier initializers that name module constants + (`K : A : 42; g : A = K;`) and diagnoses identifiers that are not usable + constants. +- A remaining non-identifier expression shape still falls through silently: + `K : Point : Point.{ x = 9, y = 4 }; g : s32 = K.x;` emits a null global + initializer payload and runs as `g=0`. + +Suspected area: +- `src/ir/lower.zig`, `Lowering.globalInitValue`. +- The `.identifier` arm now diagnoses unusable identifier initializers, but the + generic `else => null` still covers `.field_access` and other expression + forms. +- `constExprValue` currently handles literals, negative numeric literals, and + array literals; it does not evaluate field access or struct-literal module + constants. `constStructLiteral` can serialize direct struct literals when + called with the destination type, but that machinery is not used for `K.x`. + +Likely fix: +- Add a loud diagnostic for unsupported top-level global initializer expression + shapes, or implement field-access constant evaluation in the same step. +- If implementing it, resolve the base identifier through + `ProgramIndex.module_const_map`, materialize the base constant with its + recorded type, then extract the named field using the IR struct layout. +- Do not replace the current bug with a real-type sentinel such as `.void`; an + unsupported initializer should either produce a concrete `ConstantValue` or a + user-visible diagnostic. +- Preserve the issue-0071 behavior: + `K : A : 42; g : A = K;` must still emit `g=42`. +- Preserve the issue-0070/0068 regressions: + `examples/0133-types-forward-alias-global.sx` and + `examples/1117-diagnostics-value-const-as-type-rejected.sx` must stay green. +- Scope-check enum-literal globals separately. `OS : OperatingSystem = .unknown` + currently relies on compile-time constant injection and pre-existing zero-init + behavior; decide explicitly whether that stays carved out or gets its own + separate issue. + +Verification: +- Repro above should either print `g=9` and exit `9`, or fail with a clear + "global initializer must be a compile-time constant" diagnostic. It must not + print `g=0`. +- Run: + +```sh +zig build +zig build test +bash tests/run_examples.sh +``` diff --git a/src/ir/lower.zig b/src/ir/lower.zig index 772d56b..9e93991 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -1295,6 +1295,7 @@ pub const Lowering = struct { const v = vd.value orelse return null; return switch (v.data) { .undef_literal => .zeroinit, + .null_literal => .null_val, .int_literal => |il| .{ .int = il.value }, .bool_literal => |bl| .{ .boolean = bl.value }, .float_literal => |fl| .{ .float = fl.value }, @@ -1313,10 +1314,20 @@ pub const Lowering = struct { d.addFmt(.err, v.span, "global '{s}' must be initialized by a compile-time constant; '{s}' is not a usable constant here", .{ vd.name, id.name }); break :blk null; }, - // Other initializer shapes (enum-literal shorthand, etc.) keep their - // established static-lowering behavior; this pass only closes the - // identifier/module-const hole (issue 0071). - else => null, + // Enum-literal shorthand globals (`OS : OperatingSystem = .unknown;`) + // keep their established zero-init: it is load-bearing for + // compile-time `inline if OS == .X` in the stdlib (issue 0071 scope + // note). Carved out explicitly — not folded into a silent fallthrough. + .enum_literal => null, + // Any other initializer shape (`.field_access` on a const, a call, an + // arithmetic expression, …) is not a static constant the compiler can + // evaluate here. Diagnose loudly rather than emit a null payload that + // silently zero-initializes the global (issues 0071/0072). + else => blk: { + if (self.diagnostics) |d| + d.addFmt(.err, v.span, "global '{s}' must be initialized by a compile-time constant", .{vd.name}); + break :blk null; + }, }; }