wasm-tools icon indicating copy to clipboard operation
wasm-tools copied to clipboard

Use typed indices via a new `wasm-types` crate

Open samestep opened this issue 11 months ago • 3 comments

Following up on some conversation in #1985, this draft PR is a start on what it might look like to use typed indices throughout the whole codebase. I was originally planning to finish this before coming back to #1985, but it eventually became clear to me that this is considerably bigger than I expected.

In any case, I'm opening this PR to gauge whether there's any interest in going further down this path, or if I should just call this an interesting experiment and abandon it. There were some unintuitive cases, so maybe there is some potential value in these from a codebase documentation/consistency perspective? For example:

https://github.com/bytecodealliance/wasm-tools/blob/beac7de36a6747a9061b8fa497a130aa4f7da185/crates/wasm-encoder/src/reencode/component.rs#L963-L966

This code made me mark the func_ty_index field of the CanonicalFunction::ThreadSpawn variant as TypeIdx instead of ComponentTypeIdx, unlike all the fields in all the Resource variants. Is this correct? I don't know much about the component model, so maybe threads just happen to use core type indices while other similar-looking things use component type indices. Maybe it's clear from the spec (?), but it's not clear from the docstring:

https://github.com/bytecodealliance/wasm-tools/blob/beac7de36a6747a9061b8fa497a130aa4f7da185/crates/wasmparser/src/readers/component/canonicals.rs#L70-L74

Another random question I had while working on this: why do both wasm_encoder::NameSection::tag and wasm_encoder::NameSection::tags exist? Don't they do the same thing?

samestep avatar Feb 12 '25 02:02 samestep

Thanks again for your work on this! I wanted to give a heads up that I'm at least a bit swamped right now so it may take a bit before I get back to this, but it's in my inbox so I don't forget about it.

alexcrichton avatar Feb 18 '25 17:02 alexcrichton

Following up on some conversation in #1985, this draft PR is a start on what it might look like to use typed indices throughout the whole codebase. I was originally planning to finish this before coming back to #1985, but it eventually became clear to me that this is considerably bigger than I expected.

In any case, I'm opening this PR to gauge whether there's any interest in going further down this path, or if I should just call this an interesting experiment and abandon it. There were some unintuitive cases, so maybe there is some potential value in these from a codebase documentation/consistency perspective? For example:

https://github.com/bytecodealliance/wasm-tools/blob/beac7de36a6747a9061b8fa497a130aa4f7da185/crates/wasm-encoder/src/reencode/component.rs#L963-L966

This could also extend outside the codebase; it would be useful for interoperability with walrus and waffle (both the original and my fork). A similar thing could also be done with Operator. +1, though I'm not in BA

This code made me mark the func_ty_index field of the CanonicalFunction::ThreadSpawn variant as TypeIdx instead of ComponentTypeIdx, unlike all the fields in all the Resource variants. Is this correct? I don't know much about the component model, so maybe threads just happen to use core type indices while other similar-looking things use component type indices. Maybe it's clear from the spec (?), but it's not clear from the docstring:

https://github.com/bytecodealliance/wasm-tools/blob/beac7de36a6747a9061b8fa497a130aa4f7da185/crates/wasmparser/src/readers/component/canonicals.rs#L70-L74

Another random question I had while working on this: why do both wasm_encoder::NameSection::tag and wasm_encoder::NameSection::tags exist? Don't they do the same thing?

gkgoat1 avatar Jul 16 '25 17:07 gkgoat1

+1 again, I am developing a new WASM framework for storing and processing modules in-memory and this would help, again.

gkgoat1 avatar Oct 28 '25 18:10 gkgoat1