Skip to content

Commit

Permalink
Remove duplicate API on Types (bytecodealliance#1837)
Browse files Browse the repository at this point in the history
* Remove duplicate API on `Types`

This commit removes the duplicate API of `Types` and `TypesRef` in the
`wasmparser` crate by removing the API on `Types`. The source of truth
is the one on `TypesRef` and maintaining this duplication is a bit of a
burden as it's a lot of copy/pasted code. It's hoped that users can
instead use `.as_ref()` "early on" which will enable the same usage as
before. Ideally this would be a `Deref` impl but due to how `Deref`
works that's not possible right now.

* Update tests

* Fix doc tests
  • Loading branch information
alexcrichton authored Oct 2, 2024
1 parent 42e1c77 commit d4532cb
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 322 deletions.
3 changes: 2 additions & 1 deletion crates/wasm-compose/src/composer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ impl<'a> CompositionGraphBuilder<'a> {

match component.export_by_name(export) {
Some((export_index, kind, index)) if kind == ComponentExternalKind::Instance => {
let export_ty = component.types.component_instance_at(index);
let export_ty = component.types.as_ref().component_instance_at(index);

if self.graph.try_connection(
component_id,
Expand Down Expand Up @@ -289,6 +289,7 @@ impl<'a> CompositionGraphBuilder<'a> {
name,
component
.types
.as_ref()
.component_any_type_at(index)
.unwrap_instance(),
),
Expand Down
2 changes: 1 addition & 1 deletion crates/wasm-compose/src/encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ impl DependencyRegistrar<'_, '_> {

// Recurse for aliases to see edges across components, and otherwise
// recurse on the structure of the type below.
if let Some(ty) = self.types.peel_alias(ty) {
if let Some(ty) = self.types.as_ref().peel_alias(ty) {
return self.ty(ty);
}

Expand Down
15 changes: 12 additions & 3 deletions crates/wasm-compose/src/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,15 +258,21 @@ impl<'a> Component<'a> {
index: ExportIndex,
) -> Option<(&str, ComponentEntityType)> {
let (name, _kind, _index) = self.export(index)?;
Some((name, self.types.component_entity_type_of_export(name)?))
Some((
name,
self.types.as_ref().component_entity_type_of_export(name)?,
))
}

pub(crate) fn import_entity_type(
&self,
index: ImportIndex,
) -> Option<(&str, ComponentEntityType)> {
let (name, _ty) = self.import(index)?;
Some((name, self.types.component_entity_type_of_import(name)?))
Some((
name,
self.types.as_ref().component_entity_type_of_import(name)?,
))
}

/// Finds a compatible instance export on the component for the given instance type.
Expand All @@ -286,7 +292,9 @@ impl<'a> Component<'a> {

graph.try_connection(
export_component_id,
ComponentEntityType::Instance(self.types.component_instance_at(*index)),
ComponentEntityType::Instance(
self.types.as_ref().component_instance_at(*index),
),
self.types(),
ComponentEntityType::Instance(ty),
types,
Expand Down Expand Up @@ -563,6 +571,7 @@ impl<'a> CompositionGraph<'a> {
for import_name in component.imports.keys() {
let ty = component
.types
.as_ref()
.component_entity_type_of_import(import_name)
.unwrap();

Expand Down
1 change: 1 addition & 0 deletions crates/wasm-smith/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1761,6 +1761,7 @@ impl Module {
}

// For each export, add necessary prerequisites to the module.
let exports_types = exports_types.as_ref();
for export in required_exports {
let new_index = match exports_types
.entity_type_from_export(&export)
Expand Down
1 change: 1 addition & 0 deletions crates/wasm-smith/tests/exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ fn smoke_test_with_mutable_global_exports() {
fn get_func_and_global_exports(features: WasmFeatures, module: &[u8]) -> Vec<(String, ExportType)> {
let mut validator = Validator::new_with_features(features);
let types = validate(&mut validator, module);
let types = types.as_ref();
let mut exports = vec![];

for payload in Parser::new(0).parse_all(module) {
Expand Down
11 changes: 7 additions & 4 deletions crates/wasmparser/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ impl Validator {
///
/// // Validate the first Wasm module and get the ID of its type.
/// let types = validator.validate_all(&wasm1)?;
/// let id1 = types.core_type_at(0);
/// let id1 = types.as_ref().core_type_at(0);
///
/// // Reset the validator so we can parse the second wasm module inside
/// // this validator's same context.
Expand All @@ -411,7 +411,7 @@ impl Validator {
/// // Validate the second Wasm module and get the ID of its second type,
/// // which is the same type as the first Wasm module's only type.
/// let types = validator.validate_all(&wasm2)?;
/// let id2 = types.core_type_at(1);
/// let id2 = types.as_ref().core_type_at(1);
///
/// // Because both modules were processed in the same `Validator`, they
/// // share the same types context and therefore the same type defined
Expand Down Expand Up @@ -1530,12 +1530,13 @@ mod tests {
Validator::new_with_features(WasmFeatures::default() | WasmFeatures::EXCEPTIONS);

let types = validator.validate_all(&bytes)?;
let types = types.as_ref();

assert_eq!(types.type_count(), 2);
assert_eq!(types.core_type_count(), 2);
assert_eq!(types.memory_count(), 1);
assert_eq!(types.table_count(), 1);
assert_eq!(types.global_count(), 1);
assert_eq!(types.core_function_count(), 1);
assert_eq!(types.function_count(), 1);
assert_eq!(types.tag_count(), 1);
assert_eq!(types.element_count(), 1);
assert_eq!(types.module_count(), 0);
Expand Down Expand Up @@ -1621,6 +1622,7 @@ mod tests {
Validator::new_with_features(WasmFeatures::default() | WasmFeatures::COMPONENT_MODEL);

let types = validator.validate_all(&bytes)?;
let types = types.as_ref();

let t_id = types.component_defined_type_at(0);
let a1_id = types.component_defined_type_at(1);
Expand Down Expand Up @@ -1654,6 +1656,7 @@ mod tests {
Validator::new_with_features(WasmFeatures::default() | WasmFeatures::COMPONENT_MODEL);

let types = validator.validate_all(&bytes)?;
let types = types.as_ref();

let t_id = types.component_defined_type_at(0);
let a1_id = types.component_defined_type_at(1);
Expand Down
Loading

0 comments on commit d4532cb

Please sign in to comment.