Repository: bytecodealliance/wasm-tools
Pull request #1182 (2023-08-25 15:11:21)

Add a helper method to parse types without GC types 
This commit updates the API of the `wasmparser` crate to make it easier
to iterate over a type section without working with GC types. A new
method `into_iter_err_on_gc_types` is added which automatically
"unwraps" any GC features and returns an error, instead returning only
the `FuncType` which is possible with non-GC wasm. This is intended to
make it easier for tools that don't support the GC proposal to continue
to use `wasmparser` and avoid API changes related to the GC proposal
support.

Many users in this repository have been updated as well. For example
those that are translating from one encoding to another can't ignore
`rec` groups as they currently do, instead the `rec` group structure
itself would have to be mirrored which isn't supported right now.


Patch:
diff --git a/crates/wasm-mutate/src/info.rs b/crates/wasm-mutate/src/info.rs
index 3d2415ee..3e2bf0b6 100644
--- a/crates/wasm-mutate/src/info.rs
+++ b/crates/wasm-mutate/src/info.rs
@@ -1,6 +1,6 @@
 use crate::{
     module::{PrimitiveTypeInfo, TypeInfo},
-    Error, Result,
+    Result,
 };
 use std::collections::HashSet;
 use std::convert::TryFrom;
@@ -92,20 +92,8 @@ impl<'a> ModuleInfo<'a> {
                     info.section(SectionId::Type.into(), reader.range(), input_wasm);
 
                     // Save function types
-                    for rec_group in reader {
-                        if let Ok(rg) = rec_group {
-                            if rg.types().len() != 1 {
-                                return Err(Error::unsupported("GC types not supported yet"));
-                            }
-                            for st in rg.types() {
-                                if st.is_final || st.supertype_idx.is_some() {
-                                    return Err(Error::unsupported("GC types not supported yet"));
-                                }
-                                let typeinfo =
-                                    TypeInfo::try_from(st.clone().structural_type).unwrap();
-                                info.types_map.push(typeinfo);
-                            }
-                        }
+                    for ty in reader.into_iter_err_on_gc_types() {
+                        info.types_map.push(ty?.into());
                     }
                 }
                 Payload::ImportSection(reader) => {
diff --git a/crates/wasm-mutate/src/module.rs b/crates/wasm-mutate/src/module.rs
index 4b997df2..4431a14f 100644
--- a/crates/wasm-mutate/src/module.rs
+++ b/crates/wasm-mutate/src/module.rs
@@ -1,5 +1,4 @@
-use crate::{Error, Result};
-use std::convert::TryFrom;
+use crate::Result;
 use wasm_encoder::{BlockType, HeapType, RefType, ValType};
 
 #[derive(Debug, Copy, Clone, PartialEq)]
@@ -49,30 +48,20 @@ impl From<wasmparser::RefType> for PrimitiveTypeInfo {
     }
 }
 
-impl TryFrom<wasmparser::StructuralType> for TypeInfo {
-    type Error = Error;
-
-    fn try_from(value: wasmparser::StructuralType) -> Result<Self> {
-        match value {
-            wasmparser::StructuralType::Func(ft) => Ok(TypeInfo::Func(FuncInfo {
-                params: ft
-                    .params()
-                    .iter()
-                    .map(|&t| PrimitiveTypeInfo::from(t))
-                    .collect(),
-                returns: ft
-                    .results()
-                    .iter()
-                    .map(|&t| PrimitiveTypeInfo::from(t))
-                    .collect(),
-            })),
-            wasmparser::StructuralType::Array(_) => {
-                Err(Error::unsupported("Array types are not supported yet."))
-            }
-            wasmparser::StructuralType::Struct(_) => {
-                Err(Error::unsupported("Struct types are not supported yet."))
-            }
-        }
+impl From<wasmparser::FuncType> for TypeInfo {
+    fn from(ft: wasmparser::FuncType) -> Self {
+        TypeInfo::Func(FuncInfo {
+            params: ft
+                .params()
+                .iter()
+                .map(|&t| PrimitiveTypeInfo::from(t))
+                .collect(),
+            returns: ft
+                .results()
+                .iter()
+                .map(|&t| PrimitiveTypeInfo::from(t))
+                .collect(),
+        })
     }
 }
 
diff --git a/crates/wasm-mutate/src/mutators/add_type.rs b/crates/wasm-mutate/src/mutators/add_type.rs
index ce289412..f2fe0c3e 100644
--- a/crates/wasm-mutate/src/mutators/add_type.rs
+++ b/crates/wasm-mutate/src/mutators/add_type.rs
@@ -2,7 +2,7 @@
 
 use super::Mutator;
 use crate::module::map_type;
-use crate::{Error, Result};
+use crate::Result;
 use rand::Rng;
 use std::iter;
 
@@ -55,32 +55,21 @@ impl Mutator for AddTypeMutator {
         if let Some(old_types) = config.info().get_type_section() {
             // Copy the existing types section over into the encoder.
             let reader = wasmparser::TypeSectionReader::new(old_types.data, 0)?;
-            for rec_group in reader {
-                for ty in rec_group?.types() {
-                    match ty.clone().structural_type {
-                        wasmparser::StructuralType::Func(ty) => {
-                            let params = ty
-                                .params()
-                                .iter()
-                                .copied()
-                                .map(map_type)
-                                .collect::<Result<Vec<_>, _>>()?;
-                            let results = ty
-                                .results()
-                                .iter()
-                                .copied()
-                                .map(map_type)
-                                .collect::<Result<Vec<_>, _>>()?;
-                            types.function(params, results);
-                        }
-                        wasmparser::StructuralType::Array(_) => {
-                            return Err(Error::unsupported("Array types are not supported yet."));
-                        }
-                        wasmparser::StructuralType::Struct(_) => {
-                            return Err(Error::unsupported("Struct types are not supported yet."));
-                        }
-                    }
-                }
+            for ty in reader.into_iter_err_on_gc_types() {
+                let ty = ty?;
+                let params = ty
+                    .params()
+                    .iter()
+                    .copied()
+                    .map(map_type)
+                    .collect::<Result<Vec<_>, _>>()?;
+                let results = ty
+                    .results()
+                    .iter()
+                    .copied()
+                    .map(map_type)
+                    .collect::<Result<Vec<_>, _>>()?;
+                types.function(params, results);
             }
             // And then add our new type.
             types.function(params, results);
diff --git a/crates/wasm-mutate/src/mutators/remove_item.rs b/crates/wasm-mutate/src/mutators/remove_item.rs
index b6e84851..b3283326 100644
--- a/crates/wasm-mutate/src/mutators/remove_item.rs
+++ b/crates/wasm-mutate/src/mutators/remove_item.rs
@@ -17,7 +17,7 @@ use wasm_encoder::*;
 use wasmparser::{
     BinaryReader, CodeSectionReader, DataSectionReader, ElementSectionReader, ExportSectionReader,
     ExternalKind, FromReader, FunctionSectionReader, GlobalSectionReader, ImportSectionReader,
-    MemorySectionReader, Operator, SectionLimited, TableInit, TableSectionReader, TagSectionReader,
+    MemorySectionReader, Operator, TableInit, TableSectionReader, TagSectionReader,
     TypeSectionReader,
 };
 
@@ -134,12 +134,10 @@ impl RemoveItem {
                     self.filter_out(
                         &mut module,
                         0,
-                        TypeSectionReader::new(section.data, 0)?,
+                        TypeSectionReader::new(section.data, 0)?.into_iter_err_on_gc_types(),
                         Item::Type,
-                        |me, rec_group, section| {
-                            for ty in rec_group.types() {
-                                me.translate_type_def(ty.clone().structural_type, section)?;
-                            }
+                        |me, ty, section| {
+                            me.translate_func_type(ty,section)?;
                             Ok(())
                         },
                     )?;
@@ -378,7 +376,7 @@ impl RemoveItem {
         &mut self,
         module: &mut Module,
         offset: u32,
-        section: SectionLimited<'a, S>,
+        section: impl IntoIterator<Item = wasmparser::Result<S>>,
         section_item: Item,
         encode: impl Fn(&mut Self, S, &mut T) -> Result<()>,
     ) -> Result<()>
diff --git a/crates/wasm-mutate/src/mutators/translate.rs b/crates/wasm-mutate/src/mutators/translate.rs
index 6e6c2d28..f3044817 100644
--- a/crates/wasm-mutate/src/mutators/translate.rs
+++ b/crates/wasm-mutate/src/mutators/translate.rs
@@ -1,6 +1,6 @@
 use crate::{Error, Result};
 use wasm_encoder::*;
-use wasmparser::{DataKind, ElementKind, FunctionBody, Global, Operator, StructuralType};
+use wasmparser::{DataKind, ElementKind, FunctionBody, Global, Operator};
 
 #[derive(Debug, Hash, Eq, PartialEq, Copy, Clone)]
 pub enum Item {
@@ -26,8 +26,8 @@ pub enum ConstExprKind {
 pub trait Translator {
     fn as_obj(&mut self) -> &mut dyn Translator;
 
-    fn translate_type_def(&mut self, ty: StructuralType, s: &mut TypeSection) -> Result<()> {
-        type_def(self.as_obj(), ty, s)
+    fn translate_func_type(&mut self, ty: wasmparser::FuncType, s: &mut TypeSection) -> Result<()> {
+        func_type(self.as_obj(), ty, s)
     }
 
     fn translate_table_type(
@@ -122,24 +122,22 @@ impl Translator for DefaultTranslator {
     }
 }
 
-pub fn type_def(t: &mut dyn Translator, ty: StructuralType, s: &mut TypeSection) -> Result<()> {
-    match ty {
-        StructuralType::Func(f) => {
-            s.function(
-                f.params()
-                    .iter()
-                    .map(|ty| t.translate_ty(ty))
-                    .collect::<Result<Vec<_>>>()?,
-                f.results()
-                    .iter()
-                    .map(|ty| t.translate_ty(ty))
-                    .collect::<Result<Vec<_>>>()?,
-            );
-            Ok(())
-        }
-        StructuralType::Array(_) => Err(Error::unsupported("Array types are not supported yet.")),
-        StructuralType::Struct(_) => Err(Error::unsupported("Struct types are not supported yet.")),
-    }
+pub fn func_type(
+    t: &mut dyn Translator,
+    ty: wasmparser::FuncType,
+    s: &mut TypeSection,
+) -> Result<()> {
+    s.function(
+        ty.params()
+            .iter()
+            .map(|ty| t.translate_ty(ty))
+            .collect::<Result<Vec<_>>>()?,
+        ty.results()
+            .iter()
+            .map(|ty| t.translate_ty(ty))
+            .collect::<Result<Vec<_>>>()?,
+    );
+    Ok(())
 }
 
 pub fn table_type(
diff --git a/crates/wasm-smith/src/core.rs b/crates/wasm-smith/src/core.rs
index 6943f8b8..1eea3170 100644
--- a/crates/wasm-smith/src/core.rs
+++ b/crates/wasm-smith/src/core.rs
@@ -547,16 +547,14 @@ impl Module {
         // index in our newly generated module. Initially the option is `None` and will become a
         // `Some` when we encounter an import that uses this signature in the next portion of this
         // function. See also the `make_func_type` closure below.
-        let mut available_types = Vec::<(wasmparser::StructuralType, Option<u32>)>::new();
+        let mut available_types = Vec::new();
         let mut available_imports = Vec::<wasmparser::Import>::new();
         for payload in wasmparser::Parser::new(0).parse_all(&example_module) {
             match payload.expect("could not parse the available import payload") {
                 wasmparser::Payload::TypeSection(type_reader) => {
-                    for ty in type_reader {
-                        let rec_group = ty.expect("could not parse type section");
-                        for ty in rec_group.types() {
-                            available_types.push((ty.clone().structural_type, None));
-                        }
+                    for ty in type_reader.into_iter_err_on_gc_types() {
+                        let ty = ty.expect("could not parse type section");
+                        available_types.push((ty, None));
                     }
                 }
                 wasmparser::Payload::ImportSection(import_reader) => {
@@ -589,7 +587,7 @@ impl Module {
             let serialized_sig_idx = match available_types.get_mut(parsed_sig_idx as usize) {
                 None => panic!("signature index refers to a type out of bounds"),
                 Some((_, Some(idx))) => *idx as usize,
-                Some((wasmparser::StructuralType::Func(func_type), index_store)) => {
+                Some((func_type, index_store)) => {
                     let multi_value_required = func_type.results().len() > 1;
                     let new_index = first_type_index + new_types.len();
                     if new_index >= max_types || (multi_value_required && !multi_value_enabled) {
@@ -611,12 +609,6 @@ impl Module {
                     new_types.push(Type::Func(Rc::clone(&func_type)));
                     new_index
                 }
-                Some((wasmparser::StructuralType::Array(_array_type), _index_store)) => {
-                    unimplemented!("Array types are not supported yet.");
-                }
-                Some((wasmparser::StructuralType::Struct(_struct_type), _index_store)) => {
-                    unimplemented!("Struct types are not supported yet.");
-                }
             };
             match &new_types[serialized_sig_idx - first_type_index] {
                 Type::Func(f) => Some((serialized_sig_idx as u32, Rc::clone(f))),
diff --git a/crates/wasmparser/src/readers/core/types.rs b/crates/wasmparser/src/readers/core/types.rs
index 45f59af2..fad9d426 100644
--- a/crates/wasmparser/src/readers/core/types.rs
+++ b/crates/wasmparser/src/readers/core/types.rs
@@ -1171,6 +1171,30 @@ pub struct TagType {
 /// A reader for the type section of a WebAssembly module.
 pub type TypeSectionReader<'a> = SectionLimited<'a, RecGroup>;
 
+impl<'a> TypeSectionReader<'a> {
+    /// Returns an iterator over this type section which will only yield
+    /// function types and any usage of GC types from the GC proposal will
+    /// be translated into an error.
+    pub fn into_iter_err_on_gc_types(self) -> impl Iterator<Item = Result<FuncType>> + 'a {
+        self.into_iter_with_offsets().map(|item| {
+            let (offset, group) = item?;
+            let ty = match group {
+                RecGroup::Single(ty) => ty,
+                RecGroup::Many(_) => bail!(offset, "gc proposal not supported"),
+            };
+            if ty.is_final || ty.supertype_idx.is_some() {
+                bail!(offset, "gc proposal not supported");
+            }
+            match ty.structural_type {
+                StructuralType::Func(f) => Ok(f),
+                StructuralType::Array(_) | StructuralType::Struct(_) => {
+                    bail!(offset, "gc proposal not supported");
+                }
+            }
+        })
+    }
+}
+
 impl<'a> FromReader<'a> for StructuralType {
     fn from_reader(reader: &mut BinaryReader<'a>) -> Result<Self> {
         read_structural_type(reader.read_u8()?, reader)
diff --git a/crates/wit-component/src/gc.rs b/crates/wit-component/src/gc.rs
index 49a01471..c4e1dff6 100644
--- a/crates/wit-component/src/gc.rs
+++ b/crates/wit-component/src/gc.rs
@@ -178,7 +178,7 @@ type WorklistFunc<'a> = fn(&mut Module<'a>, u32) -> Result<()>;
 #[derive(Default)]
 struct Module<'a> {
     // Definitions found when parsing a module
-    types: Vec<SubType>,
+    types: Vec<FuncType>,
     tables: Vec<Table<'a>>,
     globals: Vec<Global<'a>>,
     memories: Vec<Memory<'a>>,
@@ -245,10 +245,8 @@ impl<'a> Module<'a> {
                 }
                 Payload::End(_) => {}
                 Payload::TypeSection(s) => {
-                    for rec_group in s {
-                        for ty in rec_group?.types() {
-                            self.types.push(ty.clone());
-                        }
+                    for ty in s.into_iter_err_on_gc_types() {
+                        self.types.push(ty?);
                     }
                 }
                 Payload::ImportSection(s) => {
@@ -482,13 +480,6 @@ impl<'a> Module<'a> {
         }
     }
 
-    fn storagety(&mut self, ty: StorageType) {
-        match ty {
-            StorageType::I8 | StorageType::I16 => {}
-            StorageType::Val(ty) => self.valty(ty),
-        }
-    }
-
     fn heapty(&mut self, ty: HeapType) {
         match ty {
             HeapType::Func
@@ -510,21 +501,10 @@ impl<'a> Module<'a> {
             return;
         }
         self.worklist.push((ty, |me, ty| {
-            match me.types[ty as usize].clone().structural_type {
-                StructuralType::Func(ty) => {
-                    for param in ty.params().iter().chain(ty.results()) {
-                        me.valty(*param);
-                    }
-                }
-                StructuralType::Array(ArrayType(ty)) => {
-                    me.storagety(ty.element_type);
-                }
-                StructuralType::Struct(ty) => {
-                    for field in ty.fields.iter() {
-                        me.storagety(field.element_type);
-                    }
-                }
-            };
+            let ty = me.types[ty as usize].clone();
+            for param in ty.params().iter().chain(ty.results()) {
+                me.valty(*param);
+            }
             Ok(())
         }));
     }
@@ -536,7 +516,7 @@ impl<'a> Module<'a> {
         Ok(())
     }
 
-    fn live_types(&self) -> impl Iterator<Item = (u32, &SubType)> + '_ {
+    fn live_types(&self) -> impl Iterator<Item = (u32, &FuncType)> + '_ {
         live_iter(&self.live_types, self.types.iter())
     }
 
@@ -576,15 +556,17 @@ impl<'a> Module<'a> {
         let mut empty_type = None;
         for (i, ty) in self.live_types() {
             map.types.push(i);
-            types.subtype(&map.subtype(ty));
+
+            types.function(
+                ty.params().iter().map(|t| map.valty(*t)),
+                ty.results().iter().map(|t| map.valty(*t)),
+            );
 
             // Keep track of the "empty type" to see if we can reuse an
             // existing one or one needs to be injected if a `start`
             // function is calculated at the end.
-            if let StructuralType::Func(ty) = &ty.structural_type {
-                if ty.params().is_empty() && ty.results().is_empty() {
-                    empty_type = Some(map.types.remap(i));
-                }
+            if ty.params().is_empty() && ty.results().is_empty() {
+                empty_type = Some(map.types.remap(i));
             }
         }
 
@@ -1113,14 +1095,6 @@ impl Encoder {
         }
     }
 
-    fn storagety(&self, ty: StorageType) -> wasm_encoder::StorageType {
-        match ty {
-            StorageType::I8 => wasm_encoder::StorageType::I8,
-            StorageType::I16 => wasm_encoder::StorageType::I16,
-            StorageType::Val(ty) => wasm_encoder::StorageType::Val(self.valty(ty)),
-        }
-    }
-
     fn refty(&self, rt: wasmparser::RefType) -> wasm_encoder::RefType {
         wasm_encoder::RefType {
             nullable: rt.is_nullable(),
@@ -1143,44 +1117,6 @@ impl Encoder {
             HeapType::Indexed(idx) => wasm_encoder::HeapType::Indexed(self.types.remap(idx)),
         }
     }
-
-    fn subtype(&self, ty: &SubType) -> wasm_encoder::SubType {
-        wasm_encoder::SubType {
-            is_final: ty.is_final,
-            supertype_idx: ty.supertype_idx,
-            structural_type: self.structural_type(&ty.structural_type),
-        }
-    }
-
-    fn structural_type(&self, ty: &StructuralType) -> wasm_encoder::StructuralType {
-        match ty {
-            StructuralType::Func(ty) => {
-                wasm_encoder::StructuralType::Func(wasm_encoder::FuncType::new(
-                    ty.params().iter().map(|t| self.valty(*t)),
-                    ty.results().iter().map(|t| self.valty(*t)),
-                ))
-            }
-
-            StructuralType::Struct(ty) => {
-                wasm_encoder::StructuralType::Struct(wasm_encoder::StructType {
-                    fields: ty
-                        .fields
-                        .iter()
-                        .map(|f| wasm_encoder::FieldType {
-                            element_type: self.storagety(f.element_type),
-                            mutable: f.mutable,
-                        })
-                        .collect(),
-                })
-            }
-            StructuralType::Array(ty) => wasm_encoder::StructuralType::Array(
-                wasm_encoder::ArrayType(wasm_encoder::FieldType {
-                    element_type: self.storagety(ty.0.element_type),
-                    mutable: ty.0.mutable,
-                }),
-            ),
-        }
-    }
 }
 
 // This is a helper macro to translate all `wasmparser` instructions to
diff --git a/crates/wit-component/src/linking/metadata.rs b/crates/wit-component/src/linking/metadata.rs
index 63a487f8..6893199f 100644
--- a/crates/wit-component/src/linking/metadata.rs
+++ b/crates/wit-component/src/linking/metadata.rs
@@ -9,7 +9,7 @@ use {
     },
     wasmparser::{
         BinaryReader, BinaryReaderError, ExternalKind, FuncType, Parser, Payload, RefType,
-        StructuralType, Subsection, Subsections, TableType, TypeRef, ValType,
+        Subsection, Subsections, TableType, TypeRef, ValType,
     },
 };
 
@@ -343,22 +343,7 @@ impl<'a> Metadata<'a> {
 
                 Payload::TypeSection(reader) => {
                     types = reader
-                        .into_iter()
-                        .flat_map(|r| match r {
-                            Err(e) => vec![Err(e)],
-                            Ok(rg) => rg
-                                .types()
-                                .iter()
-                                .map(|st| Ok(st.clone()))
-                                .collect::<Vec<_>>(),
-                        })
-                        .filter_map(|r| {
-                            r.map(|ty| match ty.clone().structural_type {
-                                StructuralType::Func(ty) => Some(ty),
-                                StructuralType::Array(_) | StructuralType::Struct(_) => None,
-                            })
-                            .transpose()
-                        })
+                        .into_iter_err_on_gc_types()
                         .collect::<Result<Vec<_>, _>>()?;
                 }
 


Test patch:
diff --git a/crates/wasm-smith/tests/core.rs b/crates/wasm-smith/tests/core.rs
index ada3efab..4a563566 100644
--- a/crates/wasm-smith/tests/core.rs
+++ b/crates/wasm-smith/tests/core.rs
@@ -129,18 +129,8 @@ fn smoke_test_imports_config() {
                 let payload = payload.unwrap();
                 if let wasmparser::Payload::TypeSection(rdr) = payload {
                     // Gather the signature types to later check function types against.
-                    for rec_group in rdr {
-                        for ty in rec_group.unwrap().types() {
-                            match ty.clone().structural_type {
-                                wasmparser::StructuralType::Func(ft) => sig_types.push(ft),
-                                wasmparser::StructuralType::Array(_) => {
-                                    unimplemented!("Array types are not supported yet.")
-                                }
-                                wasmparser::StructuralType::Struct(_) => {
-                                    unimplemented!("Struct types are not supported yet.")
-                                }
-                            }
-                        }
+                    for ty in rdr.into_iter_err_on_gc_types() {
+                        sig_types.push(ty.unwrap());
                     }
                 } else if let wasmparser::Payload::ImportSection(rdr) = payload {
                     // Read out imports, checking that they all are within the list of expected