mem: remove resolveType(null) → .s64 silent fallback

CLAUDE.md REJECTED PATTERNS forbids silent default returns where the
"reasonable-looking" value happens to match one common case (s64 = 8
bytes = pointer-sized on the host) and is silently wrong everywhere
else. `resolveType(null) → .s64` was exactly this shape: a top-level
`g_pi := 3.14;` was silently typed as `s64`, producing a wrong-typed
slot and the wrong runtime value.

`resolveType` now takes a non-optional `*const Node`. Twelve callers
were classified:

- Six were already guarded by `if (x.type_annotation != null)` blocks
  — the null branch was unreachable. Cleaned up to optional-payload
  syntax (`if (cd.type_annotation) |ta|`) so the always-non-null path
  is obvious from the type.
- Two (`#objc_call` / `#jni_call` return types) pass `FfiIntrinsicCall.
  return_type`, which is `*Node` (not optional) in the AST — the
  silent fallback couldn't be reached there either.
- One (top-level `var_decl` at lower.zig:630) DID legitimately receive
  null when the user omitted both annotation and initializer typing.
  Now mirrors `lowerVarDecl`'s local-scope behavior: explicit
  annotation → resolveType; no annotation → `inferExprType` from the
  initializer; neither → diagnose with a real error message.
- One (`lowerComptimeGlobal`, fixed in commit 82e7b04 alongside
  Phase 1.4) already infers from the comptime expression.
- Two (JNI super-call / JNI method return type) were already
  hand-rolled with `if (rt) |t| resolveType(t) else .void`.

Regression at `examples/137-toplevel-var-type-inference.sx`: `g_count
:= 42;` / `g_pi := 3.14;` / `g_flag := true;` at module scope. Pre-fix
`g_pi` got silently typed as `s64` and printed `0` or garbage; now it
prints `3.140000`. 159/159 example tests + chess clean.
This commit is contained in:
agra
2026-05-25 15:59:32 +03:00
parent 179310d62b
commit 071352e655
5 changed files with 90 additions and 21 deletions

View File

