Implement a wasmparser -> wasm-encoder trait (#1628)

* Implement a reencoder helper trait

* Use new error type in all conversions

* Centralise from impls

* Remove mut ref returns

* rename trait

* reencode the data count section

* refactor core module parsing helper

* Add some more docs

* reorder trait members

* Review comments plus a reencoder fuzzer

* Use `Reencode` in `wit-component`

* Add user error variant

* Fix Reencode impl

* Add an intersperse section hook to allow more complex user module building

* Add docs to section intersperse hook

---------

Co-authored-by: Alex Crichton <alex@alexcrichton.com>

Author: juntyr

crates/wasm-encoder/src/component/types.rs

@@ -527,27 +527,6 @@ impl Encode for PrimitiveValType {
     }
 }
 
-#[cfg(feature = "wasmparser")]
-impl From<wasmparser::PrimitiveValType> for PrimitiveValType {
-    fn from(ty: wasmparser::PrimitiveValType) -> Self {
-        match ty {
-            wasmparser::PrimitiveValType::Bool => PrimitiveValType::Bool,
-            wasmparser::PrimitiveValType::S8 => PrimitiveValType::S8,
-            wasmparser::PrimitiveValType::U8 => PrimitiveValType::U8,
-            wasmparser::PrimitiveValType::S16 => PrimitiveValType::S16,
-            wasmparser::PrimitiveValType::U16 => PrimitiveValType::U16,
-            wasmparser::PrimitiveValType::S32 => PrimitiveValType::S32,
-            wasmparser::PrimitiveValType::U32 => PrimitiveValType::U32,
-            wasmparser::PrimitiveValType::S64 => PrimitiveValType::S64,
-            wasmparser::PrimitiveValType::U64 => PrimitiveValType::U64,
-            wasmparser::PrimitiveValType::F32 => PrimitiveValType::F32,
-            wasmparser::PrimitiveValType::F64 => PrimitiveValType::F64,
-            wasmparser::PrimitiveValType::Char => PrimitiveValType::Char,
-            wasmparser::PrimitiveValType::String => PrimitiveValType::String,
-        }
-    }
-}
-
 /// Represents a component value type.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum ComponentValType {

crates/wasm-encoder/src/core/code.rs

@@ -103,30 +103,6 @@ impl CodeSection {
         self.num_added += 1;
         self
     }
