Skip to content

Commit

Permalink
wasmparser: Use type-specific identifiers pervasively
Browse files Browse the repository at this point in the history
This commit changes `wasmparser` from the uni-typed `TypeId` identifier to a
myriad of type-specific identifiers, such as `ComponentInstanceTypeId` and
`ComponentDefinedTypeId`. There are also identifiers for any kind of type within
some group, such as `ComponentAnyTypeId` for any kind of component type and
`AnyTypeId` for any kind of type, component or core.

The `wasmparser::Types` arena can be indexed by any of these leaf type
identifiers, which all implement the `TypeIdentifier` trait. The resulting
indexed data is the particular type, e.g. a `ComponentFuncType`, rather than the
uni-typed `wasmparser::Type`.

There was a lot of ocean boiling involved here. Apologies in advance for the
pain of this upgrade! That said, it should provide type safety benefits moving
forward and sets the stage for further identifier refactors for core types that
are necessary for Wasm GC.

If you are upgrading and were using `TypeId` before, and don't want to spend
time gaining type safety on your type lookups, then just use `AnyTypeId` instead
of the old `TypeId`. They are morally identical. Alternatively, if you know a
particular `TypeId` was always a component function, for example, and you always
did `types[id].unwrap_component_func()`, then you can switch to using
`ComponentFuncTypeId`.

Co-Authored-By: Alex Crichton <[email protected]>
  • Loading branch information
fitzgen and alexcrichton committed Oct 19, 2023
1 parent 044b3cf commit bf4d45c
Show file tree
Hide file tree
Showing 14 changed files with 2,190 additions and 1,271 deletions.
19 changes: 12 additions & 7 deletions crates/wasm-compose/src/composer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use anyhow::{anyhow, bail, Context, Result};
use indexmap::IndexMap;
use std::{collections::VecDeque, ffi::OsStr, path::Path};
use wasmparser::{
types::{ComponentEntityType, TypeId, TypesRef},
types::{ComponentEntityType, ComponentInstanceTypeId, TypesRef},
ComponentExternalKind, ComponentTypeRef,
};

Expand Down Expand Up @@ -198,7 +198,7 @@ impl<'a> CompositionGraphBuilder<'a> {
instance: usize,
dependent: usize,
arg_name: &str,
ty: TypeId,
ty: ComponentInstanceTypeId,
types: TypesRef,
) -> Result<Option<ExportIndex>> {
let (instance_name, instance_id) = self.instances.get_index(instance).unwrap();
Expand Down Expand Up @@ -241,7 +241,7 @@ impl<'a> CompositionGraphBuilder<'a> {
instance: usize,
dependent_path: &Path,
arg_name: &str,
ty: TypeId,
ty: ComponentInstanceTypeId,
types: TypesRef,
) -> Result<ExportIndex> {
let (_, instance_id) = self.instances.get_index(instance).unwrap();
Expand Down Expand Up @@ -271,13 +271,18 @@ impl<'a> CompositionGraphBuilder<'a> {
}

/// Resolves an import instance reference.
fn resolve_import_ref(&self, r: InstanceImportRef) -> (&Component, &str, TypeId) {
fn resolve_import_ref(
&self,
r: InstanceImportRef,
) -> (&Component, &str, ComponentInstanceTypeId) {
let component = self.graph.get_component(r.component).unwrap();
let (name, ty) = component.import(r.import).unwrap();
match ty {
ComponentTypeRef::Instance(index) => {
(component, name, component.types.component_type_at(index))
}
ComponentTypeRef::Instance(index) => (
component,
name,
component.types.component_any_type_at(index).unwrap_instance(),
),
_ => unreachable!("should not have an instance import ref to a non-instance import"),
}
}
Expand Down
Loading

0 comments on commit bf4d45c

Please sign in to comment.