Skip to content

Support compact imports in the spec interpreter - #18

Merged
bvisness merged 3 commits into
mainfrom
cimp-spec-interpreter
Jul 30, 2026
Merged

Support compact imports in the spec interpreter#18
bvisness merged 3 commits into
mainfrom
cimp-spec-interpreter

Conversation

@bvisness

@bvisness bvisness commented Mar 25, 2026

Copy link
Copy Markdown
Collaborator

Adds binary and text parsing support for the compact import section proposal.

The text parser is a bit crazy because we need to disallow identifiers when using the second compact encoding form. The actual binding of identifiers has been sort of proxied up to the "caller" as a result. I'm open to suggestions for other ways to structure this, but duplicating all the externtype parsing seemed bad, and I'm no good with Menhir.

@bvisness
bvisness requested a review from rossberg March 25, 2026 17:42
@bvisness
bvisness force-pushed the cimp-spec-interpreter branch from 13139c4 to 1196239 Compare April 27, 2026 22:00
@bvisness bvisness mentioned this pull request Apr 27, 2026
5 tasks
Adds binary and text parsing support for the compact import section
proposal. Also fixes a small section-sizing bug in the binary tests.
@bvisness
bvisness force-pushed the cimp-spec-interpreter branch from 1196239 to e848b7d Compare April 27, 2026 22:21

@rossberg rossberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I have somehow managed to miss this PR entirely!

This looks correct, I only have suggestions for simplifying the code somewhat.

The one thing I wonder is whether the encoder should also make use of the short-hands, similar to how it tries to compactify locals. But we can discuss that separately.

Comment thread interpreter/binary/decode.ml
Comment thread interpreter/text/parser.mly Outdated
Comment thread interpreter/text/parser.mly Outdated
@bvisness
bvisness merged commit 49d4d95 into main Jul 30, 2026
1 check passed
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.

2 participants