navicore/patch-seq

Fork 0

Regression: union variants with fields crash at runtime (local main branch) #376

New issue

Closed

opened 2026-03-27 14:10:26 +00:00 by navicore · 3 comments

navicore commented

2026-03-27 14:10:26 +00:00

(Migrated from github.com)

Summary

Building seqc from the local main branch (post-v4.2.1 tagged-ptr/40-bit removal work) produces a compiler that generates broken code for all union variants with fields. The published crates.io v4.2.1 is unaffected — this is a regression on main that must be fixed before the next crates.io publish.

Discovered while debugging seq-lisp test failures.

What's broken

Three distinct failures, all related to variant field handling:

1. `Make-*` constructors with fields panic

union MyVal {
  MV { a: Int }
}

: main ( -- )
  42 Make-MV MV-a int->string io.write-line
;

thread panicked at crates/core/src/stack_old.rs:219:18:
Invalid discriminant: 4310895856

The discriminant values (e.g. 4310895856, 4322305760) look like raw pointer values being misinterpreted as discriminant tags — suggests the new tagged-pointer representation is not being decoded correctly by the old variant construction path.

2. Generated field accessors panic

variant-field-at: expected Variant, got Int(0)

at crates/runtime/src/variant_ops.rs:126:18

3. `is-*?` predicates return wrong results

union Color { Red  Green  Blue }

: main ( -- )
  Make-Red is-Red? if "is red" io.write-line else "not red" io.write-line then
;

Output: not red — the predicate fails to recognize a value it just constructed.

What still works

Nullary variant construction: Make-Red, Make-Nothing — no crash
match on nullary variants: pattern matching dispatches correctly
Published crates.io v4.2.1: all seq-lisp tests pass (489 Lisp tests, 8 Seq tests, 8 LSP tests)

Root cause (likely)

The v4.1.0 "tagged-ptr default" and v4.2.0 "remove 40-bit support" changes altered the variant memory layout. It appears that:

match was updated to use the new representation
Make-* (with fields), is-*?, and generated field accessors (Variant-field, variant.field-at) were not fully updated

How we found it

seq-lisp's first Seq unit test (test_error_handling) crashed immediately on "(car)" tokenize — the tokenizer uses TokState, TokPos, and TokAccum union types heavily. The crash traced to _patch_seq_variant_field_at inside _seq_tokenize_loop.

Minimal repro narrowed it down to any union variant with at least one field.

How to reproduce

# From the patch-seq working tree (main branch, post-4.2.1):
cargo install --path crates/compiler --force

# Then compile and run any program with union fields:
cat > /tmp/test.seq << 'SEQEOF'
union MyVal {
  MV { a: Int }
}
: main ( -- )
  42 Make-MV MV-a int->string io.write-line
;
SEQEOF

seqc build /tmp/test.seq -o /tmp/test && /tmp/test
# Expected: "42"
# Actual: panic — Invalid discriminant

Suggested fix

Add a CI test that covers union variants with fields (construction, field access, predicates, dup + predicate). The current test suite apparently doesn't cover this — a single-field Make-* / accessor round-trip test would have caught this immediately.
Ensure Make-*, is-*?, generated accessors, and variant.field-at all use the same tagged-pointer representation that match uses.

Environment

