d515696e61
`Analyzer.resolveTypeNode` read the array `.length` node's `.int_literal` union field unconditionally. For a named-const dimension (`MAX :: 4; [MAX]u8`) that node is an `identifier`, so the access tripped Zig's checked-union panic and `sx lsp` aborted on didOpen. The main compiler was unaffected (it folds the dim through the IR). - New `arrayDimLength` helper switches on the dimension node tag: int_literal → value; identifier → a recorded module-const int value; anything else / out-of-u32-range → unknown. Never assumes a node shape. - `Type.ArrayTypeInfo.length` is now `?u32`; null is an explicit "editor couldn't fold this dimension" marker (rendered `[_]T`), never a fabricated concrete length. - New `const_int_values` registry records integer-literal consts at registration time for the identifier path. Regression: first `src/lsp/*.test.zig` (the minimal LSP harness), wired into the test graph via `src/root.zig`. Drives `analyzeDocument` over `[MAX]u8` (folds to 4, no panic), `[64]u8` (happy-path guard), and `[N]u8` (explicit unknown). Fail-before/pass-after verified. Sibling audit of the resolveTypeNode/fieldType family: the array dim was the only unchecked union-field access; all other arms recurse or tag-check first. Noted a non-crashing display gap in server.zig hover rendering for step B.
101 lines
5.0 KiB
Markdown
101 lines
5.0 KiB
Markdown
# 0099 — LSP analyzer panics on an identifier array dimension (`[MAX]u8`)
|
|
|
|
**RESOLVED.** The editor analyzer (`src/sema.zig`) read the array-dimension node's
|
|
`.int_literal` union field unconditionally. When the dimension is a **named const**
|
|
(`[MAX]u8`), the `.length` node is an `identifier`, so the access tripped Zig's
|
|
checked-union panic and `sx lsp` aborted (`SIGABRT`) the moment the file was opened
|
|
(didOpen → `analyzeDocument`). The main compiler (`sx run`) was never affected — it
|
|
resolves `[MAX]u8` through the IR's `foldArrayDim` machinery.
|
|
|
|
## Symptom
|
|
|
|
- **Observed:** opening a buffer with a named-const dimension in an editor crashes the
|
|
language server: `src/sema.zig` aborts with
|
|
`access of union field 'int_literal' while field 'identifier' is active`.
|
|
- **Expected:** the analyzer resolves `[MAX]u8` to length 4 (or, when it can't fold the
|
|
dimension, records an explicit "unknown" length) and never crashes.
|
|
|
|
## Reproduction
|
|
|
|
```sx
|
|
MAX :: 4;
|
|
Thing :: struct { buf: [MAX]u8; }
|
|
```
|
|
|
|
`sx run` compiles this fine; `sx lsp` aborts on didOpen. (The crash is reached via
|
|
`registerTopLevelDecl` → `fieldType` → `resolveTypeNode` while building the editor
|
|
index — not a diagnostic pass.)
|
|
|
|
## Root cause
|
|
|
|
`Analyzer.resolveTypeNode` in `src/sema.zig`, the `array_type_expr` arm:
|
|
|
|
```zig
|
|
const length: u32 = @intCast(ate.length.data.int_literal.value);
|
|
```
|
|
|
|
`ate.length` is an arbitrary expression node. For a literal `[64]u8` it is
|
|
`int_literal`; for `[MAX]u8` it is `identifier`. The code assumed `int_literal`
|
|
unconditionally — a textbook unchecked-union-field access. `@intCast` of the
|
|
non-existent payload never even runs; the union-field read panics first.
|
|
|
|
## Fix
|
|
|
|
- New helper `Analyzer.arrayDimLength(len_node)` switches on the dimension node's tag:
|
|
`int_literal` → its value; `identifier` → the value recorded for that name in a new
|
|
`const_int_values` registry (populated when a top-level `const_decl` has an
|
|
`int_literal` value); anything else (unknown name, non-const expression) → `null`. A
|
|
value outside `u32` range also yields `null`. No node shape is ever assumed without a
|
|
tag check, so the path cannot panic.
|
|
- `Type.ArrayTypeInfo.length` changed from `u32` to `?u32`. `null` is the explicit
|
|
"editor couldn't fold this dimension" marker — never a fabricated concrete length
|
|
(this is hover/metadata, not codegen). `displayName` renders an unknown length as
|
|
`[_]T`.
|
|
- `src/sema.zig` `substType` passes the (now optional) length through unchanged; the
|
|
only other consumer of the editor `ArrayTypeInfo` is `displayName`, updated above.
|
|
|
|
The named-const value is captured at registration time, so resolution shares the
|
|
analyzer's existing intra-pass forward-reference limitation: a const declared *after* the
|
|
struct that uses it resolves to the safe "unknown" length rather than its value. This
|
|
matches how the analyzer already treats a struct field that references a later-declared
|
|
struct type, and never crashes.
|
|
|
|
## Sibling audit (`resolveTypeNode` / `fieldType` family)
|
|
|
|
Audited every arm for the same unchecked `.data.<field>` pattern. The array dimension
|
|
was the **only** crashing site:
|
|
|
|
- `slice_type_expr`, `optional_type_expr`, `pointer_type_expr`,
|
|
`many_pointer_type_expr`, `function_type_expr` — all recurse through
|
|
`resolveTypeNode`, which dispatches on the node tag; no raw field access.
|
|
- `parameterized_type_expr` returns `.void_type`; `type_expr` / `identifier` read
|
|
`.name` / `.is_raw` only *after* `tn.data == .type_expr or tn.data == .identifier`.
|
|
- `fieldType`, `typeExprName`, `typeExprIsRaw`, `resolveTypeAnnotation`,
|
|
`resolveTypeRef` all tag-check (switch capture or explicit `==`) before any payload
|
|
read; `resolveTypeAnnotation`'s `array_type_expr` case delegates to `resolveTypeNode`,
|
|
so it is fixed transitively.
|
|
|
|
**Non-crashing display gap noted (not fixed here — out of scope for A):**
|
|
`src/lsp/server.zig` (~line 2801) renders a struct's array field in hover/completion
|
|
detail directly off the AST. It already guards `if (ate.length.data == .int_literal)`,
|
|
so it does **not** crash, but it renders a named-const dimension as an empty `[]u8`
|
|
instead of `[4]u8`. Tracked for the broad LSP `.data` sweep (next step B), not addressed
|
|
in this crash fix.
|
|
|
|
## Regression
|
|
|
|
First `src/lsp/*.test.zig` — establishes the minimal LSP test harness
|
|
(`src/lsp/document.test.zig`), wired into the `zig build test` graph via
|
|
`src/root.zig`'s `lsp.document_tests` reference. Tests drive the real didOpen path
|
|
(`DocumentStore.analyzeDocument`) and inspect the editor index:
|
|
|
|
- `[MAX]u8` (named const) → no panic; folded length is `4`, element `u8`.
|
|
- `[64]u8` (int literal) → length `64` (happy-path guard).
|
|
- `[N]u8` (undeclared name) → explicit unknown length (`null`), no panic, no fabricated
|
|
value.
|
|
|
|
Verified fail-before / pass-after: with the pre-fix unconditional `.int_literal`
|
|
dereference restored, the identifier test aborts with the exact original panic; with the
|
|
tag-switching fix it passes. Full gate green: `zig build`, `zig build test` (incl. the
|
|
new LSP test), `bash tests/run_examples.sh` (453 passed, 0 failed).
|