-
-    /// Parses the input `section` given from the `wasmparser` crate and adds
-    /// all the code to this section.
-    #[cfg(feature = "wasmparser")]
-    pub fn parse_section(
-        &mut self,
-        section: wasmparser::CodeSectionReader<'_>,
-    ) -> wasmparser::Result<&mut Self> {
-        for code in section {
-            self.parse(code?)?;
-        }
-        Ok(self)
-    }
-
-    /// Parses a single [`wasmparser::Code`] and adds it to this section.
-    #[cfg(feature = "wasmparser")]
-    pub fn parse(&mut self, func: wasmparser::FunctionBody<'_>) -> wasmparser::Result<&mut Self> {
-        let mut f = Function::new_parsed_locals(&func)?;
-        let mut reader = func.get_operators_reader()?;
-        while !reader.eof() {
-            f.parse(&mut reader)?;
-        }
-        Ok(self.function(&f))
-    }
 }
 
 impl Encode for CodeSection {
@@ -238,18 +214,6 @@ impl Function {
         Function::new(locals_collected)
     }
 
-    /// Create a new [`Function`] by parsing the locals declarations from the
-    /// provided [`wasmparser::FunctionBody`].
-    #[cfg(feature = "wasmparser")]
-    pub fn new_parsed_locals(func: &wasmparser::FunctionBody<'_>) -> wasmparser::Result<Self> {
-        let mut locals = Vec::new();
-        for pair in func.get_locals_reader()? {
-            let (cnt, ty) = pair?;
-            locals.push((cnt, ty.try_into().unwrap()));
-        }
-        Ok(Function::new(locals))
-    }
-
     /// Write an instruction into this function body.
     pub fn instruction(&mut self, instruction: &Instruction) -> &mut Self {
         instruction.encode(&mut self.bytes);
@@ -312,15 +276,6 @@ impl Function {
     pub fn into_raw_body(self) -> Vec<u8> {
         self.bytes
     }
-
-    /// Parses a single instruction from `reader` and adds it to `self`.
-    #[cfg(feature = "wasmparser")]
-    pub fn parse(
-        &mut self,
-        reader: &mut wasmparser::OperatorsReader<'_>,
-    ) -> wasmparser::Result<&mut Self> {
-        Ok(self.instruction(&reader.read()?.try_into().unwrap()))
-    }
 }
 
 impl Encode for Function {
@@ -360,17 +315,6 @@ impl Encode for MemArg {
     }
 }
 
-#[cfg(feature = "wasmparser")]
-impl From<wasmparser::MemArg> for MemArg {
-    fn from(arg: wasmparser::MemArg) -> MemArg {
-        MemArg {
-            offset: arg.offset,
-            align: arg.align.into(),
-            memory_index: arg.memory,
-        }
-    }
-}
-
 /// The memory ordering for atomic instructions.
 ///
 /// For an in-depth explanation of memory orderings, see the C++ documentation
@@ -399,16 +343,6 @@ impl Encode for Ordering {
     }
 }
 
-#[cfg(feature = "wasmparser")]
-impl From<wasmparser::Ordering> for Ordering {
-    fn from(arg: wasmparser::Ordering) -> Ordering {
-        match arg {
-            wasmparser::Ordering::SeqCst => Ordering::SeqCst,
-            wasmparser::Ordering::AcqRel => Ordering::AcqRel,
-        }
-    }
-}
-
 /// Describe an unchecked SIMD lane index.
 pub type Lane = u8;
 
@@ -433,18 +367,6 @@ impl Encode for BlockType {
     }
 }
 
-#[cfg(feature = "wasmparser")]
-impl TryFrom<wasmparser::BlockType> for BlockType {
-    type Error = ();
-    fn try_from(arg: wasmparser::BlockType) -> Result<BlockType, ()> {
-        match arg {
-            wasmparser::BlockType::Empty => Ok(BlockType::Empty),
-            wasmparser::BlockType::FuncType(n) => Ok(BlockType::FunctionType(n)),
-            wasmparser::BlockType::Type(t) => Ok(BlockType::Result(t.try_into()?)),
-        }
-    }
-}
-
 /// WebAssembly instructions.
 #[derive(Clone, Debug)]
 #[non_exhaustive]
@@ -3401,64 +3323,6 @@ impl Encode for Instruction<'_> {
     }
 }
 