@@ -5,6 +5,27 @@ Tracking checkpoint for the mem.sx Zig-aligned implementation
## Last completed step
- **`resolveType(null) → .s64` silent fallback removed.** `resolveType`
now takes a non-optional `*const Node`; the `null → .s64` branch is
gone. Callers that legitimately had no annotation handle it
themselves: top-level `var_decl` at `lower.zig:630` infers from the
initializer or diagnoses if neither is present (matches the
`lowerVarDecl` pattern that already existed for locals).
`FfiIntrinsicCall.return_type` is non-optional in the AST so its
callers (`#objc_call`, `#jni_call`) didn't actually need the
fallback. JNI super-call and JNI method paths were already guarded
with `if (rt) |t| ... else .void`.
Caller cleanup also dropped the explicit `if (x != null)` guards in
favour of optional-payload syntax (`if (cd.type_annotation) |ta|
...`), which makes the always-non-null path obvious from the type.
Real-world impact: `g_pi := 3.14;` at the top level used to be
silently typed as `s64`. Now it infers as `f64`. Regression at
`examples/137-toplevel-var-type-inference.sx` (count/pi/flag — int /
float / bool inferred correctly). 159/159 example tests + chess
clean.
- **Phase 1.4a — IR `TypeId` threaded through `valueToLLVMConst`;
string/slice fat-pointer aggregates serialize by reading host
memory.** The Phase 1.4 serializer bailed on `heap_ptr` / `byte_ptr`
@@ -267,15 +288,17 @@ Open follow-ups, in roughly the order they make sense:
- **`.int → ptr` heap-walk follow-up.** Phase 1.4a handles the
fat-pointer aggregate case. A `.int` host-address landing in a
bare ptr field (e.g. a struct with a raw `[*]u8` member) still
bails. Requires recursive struct walking with cycle detection on
`(heap_id, type_id)` visited pairs. No practical trigger
in-tree; defer until a real `#run` site surfaces the need.
- **`resolveType(null) -> .s64` audit.** The silent fallback at
`lower.zig:8387` is still in place for every caller other than
`lowerComptimeGlobal`. CLAUDE.md REJECTED PATTERNS forbids this
shape. Survey callers; either make the default an error
diagnostic or thread an inferred type per call site.
bare ptr field (e.g. a struct with `*Inner` or `[*]u8` members)
still bails. Investigated this session — the practical blocker
is interp-level: `struct_gep` / load / store through raw integer
pointers aren't wired (only `index_gep` is), so users can't even
construct most cases. Two-step plan if a real trigger surfaces:
(1) lift the interp's struct_gep+load+store-through-raw-int limit
using `FieldAccess.base_type` for the field offset table; (2) add
recursive heap-walk in `valueToLLVMConst` with cycle detection on
`(addr, type_id)` visited pairs. No practical trigger in-tree —
the canonical buffer/string case is already handled by `[]T` /
`string`.
## Phase 0.3 audit findings — chess allocator usage (closed)
@@ -301,7 +324,18 @@ Allocator value naturally.
## Log
- **2026-05-25 (latest)** — Phase 1.4a shipped. `valueToLLVMConst`
- **2026-05-25 (latest)** — `resolveType(null) → .s64` removed.
Signature changed to non-optional `*const Node`; 12 callers
surveyed and classified. The three unguarded ones — top-level
`var_decl` at `lower.zig:630` (now mirrors lowerVarDecl's
infer-from-initializer pattern), `#objc_call` / `#jni_call` return
types (already non-optional in AST so no actual silent fallback
was reached) — handle null explicitly. The rest were guarded by
`if (x != null)` blocks; cleaned up to optional-payload syntax.
`examples/137-toplevel-var-type-inference.sx` proves the visible
win: `g_pi := 3.14;` at module scope now infers `f64` (used to be
silent `s64`). 159/159 + chess clean.
- **2026-05-25 (penultimate)** — Phase 1.4a shipped. `valueToLLVMConst`
takes IR `TypeId` (not LLVM type) + an interpreter handle.
String/slice fat pointers are serialized by capturing the
pointed-to bytes (via `interp.heapSlice` for heap_ptr, raw

View File

@@ -0,0 +1,21 @@
// Pre-fix: `resolveType(null)` silently returned `.s64`, so a top-level
// var without a type annotation got typed as `s64` regardless of what
// the initializer was. For `g_pi := 3.14;` this meant the float literal
// was assigned to an s64 slot, producing a wrong value at runtime or
// the wrong codegen shape.
//
// After the fix `lowerVarDecl` at the top level mirrors the local-scope
// path: explicit annotation → resolveType; no annotation → infer from
// the initializer's type. Mirrors how `:=` already worked for locals.
#import "modules/std.sx";
g_count := 42; // inferred s64
g_pi := 3.14; // inferred f64 — used to silently become s64
g_flag := true; // inferred bool
main :: () -> s32 {
print("count = {}\n", g_count);
print("pi = {}\n", g_pi);
print("flag = {}\n", g_flag);
return 0;
}

View File

@@ -574,10 +574,10 @@ pub const Lowering = struct {
// comptime_expr handled in Pass 2
// Simple value constants with type annotation (e.g. AF_INET :s32: 2)
if (cd.type_annotation != null) {
if (cd.type_annotation) |ta| {
switch (cd.value.data) {
.int_literal, .float_literal, .bool_literal, .string_literal, .undef_literal, .null_literal => {
const ty = self.resolveType(cd.type_annotation);
const ty = self.resolveType(ta);
self.module_const_map.put(cd.name, .{ .value = cd.value, .ty = ty }) catch {};
},
else => {},
@@ -630,8 +630,19 @@ pub const Lowering = struct {
.var_decl => |vd| {
// Top-level mutable global (e.g., `context : Context = ---;`)
// Use self.resolveType so type aliases like `Handle :: u32;` resolve
// to their target type (not a synthetic empty struct).
const var_ty = self.resolveType(vd.type_annotation);
// to their target type (not a synthetic empty struct). When the
// user omitted the annotation, infer from the initializer
// expression; foreign globals with no annotation are diagnosed
// because their type can't be inferred without an initializer.
const var_ty: TypeId = if (vd.type_annotation) |ta|
self.resolveType(ta)
else if (vd.value) |val|
self.inferExprType(val)
else blk: {
if (self.diagnostics) |d|
d.addFmt(.err, null, "top-level var '{s}' has no type annotation and no initializer to infer from", .{vd.name});
break :blk .void;
};
// Foreign globals reference a symbol defined in libSystem etc.
// (`_NSConcreteStackBlock : *void #foreign;`). The C symbol
// name is the optional override or the sx name itself.
@@ -1338,9 +1349,9 @@ pub const Lowering = struct {
}
fn lowerVarDecl(self: *Lowering, vd: *const ast.VarDecl) void {
if (vd.type_annotation != null) {
if (vd.type_annotation) |ta| {
// Explicit type annotation — resolve type first, then lower value
const ty = self.resolveType(vd.type_annotation);
const ty = self.resolveType(ta);
const slot = self.builder.alloca(ty);
if (vd.value) |val| {
// = --- (undef_literal) on tuple types: zero-initialize
@@ -1478,8 +1489,8 @@ pub const Lowering = struct {
const ref = self.lowerExpr(cd.value);
// If there's an explicit type annotation, use it. Otherwise, infer from the expression.
const ty = if (cd.type_annotation != null)
self.resolveType(cd.type_annotation)
const ty = if (cd.type_annotation) |ta|
self.resolveType(ta)
else
self.builder.getRefType(ref);
@@ -8389,9 +8400,8 @@ pub const Lowering = struct {
return elem_ty;
}
fn resolveType(self: *Lowering, type_ann: ?*const Node) TypeId {
if (type_ann) |n| return self.resolveTypeWithBindings(n);
return .s64;
fn resolveType(self: *Lowering, type_ann: *const Node) TypeId {
return self.resolveTypeWithBindings(type_ann);
}
/// Resolve a type node, checking type_bindings first for generic type params.

View File

@@ -0,0 +1 @@
0

View File

@@ -0,0 +1,3 @@
count = 42
pi = 3.140000
flag = true