Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor interface type representation in wast #607

Conversation

alexcrichton
Copy link
Member

This commit updates the wast representation of interface types to
match more closely what wasmparser does. Previously wast primarily
used ComponentTypeUse<'_, InterType<'_>> for references and
InterType<'_> for definitions. This meant, though, that references
could not use primitives by-value. Additionally it also meant that the
parsing for ComponentTypeUse actually used IndexOrRef to parse
either $t or (type $t) but ComponentTypeUse is used for many other
constructs where it would not be appropriate for a bare index $t to be
parsed, for example this should not parse:

(component
  (type $ty (module))
  (import "" (module $m $ty)))

This commit instead refactors all uses of
ComponentTypeUse<'_, InterType<'_>> to instead use a new
InterTypeRef<'_> construct. Additionally a Primitive was split out
from the existing InterType<'_> and added as a variant to the
InterTypeRef<'_> enum as well. This reflects the organization in the
wasmparser crate and means that primitive types can be used inline
instead of references and such.

Closes #600

This commit updates the `wast` representation of interface types to
match more closely what `wasmparser` does. Previously `wast` primarily
used `ComponentTypeUse<'_, InterType<'_>>` for references and
`InterType<'_>` for definitions. This meant, though, that references
could not use primitives by-value. Additionally it also meant that the
parsing for `ComponentTypeUse` actually used `IndexOrRef` to parse
either `$t` or `(type $t)` but `ComponentTypeUse` is used for many other
constructs where it would not be appropriate for a bare index `$t` to be
parsed, for example this should not parse:

    (component
      (type $ty (module))
      (import "" (module $m $ty)))

This commit instead refactors all uses of
`ComponentTypeUse<'_, InterType<'_>>` to instead use a new
`InterTypeRef<'_>` construct. Additionally a `Primitive` was split out
from the existing `InterType<'_>` and added as a variant to the
`InterTypeRef<'_>` enum as well. This reflects the organization in the
`wasmparser` crate and means that primitive types can be used inline
instead of references and such.

Closes bytecodealliance#600
}
// And if this type isn't already defined we append it to the index
// space with a fresh and unique name.
let span = Span::from_offset(0); // FIXME: don't manufacture
Copy link
Member

Choose a reason for hiding this comment

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

Do we need an issue to track this FIXME?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure yeah, I'll tag a few places with this.

@alexcrichton alexcrichton merged commit cf92a1e into bytecodealliance:main May 18, 2022
@alexcrichton alexcrichton deleted the intertype-primitive-not-through-ref branch May 18, 2022 23:52
code-terror pushed a commit to code-terror/wasm-tools that referenced this pull request Aug 24, 2022
* Refactor interface type representation in `wast`

This commit updates the `wast` representation of interface types to
match more closely what `wasmparser` does. Previously `wast` primarily
used `ComponentTypeUse<'_, InterType<'_>>` for references and
`InterType<'_>` for definitions. This meant, though, that references
could not use primitives by-value. Additionally it also meant that the
parsing for `ComponentTypeUse` actually used `IndexOrRef` to parse
either `$t` or `(type $t)` but `ComponentTypeUse` is used for many other
constructs where it would not be appropriate for a bare index `$t` to be
parsed, for example this should not parse:

    (component
      (type $ty (module))
      (import "" (module $m $ty)))

This commit instead refactors all uses of
`ComponentTypeUse<'_, InterType<'_>>` to instead use a new
`InterTypeRef<'_>` construct. Additionally a `Primitive` was split out
from the existing `InterType<'_>` and added as a variant to the
`InterTypeRef<'_>` enum as well. This reflects the organization in the
`wasmparser` crate and means that primitive types can be used inline
instead of references and such.

Closes bytecodealliance#600

* List an issue in FIXME annotations
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.

components: Encode primitive interface types inline in function type definitions
2 participants