-#[cfg(feature = "wasmparser")]
-impl TryFrom<wasmparser::Operator<'_>> for Instruction<'_> {
-    type Error = ();
-
-    fn try_from(arg: wasmparser::Operator<'_>) -> Result<Self, ()> {
-        use Instruction::*;
-
-        macro_rules! define_match {
-            ($(@$p:ident $op:ident $({ $($arg:ident: $argty:ty),* })? => $visit:ident)*) => {
-                match arg {
-                    $(
-                        wasmparser::Operator::$op $({ $($arg),* })? => {
-                            $(
-                                $(let $arg = define_match!(map $arg $arg);)*
-                            )?
-                            Ok(define_match!(mk $op $($($arg)*)?))
-                        }
-                    )*
-                }
-            };
-
-            // No-payload instructions are named the same in wasmparser as they are in
-            // wasm-encoder
-            (mk $op:ident) => ($op);
-
-            // Instructions which need "special care" to map from wasmparser to
-            // wasm-encoder
-            (mk BrTable $arg:ident) => ({BrTable($arg.0, $arg.1)});
-            (mk TryTable $arg:ident) => ({TryTable($arg.0, $arg.1)});
-
-            // Catch-all for the translation of one payload argument which is typically
-            // represented as a tuple-enum in wasm-encoder.
-            (mk $op:ident $arg:ident) => ($op($arg));
-
-            // Catch-all of everything else where the wasmparser fields are simply
-            // translated to wasm-encoder fields.
-            (mk $op:ident $($arg:ident)*) => ($op { $($arg),* });
-
-            // Special-case BrTable/TryTable conversion of arguments.
-            (map $arg:ident targets) => ((
-                $arg.targets().map(|i| i.unwrap()).collect::<Vec<_>>().into(),
-                $arg.default(),
-            ));
-            (map $arg:ident try_table) => ((
-                $arg.ty.try_into().unwrap(),
-                $arg.catches.into_iter().map(|i| i.into()).collect::<Vec<_>>().into(),
-            ));
-
-            // Everything else is converted with `TryFrom`/`From`. Note that the
-            // fallibility here has to do with how indexes are represented in
-            // `wasmparser` which we know when reading directly we'll never hit the
-            // erroneous cases here, hence the unwrap.
-            (map $arg:ident $other:ident) => {$other.try_into().unwrap()};
-        }
-        wasmparser::for_each_operator!(define_match)
-    }
-}
-
 #[derive(Clone, Debug)]
 #[allow(missing_docs)]
 pub enum Catch {
@@ -3493,18 +3357,6 @@ impl Encode for Catch {
     }
 }
 
-#[cfg(feature = "wasmparser")]
-impl From<wasmparser::Catch> for Catch {
-    fn from(arg: wasmparser::Catch) -> Catch {
-        match arg {
-            wasmparser::Catch::One { tag, label } => Catch::One { tag, label },
-            wasmparser::Catch::OneRef { tag, label } => Catch::OneRef { tag, label },
-            wasmparser::Catch::All { label } => Catch::All { label },
-            wasmparser::Catch::AllRef { label } => Catch::AllRef { label },
-        }
-    }
-}
-
 /// A constant expression.
 ///
 /// Usable in contexts such as offsets or initializers.
@@ -3669,101 +3521,6 @@ impl Encode for ConstExpr {
     }
 }
 
