fix: xx pack[i] to a protocol target heap-copies the element
Erasing a single comptime-pack element to a protocol value (`xx sources[0]` with a protocol target) tripped the pack-as-value error: buildProtocolErasure treated the index_expr as an lvalue and took its address via lowerExprAsPtr, whose .index_expr arm lowers the bare pack as a value (a pack is comptime-only with no runtime storage). isLvalueExpr now reports a comptime pack index as an rvalue, decided via the same packArgNodeAt predicate the value path uses — so the value and lvalue paths can't diverge on what counts as a pack element — and erasure heap-copies the already-materialized element instead. Resolves issue 0135. Regression tests: examples/0547, 0548.
This commit is contained in:
agra
@@ -0,0 +1,174 @@
|
||||
# 0135 — `xx <pack>[i]` to a protocol target lowers the pack as a value ("pack has no runtime value")
|
||||
|
||||
> **RESOLVED (2026-06-13).** Root cause: `buildProtocolErasure`
|
||||
> (`src/ir/lower/coerce.zig`) treated `pack[i]` as an lvalue (any `index_expr`
|
||||
> returned true from `isLvalueExpr`) and tried to take its address via
|
||||
> `lowerExprAsPtr`, whose `.index_expr` arm lowers the bare pack as a value →
|
||||
> the pack-as-value error. Fix (preferred option 1): `isLvalueExpr` now reports
|
||||
> a comptime pack index as an **rvalue**, so erasure falls into its heap-copy
|
||||
> branch and copies the already-materialized element. It decides pack-ness with
|
||||
> the SAME predicate the value path uses — `packArgNodeAt` (the `pack_arg_nodes`
|
||||
> map) — not `isPackName` (`pack_param_count`), since the comptime-call path
|
||||
> installs only `pack_arg_nodes`; sharing one predicate keeps the value and
|
||||
> lvalue paths from diverging on what counts as a pack element. Regression tests:
|
||||
> [examples/0547-packs-xx-pack-index-to-protocol.sx](../examples/0547-packs-xx-pack-index-to-protocol.sx)
|
||||
> (single element) and
|
||||
> [examples/0548-packs-xx-pack-index-two-elements.sx](../examples/0548-packs-xx-pack-index-two-elements.sx)
|
||||
> (two distinct concrete types, each resolving to its own vtable). This also
|
||||
> unblocked [issue 0133](0133-union-member-struct-literal-assign-unresolved-panic.md),
|
||||
> now landed. The standalone repro `.sx` was removed (superseded by 0547).
|
||||
|
||||
## Symptom
|
||||
|
||||
One-line: erasing a single comptime-pack element to a protocol value —
|
||||
`xx sources[0]` where the target type is a protocol (`VL(i64)`) — spuriously
|
||||
errors with **"pack 'sources' has no runtime value — a pack is comptime-only
|
||||
and can't be used as a value here"**, pointing at the pack name.
|
||||
|
||||
- **Observed:** the pack-as-value diagnostic fires on `sources` in
|
||||
`x : VL(i64) = xx sources[0];`, even though `sources[0]` is a valid
|
||||
compile-time pack index.
|
||||
- **Expected:** `sources[0]` resolves to the call-site arg (the concrete
|
||||
`IntCell`), gets erased to the protocol `VL(i64)`, and the program prints
|
||||
`7`, exit 0.
|
||||
|
||||
This is **PRE-EXISTING** and reproduces on clean `master`, independent of any
|
||||
union / tuple / issue-0133 work. Issue 0053 added the `xx <whole-pack>` →
|
||||
`[]Any`/`[]P` slice bridge (`lowerPackToSlice`), but **single-element**
|
||||
`xx pack[i]` erasure to a protocol scalar was never handled.
|
||||
|
||||
## Reproduction
|
||||
|
||||
Minimal, standalone (only `modules/std.sx`):
|
||||
|
||||
```sx
|
||||
#import "modules/std.sx";
|
||||
|
||||
VL :: protocol(T: Type) { get :: () -> T; }
|
||||
IntCell :: struct { v: i64; }
|
||||
impl VL(i64) for IntCell { get :: (self: *IntCell) -> i64 => self.v; }
|
||||
|
||||
make :: (..sources: VL) -> i64 {
|
||||
x : VL(i64) = xx sources[0]; // protocol local <- xx pack[0]
|
||||
return x.get();
|
||||
}
|
||||
|
||||
main :: () -> i32 {
|
||||
print("{}\n", make(IntCell.{ v = 7 })); // 7
|
||||
0
|
||||
}
|
||||
```
|
||||
|
||||
Run: `./zig-out/bin/sx run issues/0135-xx-pack-index-protocol-erasure-lowers-pack-as-value.sx`
|
||||
→ errors today; the fix should make it print `7`, exit 0.
|
||||
|
||||
### Root cause (traced)
|
||||
|
||||
The error chain (from a stack trace at the diagnostic site):
|
||||
|
||||
```
|
||||
lowerAssignment / lowerVarDecl sets target_type = VL(i64) for the RHS
|
||||
lowerExpr(xx sources[0]) unary_op .xx
|
||||
operand = lowerExpr(sources[0]) → packArgNodeAt resolves to the
|
||||
call-site arg IntCell.{v=7} (OK)
|
||||
lowerXX(operand, sources[0]) classifies .erase_protocol
|
||||
buildProtocolErasure(..., operand_node = sources[0], dst = VL(i64))
|
||||
isLvalueExpr(sources[0]) == true (it's an index_expr)
|
||||
concrete_ptr = lowerExprAsPtr(sources[0]) ← HERE
|
||||
lowerExprAsPtr .index_expr arm: lowerExpr(ie.object)
|
||||
lowerExpr(sources) → bare pack name → diagPackAsValue ✗
|
||||
```
|
||||
|
||||
The defect: **`lowerExprAsPtr`'s `.index_expr` arm does NOT perform the
|
||||
pack-arg-node substitution that `lowerIndexExpr` does.** `lowerIndexExpr`
|
||||
intercepts `<pack>[<comptime-int>]` via `packArgNodeAt` and lowers the
|
||||
call-site arg node directly; `lowerExprAsPtr` skips straight to
|
||||
`lowerExpr(ie.object)`, lowering the bare pack `sources` as a value — which is
|
||||
the (correct) pack-as-value error for a context where there is genuinely no
|
||||
pointer to take.
|
||||
|
||||
So the value path (`lowerIndexExpr`) handles a pack index but the
|
||||
address-of path (`lowerExprAsPtr`) does not — a two-resolver divergence on
|
||||
the pack-index case. `buildProtocolErasure` only reaches the address-of path
|
||||
because `isLvalueExpr(sources[0])` returns `true` (any index_expr looks like
|
||||
an lvalue), so it tries to alias the operand's storage instead of heap-copying
|
||||
the already-materialized rvalue.
|
||||
|
||||
## Investigation prompt
|
||||
|
||||
> `xx <pack>[i]` erased to a protocol target spuriously errors with "pack
|
||||
> '<name>' has no runtime value". Repro:
|
||||
> `issues/0135-xx-pack-index-protocol-erasure-lowers-pack-as-value.sx`
|
||||
> (errors today; the fix should make it print `7`, exit 0).
|
||||
>
|
||||
> Root cause: `buildProtocolErasure` (`src/ir/lower/coerce.zig`, ~line 389)
|
||||
> sees `isLvalueExpr(operand_node) == true` for the index_expr `sources[0]`
|
||||
> and takes the alias-the-storage branch:
|
||||
> `concrete_ptr = self.lowerExprAsPtr(operand_node)`. But
|
||||
> `lowerExprAsPtr`'s `.index_expr` arm (`src/ir/lower/stmt.zig`, the
|
||||
> `.index_expr => |ie|` case, `self.lowerExpr(ie.object)` at stmt.zig:1005
|
||||
> on clean master) does
|
||||
> NOT do the pack-arg-node substitution that `lowerIndexExpr`
|
||||
> (`src/ir/lower/expr.zig:1343`, via `packArgNodeAt`) performs. It lowers the
|
||||
> bare pack `sources` as a value → `diagPackAsValue` (the pack-as-value
|
||||
> error at `src/ir/lower/expr.zig:1722`).
|
||||
>
|
||||
> A comptime pack index has no addressable storage of its own — `sources[0]`
|
||||
> is the call-site arg node, which only acquires storage when lowered as a
|
||||
> value. So the address-of path is the wrong path for a pack index.
|
||||
>
|
||||
> Suspected fix (pick the one that keeps the value/address paths from
|
||||
> diverging — the recurring two-resolver defect class in this codebase):
|
||||
> 1. **Preferred — teach `isLvalueExpr` that a comptime pack index is NOT
|
||||
> an lvalue.** Add a check: an `index_expr` whose object is a pack name
|
||||
> (`isPackName(ie.object...)` / a `packArgNodeAt(ie) != null` hit) is an
|
||||
> rvalue. Then `buildProtocolErasure` falls into its existing
|
||||
> `else { heap_copy = true; alloca + store(operand) }` branch and erases
|
||||
> the already-materialized `IntCell` value correctly. Smallest, and
|
||||
> matches the semantic truth (a pack element is a comptime rvalue).
|
||||
> 2. Alternatively, make `lowerExprAsPtr`'s `.index_expr` arm resolve a
|
||||
> pack index the way `lowerIndexExpr` does (`packArgNodeAt` → lower the
|
||||
> arg node as a value → `addr_of` an alloca holding it). More plumbing,
|
||||
> and it manufactures storage the caller could already manufacture.
|
||||
> Do NOT paper over with a silent default — per CLAUDE.md, resolve the real
|
||||
> path or emit a diagnostic.
|
||||
>
|
||||
> Verification: the repro prints `7`, exit 0; then `zig build &&
|
||||
> zig build test` green. Add positive coverage (a new
|
||||
> `examples/05xx-packs-xx-pack-index-to-protocol.sx`, packs category) and a
|
||||
> sibling that erases two distinct pack elements. When resolved, this also
|
||||
> UNBLOCKS issue 0133 (see below) — re-apply the 0133 unified-resolver fix
|
||||
> and confirm `examples/0540-packs-pack-type-arg-spread.sx` stays green.
|
||||
|
||||
## Relationship to issue 0133
|
||||
|
||||
Surfaced while fixing **issue 0133** (assigning a struct literal to a union
|
||||
member panics — the RHS never gets its target type). The clean 0133 fix
|
||||
(per its own investigation prompt) unifies the lvalue field resolver so the
|
||||
**target-type path** and the **lvalue-pointer path** share one matcher
|
||||
(`fieldLvalueResolve`), which then resolves *tuple element* LHS types too
|
||||
(not just structs). That makes `c.sources.0 = xx sources[0]` in
|
||||
`examples/0540-packs-pack-type-arg-spread.sx` set `target_type = VL(i64)`
|
||||
for the RHS — which routes `xx sources[0]` through `buildProtocolErasure`
|
||||
(it previously erased later, at the store, via `coerceToType` on the
|
||||
already-materialized value). That is exactly this bug.
|
||||
|
||||
So **issue 0133's correct (unified-resolver) fix is BLOCKED on this issue.**
|
||||
The ready-to-apply 0133 patch is recorded in
|
||||
`issues/0133-union-member-struct-literal-assign-unresolved-panic.md`; after
|
||||
0135 lands, re-apply it and confirm both the 0133 union repro (`code=9`) and
|
||||
`examples/0540` stay green.
|
||||
|
||||
## Notes
|
||||
|
||||
- Diagnostic site (symptom): `src/ir/lower/expr.zig:1723` (`isPackName` at
|
||||
1722 → `diagPackAsValue`, `.generic`) via `src/ir/lower/expr.zig:1386`
|
||||
(`lowerExpr(ie.object)` in `lowerIndexExpr`) — reached here through
|
||||
`lowerExprAsPtr`'s `.index_expr` arm, NOT `lowerIndexExpr`.
|
||||
- Root area (cause): `buildProtocolErasure` (`src/ir/lower/coerce.zig:389-390`)
|
||||
+ `lowerExprAsPtr` `.index_expr` arm (`src/ir/lower/stmt.zig:1005`) +
|
||||
`isLvalueExpr`.
|
||||
- Prior art (RESOLVED, not duplicates): 0052 (slice-of-protocol variadic
|
||||
erasure), 0053 (`xx <whole-pack>` → slice bridge), 0054 (generic-struct →
|
||||
param protocol erasure). None handle single-element `xx pack[i]` → protocol
|
||||
scalar.
|
||||
Reference in New Issue
Block a user