macOS 15.3, arm64
rustc 1.93.0 (LLVM 21.1.8)
seqc 4.2.1 built from local path (path+file:///...patch-seq/crates/compiler)

## Summary Building `seqc` from the local `main` branch (post-v4.2.1 tagged-ptr/40-bit removal work) produces a compiler that generates broken code for **all union variants with fields**. The published crates.io v4.2.1 is unaffected — this is a regression on `main` that must be fixed before the next crates.io publish. Discovered while debugging seq-lisp test failures. ## What's broken Three distinct failures, all related to variant field handling: ### 1. `Make-*` constructors with fields panic ```seq union MyVal { MV { a: Int } } : main ( -- ) 42 Make-MV MV-a int->string io.write-line ; ``` ``` thread panicked at crates/core/src/stack_old.rs:219:18: Invalid discriminant: 4310895856 ``` The discriminant values (e.g. `4310895856`, `4322305760`) look like raw pointer values being misinterpreted as discriminant tags — suggests the new tagged-pointer representation is not being decoded correctly by the old variant construction path. ### 2. Generated field accessors panic ``` variant-field-at: expected Variant, got Int(0) ``` at `crates/runtime/src/variant_ops.rs:126:18` ### 3. `is-*?` predicates return wrong results ```seq union Color { Red Green Blue } : main ( -- ) Make-Red is-Red? if "is red" io.write-line else "not red" io.write-line then ; ``` Output: `not red` — the predicate fails to recognize a value it just constructed. ## What still works - **Nullary variant construction**: `Make-Red`, `Make-Nothing` — no crash - **`match` on nullary variants**: pattern matching dispatches correctly - **Published crates.io v4.2.1**: all seq-lisp tests pass (489 Lisp tests, 8 Seq tests, 8 LSP tests) ## Root cause (likely) The v4.1.0 "tagged-ptr default" and v4.2.0 "remove 40-bit support" changes altered the variant memory layout. It appears that: - `match` was updated to use the new representation - `Make-*` (with fields), `is-*?`, and generated field accessors (`Variant-field`, `variant.field-at`) were **not** fully updated ## How we found it seq-lisp's first Seq unit test (`test_error_handling`) crashed immediately on `"(car)" tokenize` — the tokenizer uses `TokState`, `TokPos`, and `TokAccum` union types heavily. The crash traced to `_patch_seq_variant_field_at` inside `_seq_tokenize_loop`. Minimal repro narrowed it down to any union variant with at least one field. ## How to reproduce ```bash # From the patch-seq working tree (main branch, post-4.2.1): cargo install --path crates/compiler --force # Then compile and run any program with union fields: cat > /tmp/test.seq << 'SEQEOF' union MyVal { MV { a: Int } } : main ( -- ) 42 Make-MV MV-a int->string io.write-line ; SEQEOF seqc build /tmp/test.seq -o /tmp/test && /tmp/test # Expected: "42" # Actual: panic — Invalid discriminant ``` ## Suggested fix - Add a CI test that covers union variants with fields (construction, field access, predicates, `dup` + predicate). The current test suite apparently doesn't cover this — a single-field `Make-*` / accessor round-trip test would have caught this immediately. - Ensure `Make-*`, `is-*?`, generated accessors, and `variant.field-at` all use the same tagged-pointer representation that `match` uses. ## Environment - macOS 15.3, arm64 - rustc 1.93.0 (LLVM 21.1.8) - seqc 4.2.1 built from local path (`path+file:///...patch-seq/crates/compiler`)

navicore commented

2026-03-27 15:02:26 +00:00

(Migrated from github.com)

@navicore-bot please take a look at this issue

navicore-bot commented

2026-03-27 15:16:29 +00:00

(Migrated from github.com)

Investigation Results (Rocky Linux 9.7, x86_64, rustc 1.94.0)

After a clean rebuild from the latest main (commit f667acc), I cannot reproduce this issue. All three failure scenarios from the report now work correctly:

1. `Make-*` constructors with fields ✅

union MyVal { MV { a: Int } }
: main ( -- ) 42 Make-MV MV-a int->string io.write-line ;

Output: 42 (expected)

2. `is-*?` predicates ✅

union Color { Red  Green  Blue }
: main ( -- ) Make-Red is-Red? if "is red" io.write-line else "not red" io.write-line then ;

Output: is red (expected)

3. Multi-field variant access ✅

union Pair { P { a: Int  b: Int } }
: main ( -- ) 10 20 Make-P dup P-a int->string io.write-line P-b int->string io.write-line ;

Output: 10\n20 (expected)

Full test suite

Unit tests: 943 passed, 0 failed
Integration tests: 438/438 passed

Likely root cause

The original crash referenced crates/core/src/stack_old.rs:219:18 — a file that no longer exists on main (removed in the 40-bit cleanup). This strongly suggests the crash was caused by a stale embedded runtime: the compiler binary had the old 40-bit runtime .a file baked in, while the rest of the compiler was updated for the new tagged-ptr layout.

A cargo clean && cargo build --release should resolve this. The build script (crates/compiler/build.rs) embeds libseq_runtime.a at compile time via include_bytes!, so an incremental build can pick up the old runtime if only the compiler crate changed.

Recommendation

This may be worth adding a safeguard for — e.g., embedding a version hash in the runtime .a and checking it at load time, or having the build script always rebuild the runtime when the compiler version changes. But functionally, a clean build fixes the issue.

## Investigation Results (Rocky Linux 9.7, x86_64, rustc 1.94.0) After a **clean rebuild** from the latest `main` (commit `f667acc`), I **cannot reproduce** this issue. All three failure scenarios from the report now work correctly: ### 1. `Make-*` constructors with fields ✅ ```seq union MyVal { MV { a: Int } } : main ( -- ) 42 Make-MV MV-a int->string io.write-line ; ``` Output: `42` (expected) ### 2. `is-*?` predicates ✅ ```seq union Color { Red Green Blue } : main ( -- ) Make-Red is-Red? if "is red" io.write-line else "not red" io.write-line then ; ``` Output: `is red` (expected) ### 3. Multi-field variant access ✅ ```seq union Pair { P { a: Int b: Int } } : main ( -- ) 10 20 Make-P dup P-a int->string io.write-line P-b int->string io.write-line ; ``` Output: `10\n20` (expected) ### Full test suite - **Unit tests**: 943 passed, 0 failed - **Integration tests**: 438/438 passed ### Likely root cause The original crash referenced `crates/core/src/stack_old.rs:219:18` — a file that **no longer exists** on `main` (removed in the 40-bit cleanup). This strongly suggests the crash was caused by a **stale embedded runtime**: the compiler binary had the old 40-bit runtime `.a` file baked in, while the rest of the compiler was updated for the new tagged-ptr layout. A `cargo clean && cargo build --release` should resolve this. The build script (`crates/compiler/build.rs`) embeds `libseq_runtime.a` at compile time via `include_bytes!`, so an incremental build can pick up the old runtime if only the compiler crate changed. ### Recommendation This may be worth adding a safeguard for — e.g., embedding a version hash in the runtime `.a` and checking it at load time, or having the build script always rebuild the runtime when the compiler version changes. But functionally, a clean build fixes the issue.

👍 1

navicore commented

2026-03-27 16:27:42 +00:00

(Migrated from github.com)

I've reinstalled from main and the regression went away. Must have been a partial install or an install of an abandoned change that was reverted but no new "just install" run.