Fix #559: nested type aliases through Array<...> compile and run#666
Merged
Conversation
Pre-fix `vera/wasm/inference.py::_alias_array_element` extracted
the array element type but did not canonicalise it. For
`type Row = Array<Int>; type Grid = Array<Row>;`, indexing
`@Grid.0` walked `Grid -> Array<Row>` and returned
`NamedType("Row")` rather than the canonical
`NamedType("Array", (Int,))`.
Two downstream consequences:
1. Chained-indexing in `_infer_index_element_type_expr` checks
`inner_te.name == "Array"` — on `"Row"` this falls through to
`return None`, so `@Grid.0[i][j]` silently dropped the
enclosing function via `[E602]`.
2. The IndexExpr WASM-emit path looked up the element WASM type
by name; for the opaque alias `Row` it returned scalar shape
and emitted a load-as-i32 + `i64.extend_i32_u` against what is
actually a heap pointer to an i32_pair. WASM validation
rejected the module with `type mismatch: expected a type but
nothing on stack`.
When the bug surfaced on a private helper (e.g. the
`cell_alive(@grid, @int, @int -> @int)` from the Conway's Life
program in the issue body), the failure mode was the misleading
`unknown func: $caller` symptom because the helper had been
dropped via [E602] earlier in the pipeline and the call site
referenced a non-existent symbol.
Post-fix the helper runs the extracted element through the
existing `_canonical_named_type` walker (the #630
canonicalisation seam, which iteratively peels `RefinementType`
layers and follows alias chains). A `Row` element resolves to
`Array<Int>`, the chained-indexing branch matches `"Array"`, and
downstream size lookups see the real i32_pair shape.
Fallback: `_canonical_named_type` returns `None` when the walk
terminates at a non-NamedType (e.g. element is a `FnType` — not
a useful array shape today). In that case we fall back to the
original unresolved NamedType so we don't regress the pre-#559
contract for the direct `Array<T>` path.
Two new regression tests in
`tests/test_codegen.py::TestCompoundArrays`:
- `test_nested_alias_array_length_559` — pins
`array_length(@Grid.0[0])` returning 1 for `[[10]]`.
- `test_nested_alias_2d_index_559` — pins `@Grid.0[1][0]`
returning 30 for `[[10, 20], [30, 40]]` (chained index branch).
Validation
- pytest tests/ -q -> 3,811 passed, 14 skipped (pre-version-bump
number; post bump will be 3,813)
- mypy vera/ -> clean
- vera run on issue's repro -> 1 (was: type mismatch trap)
- vera run on Conway's-Life helper pattern -> 3 (was: skipped)
Closes #559
Co-Authored-By: Claude <noreply@anthropic.invalid>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #666 +/- ##
=======================================
Coverage 91.06% 91.06%
=======================================
Files 60 60
Lines 23239 23240 +1
Branches 259 259
=======================================
+ Hits 21163 21164 +1
Misses 2069 2069
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
💤 Files with no reviewable changes (1)
WalkthroughRelease v0.0.150 fixes issue ChangesNested Type-Alias Canonicalisation and v0.0.150 Release
Possibly Related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Array<...>(e.g.type Row = Array<Int>; type Grid = Array<Row>;) now compile and run correctly when indexed through both layers.vera/wasm/inference.py::_alias_array_elementreturnedNamedType("Row")rather than the canonicalNamedType("Array", (Int,))for@Grid.0's element type. Downstream consumers either fell through (chained-indexing checkinner_te.name == "Array"fails on"Row") or emitted load-as-i32 +i64.extend_i32_uagainst a heap pointer to an i32_pair — producingtype mismatch: expected a type but nothing on stackat WASM validation, or the misleadingunknown func: $callersymptom when the bug surfaced on a private helper._canonical_named_typewalker (the Cross-cutting: centralise canonical Vera-type-name resolution to close the #602 bug class structurally #630 canonicalisation seam). Fallback preserves the pre-fix contract for non-NamedType element types.tests/test_codegen.py::TestCompoundArrays(test_nested_alias_array_length_559,test_nested_alias_2d_index_559) pin both the single-level array_length shape and the chained-indexing shape from the issue body's Conway's-Life context.Why this shape
The codebase already had the right primitive —
_canonical_named_typewas introduced in #630 to consolidate alias-walking logic._alias_array_elementslipped through that consolidation: it returned the element type but didn't re-canonicalise. The fix is one call to the existing walker plus aNonefallback so non-NamedType elements (e.g.FnType) don't regress the directArray<T>path. Minimal blast radius, no new abstraction.Test plan
pytest tests/ -q→ 3,811 passed, 14 skipped (pre-bump count; 3,813 post-bump)mypy vera/→ cleanvera runon the issue's reproducer (array_length(@Grid.0[0])) →1(was: type-mismatch trap)vera runon the Conway's-Life helper pattern (@Grid.0[@Int.1][@Int.0]inside a private fn) → returns expected value (was: skipped, misleadingunknown func: $caller)Closes #559