-/// An error when converting a `wasmparser::ConstExpr` into a
-/// `wasm_encoder::ConstExpr`.
-#[cfg(feature = "wasmparser")]
-#[derive(Debug)]
-pub enum ConstExprConversionError {
-    /// There was an error when parsing the const expression.
-    ParseError(wasmparser::BinaryReaderError),
-
-    /// The const expression is invalid: not actually constant or something like
-    /// that.
-    Invalid,
-
-    /// There was a type reference that was canonicalized and no longer
-    /// references an index into a module's types space, so we cannot encode it
-    /// into a Wasm binary again.
-    CanonicalizedTypeReference,
-}
-
-#[cfg(feature = "wasmparser")]
-impl From<wasmparser::BinaryReaderError> for ConstExprConversionError {
-    fn from(err: wasmparser::BinaryReaderError) -> ConstExprConversionError {
-        ConstExprConversionError::ParseError(err)
-    }
-}
-
-#[cfg(feature = "wasmparser")]
-impl std::fmt::Display for ConstExprConversionError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Self::ParseError(_e) => {
-                write!(f, "There was an error when parsing the const expression")
-            }
-            Self::Invalid => write!(f, "The const expression was invalid"),
-            Self::CanonicalizedTypeReference => write!(
-                f,
-                "There was a canonicalized type reference without type index information"
-            ),
-        }
-    }
-}
-
-#[cfg(feature = "wasmparser")]
-impl std::error::Error for ConstExprConversionError {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        match self {
-            Self::ParseError(e) => Some(e),
-            Self::Invalid | Self::CanonicalizedTypeReference => None,
-        }
-    }
-}
-
-#[cfg(feature = "wasmparser")]
-impl<'a> TryFrom<wasmparser::ConstExpr<'a>> for ConstExpr {
-    type Error = ConstExprConversionError;
-
-    fn try_from(const_expr: wasmparser::ConstExpr) -> Result<Self, Self::Error> {
-        let mut ops = const_expr.get_operators_reader().into_iter();
-
-        let result = match ops.next() {
-            Some(Ok(wasmparser::Operator::I32Const { value })) => ConstExpr::i32_const(value),
-            Some(Ok(wasmparser::Operator::I64Const { value })) => ConstExpr::i64_const(value),
-            Some(Ok(wasmparser::Operator::F32Const { value })) => {
-                ConstExpr::f32_const(f32::from_bits(value.bits()))
-            }
-            Some(Ok(wasmparser::Operator::F64Const { value })) => {
-                ConstExpr::f64_const(f64::from_bits(value.bits()))
-            }
-            Some(Ok(wasmparser::Operator::V128Const { value })) => {
-                ConstExpr::v128_const(i128::from_le_bytes(*value.bytes()))
-            }
-            Some(Ok(wasmparser::Operator::RefNull { hty })) => ConstExpr::ref_null(
-                HeapType::try_from(hty)
-                    .map_err(|_| ConstExprConversionError::CanonicalizedTypeReference)?,
-            ),
-            Some(Ok(wasmparser::Operator::RefFunc { function_index })) => {
-                ConstExpr::ref_func(function_index)
-            }
-            Some(Ok(wasmparser::Operator::GlobalGet { global_index })) => {
-                ConstExpr::global_get(global_index)
-            }
-
-            // TODO: support the extended-const proposal.
-            Some(Ok(_op)) => return Err(ConstExprConversionError::Invalid),
-
-            Some(Err(e)) => return Err(ConstExprConversionError::ParseError(e)),
-            None => return Err(ConstExprConversionError::Invalid),
-        };
-
-        match (ops.next(), ops.next()) {
-            (Some(Ok(wasmparser::Operator::End)), None) => Ok(result),
-            _ => Err(ConstExprConversionError::Invalid),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     #[test]

crates/wasm-encoder/src/core/custom.rs

@@ -44,16 +44,6 @@ impl Section for RawCustomSection<'_> {
     }
 }
 
-#[cfg(feature = "wasmparser")]
-impl<'a> From<wasmparser::CustomSectionReader<'a>> for CustomSection<'a> {
-    fn from(section: wasmparser::CustomSectionReader<'a>) -> Self {
-        CustomSection {
-            data: section.data().into(),
-            name: section.name().into(),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

crates/wasm-encoder/src/core/data.rs

@@ -151,38 +151,6 @@ impl DataSection {
         self.num_added += 1;
         self
     }
-
-    /// Parses the input `section` given from the `wasmparser` crate and adds
-    /// all the data to this section.
-    #[cfg(feature = "wasmparser")]
-    pub fn parse_section(
-        &mut self,
-        section: wasmparser::DataSectionReader<'_>,
-    ) -> Result<&mut Self, crate::ConstExprConversionError> {
-        for data in section {
-            self.parse(data?)?;
-        }
-        Ok(self)
-    }
-
-    /// Parses a single [`wasmparser::Data`] and adds it to this section.
-    #[cfg(feature = "wasmparser")]
-    pub fn parse(
-        &mut self,
-        data: wasmparser::Data<'_>,
-    ) -> Result<&mut Self, crate::ConstExprConversionError> {
-        match data.kind {
-            wasmparser::DataKind::Active {
-                memory_index,
-                offset_expr,
-            } => Ok(self.active(
-                memory_index,
-                &ConstExpr::try_from(offset_expr)?,
-                data.data.iter().copied(),
-            )),
-            wasmparser::DataKind::Passive => Ok(self.passive(data.data.iter().copied())),
-        }
-    }
 }
 
 impl Encode for DataSection {