Skip to content

fix(data): convert constructor arguments to declared field types - #83

Merged
Roger-luo merged 1 commit into
mainfrom
fix/container-type-convert
Jul 12, 2026
Merged

Roger-luo merged 1 commit into
mainfrom
fix/container-type-convert

Conversation

@Roger-luo

Copy link
Copy Markdown
Owner

Summary

Fixes #32 — TypeError in getproperty on container-typed fields.

Root cause

Variant storage structs use Any for a field whenever its declared type "collapses" in guess_self_as_any (src/data/scan.jl):

  • A genuine Any type parameter — e.g. Vector{Any} becomes an Any storage field.
  • A self-referential container — e.g. Vector{Self} must use Any because the wrapper Type isn't defined yet when the storage struct is emitted (removing the collapse, as suggested in the issue thread, breaks these).

Because the storage field is Any, Julia's inner constructor performs no conversion, so the stored value keeps its original type (Vector{Int}). But getproperty/variant_getfield type-assert against the declared annotation (Vector{Any}), producing:

ERROR: TypeError: in typeassert, expected Vector{Any}, got a value of type Vector{Int64}

Fix

Convert each argument to its declared annotation type in the positional (emit_each_variant_cons) and keyword (emit_each_variant_kw_cons) constructors, restoring the normal Julia struct-construction semantics that were lost to the Any storage. The conversion is a no-op when the value already matches, so construction stays type stable (verified via Base.return_types).

This resolves all three failure modes from the issue:

  • OptionVec.Some{String}("hi", [1, 2]) (the report)
  • M.V([5]) (anonymous variant with Vector{Any})
  • Pattern.Row([]) (self-referential Vector{Self})

It also fixes a latent bug in the explicit-brace form of self-referential constructors: Tree.Node{Int}(5, Tree.Leaf(3), Tree.Empty()) now promotes the parametric singleton bottom correctly.

Not changed

The strict type-inferring constructor (Some("hi", [1,2]) without {String}) still requires exact argument types — this is pre-existing behavior shared by all concrete field types (it also rejects Int for a Float64 field), not the reported bug, and that path is deliberately delicate for type-parameter inference (see #34).

Tests

Adds test/data/emit/container.jl covering generic/anonymous/named/keyword Vector{Any} fields, self-referential Vector{Self} conversion, and explicit-brace self-ref promotion. Full suite passes (data 285 → 304).

🤖 Generated with Claude Code

Variant storage structs use `Any` for a field whenever its declared type
collapses -- a `Vector{Any}` field or a self-referential `Vector{Self}`
(which cannot reference the not-yet-defined wrapper `Type`). Julia's inner
constructor then performs no conversion, so the stored value keeps its
original type while `getproperty`/`variant_getfield` type-assert against the
declared annotation, raising a `TypeError`.

Convert each argument to its declared annotation type in the positional and
keyword constructors, restoring normal struct-construction semantics. The
conversion is a no-op when the value already matches, so construction stays
type stable. This also lets the explicit-brace form of self-referential
constructors promote a parametric singleton bottom (e.g. `Empty()`).

Fixes #32

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
moshi-jl Ignored Ignored Jul 12, 2026 6:01pm

@codecov

codecov Bot commented Jul 12, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.70%. Comparing base (a74db69) to head (8da1b32).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #83   +/-   ##
=======================================
  Coverage   91.69%   91.70%           
=======================================
  Files          43       43           
  Lines        1661     1663    +2     
=======================================
+ Hits         1523     1525    +2     
  Misses        138      138           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown
Contributor

Benchmark Results (Julia v1)

Time benchmarks
main 8da1b32... main / 8da1b32...
adt_transform/transform n=100 2.67 ± 0.19 μs 6.67 ± 0.15 μs 0.399 ± 0.03
adt_transform/transform n=1000 29 ± 0.69 μs 0.0648 ± 0.00091 ms 0.448 ± 0.012
linked_list/sum n=100 0.471 ± 0 μs 0.501 ± 0.01 μs 0.94 ± 0.019
linked_list/sum n=1000 5.14 ± 0.04 μs 5.44 ± 0.06 μs 0.945 ± 0.013
time_to_load 0.0779 ± 0.00096 s 0.0784 ± 0.00073 s 0.994 ± 0.015
Memory benchmarks
main 8da1b32... main / 8da1b32...
adt_transform/transform n=100 0.178 k allocs: 5.94 kB 0.204 k allocs: 6.34 kB 0.936
adt_transform/transform n=1000 1.79 k allocs: 0.0583 MB 2.02 k allocs: 0.0619 MB 0.943
linked_list/sum n=100 0 allocs: 0 B 0 allocs: 0 B
linked_list/sum n=1000 0 allocs: 0 B 0 allocs: 0 B
time_to_load 0.145 k allocs: 11 kB 0.145 k allocs: 11 kB 1

@Roger-luo
Roger-luo merged commit 16c3497 into main Jul 12, 2026
8 checks passed
@Roger-luo
Roger-luo deleted the fix/container-type-convert branch July 12, 2026 18:08
github-actions Bot referenced this pull request Jul 12, 2026
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeError in getproperty on container types

1 participant