From b30aa65243bfe82cefd7d1aeddf3c3fba8c2aafa Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 25 Jun 2024 14:43:26 -0400 Subject: [PATCH 01/31] upgrade wasmparser & wasm-encoder to v0.211.0 --- Cargo.toml | 4 +- src/init_expr.rs | 9 +- src/ir/mod.rs | 4 +- src/module/data.rs | 4 +- src/module/debug/expression.rs | 16 +- src/module/elements.rs | 48 ++--- src/module/exports.rs | 7 +- src/module/functions/local_function/emit.rs | 14 +- src/module/functions/local_function/mod.rs | 136 ++++++------- src/module/functions/mod.rs | 7 +- src/module/globals.rs | 22 ++- src/module/imports.rs | 59 +++--- src/module/memories.rs | 32 ++- src/module/mod.rs | 204 ++++++++++---------- src/module/producers.rs | 2 +- src/module/tables.rs | 35 +++- src/module/types.rs | 11 +- src/ty.rs | 27 ++- 18 files changed, 344 insertions(+), 297 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bb3c7d78..501df342 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,8 +29,8 @@ leb128 = "0.2.4" log = "0.4.8" rayon = { version = "1.1.0", optional = true } walrus-macro = { path = './crates/macro', version = '=0.19.0' } -wasm-encoder = "0.41.0" -wasmparser = "0.80.2" +wasm-encoder = "0.211.0" +wasmparser = "0.211.0" gimli = "0.26.0" [features] diff --git a/src/init_expr.rs b/src/init_expr.rs index 5c9f9bc8..391db297 100644 --- a/src/init_expr.rs +++ b/src/init_expr.rs @@ -22,7 +22,7 @@ pub enum InitExpr { } impl InitExpr { - pub(crate) fn eval(init: &wasmparser::InitExpr, ids: &IndicesToIds) -> Result { + pub(crate) fn eval(init: &wasmparser::ConstExpr, ids: &IndicesToIds) -> Result { use wasmparser::Operator::*; let mut reader = init.get_operators_reader(); let val = match reader.read()? { @@ -32,7 +32,8 @@ impl InitExpr { F64Const { value } => InitExpr::Value(Value::F64(f64::from_bits(value.bits()))), V128Const { value } => InitExpr::Value(Value::V128(v128_to_u128(&value))), GlobalGet { global_index } => InitExpr::Global(ids.get_global(global_index)?), - RefNull { ty } => InitExpr::RefNull(ValType::parse(&ty)?), + // TODO: handle RefNull + // RefNull { hty } => InitExpr::RefNull(ValType::parse(&hty)?), RefFunc { function_index } => InitExpr::RefFunc(ids.get_func(function_index)?), _ => bail!("invalid constant expression"), }; @@ -57,8 +58,8 @@ impl InitExpr { wasm_encoder::ConstExpr::global_get(cx.indices.get_global_index(*g)) } InitExpr::RefNull(ty) => wasm_encoder::ConstExpr::ref_null(match ty { - ValType::Externref => wasm_encoder::HeapType::Extern, - ValType::Funcref => wasm_encoder::HeapType::Func, + ValType::Externref => wasm_encoder::HeapType::Abstract { shared: false, ty: wasm_encoder::AbstractHeapType::Extern }, + ValType::Funcref => wasm_encoder::HeapType::Abstract { shared: false, ty: wasm_encoder::AbstractHeapType::Func }, _ => unreachable!(), }), InitExpr::RefFunc(f) => { diff --git a/src/ir/mod.rs b/src/ir/mod.rs index e02ef54f..c4dfc251 100644 --- a/src/ir/mod.rs +++ b/src/ir/mod.rs @@ -830,8 +830,8 @@ pub enum BinaryOp { I8x16NarrowI16x8U, I16x8NarrowI32x4S, I16x8NarrowI32x4U, - I8x16RoundingAverageU, - I16x8RoundingAverageU, + I8x16AvgrU, + I16x8AvgrU, I8x16MinS, I8x16MinU, diff --git a/src/module/data.rs b/src/module/data.rs index 57139227..d4fc10c9 100644 --- a/src/module/data.rs +++ b/src/module/data.rs @@ -222,7 +222,7 @@ impl Module { } wasmparser::DataKind::Active { memory_index, - init_expr, + offset_expr, } => { data.value = segment.data.to_vec(); @@ -230,7 +230,7 @@ impl Module { let memory = self.memories.get_mut(memory_id); memory.data_segments.insert(data.id); - let offset = InitExpr::eval(&init_expr, ids) + let offset = InitExpr::eval(&offset_expr, ids) .with_context(|| format!("in segment {}", i))?; data.kind = DataKind::Active(ActiveData { memory: memory_id, diff --git a/src/module/debug/expression.rs b/src/module/debug/expression.rs index 3c95bbd8..582ff828 100644 --- a/src/module/debug/expression.rs +++ b/src/module/debug/expression.rs @@ -1,6 +1,6 @@ use crate::{CodeTransform, Function, InstrLocId, ModuleFunctions}; use id_arena::Id; -use std::cmp::Ordering; +use std::{cmp::Ordering, ops::Range}; use super::dwarf::AddressSearchPreference; @@ -22,7 +22,7 @@ pub(crate) enum CodeAddress { /// Converts original code address to CodeAddress pub(crate) struct CodeAddressGenerator { /// Function range based convert table - address_convert_table: Vec<(wasmparser::Range, Id)>, + address_convert_table: Vec<(Range, Id)>, /// Instrument based convert table instrument_address_convert_table: Vec<(usize, InstrLocId)>, } @@ -31,7 +31,7 @@ impl CodeAddressGenerator { pub(crate) fn new(funcs: &ModuleFunctions) -> Self { let mut address_convert_table = funcs .iter_local() - .filter_map(|(func_id, func)| func.original_range.map(|range| (range, func_id))) + .filter_map(|(func_id, func)| func.original_range.clone().map(|range| (range, func_id))) .collect::>(); let mut instrument_address_convert_table = funcs @@ -75,7 +75,7 @@ impl CodeAddressGenerator { }; // If the address is not mapped to any instruction, falling back to function-range-based comparison. - let inclusive_range_comparor = |range: &(wasmparser::Range, Id)| { + let inclusive_range_comparor = |range: &(Range, Id)| { // range.start < address <= range.end if range.0.end < address { Ordering::Less @@ -85,7 +85,7 @@ impl CodeAddressGenerator { Ordering::Equal } }; - let exclusive_range_comparor = |range: &(wasmparser::Range, Id)| { + let exclusive_range_comparor = |range: &(Range, Id)| { // normal comparison: range.start <= address < range.end if range.0.end <= address { Ordering::Less @@ -189,7 +189,7 @@ mod tests { crate::FunctionBuilder::new(&mut module.types, &[], &[]), ); - func1.original_range = Some(wasmparser::Range { start: 20, end: 30 }); + func1.original_range = Some(Range { start: 20, end: 30 }); let id1 = module.funcs.add_local(func1); @@ -198,7 +198,7 @@ mod tests { crate::FunctionBuilder::new(&mut module.types, &[], &[]), ); - func2.original_range = Some(wasmparser::Range { start: 30, end: 50 }); + func2.original_range = Some(Range { start: 30, end: 50 }); let id2 = module.funcs.add_local(func2); @@ -262,7 +262,7 @@ mod tests { { code_transform .function_ranges - .push((id1, wasmparser::Range { start: 50, end: 80 })); + .push((id1, Range { start: 50, end: 80 })); code_transform.instruction_map.push((instr_id1, 60)); code_transform.instruction_map.push((instr_id2, 65)); } diff --git a/src/module/elements.rs b/src/module/elements.rs index e5e8750d..6b444f65 100644 --- a/src/module/elements.rs +++ b/src/module/elements.rs @@ -110,36 +110,35 @@ impl Module { ) -> Result<()> { log::debug!("parse element section"); for (i, segment) in section.into_iter().enumerate() { - let segment = segment?; - let ty = ValType::parse(&segment.ty)?; - match ty { - ValType::Funcref => {} - _ => bail!("only funcref type allowed in element segments"), - } - let members = segment - .items - .get_items_reader()? - .into_iter() - .map(|e| -> Result<_> { - Ok(match e? { - wasmparser::ElementItem::Func(f) => Some(ids.get_func(f)?), - wasmparser::ElementItem::Null(_) => None, + let element = segment?; + + let members = match element.items { + wasmparser::ElementItems::Functions(f) => f + .into_iter() + .map(|f| -> Result<_> { + match ids.get_func(f?) { + Ok(f) => Ok(Some(f)), + Err(_) => Ok(None), + } }) - }) - .collect::>()?; + .collect::>()?, + wasmparser::ElementItems::Expressions(_, _) => todo!(), + }; + let id = self.elements.arena.next_id(); - let kind = match segment.kind { + let kind = match element.kind { wasmparser::ElementKind::Passive => ElementKind::Passive, wasmparser::ElementKind::Declared => ElementKind::Declared, wasmparser::ElementKind::Active { table_index, - init_expr, + offset_expr, } => { - let table = ids.get_table(table_index)?; + // TODO: check if this is correct + let table = ids.get_table(table_index.unwrap_or_default())?; self.tables.get_mut(table).elem_segments.insert(id); - let offset = InitExpr::eval(&init_expr, ids) + let offset = InitExpr::eval(&offset_expr, ids) .with_context(|| format!("in segment {}", i))?; match offset { InitExpr::Value(Value::I32(_)) => {} @@ -152,7 +151,7 @@ impl Module { }; self.elements.arena.alloc(Element { id, - ty, + ty: ValType::Funcref, kind, members, name: None, @@ -182,7 +181,12 @@ impl Emit for ModuleElements { Some(func) => { wasm_encoder::ConstExpr::ref_func(cx.indices.get_func_index(*func)) } - None => wasm_encoder::ConstExpr::ref_null(wasm_encoder::HeapType::Func), + None => { + wasm_encoder::ConstExpr::ref_null(wasm_encoder::HeapType::Abstract { + shared: false, + ty: wasm_encoder::AbstractHeapType::Func, + }) + } }) .collect(); let els = wasm_encoder::Elements::Expressions( diff --git a/src/module/exports.rs b/src/module/exports.rs index f51042f9..1416facc 100644 --- a/src/module/exports.rs +++ b/src/module/exports.rs @@ -164,20 +164,17 @@ impl Module { for entry in section { let entry = entry?; let item = match entry.kind { - Function => ExportItem::Function(ids.get_func(entry.index)?), + Func => ExportItem::Function(ids.get_func(entry.index)?), Table => ExportItem::Table(ids.get_table(entry.index)?), Memory => ExportItem::Memory(ids.get_memory(entry.index)?), Global => ExportItem::Global(ids.get_global(entry.index)?), - Type | Module | Instance => { - unimplemented!("module linking not supported"); - } Tag => { unimplemented!("exception handling not supported"); } }; self.exports.arena.alloc_with_id(|id| Export { id, - name: entry.field.to_string(), + name: entry.name.to_string(), item, }); } diff --git a/src/module/functions/local_function/emit.rs b/src/module/functions/local_function/emit.rs index 226f5a46..be3127df 100644 --- a/src/module/functions/local_function/emit.rs +++ b/src/module/functions/local_function/emit.rs @@ -356,7 +356,7 @@ impl<'instr> Visitor<'instr> for Emit<'_> { I8x16MinU => Instruction::I8x16MinU, I8x16MaxS => Instruction::I8x16MaxS, I8x16MaxU => Instruction::I8x16MaxU, - I8x16RoundingAverageU => Instruction::I8x16AvgrU, + I8x16AvgrU => Instruction::I8x16AvgrU, I16x8NarrowI32x4S => Instruction::I16x8NarrowI32x4S, I16x8NarrowI32x4U => Instruction::I16x8NarrowI32x4U, @@ -374,7 +374,7 @@ impl<'instr> Visitor<'instr> for Emit<'_> { I16x8MinU => Instruction::I16x8MinU, I16x8MaxS => Instruction::I16x8MaxS, I16x8MaxU => Instruction::I16x8MaxU, - I16x8RoundingAverageU => Instruction::I16x8AvgrU, + I16x8AvgrU => Instruction::I16x8AvgrU, I32x4Shl => Instruction::I32x4Shl, I32x4ShrS => Instruction::I32x4ShrS, @@ -772,8 +772,14 @@ impl<'instr> Visitor<'instr> for Emit<'_> { TableSize(e) => Instruction::TableSize(self.indices.get_table_index(e.table)), TableFill(e) => Instruction::TableFill(self.indices.get_table_index(e.table)), RefNull(e) => Instruction::RefNull(match &e.ty { - crate::ValType::Externref => wasm_encoder::HeapType::Extern, - crate::ValType::Funcref => wasm_encoder::HeapType::Func, + crate::ValType::Externref => wasm_encoder::HeapType::Abstract { + shared: false, + ty: wasm_encoder::AbstractHeapType::Extern, + }, + crate::ValType::Funcref => wasm_encoder::HeapType::Abstract { + shared: false, + ty: wasm_encoder::AbstractHeapType::Func, + }, _ => unreachable!(), }), RefIsNull(_) => Instruction::RefIsNull, diff --git a/src/module/functions/local_function/mod.rs b/src/module/functions/local_function/mod.rs index 19931cfc..38d7592c 100644 --- a/src/module/functions/local_function/mod.rs +++ b/src/module/functions/local_function/mod.rs @@ -10,7 +10,8 @@ use crate::map::{IdHashMap, IdHashSet}; use crate::parse::IndicesToIds; use crate::{Data, DataId, FunctionBuilder, FunctionId, MemoryId, Module, Result, TypeId, ValType}; use std::collections::BTreeMap; -use wasmparser::{FuncValidator, Operator, Range, ValidatorResources}; +use std::ops::Range; +use wasmparser::{FuncValidator, Operator, ValidatorResources}; /// A function defined locally within the wasm module. #[derive(Debug)] @@ -25,7 +26,7 @@ pub struct LocalFunction { pub instruction_mapping: Vec<(usize, InstrLocId)>, /// Original function binary range. - pub original_range: Option, + pub original_range: Option>, } impl LocalFunction { @@ -62,7 +63,7 @@ impl LocalFunction { builder: FunctionBuilder::without_entry(ty), args, instruction_mapping: Vec::new(), - original_range: Some(wasmparser::Range { + original_range: Some(Range:: { start: body.range().start - code_address_offset - (function_body_size_bit as usize), end: body.range().end - code_address_offset, }), @@ -276,29 +277,25 @@ impl LocalFunction { } } -fn block_result_tys( - ctx: &ValidationContext, - ty: wasmparser::TypeOrFuncType, -) -> Result> { +fn block_result_tys(ctx: &ValidationContext, ty: wasmparser::BlockType) -> Result> { match ty { - wasmparser::TypeOrFuncType::Type(ty) => ValType::from_wasmparser_type(ty).map(Into::into), - wasmparser::TypeOrFuncType::FuncType(idx) => { + wasmparser::BlockType::Type(ty) => ValType::from_wasmparser_type(ty).map(Into::into), + wasmparser::BlockType::FuncType(idx) => { let ty = ctx.indices.get_type(idx)?; Ok(ctx.module.types.results(ty).into()) } + wasmparser::BlockType::Empty => Ok(Box::new([])), } } -fn block_param_tys( - ctx: &ValidationContext, - ty: wasmparser::TypeOrFuncType, -) -> Result> { +fn block_param_tys(ctx: &ValidationContext, ty: wasmparser::BlockType) -> Result> { match ty { - wasmparser::TypeOrFuncType::Type(_) => Ok([][..].into()), - wasmparser::TypeOrFuncType::FuncType(idx) => { + wasmparser::BlockType::Type(_) => Ok([][..].into()), + wasmparser::BlockType::FuncType(idx) => { let ty = ctx.indices.get_type(idx)?; Ok(ctx.module.types.params(ty).into()) } + wasmparser::BlockType::Empty => Ok(Box::new([])), } } @@ -325,16 +322,15 @@ fn append_instruction<'context>( ctx.alloc_instr(Binop { op }, loc); }; - let mem_arg = - |ctx: &mut ValidationContext, arg: &wasmparser::MemoryImmediate| -> (MemoryId, MemArg) { - ( - ctx.indices.get_memory(arg.memory).unwrap(), - MemArg { - align: 1 << (arg.align as i32), - offset: arg.offset as u32, - }, - ) - }; + let mem_arg = |ctx: &mut ValidationContext, arg: &wasmparser::MemArg| -> (MemoryId, MemArg) { + ( + ctx.indices.get_memory(arg.memory).unwrap(), + MemArg { + align: 1 << (arg.align as i32), + offset: arg.offset as u32, + }, + ) + }; let load = |ctx: &mut ValidationContext, arg, kind| { let (memory, arg) = mem_arg(ctx, &arg); @@ -373,8 +369,11 @@ fn append_instruction<'context>( let func = ctx.indices.get_func(function_index).unwrap(); ctx.alloc_instr(Call { func }, loc); } - Operator::CallIndirect { index, table_index } => { - let type_id = ctx.indices.get_type(index).unwrap(); + Operator::CallIndirect { + type_index, + table_index, + } => { + let type_id = ctx.indices.get_type(type_index).unwrap(); let table = ctx.indices.get_table(table_index).unwrap(); ctx.alloc_instr(CallIndirect { table, ty: type_id }, loc); } @@ -562,25 +561,25 @@ fn append_instruction<'context>( ctx.alloc_instr(Unreachable {}, loc); ctx.unreachable(); } - Operator::Block { ty } => { - let param_tys = block_param_tys(ctx, ty).unwrap(); - let result_tys = block_result_tys(ctx, ty).unwrap(); + Operator::Block { blockty } => { + let param_tys = block_param_tys(ctx, blockty).unwrap(); + let result_tys = block_result_tys(ctx, blockty).unwrap(); let seq = ctx .push_control(BlockKind::Block, param_tys, result_tys) .unwrap(); ctx.alloc_instr_in_control(1, Block { seq }, loc).unwrap(); } - Operator::Loop { ty } => { - let result_tys = block_result_tys(ctx, ty).unwrap(); - let param_tys = block_param_tys(ctx, ty).unwrap(); + Operator::Loop { blockty } => { + let result_tys = block_result_tys(ctx, blockty).unwrap(); + let param_tys = block_param_tys(ctx, blockty).unwrap(); let seq = ctx .push_control(BlockKind::Loop, param_tys, result_tys) .unwrap(); ctx.alloc_instr_in_control(1, Loop { seq }, loc).unwrap(); } - Operator::If { ty } => { - let result_tys = block_result_tys(ctx, ty).unwrap(); - let param_tys = block_param_tys(ctx, ty).unwrap(); + Operator::If { blockty } => { + let result_tys = block_result_tys(ctx, blockty).unwrap(); + let param_tys = block_param_tys(ctx, blockty).unwrap(); let consequent = ctx .push_control(BlockKind::If, param_tys, result_tys) @@ -672,22 +671,19 @@ fn append_instruction<'context>( ctx.alloc_instr(BrIf { block }, loc); } - Operator::BrTable { table } => { - let mut blocks = Vec::with_capacity(table.len()); - let mut default = None; - for pair in table.targets() { - let (target, is_default) = pair.unwrap(); + Operator::BrTable { targets } => { + let mut blocks = Vec::with_capacity(targets.len() as usize); + let default = ctx.control(targets.default() as usize).unwrap().block; + for target in targets.targets() { + let target = target.unwrap(); let control = ctx.control(target as usize).unwrap(); - if is_default { - default = Some(control.block); - } else { - blocks.push(control.block); - } + + blocks.push(control.block); } ctx.alloc_instr( BrTable { blocks: blocks.into(), - default: default.unwrap(), + default, }, loc, ); @@ -702,18 +698,18 @@ fn append_instruction<'context>( let memory = ctx.indices.get_memory(mem).unwrap(); ctx.alloc_instr(MemoryGrow { memory }, loc); } - Operator::MemoryInit { segment, mem } => { + Operator::MemoryInit { data_index, mem } => { let memory = ctx.indices.get_memory(mem).unwrap(); - let data = ctx.indices.get_data(segment).unwrap(); + let data = ctx.indices.get_data(data_index).unwrap(); ctx.alloc_instr(MemoryInit { memory, data }, loc); } - Operator::DataDrop { segment } => { - let data = ctx.indices.get_data(segment).unwrap(); + Operator::DataDrop { data_index } => { + let data = ctx.indices.get_data(data_index).unwrap(); ctx.alloc_instr(DataDrop { data }, loc); } - Operator::MemoryCopy { src, dst } => { - let src = ctx.indices.get_memory(src).unwrap(); - let dst = ctx.indices.get_memory(dst).unwrap(); + Operator::MemoryCopy { dst_mem, src_mem } => { + let src = ctx.indices.get_memory(src_mem).unwrap(); + let dst = ctx.indices.get_memory(dst_mem).unwrap(); ctx.alloc_instr(MemoryCopy { src, dst }, loc); } Operator::MemoryFill { mem } => { @@ -750,7 +746,7 @@ fn append_instruction<'context>( Operator::I64Store16 { memarg } => store(ctx, memarg, StoreKind::I64_16 { atomic: false }), Operator::I64Store32 { memarg } => store(ctx, memarg, StoreKind::I64_32 { atomic: false }), - Operator::AtomicFence { flags: _ } => ctx.alloc_instr(AtomicFence {}, loc), + Operator::AtomicFence => ctx.alloc_instr(AtomicFence {}, loc), Operator::I32AtomicLoad { memarg } => load(ctx, memarg, LoadKind::I32 { atomic: true }), Operator::I64AtomicLoad { memarg } => load(ctx, memarg, LoadKind::I64 { atomic: true }), @@ -1002,9 +998,13 @@ fn append_instruction<'context>( let table = ctx.indices.get_table(table).unwrap(); ctx.alloc_instr(TableFill { table }, loc); } - Operator::RefNull { ty } => { - let ty = ValType::parse(&ty).unwrap(); - ctx.alloc_instr(RefNull { ty }, loc); + Operator::RefNull { hty: _ } => { + // TODO: + // RefNull definition has changed in wasmparser. + // Should we update accordingly? + + // let ty = ValType::parse(&hty).unwrap(); + // ctx.alloc_instr(RefNull { ty }, loc); } Operator::RefIsNull => { ctx.alloc_instr(RefIsNull {}, loc); @@ -1273,8 +1273,8 @@ fn append_instruction<'context>( Operator::V128Load16x4U { memarg } => load_simd(ctx, memarg, LoadSimdKind::V128Load16x4U), Operator::V128Load32x2S { memarg } => load_simd(ctx, memarg, LoadSimdKind::V128Load32x2S), Operator::V128Load32x2U { memarg } => load_simd(ctx, memarg, LoadSimdKind::V128Load32x2U), - Operator::I8x16RoundingAverageU => binop(ctx, BinaryOp::I8x16RoundingAverageU), - Operator::I16x8RoundingAverageU => binop(ctx, BinaryOp::I16x8RoundingAverageU), + Operator::I8x16AvgrU => binop(ctx, BinaryOp::I8x16AvgrU), + Operator::I16x8AvgrU => binop(ctx, BinaryOp::I16x8AvgrU), Operator::I8x16MinS => binop(ctx, BinaryOp::I8x16MinS), Operator::I8x16MinU => binop(ctx, BinaryOp::I8x16MinU), @@ -1305,26 +1305,28 @@ fn append_instruction<'context>( ctx.alloc_instr(TableCopy { src, dst }, loc); } - Operator::TableInit { segment, table } => { - let elem = ctx.indices.get_element(segment).unwrap(); + Operator::TableInit { elem_index, table } => { + let elem = ctx.indices.get_element(elem_index).unwrap(); let table = ctx.indices.get_table(table).unwrap(); ctx.alloc_instr(TableInit { elem, table }, loc); } - Operator::ElemDrop { segment } => { - let elem = ctx.indices.get_element(segment).unwrap(); + Operator::ElemDrop { elem_index } => { + let elem = ctx.indices.get_element(elem_index).unwrap(); ctx.alloc_instr(ElemDrop { elem }, loc); } Operator::ReturnCall { .. } | Operator::ReturnCallIndirect { .. } - | Operator::Try { ty: _ } - | Operator::Catch { index: _ } - | Operator::Throw { index: _ } + | Operator::Try { blockty: _ } + | Operator::Catch { tag_index: _ } + | Operator::Throw { tag_index: _ } | Operator::Rethrow { relative_depth: _ } | Operator::Delegate { relative_depth: _ } | Operator::CatchAll => { unimplemented!("not supported") } + + _ => todo!("Many operators are not yet implemented"), } } diff --git a/src/module/functions/mod.rs b/src/module/functions/mod.rs index 530ce720..4d4d4625 100644 --- a/src/module/functions/mod.rs +++ b/src/module/functions/mod.rs @@ -2,10 +2,11 @@ use std::cmp; use std::collections::BTreeMap; +use std::ops::Range; use anyhow::{bail, Context}; use wasm_encoder::Encode; -use wasmparser::{FuncValidator, FunctionBody, Range, ValidatorResources}; +use wasmparser::{FuncValidator, FunctionBody, ValidatorResources}; #[cfg(feature = "parallel")] use rayon::prelude::*; @@ -383,7 +384,7 @@ impl Module { for _ in 0..reader.read_var_u32()? { let pos = reader.original_position(); let count = reader.read_var_u32()?; - let ty = reader.read_type()?; + let ty = reader.read()?; validator.define_locals(pos, count, ty)?; let ty = ValType::parse(&ty)?; for _ in 0..count { @@ -689,7 +690,7 @@ mod tests { #[test] fn get_memory_id() { let mut module = Module::default(); - let expected_id = module.memories.add_local(false, 0, None); + let expected_id = module.memories.add_local(false, false, 0, None); assert!(module.get_memory_id().is_ok_and(|id| id == expected_id)); } diff --git a/src/module/globals.rs b/src/module/globals.rs index 399f04e7..ed3de487 100644 --- a/src/module/globals.rs +++ b/src/module/globals.rs @@ -17,6 +17,8 @@ pub struct Global { pub ty: ValType, /// Whether this global is mutable or not. pub mutable: bool, + /// Whether this global is shared or not. + pub shared: bool, /// The kind of global this is pub kind: GlobalKind, /// The name of this data, used for debugging purposes in the `name` @@ -51,11 +53,18 @@ pub struct ModuleGlobals { impl ModuleGlobals { /// Adds a new imported global to this list. - pub fn add_import(&mut self, ty: ValType, mutable: bool, import_id: ImportId) -> GlobalId { + pub fn add_import( + &mut self, + ty: ValType, + mutable: bool, + shared: bool, + import_id: ImportId, + ) -> GlobalId { self.arena.alloc_with_id(|id| Global { id, ty, mutable, + shared, kind: GlobalKind::Import(import_id), name: None, }) @@ -63,11 +72,18 @@ impl ModuleGlobals { /// Construct a new global, that does not originate from any of the input /// wasm globals. - pub fn add_local(&mut self, ty: ValType, mutable: bool, init: InitExpr) -> GlobalId { + pub fn add_local( + &mut self, + ty: ValType, + mutable: bool, + shared: bool, + init: InitExpr, + ) -> GlobalId { self.arena.alloc_with_id(|id| Global { id, ty, mutable, + shared, kind: GlobalKind::Local(init), name: None, }) @@ -110,6 +126,7 @@ impl Module { let id = self.globals.add_local( ValType::parse(&g.ty.content_type)?, g.ty.mutable, + g.ty.shared, InitExpr::eval(&g.init_expr, ids)?, ); ids.push_global(id); @@ -144,6 +161,7 @@ impl Emit for ModuleGlobals { wasm_encoder::GlobalType { val_type: global.ty.to_wasmencoder_type(), mutable: global.mutable, + shared: global.shared, }, &local.to_wasmencoder_type(cx), ); diff --git a/src/module/imports.rs b/src/module/imports.rs index e7e02289..3ff6c1bf 100644 --- a/src/module/imports.rs +++ b/src/module/imports.rs @@ -153,53 +153,48 @@ impl Module { for entry in section { let entry = entry?; match entry.ty { - wasmparser::ImportSectionEntryType::Function(idx) => { + wasmparser::TypeRef::Func(idx) => { let ty = ids.get_type(idx)?; - let id = self.add_import_func( - entry.module, - entry.field.expect("module linking not supported"), - ty, - ); + let id = self.add_import_func(entry.module, entry.name, ty); ids.push_func(id.0); } - wasmparser::ImportSectionEntryType::Table(t) => { - let ty = ValType::parse(&t.element_type)?; + wasmparser::TypeRef::Table(t) => { + let ty = ValType::parse(&wasmparser::ValType::Ref(t.element_type))?; let id = self.add_import_table( entry.module, - entry.field.expect("module linking not supported"), + entry.name, + t.table64, t.initial, t.maximum, ty, ); ids.push_table(id.0); } - wasmparser::ImportSectionEntryType::Memory(m) => { + wasmparser::TypeRef::Memory(m) => { if m.memory64 { bail!("64-bit memories not supported") }; let id = self.add_import_memory( entry.module, - entry.field.expect("module linking not supported"), + entry.name, m.shared, - m.initial as u32, - m.maximum.map(|m| m as u32), + m.memory64, + m.initial, + m.maximum, ); ids.push_memory(id.0); } - wasmparser::ImportSectionEntryType::Global(g) => { + wasmparser::TypeRef::Global(g) => { let id = self.add_import_global( entry.module, - entry.field.expect("module linking not supported"), + entry.name, ValType::parse(&g.content_type)?, g.mutable, + g.shared, ); ids.push_global(id.0); } - wasmparser::ImportSectionEntryType::Module(_) - | wasmparser::ImportSectionEntryType::Instance(_) => { - unimplemented!("component model not implemented"); - } - wasmparser::ImportSectionEntryType::Tag(_) => { + wasmparser::TypeRef::Tag(_) => { unimplemented!("exception handling not implemented"); } } @@ -227,11 +222,14 @@ impl Module { module: &str, name: &str, shared: bool, - initial: u32, - maximum: Option, + memory64: bool, + initial: u64, + maximum: Option, ) -> (MemoryId, ImportId) { let import = self.imports.arena.next_id(); - let mem = self.memories.add_import(shared, initial, maximum, import); + let mem = self + .memories + .add_import(shared, memory64, initial, maximum, import); self.imports.add(module, name, mem); (mem, import) } @@ -241,12 +239,15 @@ impl Module { &mut self, module: &str, name: &str, - initial: u32, - max: Option, + table64: bool, + initial: u64, + maximum: Option, ty: ValType, ) -> (TableId, ImportId) { let import = self.imports.arena.next_id(); - let table = self.tables.add_import(initial, max, ty, import); + let table = self + .tables + .add_import(table64, initial, maximum, ty, import); self.imports.add(module, name, table); (table, import) } @@ -258,9 +259,10 @@ impl Module { name: &str, ty: ValType, mutable: bool, + shared: bool, ) -> (GlobalId, ImportId) { let import = self.imports.arena.next_id(); - let global = self.globals.add_import(ty, mutable, import); + let global = self.globals.add_import(ty, mutable, shared, import); self.imports.add(module, name, global); (global, import) } @@ -297,6 +299,7 @@ impl Emit for ModuleImports { ValType::Funcref => wasm_encoder::RefType::FUNCREF, _ => panic!("Unexpected table type"), }, + table64: table.table64, minimum: table.initial, maximum: table.maximum, }) @@ -309,6 +312,7 @@ impl Emit for ModuleImports { maximum: mem.maximum.map(|v| v as u64), memory64: false, shared: mem.shared, + page_size_log2: None, }) } ImportKind::Global(id) => { @@ -317,6 +321,7 @@ impl Emit for ModuleImports { wasm_encoder::EntityType::Global(wasm_encoder::GlobalType { val_type: g.ty.to_wasmencoder_type(), mutable: g.mutable, + shared: g.shared, }) } }, diff --git a/src/module/memories.rs b/src/module/memories.rs index 48acc339..eb6d2761 100644 --- a/src/module/memories.rs +++ b/src/module/memories.rs @@ -16,10 +16,12 @@ pub struct Memory { id: MemoryId, /// Is this memory shared? pub shared: bool, + /// Whether or not this is a 64-bit memory. + pub memory64: bool, /// The initial page size for this memory. - pub initial: u32, + pub initial: u64, /// The maximum page size for this memory. - pub maximum: Option, + pub maximum: Option, /// Whether or not this memory is imported, and if so from where. pub import: Option, /// Active data segments that will be used to initialize this memory. @@ -53,14 +55,16 @@ impl ModuleMemories { pub fn add_import( &mut self, shared: bool, - initial: u32, - maximum: Option, + memory64: bool, + initial: u64, + maximum: Option, import: ImportId, ) -> MemoryId { let id = self.arena.next_id(); let id2 = self.arena.alloc(Memory { id, shared, + memory64, initial, maximum, import: Some(import), @@ -73,11 +77,18 @@ impl ModuleMemories { /// Construct a new memory, that does not originate from any of the input /// wasm memories. - pub fn add_local(&mut self, shared: bool, initial: u32, maximum: Option) -> MemoryId { + pub fn add_local( + &mut self, + shared: bool, + memory64: bool, + initial: u64, + maximum: Option, + ) -> MemoryId { let id = self.arena.next_id(); let id2 = self.arena.alloc(Memory { id, shared, + memory64, initial, maximum, import: None, @@ -135,9 +146,9 @@ impl Module { if m.memory64 { bail!("64-bit memories not supported") }; - let id = - self.memories - .add_local(m.shared, m.initial as u32, m.maximum.map(|m| m as u32)); + let id = self + .memories + .add_local(m.shared, m.memory64, m.initial, m.maximum); ids.push_memory(id); } Ok(()) @@ -164,6 +175,7 @@ impl Emit for ModuleMemories { maximum: memory.maximum.map(|v| v as u64), memory64: false, shared: memory.shared, + page_size_log2: None, // No support for the custom-page-sizes proposal }); } @@ -180,10 +192,10 @@ mod tests { let mut module = Module::default(); assert_eq!(module.memories.len(), 0); - module.memories.add_local(false, 0, Some(1024)); + module.memories.add_local(false, false, 0, Some(1024)); assert_eq!(module.memories.len(), 1); - module.memories.add_local(true, 1024, Some(2048)); + module.memories.add_local(true, true, 1024, Some(2048)); assert_eq!(module.memories.len(), 2); } } diff --git a/src/module/mod.rs b/src/module/mod.rs index 881b30f9..625b040e 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -43,8 +43,9 @@ use id_arena::Id; use log::warn; use std::fs; use std::mem; +use std::ops::Range; use std::path::Path; -use wasmparser::{Parser, Payload, Validator, WasmFeatures}; +use wasmparser::{BinaryReader, Parser, Payload, Validator, WasmFeatures}; pub use self::config::ModuleConfig; @@ -93,7 +94,7 @@ pub struct CodeTransform { pub code_section_start: usize, /// Emitted binary ranges of functions - pub function_ranges: Vec<(Id, wasmparser::Range)>, + pub function_ranges: Vec<(Id, Range)>, } impl Module { @@ -132,24 +133,23 @@ impl Module { let mut ret = Module::default(); ret.config = config.clone(); let mut indices = IndicesToIds::default(); - let mut validator = Validator::new(); - validator.wasm_features(WasmFeatures { - reference_types: !config.only_stable_features, - multi_value: true, - bulk_memory: !config.only_stable_features, - simd: !config.only_stable_features, - threads: !config.only_stable_features, - multi_memory: !config.only_stable_features, - ..WasmFeatures::default() - }); + let wasm_features = match config.only_stable_features { + true => WasmFeatures::default(), + false => WasmFeatures::all(), + }; + let mut validator = Validator::new_with_features(wasm_features); let mut local_functions = Vec::new(); let mut debug_sections = Vec::new(); for payload in Parser::new(0).parse_all(wasm) { match payload? { - Payload::Version { num, range } => { - validator.version(num, &range)?; + Payload::Version { + num, + encoding, + range, + } => { + validator.version(num, encoding, &range)?; } Payload::DataSection(s) => { validator @@ -218,40 +218,43 @@ impl Module { ret.funcs.code_section_offset = range.start; } Payload::CodeSectionEntry(body) => { - let validator = validator.code_section_entry()?; + let validator = validator + .code_section_entry(&body)? + .into_validator(Default::default()); local_functions.push((body, validator)); } - Payload::CustomSection { - name, - data, - data_offset, - range: _range, - } => { - let result = match name { - "producers" => wasmparser::ProducersSectionReader::new(data, data_offset) + Payload::CustomSection(s) => { + let result = + match s.name() { + "producers" => wasmparser::ProducersSectionReader::new( + BinaryReader::new(s.data(), s.data_offset(), wasm_features), + ) .map_err(anyhow::Error::from) .and_then(|s| ret.parse_producers_section(s)), - "name" => wasmparser::NameSectionReader::new(data, data_offset) - .map_err(anyhow::Error::from) - .and_then(|r| ret.parse_name_section(r, &indices)), - _ => { - log::debug!("parsing custom section `{}`", name); - if name.starts_with(".debug") { - debug_sections.push(RawCustomSection { - name: name.to_string(), - data: data.to_vec(), - }); - } else { - ret.customs.add(RawCustomSection { - name: name.to_string(), - data: data.to_vec(), - }); + "name" => { + let name_section_reader = wasmparser::NameSectionReader::new( + BinaryReader::new(s.data(), s.data_offset(), wasm_features), + ); + ret.parse_name_section(name_section_reader, &indices) } - continue; - } - }; + name => { + log::debug!("parsing custom section `{}`", name); + if name.starts_with(".debug") { + debug_sections.push(RawCustomSection { + name: name.to_string(), + data: s.data().to_vec(), + }); + } else { + ret.customs.add(RawCustomSection { + name: name.to_string(), + data: s.data().to_vec(), + }); + } + continue; + } + }; if let Err(e) = result { - log::warn!("failed to parse `{}` custom section {}", name, e); + log::warn!("failed to parse `{}` custom section {}", s.name(), e); } } Payload::UnknownSection { id, range, .. } => { @@ -259,32 +262,26 @@ impl Module { unreachable!() } - Payload::End => validator.end()?, - - // the module linking proposal is not implemented yet. - Payload::AliasSection(s) => { - validator.alias_section(&s)?; - bail!("not supported yet"); + Payload::End(offset) => { + validator.end(offset)?; + continue; } - Payload::InstanceSection(s) => { - validator.instance_section(&s)?; - bail!("not supported yet"); - } - Payload::ModuleSectionEntry { - parser: _, - range: _, - } => { - validator.module_section_entry(); - bail!("not supported yet"); - } - Payload::ModuleSectionStart { - count, - range, - size: _, - } => { - validator.module_section_start(count, &range)?; + + // component module proposal is not implemented yet. + Payload::ModuleSection { .. } + | Payload::InstanceSection(..) + | Payload::CoreTypeSection(..) + | Payload::ComponentSection { .. } + | Payload::ComponentInstanceSection(..) + | Payload::ComponentAliasSection(..) + | Payload::ComponentTypeSection(..) + | Payload::ComponentCanonicalSection(..) + | Payload::ComponentStartSection { .. } + | Payload::ComponentImportSection(..) + | Payload::ComponentExportSection(..) => { bail!("not supported yet"); } + // exception handling is not implemented yet. Payload::TagSection(s) => { validator.tag_section(&s)?; @@ -413,38 +410,35 @@ impl Module { indices: &IndicesToIds, ) -> Result<()> { log::debug!("parse name section"); - for name in names { - match name? { - wasmparser::Name::Module(m) => { - self.name = Some(m.get_name()?.to_string()); + for subsection in names { + match subsection? { + wasmparser::Name::Module { + name, + name_range: _, + } => { + self.name = Some(name.to_string()); } - wasmparser::Name::Function(f) => { - let mut map = f.get_map()?; - for _ in 0..map.get_count() { - let naming = map.read()?; + wasmparser::Name::Function(names) => { + for name in names { + let naming = name?; match indices.get_func(naming.index) { Ok(id) => self.funcs.get_mut(id).name = Some(naming.name.to_string()), - // If some tool fails to GC function names properly, - // it doesn't really hurt anything to ignore the - // broken references and keep going. Err(e) => warn!("in name section: {}", e), } } } - wasmparser::Name::Type(t) => { - let mut map = t.get_map()?; - for _ in 0..map.get_count() { - let naming = map.read()?; + wasmparser::Name::Type(names) => { + for name in names { + let naming = name?; match indices.get_type(naming.index) { Ok(id) => self.types.get_mut(id).name = Some(naming.name.to_string()), Err(e) => warn!("in name section: {}", e), } } } - wasmparser::Name::Memory(m) => { - let mut map = m.get_map()?; - for _ in 0..map.get_count() { - let naming = map.read()?; + wasmparser::Name::Memory(names) => { + for name in names { + let naming = name?; match indices.get_memory(naming.index) { Ok(id) => { self.memories.get_mut(id).name = Some(naming.name.to_string()) @@ -453,30 +447,27 @@ impl Module { } } } - wasmparser::Name::Table(t) => { - let mut map = t.get_map()?; - for _ in 0..map.get_count() { - let naming = map.read()?; + wasmparser::Name::Table(names) => { + for name in names { + let naming = name?; match indices.get_table(naming.index) { Ok(id) => self.tables.get_mut(id).name = Some(naming.name.to_string()), Err(e) => warn!("in name section: {}", e), } } } - wasmparser::Name::Data(d) => { - let mut map = d.get_map()?; - for _ in 0..map.get_count() { - let naming = map.read()?; + wasmparser::Name::Data(names) => { + for name in names { + let naming = name?; match indices.get_data(naming.index) { Ok(id) => self.data.get_mut(id).name = Some(naming.name.to_string()), Err(e) => warn!("in name section: {}", e), } } } - wasmparser::Name::Element(e) => { - let mut map = e.get_map()?; - for _ in 0..map.get_count() { - let naming = map.read()?; + wasmparser::Name::Element(names) => { + for name in names { + let naming = name?; match indices.get_element(naming.index) { Ok(id) => { self.elements.get_mut(id).name = Some(naming.name.to_string()) @@ -485,10 +476,9 @@ impl Module { } } } - wasmparser::Name::Global(e) => { - let mut map = e.get_map()?; - for _ in 0..map.get_count() { - let naming = map.read()?; + wasmparser::Name::Global(names) => { + for name in names { + let naming = name?; match indices.get_global(naming.index) { Ok(id) => self.globals.get_mut(id).name = Some(naming.name.to_string()), Err(e) => warn!("in name section: {}", e), @@ -496,13 +486,11 @@ impl Module { } } wasmparser::Name::Local(l) => { - let mut reader = l.get_indirect_map()?; - for _ in 0..reader.get_indirect_count() { - let name = reader.read()?; - let func_id = indices.get_func(name.indirect_index)?; - let mut map = name.get_map()?; - for _ in 0..map.get_count() { - let naming = map.read()?; + for f in l { + let f = f?; + let func_id = indices.get_func(f.index)?; + for name in f.names { + let naming = name?; // Looks like tools like `wat2wasm` generate empty // names for locals if they aren't specified, so // just ignore empty names which would in theory @@ -526,6 +514,8 @@ impl Module { } wasmparser::Name::Unknown { ty, .. } => warn!("unknown name subsection {}", ty), wasmparser::Name::Label(_) => warn!("labels name subsection ignored"), + wasmparser::Name::Field(_) => warn!("fields name subsection ignored"), + wasmparser::Name::Tag(_) => warn!("tags name subsection ignored"), } } Ok(()) diff --git a/src/module/producers.rs b/src/module/producers.rs index d6570bfd..8dae0c96 100644 --- a/src/module/producers.rs +++ b/src/module/producers.rs @@ -83,7 +83,7 @@ impl Module { for field in data { let field = field?; let mut values = Vec::new(); - for value in field.get_producer_field_values_reader()? { + for value in field.values { let value = value?; values.push(Value { name: value.name.to_string(), diff --git a/src/module/tables.rs b/src/module/tables.rs index fbeb2b3c..835aba6d 100644 --- a/src/module/tables.rs +++ b/src/module/tables.rs @@ -14,10 +14,12 @@ pub type TableId = Id; #[derive(Debug)] pub struct Table { id: TableId, + /// Whether or not this is a 64-bit table. + pub table64: bool, /// The initial size of this table - pub initial: u32, + pub initial: u64, /// The maximum size of this table - pub maximum: Option, + pub maximum: Option, /// The type of the elements in this table pub element_ty: ValType, /// Whether or not this table is imported, and if so what imports it. @@ -49,16 +51,18 @@ impl ModuleTables { /// Adds a new imported table to this list of tables pub fn add_import( &mut self, - initial: u32, - max: Option, + table64: bool, + initial: u64, + maximum: Option, element_ty: ValType, import: ImportId, ) -> TableId { let id = self.arena.next_id(); self.arena.alloc(Table { id, + table64, initial, - maximum: max, + maximum, element_ty, import: Some(import), elem_segments: Default::default(), @@ -68,12 +72,19 @@ impl ModuleTables { /// Construct a new table, that does not originate from any of the input /// wasm tables. - pub fn add_local(&mut self, initial: u32, max: Option, element_ty: ValType) -> TableId { + pub fn add_local( + &mut self, + table64: bool, + initial: u64, + maximum: Option, + element_ty: ValType, + ) -> TableId { let id = self.arena.next_id(); let id2 = self.arena.alloc(Table { id, + table64, initial, - maximum: max, + maximum, element_ty, import: None, elem_segments: Default::default(), @@ -144,9 +155,12 @@ impl Module { log::debug!("parse table section"); for t in section { let t = t?; - let id = self - .tables - .add_local(t.initial, t.maximum, ValType::parse(&t.element_type)?); + let id = self.tables.add_local( + t.ty.table64, + t.ty.initial, + t.ty.maximum, + ValType::parse(&wasmparser::ValType::Ref(t.ty.element_type))?, + ); ids.push_table(id); } Ok(()) @@ -169,6 +183,7 @@ impl Emit for ModuleTables { cx.indices.push_table(table.id()); wasm_table_section.table(wasm_encoder::TableType { + table64: table.table64, minimum: table.initial, maximum: table.maximum, element_type: match table.element_ty { diff --git a/src/module/types.rs b/src/module/types.rs index f48fcaae..23aec534 100644 --- a/src/module/types.rs +++ b/src/module/types.rs @@ -118,20 +118,17 @@ impl Module { ids: &mut IndicesToIds, ) -> Result<()> { log::debug!("parsing type section"); - for ty in section { - let fun_ty = match ty? { - wasmparser::TypeDef::Func(ty) => ty, - _ => unimplemented!("module linking not supported"), - }; + for ty in section.into_iter_err_on_gc_types() { + let fun_ty = ty?; let id = self.types.arena.next_id(); let params = fun_ty - .params + .params() .iter() .map(ValType::parse) .collect::>>()? .into_boxed_slice(); let results = fun_ty - .returns + .results() .iter() .map(ValType::parse) .collect::>>()? diff --git a/src/ty.rs b/src/ty.rs index bcbede96..73d59813 100644 --- a/src/ty.rs +++ b/src/ty.rs @@ -142,11 +142,8 @@ pub enum ValType { } impl ValType { - pub(crate) fn from_wasmparser_type(ty: wasmparser::Type) -> Result> { - let v = match ty { - wasmparser::Type::EmptyBlockType => Vec::new(), - _ => vec![ValType::parse(&ty)?], - }; + pub(crate) fn from_wasmparser_type(ty: wasmparser::ValType) -> Result> { + let v = vec![ValType::parse(&ty)?]; Ok(v.into_boxed_slice()) } @@ -162,16 +159,18 @@ impl ValType { } } - pub(crate) fn parse(input: &wasmparser::Type) -> Result { + pub(crate) fn parse(input: &wasmparser::ValType) -> Result { match input { - wasmparser::Type::I32 => Ok(ValType::I32), - wasmparser::Type::I64 => Ok(ValType::I64), - wasmparser::Type::F32 => Ok(ValType::F32), - wasmparser::Type::F64 => Ok(ValType::F64), - wasmparser::Type::V128 => Ok(ValType::V128), - wasmparser::Type::ExternRef => Ok(ValType::Externref), - wasmparser::Type::FuncRef => Ok(ValType::Funcref), - _ => bail!("not a value type"), + wasmparser::ValType::I32 => Ok(ValType::I32), + wasmparser::ValType::I64 => Ok(ValType::I64), + wasmparser::ValType::F32 => Ok(ValType::F32), + wasmparser::ValType::F64 => Ok(ValType::F64), + wasmparser::ValType::V128 => Ok(ValType::V128), + wasmparser::ValType::Ref(ref_type) => match *ref_type { + wasmparser::RefType::EXTERNREF => Ok(ValType::Externref), + wasmparser::RefType::FUNCREF => Ok(ValType::Funcref), + _ => bail!("unsupported ref type {:?}", ref_type), + }, } } } From 2df4d06af1a45f35c33165440725e3c6761b4119 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 25 Jun 2024 14:57:05 -0400 Subject: [PATCH 02/31] fmt --- src/init_expr.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/init_expr.rs b/src/init_expr.rs index 391db297..d4b5ea80 100644 --- a/src/init_expr.rs +++ b/src/init_expr.rs @@ -58,8 +58,14 @@ impl InitExpr { wasm_encoder::ConstExpr::global_get(cx.indices.get_global_index(*g)) } InitExpr::RefNull(ty) => wasm_encoder::ConstExpr::ref_null(match ty { - ValType::Externref => wasm_encoder::HeapType::Abstract { shared: false, ty: wasm_encoder::AbstractHeapType::Extern }, - ValType::Funcref => wasm_encoder::HeapType::Abstract { shared: false, ty: wasm_encoder::AbstractHeapType::Func }, + ValType::Externref => wasm_encoder::HeapType::Abstract { + shared: false, + ty: wasm_encoder::AbstractHeapType::Extern, + }, + ValType::Funcref => wasm_encoder::HeapType::Abstract { + shared: false, + ty: wasm_encoder::AbstractHeapType::Func, + }, _ => unreachable!(), }), InitExpr::RefFunc(f) => { From 96a546ba01bb3953b6f039825c5f33b2f07066ba Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 25 Jun 2024 15:17:19 -0400 Subject: [PATCH 03/31] cleanup workflow yml --- .github/workflows/main.yml | 45 ++++++++++++++------------------------ 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index daa88a39..ccc56e26 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -9,32 +9,21 @@ jobs: matrix: rust: [stable, beta, nightly] steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v4 with: submodules: true - name: Install Rust run: rustup update ${{ matrix.rust }} && rustup default ${{ matrix.rust }} - - # Build an up-to-date version of wabt since the releases don't always have - # all the features we want. - - uses: actions/checkout@v2 - with: - repository: WebAssembly/wabt - ref: aa0515b3c808da880942db8658abeaa969534667 - path: wabt - - name: Build wabt + - name: Install wabt run: | set -e - cd wabt - cmake . -DBUILD_TESTS=OFF - make -j$(nproc) wast2json spectest-interp wasm-interp - echo `pwd` > $GITHUB_PATH - + curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - + echo "`pwd`/wabt-1.0.35" > $GITHUB_PATH - name: Install binaryen run: | set -e - curl -L https://github.com/WebAssembly/binaryen/releases/download/1.39.1/binaryen-1.39.1-x86_64-linux.tar.gz | tar xzf - - echo "`pwd`/binaryen-1.39.1" > $GITHUB_PATH + curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - + echo "`pwd`/binaryen-version_117" > $GITHUB_PATH - run: cargo build --all - run: cargo test --all - run: cargo check --benches @@ -45,19 +34,19 @@ jobs: name: Fuzz Crate runs-on: ubuntu-latest steps: - - uses: actions/checkout@master + - uses: actions/checkout@v4 - name: Install Rust run: rustup update stable && rustup default stable - name: Install wabt run: | set -e - curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.13/wabt-1.0.13-linux.tar.gz | tar xzf - - echo "`pwd`/wabt-1.0.13" > $GITHUB_PATH + curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - + echo "`pwd`/wabt-1.0.35" > $GITHUB_PATH - name: Install binaryen run: | set -e - curl -L https://github.com/WebAssembly/binaryen/releases/download/1.39.1/binaryen-1.39.1-x86_64-linux.tar.gz | tar xzf - - echo "`pwd`/binaryen-1.39.1" > $GITHUB_PATH + curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - + echo "`pwd`/binaryen-version_117" > $GITHUB_PATH - name: Run fuzzer run: cargo test -p walrus-fuzz-utils > fuzz.log || (tail -n 1000 fuzz.log && exit 1) env: @@ -71,20 +60,20 @@ jobs: matrix: test: [watgen, wasm-opt-ttf, raw] steps: - - uses: actions/checkout@master + - uses: actions/checkout@v4 - name: Install Rust run: rustup update nightly && rustup default nightly - run: cargo install cargo-fuzz - name: Install wabt run: | set -e - curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.13/wabt-1.0.13-linux.tar.gz | tar xzf - - echo "`pwd`/wabt-1.0.13" > $GITHUB_PATH + curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - + echo "`pwd`/wabt-1.0.35" > $GITHUB_PATH - name: Install binaryen run: | set -e - curl -L https://github.com/WebAssembly/binaryen/releases/download/1.39.1/binaryen-1.39.1-x86_64-linux.tar.gz | tar xzf - - echo "`pwd`/binaryen-1.39.1" > $GITHUB_PATH + curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - + echo "`pwd`/binaryen-version_117" > $GITHUB_PATH - name: Run fuzzer run: | cargo fuzz run ${{ matrix.test }} -- -max_total_time=300 -rss_limit_mb=4096 > fuzz.log 2>&1 || (tail -n 1000 fuzz.log && exit 1) @@ -93,7 +82,7 @@ jobs: name: Rustfmt runs-on: ubuntu-latest steps: - - uses: actions/checkout@master + - uses: actions/checkout@v4 - name: Install Rust run: rustup update stable && rustup default stable && rustup component add rustfmt - run: cargo fmt -- --check From 2a6023c6b95fdc555ba709a92a4fc8afb3027b94 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Thu, 27 Jun 2024 14:42:08 -0400 Subject: [PATCH 04/31] update testsuite and skip all proposals --- crates/tests/tests/spec-tests | 2 +- crates/tests/tests/spec-tests.rs | 41 +++++--------------------------- 2 files changed, 7 insertions(+), 36 deletions(-) diff --git a/crates/tests/tests/spec-tests b/crates/tests/tests/spec-tests index 01efde81..e0536507 160000 --- a/crates/tests/tests/spec-tests +++ b/crates/tests/tests/spec-tests @@ -1 +1 @@ -Subproject commit 01efde81028c5b0d099eb836645a2dc5e7755449 +Subproject commit e05365077e13a1d86ffe77acfb1a835b7aa78422 diff --git a/crates/tests/tests/spec-tests.rs b/crates/tests/tests/spec-tests.rs index 1b1b9e28..9343f423 100644 --- a/crates/tests/tests/spec-tests.rs +++ b/crates/tests/tests/spec-tests.rs @@ -16,41 +16,12 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { env_logger::init(); }); - let proposal = wast - .iter() - .skip_while(|part| *part != "proposals") - .skip(1) - .next() - .map(|s| s.to_str().unwrap()); - let extra_args: &[&str] = match proposal { - // stable features - None - | Some("multi-value") - | Some("nontrapping-float-to-int-conversions") - | Some("sign-extension-ops") - | Some("mutable-global") => &[], - - Some("simd") => &["--enable-simd"], - Some("bulk-memory-operations") => &["--enable-bulk-memory"], - - Some("reference-types") => &["--enable-reference-types", "--enable-bulk-memory"], - - // TODO: should get threads working - Some("threads") => return Ok(()), - // TODO: should get tail-call working - Some("tail-call") => return Ok(()), - - // not a walrus thing, but not implemented in wabt fully yet anyway - Some("annotations") => return Ok(()), - - // not implemented in walrus yet - Some("function-references") => return Ok(()), - Some("exception-handling") => return Ok(()), - Some("memory64") => return Ok(()), - - // Some("threads") => &["--enable-threads"], - Some(other) => panic!("unknown wasm proposal: {}", other), - }; + // Skip proposals tests for now + if wast.components().any(|c| c.as_os_str() == "proposals") { + return Ok(()); + } + + let extra_args = &[]; let tempdir = TempDir::new()?; let json = tempdir.path().join("foo.json"); From 58b33f6475ced87372e2992a0a11009f995b0d75 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Thu, 27 Jun 2024 14:42:52 -0400 Subject: [PATCH 05/31] proc-macro has dash now --- crates/macro/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/macro/Cargo.toml b/crates/macro/Cargo.toml index 98a44ab0..8fd4435b 100644 --- a/crates/macro/Cargo.toml +++ b/crates/macro/Cargo.toml @@ -19,4 +19,4 @@ quote = "1.0.2" syn = { version = "2.0.0", features = ['extra-traits'] } [lib] -proc_macro = true +proc-macro = true From 443462db17a0f5ff2edd1d9795c0f767653a4f14 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 28 Jun 2024 11:18:18 -0400 Subject: [PATCH 06/31] fix for bulk.wast by refresh elements --- crates/tests/tests/spec-tests.rs | 15 ++-- src/dot.rs | 6 +- src/init_expr.rs | 13 ++- src/module/elements.rs | 136 ++++++++++++++++++------------- src/module/mod.rs | 2 +- src/passes/used.rs | 10 +-- 6 files changed, 106 insertions(+), 76 deletions(-) diff --git a/crates/tests/tests/spec-tests.rs b/crates/tests/tests/spec-tests.rs index 9343f423..fd45be2e 100644 --- a/crates/tests/tests/spec-tests.rs +++ b/crates/tests/tests/spec-tests.rs @@ -35,6 +35,11 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { assert!(status.success()); let wabt_ok = run_spectest_interp(tempdir.path(), extra_args).is_ok(); + // If wabt didn't succeed before we ran walrus there's no hope of it passing + // after we run walrus. + if !wabt_ok { + return Ok(()); + } let contents = fs::read_to_string(&json).context("failed to read file")?; let test: Test = serde_json::from_str(&contents).context("failed to parse file")?; @@ -45,6 +50,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { config.only_stable_features(true); } + let mut false_positives = vec![]; for command in test.commands { let filename = match command.get("filename") { Some(name) => name.as_str().unwrap().to_string(), @@ -63,7 +69,6 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { continue; } let wasm = fs::read(&path)?; - println!("{:?}", command); if config.parse(&wasm).is_ok() { // A few spec tests assume multi-value isn't implemented, // but we implement it, so basically just skip those tests. @@ -79,7 +84,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { if wast.ends_with("unreached-invalid.wast") && line == 539 { continue; } - panic!("wasm parsed when it shouldn't (line {})", line); + false_positives.push(line.as_u64().unwrap()); } } "assert_unlinkable" if wast.file_name() == Some("elem.wast".as_ref()) => { @@ -111,10 +116,8 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { } } - // If wabt didn't succeed before we ran walrus there's no hope of it passing - // after we run walrus. - if !wabt_ok { - return Ok(()); + if !false_positives.is_empty() { + panic!("wasm parsed when it shouldn't (line {:?})", false_positives); } // First up run the spec-tests as-is after we round-tripped through walrus. diff --git a/src/dot.rs b/src/dot.rs index 7a710308..1729242c 100644 --- a/src/dot.rs +++ b/src/dot.rs @@ -526,10 +526,8 @@ impl DotNode for Element { } fn edges(&self, edges: &mut impl EdgeAggregator) { - for m in self.members.iter() { - if let Some(m) = m { - edges.add_edge(m); - } + if let ElementItems::Functions(function_ids) = &self.items { + function_ids.iter().for_each(|f| edges.add_edge(f)); } } } diff --git a/src/init_expr.rs b/src/init_expr.rs index d4b5ea80..aa1b03c8 100644 --- a/src/init_expr.rs +++ b/src/init_expr.rs @@ -32,8 +32,17 @@ impl InitExpr { F64Const { value } => InitExpr::Value(Value::F64(f64::from_bits(value.bits()))), V128Const { value } => InitExpr::Value(Value::V128(v128_to_u128(&value))), GlobalGet { global_index } => InitExpr::Global(ids.get_global(global_index)?), - // TODO: handle RefNull - // RefNull { hty } => InitExpr::RefNull(ValType::parse(&hty)?), + RefNull { hty } => { + let val_type = match hty { + wasmparser::HeapType::Abstract { shared: _, ty } => match ty { + wasmparser::AbstractHeapType::Func => ValType::Funcref, + wasmparser::AbstractHeapType::Extern => ValType::Externref, + _ => bail!("invalid constant expression"), + }, + wasmparser::HeapType::Concrete(_) => bail!("invalid constant expression"), + }; + InitExpr::RefNull(val_type) + } RefFunc { function_index } => InitExpr::RefFunc(ids.get_func(function_index)?), _ => bail!("invalid constant expression"), }; diff --git a/src/module/elements.rs b/src/module/elements.rs index 6b444f65..1e9829ba 100644 --- a/src/module/elements.rs +++ b/src/module/elements.rs @@ -13,23 +13,38 @@ pub type ElementId = Id; #[derive(Debug)] pub struct Element { id: Id, - /// Whether this segment is passive or active. + /// The kind of the element segment. pub kind: ElementKind, - /// The type of elements in this segment - pub ty: ValType, - /// The function members of this passive elements segment. - pub members: Vec>, + /// The initial elements of the element segment. + pub items: ElementItems, /// The name of this element, used for debugging purposes in the `name` /// custom section. pub name: Option, } -#[allow(missing_docs)] +/// The kind of element segment. #[derive(Debug, Copy, Clone)] pub enum ElementKind { + /// The element segment is passive. Passive, + /// The element segment is declared. Declared, - Active { table: TableId, offset: InitExpr }, + /// The element segment is active. + Active { + /// The ID of the table being initialized. + table: TableId, + /// A constant defining an offset into that table. + offset: InitExpr, + }, +} + +/// Represents the items of an element segment. +#[derive(Debug, Clone)] +pub enum ElementItems { + /// This element contains function indices. + Functions(Vec), + /// This element contains constant expressions used to initialize the table. + Expressions(ValType, Vec), } impl Element { @@ -41,7 +56,7 @@ impl Element { impl Tombstone for Element { fn on_delete(&mut self) { - self.members = Vec::new(); + // Nothing to do here } } @@ -82,18 +97,12 @@ impl ModuleElements { } /// Add an element segment - pub fn add( - &mut self, - kind: ElementKind, - ty: ValType, - members: Vec>, - ) -> ElementId { + pub fn add(&mut self, kind: ElementKind, items: ElementItems) -> ElementId { let id = self.arena.next_id(); let id2 = self.arena.alloc(Element { id, kind, - ty, - members, + items, name: None, }); debug_assert_eq!(id, id2); @@ -112,17 +121,36 @@ impl Module { for (i, segment) in section.into_iter().enumerate() { let element = segment?; - let members = match element.items { - wasmparser::ElementItems::Functions(f) => f - .into_iter() - .map(|f| -> Result<_> { - match ids.get_func(f?) { - Ok(f) => Ok(Some(f)), - Err(_) => Ok(None), + let items = match element.items { + wasmparser::ElementItems::Functions(funcs) => { + let mut function_ids = Vec::with_capacity(funcs.count() as usize); + for func in funcs { + match ids.get_func(func?) { + Ok(func) => function_ids.push(func), + Err(_) => bail!("invalid function index in element segment {}", i), } - }) - .collect::>()?, - wasmparser::ElementItems::Expressions(_, _) => todo!(), + } + ElementItems::Functions(function_ids) + } + wasmparser::ElementItems::Expressions(ref_type, items) => { + let ty = match ref_type { + wasmparser::RefType::FUNCREF => ValType::Funcref, + wasmparser::RefType::EXTERNREF => ValType::Externref, + _ => bail!("unsupported ref type in element segment {}", i), + }; + let mut init_exprs = Vec::with_capacity(items.count() as usize); + for item in items { + let const_expr = item?; + let expr = InitExpr::eval(&const_expr, ids).with_context(|| { + format!( + "Failed to evaluate a const expr in element segment {}:\n{:?}", + i, const_expr + ) + })?; + init_exprs.push(expr); + } + ElementItems::Expressions(ty, init_exprs) + } }; let id = self.elements.arena.next_id(); @@ -134,7 +162,7 @@ impl Module { table_index, offset_expr, } => { - // TODO: check if this is correct + // TODO: Why table_index is Option? let table = ids.get_table(table_index.unwrap_or_default())?; self.tables.get_mut(table).elem_segments.insert(id); @@ -151,9 +179,8 @@ impl Module { }; self.elements.arena.alloc(Element { id, - ty: ValType::Funcref, kind, - members, + items, name: None, }); ids.push_element(id); @@ -173,35 +200,28 @@ impl Emit for ModuleElements { for (id, element) in self.arena.iter() { cx.indices.push_element(id); - if element.members.iter().any(|i| i.is_none()) { - let els_vec: Vec = element - .members - .iter() - .map(|func| match func { - Some(func) => { - wasm_encoder::ConstExpr::ref_func(cx.indices.get_func_index(*func)) - } - None => { - wasm_encoder::ConstExpr::ref_null(wasm_encoder::HeapType::Abstract { - shared: false, - ty: wasm_encoder::AbstractHeapType::Func, - }) - } - }) - .collect(); - let els = wasm_encoder::Elements::Expressions( - wasm_encoder::RefType::FUNCREF, - els_vec.as_slice(), - ); - emit_elem(cx, &mut wasm_element_section, &element.kind, els); - } else { - let els_vec: Vec = element - .members - .iter() - .map(|func| cx.indices.get_func_index(func.unwrap())) - .collect(); - let els = wasm_encoder::Elements::Functions(els_vec.as_slice()); - emit_elem(cx, &mut wasm_element_section, &element.kind, els); + match &element.items { + ElementItems::Functions(function_ids) => { + let idx = function_ids + .iter() + .map(|&func| cx.indices.get_func_index(func)) + .collect::>(); + let els = wasm_encoder::Elements::Functions(&idx); + emit_elem(cx, &mut wasm_element_section, &element.kind, els); + } + ElementItems::Expressions(ty, init_exprs) => { + let ref_type = match ty { + ValType::Funcref => wasm_encoder::RefType::FUNCREF, + ValType::Externref => wasm_encoder::RefType::EXTERNREF, + _ => unreachable!(), + }; + let const_exprs = init_exprs + .iter() + .map(|expr| expr.to_wasmencoder_type(cx)) + .collect::>(); + let els = wasm_encoder::Elements::Expressions(ref_type, &const_exprs); + emit_elem(cx, &mut wasm_element_section, &element.kind, els); + } } fn emit_elem( diff --git a/src/module/mod.rs b/src/module/mod.rs index 625b040e..9914b603 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -24,8 +24,8 @@ pub use crate::module::custom::{ }; pub use crate::module::data::{ActiveData, ActiveDataLocation, Data, DataId, DataKind, ModuleData}; pub use crate::module::debug::ModuleDebugData; -pub use crate::module::elements::ElementKind; pub use crate::module::elements::{Element, ElementId, ModuleElements}; +pub use crate::module::elements::{ElementItems, ElementKind}; pub use crate::module::exports::{Export, ExportId, ExportItem, ModuleExports}; pub use crate::module::functions::{FuncParams, FuncResults}; pub use crate::module::functions::{Function, FunctionId, ModuleFunctions}; diff --git a/src/passes/used.rs b/src/passes/used.rs index 163e5329..204aa3e8 100644 --- a/src/passes/used.rs +++ b/src/passes/used.rs @@ -1,7 +1,7 @@ use crate::ir::*; use crate::map::IdHashSet; use crate::{ActiveDataLocation, Data, DataId, DataKind, Element, ExportItem, Function, InitExpr}; -use crate::{ElementId, ElementKind, Module, Type, TypeId}; +use crate::{ElementId, ElementItems, ElementKind, Module, Type, TypeId}; use crate::{FunctionId, FunctionKind, Global, GlobalId}; use crate::{GlobalKind, Memory, MemoryId, Table, TableId}; @@ -204,10 +204,10 @@ impl Used { while let Some(e) = stack.elements.pop() { let e = module.elements.get(e); - for func in e.members.iter() { - if let Some(func) = func { - stack.push_func(*func); - } + if let ElementItems::Functions(function_ids) = &e.items { + function_ids.iter().for_each(|f| { + stack.push_func(*f); + }); } if let ElementKind::Active { offset, table } = &e.kind { if let InitExpr::Global(g) = offset { From 8c38eb5c6730c24779fa75a56df01a35cf973cc6 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 28 Jun 2024 14:42:23 -0400 Subject: [PATCH 07/31] bump to 0.212.0 and follow the features in wasm-tools --- Cargo.toml | 4 ++-- src/module/functions/local_function/emit.rs | 9 ++++++--- src/module/mod.rs | 12 ++++++++---- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 501df342..7296f785 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,8 +29,8 @@ leb128 = "0.2.4" log = "0.4.8" rayon = { version = "1.1.0", optional = true } walrus-macro = { path = './crates/macro', version = '=0.19.0' } -wasm-encoder = "0.211.0" -wasmparser = "0.211.0" +wasm-encoder = "0.212.0" +wasmparser = "0.212.0" gimli = "0.26.0" [features] diff --git a/src/module/functions/local_function/emit.rs b/src/module/functions/local_function/emit.rs index be3127df..db775d55 100644 --- a/src/module/functions/local_function/emit.rs +++ b/src/module/functions/local_function/emit.rs @@ -603,9 +603,12 @@ impl<'instr> Visitor<'instr> for Emit<'_> { Call(e) => Instruction::Call(self.indices.get_func_index(e.func)), CallIndirect(e) => { - let ty = self.indices.get_type_index(e.ty); - let table = self.indices.get_table_index(e.table); - Instruction::CallIndirect { ty, table } + let type_index = self.indices.get_type_index(e.ty); + let table_index = self.indices.get_table_index(e.table); + Instruction::CallIndirect { + type_index, + table_index, + } } LocalGet(e) => Instruction::LocalGet(self.local_indices[&e.local]), diff --git a/src/module/mod.rs b/src/module/mod.rs index 9914b603..cfce5148 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -133,10 +133,14 @@ impl Module { let mut ret = Module::default(); ret.config = config.clone(); let mut indices = IndicesToIds::default(); - let wasm_features = match config.only_stable_features { - true => WasmFeatures::default(), - false => WasmFeatures::all(), - }; + + // The wasm-tools project is using the following features to run the spectests without proposals + // See tests/roundtrip.rs::TestState::wasmparser_validator_for + let mut wasm_features = WasmFeatures::default(); + wasm_features.remove(WasmFeatures::COMPONENT_MODEL); + wasm_features.remove(WasmFeatures::MULTI_MEMORY); + wasm_features.remove(WasmFeatures::THREADS); + let mut validator = Validator::new_with_features(wasm_features); let mut local_functions = Vec::new(); From 5eee359b6ee651bbdb8abc9dd2f0e49b5679fcfa Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 28 Jun 2024 19:40:26 -0400 Subject: [PATCH 08/31] fix elem.wast which failed in gc pass --- src/passes/used.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/passes/used.rs b/src/passes/used.rs index 204aa3e8..5e6329bc 100644 --- a/src/passes/used.rs +++ b/src/passes/used.rs @@ -209,6 +209,19 @@ impl Used { stack.push_func(*f); }); } + if let ElementItems::Expressions(crate::ValType::Funcref, items) = &e.items { + for item in items { + match item { + InitExpr::Global(g) => { + stack.push_global(*g); + } + InitExpr::RefFunc(f) => { + stack.push_func(*f); + } + _ => {} + } + } + } if let ElementKind::Active { offset, table } = &e.kind { if let InitExpr::Global(g) = offset { stack.push_global(*g); From 60444cbb7480b294a246da7cf0ac50135ecf7ba2 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 28 Jun 2024 20:38:12 -0400 Subject: [PATCH 09/31] handle RefNull --- src/module/functions/local_function/mod.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/module/functions/local_function/mod.rs b/src/module/functions/local_function/mod.rs index 38d7592c..2d9bd936 100644 --- a/src/module/functions/local_function/mod.rs +++ b/src/module/functions/local_function/mod.rs @@ -998,13 +998,18 @@ fn append_instruction<'context>( let table = ctx.indices.get_table(table).unwrap(); ctx.alloc_instr(TableFill { table }, loc); } - Operator::RefNull { hty: _ } => { - // TODO: - // RefNull definition has changed in wasmparser. - // Should we update accordingly? - - // let ty = ValType::parse(&hty).unwrap(); - // ctx.alloc_instr(RefNull { ty }, loc); + Operator::RefNull { hty } => { + let ty = match hty { + wasmparser::HeapType::Abstract { shared: _, ty } => match ty { + wasmparser::AbstractHeapType::Func => ValType::Funcref, + wasmparser::AbstractHeapType::Extern => ValType::Externref, + _ => panic!("unsupported abstract heap type for ref.null"), + }, + wasmparser::HeapType::Concrete(_) => { + panic!("unsupported concrete heap type for ref.null") + } + }; + ctx.alloc_instr(RefNull { ty }, loc); } Operator::RefIsNull => { ctx.alloc_instr(RefIsNull {}, loc); From fc4c36045c3d141c484d4179de489574c6ae1815 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 28 Jun 2024 21:47:52 -0400 Subject: [PATCH 10/31] always use default features from wasmparser --- src/module/mod.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/module/mod.rs b/src/module/mod.rs index cfce5148..471533c9 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -134,12 +134,8 @@ impl Module { ret.config = config.clone(); let mut indices = IndicesToIds::default(); - // The wasm-tools project is using the following features to run the spectests without proposals - // See tests/roundtrip.rs::TestState::wasmparser_validator_for - let mut wasm_features = WasmFeatures::default(); - wasm_features.remove(WasmFeatures::COMPONENT_MODEL); - wasm_features.remove(WasmFeatures::MULTI_MEMORY); - wasm_features.remove(WasmFeatures::THREADS); + // TODO: how should we handle config.only_stable_features? + let wasm_features = WasmFeatures::default(); let mut validator = Validator::new_with_features(wasm_features); From 704239d092b2f56eebf81b7436a1a0670abdf469 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 28 Jun 2024 21:48:47 -0400 Subject: [PATCH 11/31] overhaul spec-tests.rs --- crates/tests/tests/spec-tests.rs | 96 +++++++++++++++----------------- 1 file changed, 45 insertions(+), 51 deletions(-) diff --git a/crates/tests/tests/spec-tests.rs b/crates/tests/tests/spec-tests.rs index fd45be2e..257a382d 100644 --- a/crates/tests/tests/spec-tests.rs +++ b/crates/tests/tests/spec-tests.rs @@ -34,13 +34,6 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { .context("executing `wast2json`")?; assert!(status.success()); - let wabt_ok = run_spectest_interp(tempdir.path(), extra_args).is_ok(); - // If wabt didn't succeed before we ran walrus there's no hope of it passing - // after we run walrus. - if !wabt_ok { - return Ok(()); - } - let contents = fs::read_to_string(&json).context("failed to read file")?; let test: Test = serde_json::from_str(&contents).context("failed to parse file")?; let mut files = Vec::new(); @@ -50,65 +43,47 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { config.only_stable_features(true); } - let mut false_positives = vec![]; + let wabt_ok = run_spectest_interp(tempdir.path(), extra_args).is_ok(); + + let mut should_not_parse = vec![]; + let mut non_deterministic = vec![]; for command in test.commands { let filename = match command.get("filename") { Some(name) => name.as_str().unwrap().to_string(), None => continue, }; - let line = &command["line"]; + let line = command["line"].as_u64().unwrap(); let path = tempdir.path().join(filename); match command["type"].as_str().unwrap() { "assert_invalid" | "assert_malformed" => { - // Skip tests that are actually valid with various in-flight proposals - let text = command["text"].as_str().unwrap(); - if text == "invalid result arity" - || text == "multiple memories" - || text == "multiple tables" - { + // The multiple-memories feature is on by default in walrus (align with wasmparser::WasmFeatures::default()) + if command["text"] == "multiple memories" { continue; } let wasm = fs::read(&path)?; if config.parse(&wasm).is_ok() { - // A few spec tests assume multi-value isn't implemented, - // but we implement it, so basically just skip those tests. - let message = command["text"].as_str().unwrap(); - if message.contains("invalid result arity") { - continue; - } - - // MVP wasm considers this tests to fail, but - // reference-types-enhanced wasm considers this test to - // pass. We implement the reference-types semantics, so - // let's go forward with that. - if wast.ends_with("unreached-invalid.wast") && line == 539 { - continue; - } - false_positives.push(line.as_u64().unwrap()); + should_not_parse.push(line); } } - "assert_unlinkable" if wast.file_name() == Some("elem.wast".as_ref()) => { - // The `elem.wast` file has some unlinkable modules which place - // table elements at massive (aka negative) offsets. Our - // representation means that we try to allocate a massive amount - // of space for null elements. For now we skip these tests as an - // implementation detail. This is arguably a bug on our end - // where we should improve our representation to not allocate so - // much, but that's another bug for another day. - } cmd => { - let wasm = fs::read(&path)?; - let mut wasm = config - .parse(&wasm) + // The bytes read from the original spec test case + let bytes0 = fs::read(&path)?; + // The module parsed from bytes0 + let mut wasm1 = config + .parse(&bytes0) .with_context(|| format!("error parsing wasm (line {})", line))?; - let wasm1 = wasm.emit_wasm(); - fs::write(&path, &wasm1)?; - let wasm2 = config - .parse(&wasm1) - .map(|mut m| m.emit_wasm()) + // The bytes emitted from wasm1 + let bytes1 = wasm1.emit_wasm(); + fs::write(&path, &bytes1)?; + // The module parsed from bytes1 + let mut wasm2 = config + .parse(&bytes1) .with_context(|| format!("error re-parsing wasm (line {})", line))?; - if wasm1 != wasm2 { - panic!("wasm module at line {} isn't deterministic", line); + // The bytes emitted from wasm2 + let bytes2 = wasm2.emit_wasm(); + + if bytes1 != bytes2 { + non_deterministic.push(line); } files.push((cmd.to_string(), path.to_path_buf())); continue; @@ -116,8 +91,27 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { } } - if !false_positives.is_empty() { - panic!("wasm parsed when it shouldn't (line {:?})", false_positives); + let mut message = String::new(); + if !should_not_parse.is_empty() { + message.push_str(&format!( + "wasm parsed when it shouldn't at line: {:?}", + should_not_parse + )); + } + if !non_deterministic.is_empty() { + message.push_str(&format!( + "wasm isn't deterministic at line: {:?}", + non_deterministic + )); + } + if !message.is_empty() { + panic!("{}", message); + } + + // If wabt didn't succeed before we ran walrus there's no hope of it passing + // after we run walrus. + if !wabt_ok { + return Ok(()); } // First up run the spec-tests as-is after we round-tripped through walrus. From 26afde9b84cc96e64171a33138543bf94a66c697 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 28 Jun 2024 22:23:22 -0400 Subject: [PATCH 12/31] enable multi-memory spec-tests --- crates/tests/tests/spec-tests.rs | 37 +++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/crates/tests/tests/spec-tests.rs b/crates/tests/tests/spec-tests.rs index 257a382d..5d3c9ed3 100644 --- a/crates/tests/tests/spec-tests.rs +++ b/crates/tests/tests/spec-tests.rs @@ -16,12 +16,27 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { env_logger::init(); }); - // Skip proposals tests for now - if wast.components().any(|c| c.as_os_str() == "proposals") { - return Ok(()); - } - - let extra_args = &[]; + let proposal = wast + .iter() + .skip_while(|part| *part != "proposals") + .skip(1) + .next() + .map(|s| s.to_str().unwrap()); + + let extra_args: &[&str] = match proposal { + None => &[], + Some("annotations") => return Ok(()), + Some("exception-handling") => return Ok(()), + Some("extended-const") => return Ok(()), + Some("function-references") => return Ok(()), + Some("gc") => return Ok(()), + Some("memory64") => return Ok(()), + Some("multi-memory") => &["--enable-multi-memory"], + Some("relaxed-simd") => return Ok(()), + Some("tail-call") => return Ok(()), + Some("threads") => return Ok(()), + Some(other) => bail!("unknown wasm proposal: {}", other), + }; let tempdir = TempDir::new()?; let json = tempdir.path().join("foo.json"); @@ -56,10 +71,16 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { let path = tempdir.path().join(filename); match command["type"].as_str().unwrap() { "assert_invalid" | "assert_malformed" => { - // The multiple-memories feature is on by default in walrus (align with wasmparser::WasmFeatures::default()) - if command["text"] == "multiple memories" { + // The multiple-memories feature is on (from wasmparser::WasmFeatures::default()). + // In imports.wast and memory.wast, following cases will be parsed which should not. + if proposal.is_none() && command["text"] == "multiple memories" { continue; } + // In binary.wast, following cases will be parsed which should not. + if proposal.is_none() && command["text"] == "zero byte expected" { + continue; + } + let wasm = fs::read(&path)?; if config.parse(&wasm).is_ok() { should_not_parse.push(line); From 98ea9803e39b2fa3a2458a42912c032b0f11d13b Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 28 Jun 2024 22:31:11 -0400 Subject: [PATCH 13/31] fix wabt bin path --- .github/workflows/main.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ccc56e26..743709bd 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -18,7 +18,7 @@ jobs: run: | set -e curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - - echo "`pwd`/wabt-1.0.35" > $GITHUB_PATH + echo "`pwd`/wabt-1.0.35/bin" > $GITHUB_PATH - name: Install binaryen run: | set -e @@ -41,7 +41,7 @@ jobs: run: | set -e curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - - echo "`pwd`/wabt-1.0.35" > $GITHUB_PATH + echo "`pwd`/wabt-1.0.35/bin" > $GITHUB_PATH - name: Install binaryen run: | set -e @@ -68,7 +68,7 @@ jobs: run: | set -e curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - - echo "`pwd`/wabt-1.0.35" > $GITHUB_PATH + echo "`pwd`/wabt-1.0.35/bin" > $GITHUB_PATH - name: Install binaryen run: | set -e From 9fc633fc590740959fe58fe108d6ab3f604be9a6 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 28 Jun 2024 22:37:23 -0400 Subject: [PATCH 14/31] fix binaryen bin path --- .github/workflows/main.yml | 6 +++--- crates/tests/tests/spec-tests.rs | 2 +- src/module/mod.rs | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 743709bd..3a6c3482 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -23,7 +23,7 @@ jobs: run: | set -e curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - - echo "`pwd`/binaryen-version_117" > $GITHUB_PATH + echo "`pwd`/binaryen-version_117/bin" > $GITHUB_PATH - run: cargo build --all - run: cargo test --all - run: cargo check --benches @@ -46,7 +46,7 @@ jobs: run: | set -e curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - - echo "`pwd`/binaryen-version_117" > $GITHUB_PATH + echo "`pwd`/binaryen-version_117/bin" > $GITHUB_PATH - name: Run fuzzer run: cargo test -p walrus-fuzz-utils > fuzz.log || (tail -n 1000 fuzz.log && exit 1) env: @@ -73,7 +73,7 @@ jobs: run: | set -e curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - - echo "`pwd`/binaryen-version_117" > $GITHUB_PATH + echo "`pwd`/binaryen-version_117/bin" > $GITHUB_PATH - name: Run fuzzer run: | cargo fuzz run ${{ matrix.test }} -- -max_total_time=300 -rss_limit_mb=4096 > fuzz.log 2>&1 || (tail -n 1000 fuzz.log && exit 1) diff --git a/crates/tests/tests/spec-tests.rs b/crates/tests/tests/spec-tests.rs index 5d3c9ed3..5ff1a657 100644 --- a/crates/tests/tests/spec-tests.rs +++ b/crates/tests/tests/spec-tests.rs @@ -30,7 +30,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { Some("extended-const") => return Ok(()), Some("function-references") => return Ok(()), Some("gc") => return Ok(()), - Some("memory64") => return Ok(()), + Some("memory64") => &["--enable-memory64"], Some("multi-memory") => &["--enable-multi-memory"], Some("relaxed-simd") => return Ok(()), Some("tail-call") => return Ok(()), diff --git a/src/module/mod.rs b/src/module/mod.rs index 471533c9..b32e8ab4 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -135,7 +135,8 @@ impl Module { let mut indices = IndicesToIds::default(); // TODO: how should we handle config.only_stable_features? - let wasm_features = WasmFeatures::default(); + let mut wasm_features = WasmFeatures::default(); + wasm_features.insert(WasmFeatures::MEMORY64); let mut validator = Validator::new_with_features(wasm_features); From 6c0a24f085dd0bd0a038bc6b708c232d6b77bd1a Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 28 Jun 2024 22:40:05 -0400 Subject: [PATCH 15/31] no memory64 --- crates/tests/tests/spec-tests.rs | 2 +- src/module/mod.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/tests/tests/spec-tests.rs b/crates/tests/tests/spec-tests.rs index 5ff1a657..5d3c9ed3 100644 --- a/crates/tests/tests/spec-tests.rs +++ b/crates/tests/tests/spec-tests.rs @@ -30,7 +30,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { Some("extended-const") => return Ok(()), Some("function-references") => return Ok(()), Some("gc") => return Ok(()), - Some("memory64") => &["--enable-memory64"], + Some("memory64") => return Ok(()), Some("multi-memory") => &["--enable-multi-memory"], Some("relaxed-simd") => return Ok(()), Some("tail-call") => return Ok(()), diff --git a/src/module/mod.rs b/src/module/mod.rs index b32e8ab4..471533c9 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -135,8 +135,7 @@ impl Module { let mut indices = IndicesToIds::default(); // TODO: how should we handle config.only_stable_features? - let mut wasm_features = WasmFeatures::default(); - wasm_features.insert(WasmFeatures::MEMORY64); + let wasm_features = WasmFeatures::default(); let mut validator = Validator::new_with_features(wasm_features); From f1bedde2eda9f7de681ded93eaec54f189ee6153 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Sat, 29 Jun 2024 09:17:15 -0400 Subject: [PATCH 16/31] typo --- crates/tests/tests/spec-tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tests/tests/spec-tests.rs b/crates/tests/tests/spec-tests.rs index 5d3c9ed3..27e3ab89 100644 --- a/crates/tests/tests/spec-tests.rs +++ b/crates/tests/tests/spec-tests.rs @@ -71,7 +71,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { let path = tempdir.path().join(filename); match command["type"].as_str().unwrap() { "assert_invalid" | "assert_malformed" => { - // The multiple-memories feature is on (from wasmparser::WasmFeatures::default()). + // The multiple-memory feature is on (from wasmparser::WasmFeatures::default()). // In imports.wast and memory.wast, following cases will be parsed which should not. if proposal.is_none() && command["text"] == "multiple memories" { continue; From a098696fb11ea32f8bbb31c37cd6b6955375a15c Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Sat, 29 Jun 2024 10:13:56 -0400 Subject: [PATCH 17/31] separate RefType --- src/init_expr.rs | 21 +++++----- src/ir/mod.rs | 6 +-- src/lib.rs | 2 +- src/module/elements.rs | 13 +++---- src/module/functions/local_function/emit.rs | 7 ++-- src/module/functions/local_function/mod.rs | 10 +++-- src/module/imports.rs | 14 +++---- src/module/tables.rs | 19 ++++----- src/passes/used.rs | 4 +- src/ty.rs | 43 ++++++++++++++++----- 10 files changed, 84 insertions(+), 55 deletions(-) diff --git a/src/init_expr.rs b/src/init_expr.rs index aa1b03c8..c8a7f3b4 100644 --- a/src/init_expr.rs +++ b/src/init_expr.rs @@ -3,7 +3,7 @@ use crate::emit::EmitContext; use crate::ir::Value; use crate::parse::IndicesToIds; -use crate::ValType; +use crate::RefType; use crate::{FunctionId, GlobalId, Result}; use anyhow::bail; @@ -16,7 +16,7 @@ pub enum InitExpr { /// A constant value referenced by the global specified Global(GlobalId), /// A null reference - RefNull(ValType), + RefNull(RefType), /// A function initializer RefFunc(FunctionId), } @@ -35,11 +35,15 @@ impl InitExpr { RefNull { hty } => { let val_type = match hty { wasmparser::HeapType::Abstract { shared: _, ty } => match ty { - wasmparser::AbstractHeapType::Func => ValType::Funcref, - wasmparser::AbstractHeapType::Extern => ValType::Externref, - _ => bail!("invalid constant expression"), + wasmparser::AbstractHeapType::Func => RefType::Funcref, + wasmparser::AbstractHeapType::Extern => RefType::Externref, + other => bail!( + "unsupported abstract heap type in constant expression: {other:?}" + ), }, - wasmparser::HeapType::Concrete(_) => bail!("invalid constant expression"), + wasmparser::HeapType::Concrete(_) => { + bail!("unsupported concrete heap type in constant expression") + } }; InitExpr::RefNull(val_type) } @@ -67,15 +71,14 @@ impl InitExpr { wasm_encoder::ConstExpr::global_get(cx.indices.get_global_index(*g)) } InitExpr::RefNull(ty) => wasm_encoder::ConstExpr::ref_null(match ty { - ValType::Externref => wasm_encoder::HeapType::Abstract { + RefType::Externref => wasm_encoder::HeapType::Abstract { shared: false, ty: wasm_encoder::AbstractHeapType::Extern, }, - ValType::Funcref => wasm_encoder::HeapType::Abstract { + RefType::Funcref => wasm_encoder::HeapType::Abstract { shared: false, ty: wasm_encoder::AbstractHeapType::Func, }, - _ => unreachable!(), }), InitExpr::RefFunc(f) => { wasm_encoder::ConstExpr::ref_func(cx.indices.get_func_index(*f)) diff --git a/src/ir/mod.rs b/src/ir/mod.rs index c4dfc251..595061f7 100644 --- a/src/ir/mod.rs +++ b/src/ir/mod.rs @@ -8,8 +8,8 @@ mod traversals; pub use self::traversals::*; use crate::{ - DataId, ElementId, FunctionId, GlobalId, LocalFunction, MemoryId, ModuleTypes, TableId, TypeId, - ValType, + DataId, ElementId, FunctionId, GlobalId, LocalFunction, MemoryId, ModuleTypes, RefType, + TableId, TypeId, ValType, }; use id_arena::Id; use std::fmt; @@ -532,7 +532,7 @@ pub enum Instr { RefNull { /// The type of null that we're producing #[walrus(skip_visit)] - ty: ValType, + ty: RefType, }, /// `ref.is_null` diff --git a/src/lib.rs b/src/lib.rs index d2e52fba..c8895d66 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -38,4 +38,4 @@ pub use crate::init_expr::InitExpr; pub use crate::ir::{Local, LocalId}; pub use crate::module::*; pub use crate::parse::IndicesToIds; -pub use crate::ty::{Type, TypeId, ValType}; +pub use crate::ty::{RefType, Type, TypeId, ValType}; diff --git a/src/module/elements.rs b/src/module/elements.rs index 1e9829ba..d91c6d21 100644 --- a/src/module/elements.rs +++ b/src/module/elements.rs @@ -3,7 +3,7 @@ use crate::emit::{Emit, EmitContext}; use crate::parse::IndicesToIds; use crate::tombstone_arena::{Id, Tombstone, TombstoneArena}; -use crate::{ir::Value, FunctionId, InitExpr, Module, Result, TableId, ValType}; +use crate::{ir::Value, FunctionId, InitExpr, Module, RefType, Result, TableId, ValType}; use anyhow::{bail, Context}; /// A passive element segment identifier @@ -44,7 +44,7 @@ pub enum ElementItems { /// This element contains function indices. Functions(Vec), /// This element contains constant expressions used to initialize the table. - Expressions(ValType, Vec), + Expressions(RefType, Vec), } impl Element { @@ -134,8 +134,8 @@ impl Module { } wasmparser::ElementItems::Expressions(ref_type, items) => { let ty = match ref_type { - wasmparser::RefType::FUNCREF => ValType::Funcref, - wasmparser::RefType::EXTERNREF => ValType::Externref, + wasmparser::RefType::FUNCREF => RefType::Funcref, + wasmparser::RefType::EXTERNREF => RefType::Externref, _ => bail!("unsupported ref type in element segment {}", i), }; let mut init_exprs = Vec::with_capacity(items.count() as usize); @@ -211,9 +211,8 @@ impl Emit for ModuleElements { } ElementItems::Expressions(ty, init_exprs) => { let ref_type = match ty { - ValType::Funcref => wasm_encoder::RefType::FUNCREF, - ValType::Externref => wasm_encoder::RefType::EXTERNREF, - _ => unreachable!(), + RefType::Funcref => wasm_encoder::RefType::FUNCREF, + RefType::Externref => wasm_encoder::RefType::EXTERNREF, }; let const_exprs = init_exprs .iter() diff --git a/src/module/functions/local_function/emit.rs b/src/module/functions/local_function/emit.rs index db775d55..41447bc8 100644 --- a/src/module/functions/local_function/emit.rs +++ b/src/module/functions/local_function/emit.rs @@ -1,8 +1,8 @@ use crate::emit::IdsToIndices; -use crate::ir::*; use crate::map::IdHashMap; use crate::module::functions::LocalFunction; use crate::module::memories::MemoryId; +use crate::{ir::*, RefType}; use wasm_encoder::Instruction; pub(crate) fn run( @@ -775,15 +775,14 @@ impl<'instr> Visitor<'instr> for Emit<'_> { TableSize(e) => Instruction::TableSize(self.indices.get_table_index(e.table)), TableFill(e) => Instruction::TableFill(self.indices.get_table_index(e.table)), RefNull(e) => Instruction::RefNull(match &e.ty { - crate::ValType::Externref => wasm_encoder::HeapType::Abstract { + RefType::Externref => wasm_encoder::HeapType::Abstract { shared: false, ty: wasm_encoder::AbstractHeapType::Extern, }, - crate::ValType::Funcref => wasm_encoder::HeapType::Abstract { + RefType::Funcref => wasm_encoder::HeapType::Abstract { shared: false, ty: wasm_encoder::AbstractHeapType::Func, }, - _ => unreachable!(), }), RefIsNull(_) => Instruction::RefIsNull, RefFunc(e) => Instruction::RefFunc(self.indices.get_func_index(e.func)), diff --git a/src/module/functions/local_function/mod.rs b/src/module/functions/local_function/mod.rs index 2d9bd936..84383fd5 100644 --- a/src/module/functions/local_function/mod.rs +++ b/src/module/functions/local_function/mod.rs @@ -5,9 +5,9 @@ mod emit; use self::context::ValidationContext; use crate::emit::IdsToIndices; -use crate::ir::*; use crate::map::{IdHashMap, IdHashSet}; use crate::parse::IndicesToIds; +use crate::{ir::*, RefType}; use crate::{Data, DataId, FunctionBuilder, FunctionId, MemoryId, Module, Result, TypeId, ValType}; use std::collections::BTreeMap; use std::ops::Range; @@ -1001,9 +1001,11 @@ fn append_instruction<'context>( Operator::RefNull { hty } => { let ty = match hty { wasmparser::HeapType::Abstract { shared: _, ty } => match ty { - wasmparser::AbstractHeapType::Func => ValType::Funcref, - wasmparser::AbstractHeapType::Extern => ValType::Externref, - _ => panic!("unsupported abstract heap type for ref.null"), + wasmparser::AbstractHeapType::Func => RefType::Funcref, + wasmparser::AbstractHeapType::Extern => RefType::Externref, + other => { + panic!("unsupported abstract heap type for ref.null: {:?}", other) + } }, wasmparser::HeapType::Concrete(_) => { panic!("unsupported concrete heap type for ref.null") diff --git a/src/module/imports.rs b/src/module/imports.rs index 3ff6c1bf..248cab5d 100644 --- a/src/module/imports.rs +++ b/src/module/imports.rs @@ -1,12 +1,14 @@ //! A wasm module's imports. +use std::convert::TryInto; + use anyhow::{bail, Context}; use crate::emit::{Emit, EmitContext}; use crate::parse::IndicesToIds; use crate::tombstone_arena::{Id, Tombstone, TombstoneArena}; use crate::{FunctionId, GlobalId, MemoryId, Result, TableId}; -use crate::{Module, TypeId, ValType}; +use crate::{Module, RefType, TypeId, ValType}; /// The id of an import. pub type ImportId = Id; @@ -159,14 +161,13 @@ impl Module { ids.push_func(id.0); } wasmparser::TypeRef::Table(t) => { - let ty = ValType::parse(&wasmparser::ValType::Ref(t.element_type))?; let id = self.add_import_table( entry.module, entry.name, t.table64, t.initial, t.maximum, - ty, + t.element_type.try_into()?, ); ids.push_table(id.0); } @@ -242,7 +243,7 @@ impl Module { table64: bool, initial: u64, maximum: Option, - ty: ValType, + ty: RefType, ) -> (TableId, ImportId) { let import = self.imports.arena.next_id(); let table = self @@ -295,9 +296,8 @@ impl Emit for ModuleImports { let table = cx.module.tables.get(id); wasm_encoder::EntityType::Table(wasm_encoder::TableType { element_type: match table.element_ty { - ValType::Externref => wasm_encoder::RefType::EXTERNREF, - ValType::Funcref => wasm_encoder::RefType::FUNCREF, - _ => panic!("Unexpected table type"), + RefType::Externref => wasm_encoder::RefType::EXTERNREF, + RefType::Funcref => wasm_encoder::RefType::FUNCREF, }, table64: table.table64, minimum: table.initial, diff --git a/src/module/tables.rs b/src/module/tables.rs index 835aba6d..0f9c3aee 100644 --- a/src/module/tables.rs +++ b/src/module/tables.rs @@ -1,10 +1,12 @@ //! Tables within a wasm module. +use std::convert::TryInto; + use crate::emit::{Emit, EmitContext}; use crate::map::IdHashSet; use crate::parse::IndicesToIds; use crate::tombstone_arena::{Id, Tombstone, TombstoneArena}; -use crate::{Element, ImportId, Module, Result, ValType}; +use crate::{Element, ImportId, Module, RefType, Result}; use anyhow::bail; /// The id of a table. @@ -21,7 +23,7 @@ pub struct Table { /// The maximum size of this table pub maximum: Option, /// The type of the elements in this table - pub element_ty: ValType, + pub element_ty: RefType, /// Whether or not this table is imported, and if so what imports it. pub import: Option, /// Active data segments that will be used to initialize this memory. @@ -54,7 +56,7 @@ impl ModuleTables { table64: bool, initial: u64, maximum: Option, - element_ty: ValType, + element_ty: RefType, import: ImportId, ) -> TableId { let id = self.arena.next_id(); @@ -77,7 +79,7 @@ impl ModuleTables { table64: bool, initial: u64, maximum: Option, - element_ty: ValType, + element_ty: RefType, ) -> TableId { let id = self.arena.next_id(); let id2 = self.arena.alloc(Table { @@ -128,7 +130,7 @@ impl ModuleTables { /// /// Returns an error if there are two function tables in this module pub fn main_function_table(&self) -> Result> { - let mut tables = self.iter().filter(|t| t.element_ty == ValType::Funcref); + let mut tables = self.iter().filter(|t| t.element_ty == RefType::Funcref); let id = match tables.next() { Some(t) => t.id(), None => return Ok(None), @@ -159,7 +161,7 @@ impl Module { t.ty.table64, t.ty.initial, t.ty.maximum, - ValType::parse(&wasmparser::ValType::Ref(t.ty.element_type))?, + t.ty.element_type.try_into()?, ); ids.push_table(id); } @@ -187,9 +189,8 @@ impl Emit for ModuleTables { minimum: table.initial, maximum: table.maximum, element_type: match table.element_ty { - ValType::Externref => wasm_encoder::RefType::EXTERNREF, - ValType::Funcref => wasm_encoder::RefType::FUNCREF, - _ => unreachable!(), + RefType::Externref => wasm_encoder::RefType::EXTERNREF, + RefType::Funcref => wasm_encoder::RefType::FUNCREF, }, }); } diff --git a/src/passes/used.rs b/src/passes/used.rs index 5e6329bc..e127fac8 100644 --- a/src/passes/used.rs +++ b/src/passes/used.rs @@ -1,7 +1,7 @@ use crate::ir::*; use crate::map::IdHashSet; use crate::{ActiveDataLocation, Data, DataId, DataKind, Element, ExportItem, Function, InitExpr}; -use crate::{ElementId, ElementItems, ElementKind, Module, Type, TypeId}; +use crate::{ElementId, ElementItems, ElementKind, Module, RefType, Type, TypeId}; use crate::{FunctionId, FunctionKind, Global, GlobalId}; use crate::{GlobalKind, Memory, MemoryId, Table, TableId}; @@ -209,7 +209,7 @@ impl Used { stack.push_func(*f); }); } - if let ElementItems::Expressions(crate::ValType::Funcref, items) = &e.items { + if let ElementItems::Expressions(RefType::Funcref, items) = &e.items { for item in items { match item { InitExpr::Global(g) => { diff --git a/src/ty.rs b/src/ty.rs index 73d59813..93f368ba 100644 --- a/src/ty.rs +++ b/src/ty.rs @@ -5,6 +5,7 @@ use crate::tombstone_arena::Tombstone; use anyhow::bail; use id_arena::Id; use std::cmp::Ordering; +use std::convert::TryFrom; use std::fmt; use std::hash; @@ -135,10 +136,32 @@ pub enum ValType { F64, /// 128-bit vector. V128, - /// The `externref` opaque value type - Externref, - /// The `funcref` value type, representing a callable function + /// Reference. + Ref(RefType), +} + +/// A reference type. +/// +/// The "function references" and "gc" proposals will add more reference types. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[non_exhaustive] +pub enum RefType { + /// A nullable reference to an untyped function Funcref, + /// A nullable reference to an extern object + Externref, +} + +impl TryFrom for RefType { + type Error = anyhow::Error; + + fn try_from(ref_type: wasmparser::RefType) -> Result { + match ref_type { + wasmparser::RefType::FUNCREF => Ok(RefType::Funcref), + wasmparser::RefType::EXTERNREF => Ok(RefType::Externref), + _ => bail!("unsupported ref type {:?}", ref_type), + } + } } impl ValType { @@ -154,8 +177,10 @@ impl ValType { ValType::F32 => wasm_encoder::ValType::F32, ValType::F64 => wasm_encoder::ValType::F64, ValType::V128 => wasm_encoder::ValType::V128, - ValType::Externref => wasm_encoder::ValType::Ref(wasm_encoder::RefType::EXTERNREF), - ValType::Funcref => wasm_encoder::ValType::Ref(wasm_encoder::RefType::FUNCREF), + ValType::Ref(ref_type) => match ref_type { + RefType::Externref => wasm_encoder::ValType::Ref(wasm_encoder::RefType::EXTERNREF), + RefType::Funcref => wasm_encoder::ValType::Ref(wasm_encoder::RefType::FUNCREF), + }, } } @@ -167,8 +192,8 @@ impl ValType { wasmparser::ValType::F64 => Ok(ValType::F64), wasmparser::ValType::V128 => Ok(ValType::V128), wasmparser::ValType::Ref(ref_type) => match *ref_type { - wasmparser::RefType::EXTERNREF => Ok(ValType::Externref), - wasmparser::RefType::FUNCREF => Ok(ValType::Funcref), + wasmparser::RefType::EXTERNREF => Ok(ValType::Ref(RefType::Externref)), + wasmparser::RefType::FUNCREF => Ok(ValType::Ref(RefType::Funcref)), _ => bail!("unsupported ref type {:?}", ref_type), }, } @@ -186,8 +211,8 @@ impl fmt::Display for ValType { ValType::F32 => "f32", ValType::F64 => "f64", ValType::V128 => "v128", - ValType::Externref => "externref", - ValType::Funcref => "funcref", + ValType::Ref(RefType::Externref) => "externref", + ValType::Ref(RefType::Funcref) => "funcref", } ) } From 0f93d3ce555f2384b609b0ea84e873cbdea8bbaf Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Sat, 29 Jun 2024 10:18:40 -0400 Subject: [PATCH 18/31] rename InitExpr -> ConstExpr --- src/{init_expr.rs => const_expr.rs} | 30 +++++++++++----------- src/lib.rs | 4 +-- src/module/data.rs | 8 +++--- src/module/elements.rs | 26 +++++++++---------- src/module/functions/local_function/mod.rs | 2 +- src/module/globals.rs | 10 ++++---- src/passes/used.rs | 16 ++++++------ 7 files changed, 48 insertions(+), 48 deletions(-) rename src/{init_expr.rs => const_expr.rs} (78%) diff --git a/src/init_expr.rs b/src/const_expr.rs similarity index 78% rename from src/init_expr.rs rename to src/const_expr.rs index c8a7f3b4..018c5361 100644 --- a/src/init_expr.rs +++ b/src/const_expr.rs @@ -10,7 +10,7 @@ use anyhow::bail; /// A constant which is produced in WebAssembly, typically used in global /// initializers or element/data offsets. #[derive(Debug, Copy, Clone)] -pub enum InitExpr { +pub enum ConstExpr { /// An immediate constant value Value(Value), /// A constant value referenced by the global specified @@ -21,17 +21,17 @@ pub enum InitExpr { RefFunc(FunctionId), } -impl InitExpr { - pub(crate) fn eval(init: &wasmparser::ConstExpr, ids: &IndicesToIds) -> Result { +impl ConstExpr { + pub(crate) fn eval(init: &wasmparser::ConstExpr, ids: &IndicesToIds) -> Result { use wasmparser::Operator::*; let mut reader = init.get_operators_reader(); let val = match reader.read()? { - I32Const { value } => InitExpr::Value(Value::I32(value)), - I64Const { value } => InitExpr::Value(Value::I64(value)), - F32Const { value } => InitExpr::Value(Value::F32(f32::from_bits(value.bits()))), - F64Const { value } => InitExpr::Value(Value::F64(f64::from_bits(value.bits()))), - V128Const { value } => InitExpr::Value(Value::V128(v128_to_u128(&value))), - GlobalGet { global_index } => InitExpr::Global(ids.get_global(global_index)?), + I32Const { value } => ConstExpr::Value(Value::I32(value)), + I64Const { value } => ConstExpr::Value(Value::I64(value)), + F32Const { value } => ConstExpr::Value(Value::F32(f32::from_bits(value.bits()))), + F64Const { value } => ConstExpr::Value(Value::F64(f64::from_bits(value.bits()))), + V128Const { value } => ConstExpr::Value(Value::V128(v128_to_u128(&value))), + GlobalGet { global_index } => ConstExpr::Global(ids.get_global(global_index)?), RefNull { hty } => { let val_type = match hty { wasmparser::HeapType::Abstract { shared: _, ty } => match ty { @@ -45,9 +45,9 @@ impl InitExpr { bail!("unsupported concrete heap type in constant expression") } }; - InitExpr::RefNull(val_type) + ConstExpr::RefNull(val_type) } - RefFunc { function_index } => InitExpr::RefFunc(ids.get_func(function_index)?), + RefFunc { function_index } => ConstExpr::RefFunc(ids.get_func(function_index)?), _ => bail!("invalid constant expression"), }; match reader.read()? { @@ -60,17 +60,17 @@ impl InitExpr { pub(crate) fn to_wasmencoder_type(&self, cx: &EmitContext) -> wasm_encoder::ConstExpr { match self { - InitExpr::Value(v) => match v { + ConstExpr::Value(v) => match v { Value::I32(v) => wasm_encoder::ConstExpr::i32_const(*v), Value::I64(v) => wasm_encoder::ConstExpr::i64_const(*v), Value::F32(v) => wasm_encoder::ConstExpr::f32_const(*v), Value::F64(v) => wasm_encoder::ConstExpr::f64_const(*v), Value::V128(v) => wasm_encoder::ConstExpr::v128_const(*v as i128), }, - InitExpr::Global(g) => { + ConstExpr::Global(g) => { wasm_encoder::ConstExpr::global_get(cx.indices.get_global_index(*g)) } - InitExpr::RefNull(ty) => wasm_encoder::ConstExpr::ref_null(match ty { + ConstExpr::RefNull(ty) => wasm_encoder::ConstExpr::ref_null(match ty { RefType::Externref => wasm_encoder::HeapType::Abstract { shared: false, ty: wasm_encoder::AbstractHeapType::Extern, @@ -80,7 +80,7 @@ impl InitExpr { ty: wasm_encoder::AbstractHeapType::Func, }, }), - InitExpr::RefFunc(f) => { + ConstExpr::RefFunc(f) => { wasm_encoder::ConstExpr::ref_func(cx.indices.get_func_index(*f)) } } diff --git a/src/lib.rs b/src/lib.rs index c8895d66..5eb4223f 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,11 +18,11 @@ macro_rules! maybe_parallel { } mod arena_set; +mod const_expr; pub mod dot; mod emit; mod error; mod function_builder; -mod init_expr; pub mod ir; mod map; mod module; @@ -31,10 +31,10 @@ pub mod passes; mod tombstone_arena; mod ty; +pub use crate::const_expr::ConstExpr; pub use crate::emit::IdsToIndices; pub use crate::error::{ErrorKind, Result}; pub use crate::function_builder::{FunctionBuilder, InstrSeqBuilder}; -pub use crate::init_expr::InitExpr; pub use crate::ir::{Local, LocalId}; pub use crate::module::*; pub use crate::parse::IndicesToIds; diff --git a/src/module/data.rs b/src/module/data.rs index d4fc10c9..55a3c740 100644 --- a/src/module/data.rs +++ b/src/module/data.rs @@ -4,7 +4,7 @@ use crate::emit::{Emit, EmitContext}; use crate::ir::Value; use crate::parse::IndicesToIds; use crate::tombstone_arena::{Id, Tombstone, TombstoneArena}; -use crate::{GlobalId, InitExpr, MemoryId, Module, Result, ValType}; +use crate::{ConstExpr, GlobalId, MemoryId, Module, Result, ValType}; use anyhow::{bail, Context}; /// A passive element segment identifier @@ -230,15 +230,15 @@ impl Module { let memory = self.memories.get_mut(memory_id); memory.data_segments.insert(data.id); - let offset = InitExpr::eval(&offset_expr, ids) + let offset = ConstExpr::eval(&offset_expr, ids) .with_context(|| format!("in segment {}", i))?; data.kind = DataKind::Active(ActiveData { memory: memory_id, location: match offset { - InitExpr::Value(Value::I32(n)) => { + ConstExpr::Value(Value::I32(n)) => { ActiveDataLocation::Absolute(n as u32) } - InitExpr::Global(global) + ConstExpr::Global(global) if self.globals.get(global).ty == ValType::I32 => { ActiveDataLocation::Relative(global) diff --git a/src/module/elements.rs b/src/module/elements.rs index d91c6d21..672defbe 100644 --- a/src/module/elements.rs +++ b/src/module/elements.rs @@ -3,7 +3,7 @@ use crate::emit::{Emit, EmitContext}; use crate::parse::IndicesToIds; use crate::tombstone_arena::{Id, Tombstone, TombstoneArena}; -use crate::{ir::Value, FunctionId, InitExpr, Module, RefType, Result, TableId, ValType}; +use crate::{ir::Value, ConstExpr, FunctionId, Module, RefType, Result, TableId, ValType}; use anyhow::{bail, Context}; /// A passive element segment identifier @@ -34,7 +34,7 @@ pub enum ElementKind { /// The ID of the table being initialized. table: TableId, /// A constant defining an offset into that table. - offset: InitExpr, + offset: ConstExpr, }, } @@ -44,7 +44,7 @@ pub enum ElementItems { /// This element contains function indices. Functions(Vec), /// This element contains constant expressions used to initialize the table. - Expressions(RefType, Vec), + Expressions(RefType, Vec), } impl Element { @@ -138,18 +138,18 @@ impl Module { wasmparser::RefType::EXTERNREF => RefType::Externref, _ => bail!("unsupported ref type in element segment {}", i), }; - let mut init_exprs = Vec::with_capacity(items.count() as usize); + let mut const_exprs = Vec::with_capacity(items.count() as usize); for item in items { let const_expr = item?; - let expr = InitExpr::eval(&const_expr, ids).with_context(|| { + let expr = ConstExpr::eval(&const_expr, ids).with_context(|| { format!( "Failed to evaluate a const expr in element segment {}:\n{:?}", i, const_expr ) })?; - init_exprs.push(expr); + const_exprs.push(expr); } - ElementItems::Expressions(ty, init_exprs) + ElementItems::Expressions(ty, const_exprs) } }; @@ -166,12 +166,12 @@ impl Module { let table = ids.get_table(table_index.unwrap_or_default())?; self.tables.get_mut(table).elem_segments.insert(id); - let offset = InitExpr::eval(&offset_expr, ids) + let offset = ConstExpr::eval(&offset_expr, ids) .with_context(|| format!("in segment {}", i))?; match offset { - InitExpr::Value(Value::I32(_)) => {} - InitExpr::Global(global) if self.globals.get(global).ty == ValType::I32 => { - } + ConstExpr::Value(Value::I32(_)) => {} + ConstExpr::Global(global) + if self.globals.get(global).ty == ValType::I32 => {} _ => bail!("non-i32 constant in segment {}", i), } ElementKind::Active { table, offset } @@ -209,12 +209,12 @@ impl Emit for ModuleElements { let els = wasm_encoder::Elements::Functions(&idx); emit_elem(cx, &mut wasm_element_section, &element.kind, els); } - ElementItems::Expressions(ty, init_exprs) => { + ElementItems::Expressions(ty, const_exprs) => { let ref_type = match ty { RefType::Funcref => wasm_encoder::RefType::FUNCREF, RefType::Externref => wasm_encoder::RefType::EXTERNREF, }; - let const_exprs = init_exprs + let const_exprs = const_exprs .iter() .map(|expr| expr.to_wasmencoder_type(cx)) .collect::>(); diff --git a/src/module/functions/local_function/mod.rs b/src/module/functions/local_function/mod.rs index 84383fd5..d4c73b0f 100644 --- a/src/module/functions/local_function/mod.rs +++ b/src/module/functions/local_function/mod.rs @@ -402,7 +402,7 @@ fn append_instruction<'context>( Operator::F32Const { value } => const_(ctx, Value::F32(f32::from_bits(value.bits()))), Operator::F64Const { value } => const_(ctx, Value::F64(f64::from_bits(value.bits()))), Operator::V128Const { value } => { - let val = crate::init_expr::v128_to_u128(&value); + let val = crate::const_expr::v128_to_u128(&value); const_(ctx, Value::V128(val)) } Operator::I32Eqz => unop(ctx, UnaryOp::I32Eqz), diff --git a/src/module/globals.rs b/src/module/globals.rs index ed3de487..59392506 100644 --- a/src/module/globals.rs +++ b/src/module/globals.rs @@ -2,7 +2,7 @@ use crate::emit::{Emit, EmitContext}; use crate::parse::IndicesToIds; use crate::tombstone_arena::{Id, Tombstone, TombstoneArena}; -use crate::{ImportId, InitExpr, Module, Result, ValType}; +use crate::{ConstExpr, ImportId, Module, Result, ValType}; /// The id of a global. pub type GlobalId = Id; @@ -34,7 +34,7 @@ pub enum GlobalKind { /// An imported global without a known initializer Import(ImportId), /// A locally declare global with the specified identifier - Local(InitExpr), + Local(ConstExpr), } impl Global { @@ -77,7 +77,7 @@ impl ModuleGlobals { ty: ValType, mutable: bool, shared: bool, - init: InitExpr, + init: ConstExpr, ) -> GlobalId { self.arena.alloc_with_id(|id| Global { id, @@ -127,7 +127,7 @@ impl Module { ValType::parse(&g.ty.content_type)?, g.ty.mutable, g.ty.shared, - InitExpr::eval(&g.init_expr, ids)?, + ConstExpr::eval(&g.init_expr, ids)?, ); ids.push_global(id); } @@ -140,7 +140,7 @@ impl Emit for ModuleGlobals { log::debug!("emit global section"); let mut wasm_global_section = wasm_encoder::GlobalSection::new(); - fn get_local(global: &Global) -> Option<(&Global, &InitExpr)> { + fn get_local(global: &Global) -> Option<(&Global, &ConstExpr)> { match &global.kind { GlobalKind::Import(_) => None, GlobalKind::Local(local) => Some((global, local)), diff --git a/src/passes/used.rs b/src/passes/used.rs index e127fac8..f39a1174 100644 --- a/src/passes/used.rs +++ b/src/passes/used.rs @@ -1,6 +1,6 @@ use crate::ir::*; use crate::map::IdHashSet; -use crate::{ActiveDataLocation, Data, DataId, DataKind, Element, ExportItem, Function, InitExpr}; +use crate::{ActiveDataLocation, ConstExpr, Data, DataId, DataKind, Element, ExportItem, Function}; use crate::{ElementId, ElementItems, ElementKind, Module, RefType, Type, TypeId}; use crate::{FunctionId, FunctionKind, Global, GlobalId}; use crate::{GlobalKind, Memory, MemoryId, Table, TableId}; @@ -175,14 +175,14 @@ impl Used { while let Some(t) = stack.globals.pop() { match &module.globals.get(t).kind { GlobalKind::Import(_) => {} - GlobalKind::Local(InitExpr::Global(global)) => { + GlobalKind::Local(ConstExpr::Global(global)) => { stack.push_global(*global); } - GlobalKind::Local(InitExpr::RefFunc(func)) => { + GlobalKind::Local(ConstExpr::RefFunc(func)) => { stack.push_func(*func); } - GlobalKind::Local(InitExpr::Value(_)) - | GlobalKind::Local(InitExpr::RefNull(_)) => {} + GlobalKind::Local(ConstExpr::Value(_)) + | GlobalKind::Local(ConstExpr::RefNull(_)) => {} } } @@ -212,10 +212,10 @@ impl Used { if let ElementItems::Expressions(RefType::Funcref, items) = &e.items { for item in items { match item { - InitExpr::Global(g) => { + ConstExpr::Global(g) => { stack.push_global(*g); } - InitExpr::RefFunc(f) => { + ConstExpr::RefFunc(f) => { stack.push_func(*f); } _ => {} @@ -223,7 +223,7 @@ impl Used { } } if let ElementKind::Active { offset, table } = &e.kind { - if let InitExpr::Global(g) = offset { + if let ConstExpr::Global(g) = offset { stack.push_global(*g); } stack.push_table(*table); From 4f029f09ef4467849e5a38de88122b53b2f52fab Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Sat, 29 Jun 2024 10:29:41 -0400 Subject: [PATCH 19/31] align Memory with wasm-tools --- src/module/functions/mod.rs | 2 +- src/module/imports.rs | 8 ++++--- src/module/memories.rs | 44 +++++++++++++++++++++++++++---------- 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/module/functions/mod.rs b/src/module/functions/mod.rs index 4d4d4625..871bd5ba 100644 --- a/src/module/functions/mod.rs +++ b/src/module/functions/mod.rs @@ -690,7 +690,7 @@ mod tests { #[test] fn get_memory_id() { let mut module = Module::default(); - let expected_id = module.memories.add_local(false, false, 0, None); + let expected_id = module.memories.add_local(false, false, 0, None, None); assert!(module.get_memory_id().is_ok_and(|id| id == expected_id)); } diff --git a/src/module/imports.rs b/src/module/imports.rs index 248cab5d..b003584b 100644 --- a/src/module/imports.rs +++ b/src/module/imports.rs @@ -182,6 +182,7 @@ impl Module { m.memory64, m.initial, m.maximum, + m.page_size_log2, ); ids.push_memory(id.0); } @@ -226,11 +227,12 @@ impl Module { memory64: bool, initial: u64, maximum: Option, + page_size_log2: Option, ) -> (MemoryId, ImportId) { let import = self.imports.arena.next_id(); - let mem = self - .memories - .add_import(shared, memory64, initial, maximum, import); + let mem = + self.memories + .add_import(shared, memory64, initial, maximum, page_size_log2, import); self.imports.add(module, name, mem); (mem, import) } diff --git a/src/module/memories.rs b/src/module/memories.rs index eb6d2761..1bd8b415 100644 --- a/src/module/memories.rs +++ b/src/module/memories.rs @@ -14,14 +14,24 @@ pub type MemoryId = Id; #[derive(Debug)] pub struct Memory { id: MemoryId, - /// Is this memory shared? + /// Whether or not this is a “shared” memory + /// + /// This is part of the threads proposal. pub shared: bool, /// Whether or not this is a 64-bit memory. + /// + /// This is part of the memory64 proposal. pub memory64: bool, - /// The initial page size for this memory. + /// Initial size of this memory, in wasm pages. pub initial: u64, - /// The maximum page size for this memory. + /// Optional maximum size of this memory, in wasm pages. pub maximum: Option, + ///The log base 2 of the memory’s custom page size. + /// + /// Memory pages are, by default, 64KiB large (i.e. 216 or 65536). + /// + /// The custom-page-sizes proposal allows changing it to other values. + pub page_size_log2: Option, /// Whether or not this memory is imported, and if so from where. pub import: Option, /// Active data segments that will be used to initialize this memory. @@ -58,6 +68,7 @@ impl ModuleMemories { memory64: bool, initial: u64, maximum: Option, + page_size_log2: Option, import: ImportId, ) -> MemoryId { let id = self.arena.next_id(); @@ -67,6 +78,7 @@ impl ModuleMemories { memory64, initial, maximum, + page_size_log2, import: Some(import), data_segments: Default::default(), name: None, @@ -83,6 +95,7 @@ impl ModuleMemories { memory64: bool, initial: u64, maximum: Option, + page_size_log2: Option, ) -> MemoryId { let id = self.arena.next_id(); let id2 = self.arena.alloc(Memory { @@ -91,6 +104,7 @@ impl ModuleMemories { memory64, initial, maximum, + page_size_log2, import: None, data_segments: Default::default(), name: None, @@ -146,9 +160,13 @@ impl Module { if m.memory64 { bail!("64-bit memories not supported") }; - let id = self - .memories - .add_local(m.shared, m.memory64, m.initial, m.maximum); + let id = self.memories.add_local( + m.shared, + m.memory64, + m.initial, + m.maximum, + m.page_size_log2, + ); ids.push_memory(id); } Ok(()) @@ -171,11 +189,11 @@ impl Emit for ModuleMemories { cx.indices.push_memory(memory.id()); wasm_memory_section.memory(wasm_encoder::MemoryType { - minimum: memory.initial as u64, - maximum: memory.maximum.map(|v| v as u64), - memory64: false, + minimum: memory.initial, + maximum: memory.maximum, + memory64: memory.memory64, shared: memory.shared, - page_size_log2: None, // No support for the custom-page-sizes proposal + page_size_log2: memory.page_size_log2, }); } @@ -192,10 +210,12 @@ mod tests { let mut module = Module::default(); assert_eq!(module.memories.len(), 0); - module.memories.add_local(false, false, 0, Some(1024)); + module.memories.add_local(false, false, 0, Some(1024), None); assert_eq!(module.memories.len(), 1); - module.memories.add_local(true, true, 1024, Some(2048)); + module + .memories + .add_local(true, true, 1024, Some(2048), Some(16)); assert_eq!(module.memories.len(), 2); } } From 2d54251118e39d4f086d82dea9f42b77598322cf Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Sat, 29 Jun 2024 10:35:32 -0400 Subject: [PATCH 20/31] TODO for operators --- src/module/functions/local_function/mod.rs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/module/functions/local_function/mod.rs b/src/module/functions/local_function/mod.rs index d4c73b0f..9ff1d331 100644 --- a/src/module/functions/local_function/mod.rs +++ b/src/module/functions/local_function/mod.rs @@ -1323,17 +1323,7 @@ fn append_instruction<'context>( ctx.alloc_instr(ElemDrop { elem }, loc); } - Operator::ReturnCall { .. } - | Operator::ReturnCallIndirect { .. } - | Operator::Try { blockty: _ } - | Operator::Catch { tag_index: _ } - | Operator::Throw { tag_index: _ } - | Operator::Rethrow { relative_depth: _ } - | Operator::Delegate { relative_depth: _ } - | Operator::CatchAll => { - unimplemented!("not supported") - } - - _ => todo!("Many operators are not yet implemented"), + // TODO: The remaining operators are for some on-going proposals + other => unimplemented!("unsupported operator: {:?}", other), } } From 8272e87b7cf89969eaaa48060203432865f487f1 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Sat, 29 Jun 2024 10:36:54 -0400 Subject: [PATCH 21/31] apply clippy fix --- crates/fuzz-utils/src/lib.rs | 11 ++++---- crates/macro/src/lib.rs | 18 ++++++------- crates/tests-utils/src/lib.rs | 2 +- crates/tests/src/lib.rs | 26 +++++++++---------- crates/tests/tests/spec-tests.rs | 9 +++---- examples/round-trip.rs | 2 +- src/const_expr.rs | 18 ++++++------- src/dot.rs | 8 +++--- src/module/config.rs | 2 ++ src/module/custom.rs | 25 +++++------------- src/module/data.rs | 7 ++--- src/module/debug/dwarf.rs | 13 +++++----- src/module/debug/units.rs | 4 +-- src/module/elements.rs | 2 +- .../functions/local_function/context.rs | 4 +-- src/module/functions/local_function/emit.rs | 4 +-- src/module/functions/local_function/mod.rs | 15 ++++------- src/module/functions/mod.rs | 6 ++--- src/module/imports.rs | 5 ++-- src/module/memories.rs | 7 ++++- src/module/mod.rs | 20 +++++++------- src/module/producers.rs | 2 +- src/module/types.rs | 10 ++++--- src/parse.rs | 2 +- src/passes/used.rs | 14 +++++----- src/ty.rs | 6 ++--- 26 files changed, 117 insertions(+), 125 deletions(-) diff --git a/crates/fuzz-utils/src/lib.rs b/crates/fuzz-utils/src/lib.rs index c595f25c..a86bc028 100755 --- a/crates/fuzz-utils/src/lib.rs +++ b/crates/fuzz-utils/src/lib.rs @@ -68,7 +68,7 @@ where .join("..") // pop `crates` .join("target") .join("walrus-fuzz"); - fs::create_dir_all(&dir).expect(&format!("should create directory: {:?}", dir)); + fs::create_dir_all(&dir).unwrap_or_else(|_| panic!("should create directory: {:?}", dir)); let scratch = tempfile::NamedTempFile::new_in(dir).expect("should create temp file OK"); @@ -99,20 +99,20 @@ where } fn interp(&self, wasm: &[u8]) -> Result { - fs::write(self.scratch.path(), &wasm).context("failed to write to scratch file")?; + fs::write(self.scratch.path(), wasm).context("failed to write to scratch file")?; wasm_interp(self.scratch.path()) } fn round_trip_through_walrus(&self, wasm: &[u8]) -> Result> { let mut module = - walrus::Module::from_buffer(&wasm).context("walrus failed to parse the wasm buffer")?; + walrus::Module::from_buffer(wasm).context("walrus failed to parse the wasm buffer")?; walrus::passes::gc::run(&mut module); let buf = module.emit_wasm(); Ok(buf) } fn test_wat(&self, wat: &str) -> Result<()> { - let wasm = self.wat2wasm(&wat)?; + let wasm = self.wat2wasm(wat)?; let expected = self.interp(&wasm)?; let walrus_wasm = self.round_trip_through_walrus(&wasm)?; @@ -160,6 +160,7 @@ where Ok(()) => { // We reduced fuel as far as we could, so return the last // failing test case. + #[allow(clippy::question_mark)] if failing.is_err() { return failing; } @@ -313,7 +314,7 @@ impl WatGen { self.wat.push_str(&operator.to_string()); for op in immediates.into_iter() { - self.wat.push_str(" "); + self.wat.push(' '); self.wat.push_str(op.as_ref()); } diff --git a/crates/macro/src/lib.rs b/crates/macro/src/lib.rs index 07029f2a..36731a7a 100755 --- a/crates/macro/src/lib.rs +++ b/crates/macro/src/lib.rs @@ -103,7 +103,7 @@ impl Parse for WalrusFieldOpts { if attr == "skip_visit" { return Ok(Attr::SkipVisit); } - return Err(Error::new(attr.span(), "unexpected attribute")); + Err(Error::new(attr.span(), "unexpected attribute")) } } } @@ -144,7 +144,7 @@ impl Parse for WalrusVariantOpts { if attr == "skip_builder" { return Ok(Attr::SkipBuilder); } - return Err(Error::new(attr.span(), "unexpected attribute")); + Err(Error::new(attr.span(), "unexpected attribute")) } } } @@ -166,7 +166,7 @@ fn walrus_attrs(attrs: &mut Vec) -> TokenStream { ret.extend(group); ret.extend(quote! { , }); } - return ret.into(); + ret.into() } fn create_types(attrs: &[syn::Attribute], variants: &[WalrusVariant]) -> impl quote::ToTokens { @@ -328,10 +328,10 @@ fn create_types(attrs: &[syn::Attribute], variants: &[WalrusVariant]) -> impl qu } } -fn visit_fields<'a>( - variant: &'a WalrusVariant, +fn visit_fields( + variant: &WalrusVariant, allow_skip: bool, -) -> impl Iterator + 'a { +) -> impl Iterator + '_ { return variant .syn .fields @@ -439,7 +439,7 @@ fn create_visit(variants: &[WalrusVariant]) -> impl quote::ToTokens { } }); - let doc = format!("Visit `{}`.", name.to_string()); + let doc = format!("Visit `{}`.", name); visitor_trait_methods.push(quote! { #[doc=#doc] #[inline] @@ -723,13 +723,13 @@ fn create_builder(variants: &[WalrusVariant]) -> impl quote::ToTokens { let doc = format!( "Push a new `{}` instruction onto this builder's block.", - name.to_string() + name ); let at_doc = format!( "Splice a new `{}` instruction into this builder's block at the given index.\n\n\ # Panics\n\n\ Panics if `position > self.instrs.len()`.", - name.to_string() + name ); let arg_names = &arg_names; diff --git a/crates/tests-utils/src/lib.rs b/crates/tests-utils/src/lib.rs index 6840644f..4bad6b49 100644 --- a/crates/tests-utils/src/lib.rs +++ b/crates/tests-utils/src/lib.rs @@ -67,7 +67,7 @@ where cmd.arg(input); cmd.arg("-o"); cmd.arg(tmp.path()); - cmd.args(&[ + cmd.args([ "--enable-threads", "--enable-bulk-memory", // "--enable-reference-types", diff --git a/crates/tests/src/lib.rs b/crates/tests/src/lib.rs index 63dd2584..b167671a 100644 --- a/crates/tests/src/lib.rs +++ b/crates/tests/src/lib.rs @@ -79,11 +79,11 @@ impl FileCheck { let mut iter = contents.lines().map(str::trim); while let Some(line) = iter.next() { if line.starts_with("(; CHECK-ALL:") { - if patterns.len() != 0 { + if !patterns.is_empty() { panic!("CHECK cannot be used with CHECK-ALL"); } let mut pattern = Vec::new(); - while let Some(line) = iter.next() { + for line in iter.by_ref() { if line == ";)" { break; } @@ -94,19 +94,17 @@ impl FileCheck { } return FileCheck::Exhaustive(pattern, path.to_path_buf()); } - - if line.starts_with(";; CHECK:") { - let p = line[";; CHECK:".len()..].to_string(); - patterns.push(vec![p]); + if let Some(p) = line.strip_prefix(";; CHECK:") { + patterns.push(vec![p.to_string()]); } - if line.starts_with(";; NEXT:") { - let p = patterns + if let Some(p) = line.strip_prefix(";; NEXT:") { + let v = patterns .last_mut() .expect("NEXT should never come before CHECK"); - p.push(line[";; NEXT:".len()..].to_string()); + v.push(p.to_string()); } } - if patterns.len() == 0 { + if patterns.is_empty() { FileCheck::None(path.to_path_buf()) } else { FileCheck::Patterns(patterns) @@ -125,7 +123,7 @@ impl FileCheck { 'inner: while let Some(pos) = output_lines[start..] .iter() - .position(|l| matches(*l, first_line)) + .position(|l| matches(l, first_line)) { start = pos + 1; if output_lines[pos..].len() + 1 < pattern.len() { @@ -155,11 +153,11 @@ impl FileCheck { } } FileCheck::None(_) => { - println!(""); + println!(); println!("no test assertions were found in this file, but"); println!("you can rerun tests with `WALRUS_BLESS=1` to"); println!("automatically add assertions to this file"); - println!(""); + println!(); panic!("no tests to run") } } @@ -210,7 +208,7 @@ fn update_output(path: &Path, output: &str) { new_output.push_str(" "); new_output.push_str(line.trim_end()); } - new_output.push_str("\n"); + new_output.push('\n'); } let new = format!( "{}\n\n(; CHECK-ALL:\n{}\n;)\n", diff --git a/crates/tests/tests/spec-tests.rs b/crates/tests/tests/spec-tests.rs index 27e3ab89..76913363 100644 --- a/crates/tests/tests/spec-tests.rs +++ b/crates/tests/tests/spec-tests.rs @@ -19,8 +19,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { let proposal = wast .iter() .skip_while(|part| *part != "proposals") - .skip(1) - .next() + .nth(1) .map(|s| s.to_str().unwrap()); let extra_args: &[&str] = match proposal { @@ -54,7 +53,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { let mut files = Vec::new(); let mut config = walrus::ModuleConfig::new(); - if extra_args.len() == 0 { + if extra_args.is_empty() { config.only_stable_features(true); } @@ -154,7 +153,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { } let wasm = module.emit_wasm(); - fs::write(&file, wasm)?; + fs::write(file, wasm)?; } run_spectest_interp(tempdir.path(), extra_args)?; @@ -177,7 +176,7 @@ fn run_spectest_interp(cwd: &Path, extra_args: &[&str]) -> Result<(), anyhow::Er let stdout = String::from_utf8_lossy(&output.stdout); if let Some(line) = stdout.lines().find(|l| l.ends_with("tests passed.")) { let part = line.split_whitespace().next().unwrap(); - let mut parts = part.split("/"); + let mut parts = part.split('/'); let a = parts.next().unwrap().parse::(); let b = parts.next().unwrap().parse::(); if a == b { diff --git a/examples/round-trip.rs b/examples/round-trip.rs index 716f22fc..80f20c5d 100644 --- a/examples/round-trip.rs +++ b/examples/round-trip.rs @@ -5,7 +5,7 @@ fn main() -> anyhow::Result<()> { let a = std::env::args() .nth(1) .ok_or_else(|| anyhow::anyhow!("must provide the input wasm file as the first argument"))?; - let mut m = walrus::Module::from_file(&a)?; + let mut m = walrus::Module::from_file(a)?; let wasm = m.emit_wasm(); if let Some(destination) = std::env::args().nth(2) { std::fs::write(destination, wasm)?; diff --git a/src/const_expr.rs b/src/const_expr.rs index 018c5361..98d03812 100644 --- a/src/const_expr.rs +++ b/src/const_expr.rs @@ -58,17 +58,17 @@ impl ConstExpr { Ok(val) } - pub(crate) fn to_wasmencoder_type(&self, cx: &EmitContext) -> wasm_encoder::ConstExpr { + pub(crate) fn to_wasmencoder_type(self, cx: &EmitContext) -> wasm_encoder::ConstExpr { match self { ConstExpr::Value(v) => match v { - Value::I32(v) => wasm_encoder::ConstExpr::i32_const(*v), - Value::I64(v) => wasm_encoder::ConstExpr::i64_const(*v), - Value::F32(v) => wasm_encoder::ConstExpr::f32_const(*v), - Value::F64(v) => wasm_encoder::ConstExpr::f64_const(*v), - Value::V128(v) => wasm_encoder::ConstExpr::v128_const(*v as i128), + Value::I32(v) => wasm_encoder::ConstExpr::i32_const(v), + Value::I64(v) => wasm_encoder::ConstExpr::i64_const(v), + Value::F32(v) => wasm_encoder::ConstExpr::f32_const(v), + Value::F64(v) => wasm_encoder::ConstExpr::f64_const(v), + Value::V128(v) => wasm_encoder::ConstExpr::v128_const(v as i128), }, ConstExpr::Global(g) => { - wasm_encoder::ConstExpr::global_get(cx.indices.get_global_index(*g)) + wasm_encoder::ConstExpr::global_get(cx.indices.get_global_index(g)) } ConstExpr::RefNull(ty) => wasm_encoder::ConstExpr::ref_null(match ty { RefType::Externref => wasm_encoder::HeapType::Abstract { @@ -81,7 +81,7 @@ impl ConstExpr { }, }), ConstExpr::RefFunc(f) => { - wasm_encoder::ConstExpr::ref_func(cx.indices.get_func_index(*f)) + wasm_encoder::ConstExpr::ref_func(cx.indices.get_func_index(f)) } } } @@ -89,7 +89,7 @@ impl ConstExpr { pub(crate) fn v128_to_u128(value: &wasmparser::V128) -> u128 { let n = value.bytes(); - ((n[0] as u128) << 0) + (n[0] as u128) | ((n[1] as u128) << 8) | ((n[2] as u128) << 16) | ((n[3] as u128) << 24) diff --git a/src/dot.rs b/src/dot.rs index 1729242c..4dfe4428 100644 --- a/src/dot.rs +++ b/src/dot.rs @@ -103,7 +103,7 @@ impl Dot for T { impl FieldAggregator for AppendFields<'_> { fn add_field(&mut self, field: &[&str]) { - assert!(field.len() > 0); + assert!(!field.is_empty()); self.out.push_str(""); for f in field { self.out.push_str(""); self.out.push_str(""); for f in field { self.out.push_str(""); self.out.push_str(""); for f in field { self.out.push_str(""); self.out.push_str("
"); @@ -114,7 +114,7 @@ impl Dot for T { } fn add_field_with_port(&mut self, port: &str, field: &str) { - assert!(field.len() > 0); + assert!(!field.is_empty()); self.out.push_str("
Dot for T { fn add_edge_from_port(&mut self, port: &str, to: &impl DotName) { self.out.push_str(" "); self.out.push_str(self.from); - self.out.push_str(":"); + self.out.push(':'); self.out.push_str(port); self.out.push_str(" -> "); self.out.push_str(&to.dot_name()); @@ -174,7 +174,7 @@ impl Dot for Module { // self.name.dot(out); // self.config.dot(out); - out.push_str("}"); + out.push('}'); } } diff --git a/src/module/config.rs b/src/module/config.rs index 66470a65..9e9c3ce0 100644 --- a/src/module/config.rs +++ b/src/module/config.rs @@ -15,8 +15,10 @@ pub struct ModuleConfig { pub(crate) skip_producers_section: bool, pub(crate) skip_name_section: bool, pub(crate) preserve_code_transform: bool, + #[allow(clippy::type_complexity)] pub(crate) on_parse: Option Result<()> + Sync + Send + 'static>>, + #[allow(clippy::type_complexity)] pub(crate) on_instr_loc: Option InstrLocId + Sync + Send + 'static>>, } diff --git a/src/module/custom.rs b/src/module/custom.rs index 89469ee7..f1ade53f 100644 --- a/src/module/custom.rs +++ b/src/module/custom.rs @@ -193,10 +193,7 @@ where T: CustomSection, { fn clone(&self) -> Self { - TypedCustomSectionId { - id: self.id, - _phantom: PhantomData, - } + *self } } @@ -366,26 +363,18 @@ impl ModuleCustomSections { /// Iterate over shared references to custom sections and their ids. pub fn iter(&self) -> impl Iterator { - self.arena.iter().flat_map(|(id, s)| { - if let Some(s) = s.as_ref() { - Some((UntypedCustomSectionId(id), &**s)) - } else { - None - } - }) + self.arena + .iter() + .flat_map(|(id, s)| s.as_ref().map(|s| (UntypedCustomSectionId(id), &**s))) } /// Iterate over exclusive references to custom sections and their ids. pub fn iter_mut( &mut self, ) -> impl Iterator { - self.arena.iter_mut().flat_map(|(id, s)| { - if let Some(s) = s.as_mut() { - Some((UntypedCustomSectionId(id), &mut **s)) - } else { - None - } - }) + self.arena + .iter_mut() + .flat_map(|(id, s)| s.as_mut().map(|s| (UntypedCustomSectionId(id), &mut **s))) } /// Remove a custom section (by type) from the module. diff --git a/src/module/data.rs b/src/module/data.rs index 55a3c740..4fa02c3f 100644 --- a/src/module/data.rs +++ b/src/module/data.rs @@ -78,10 +78,7 @@ impl Data { /// Is this a passive data segment? pub fn is_passive(&self) -> bool { - match self.kind { - DataKind::Passive => true, - _ => false, - } + matches!(self.kind, DataKind::Passive) } } @@ -143,7 +140,7 @@ impl ModuleData { let mut any_passive = false; for data in self.iter() { - cx.indices.set_data_index(data.id(), count as u32); + cx.indices.set_data_index(data.id(), count); count += 1; any_passive |= data.is_passive(); } diff --git a/src/module/debug/dwarf.rs b/src/module/debug/dwarf.rs index 5d571e82..dc9f3ea9 100644 --- a/src/module/debug/dwarf.rs +++ b/src/module/debug/dwarf.rs @@ -324,9 +324,9 @@ mod tests { use gimli::*; use std::cell::RefCell; - fn make_test_debug_line<'a>( - debug_line: &'a mut write::DebugLine>, - ) -> IncompleteLineProgram> { + fn make_test_debug_line( + debug_line: &mut write::DebugLine>, + ) -> IncompleteLineProgram> { let encoding = Encoding { format: Format::Dwarf32, version: 4, @@ -368,11 +368,10 @@ mod tests { .unwrap(); let debug_line = read::DebugLine::new(debug_line.slice(), LittleEndian); - let incomplete_debug_line = debug_line - .program(DebugLineOffset(0), 4, None, None) - .unwrap(); - incomplete_debug_line + debug_line + .program(DebugLineOffset(0), 4, None, None) + .unwrap() } #[test] diff --git a/src/module/debug/units.rs b/src/module/debug/units.rs index 10030d88..8905de68 100644 --- a/src/module/debug/units.rs +++ b/src/module/debug/units.rs @@ -76,10 +76,10 @@ mod tests { let mut cursor = DebuggingInformationCursor::new(&mut unit1); - assert_eq!(cursor.current().is_none(), true); + assert!(cursor.current().is_none()); assert_eq!(cursor.next_dfs().unwrap().id(), root_id); assert_eq!(cursor.next_dfs().unwrap().id(), child1_id); assert_eq!(cursor.next_dfs().unwrap().id(), child2_id); - assert_eq!(cursor.next_dfs().is_none(), true); + assert!(cursor.next_dfs().is_none()); } } diff --git a/src/module/elements.rs b/src/module/elements.rs index 672defbe..cd4b3808 100644 --- a/src/module/elements.rs +++ b/src/module/elements.rs @@ -237,7 +237,7 @@ impl Emit for ModuleElements { Some(cx.indices.get_table_index(*table)).filter(|&index| index != 0); wasm_element_section.active( table_index, - &offset.to_wasmencoder_type(&cx), + &offset.to_wasmencoder_type(cx), els, ); } diff --git a/src/module/functions/local_function/context.rs b/src/module/functions/local_function/context.rs index 1902ae60..a29700c7 100644 --- a/src/module/functions/local_function/context.rs +++ b/src/module/functions/local_function/context.rs @@ -110,7 +110,7 @@ impl<'a> ValidationContext<'a> { } pub fn pop_control(&mut self) -> Result<(ControlFrame, InstrSeqId)> { - let frame = impl_pop_control(&mut self.controls)?; + let frame = impl_pop_control(self.controls)?; let block = frame.block; Ok((frame, block)) } @@ -214,7 +214,7 @@ fn impl_push_control_with_ty( fn impl_pop_control(controls: &mut ControlStack) -> Result { controls .last() - .ok_or_else(|| ErrorKind::InvalidWasm) + .ok_or(ErrorKind::InvalidWasm) .context("attempted to pop a frame from an empty control stack")?; let frame = controls.pop().unwrap(); Ok(frame) diff --git a/src/module/functions/local_function/emit.rs b/src/module/functions/local_function/emit.rs index 41447bc8..7f5da78d 100644 --- a/src/module/functions/local_function/emit.rs +++ b/src/module/functions/local_function/emit.rs @@ -87,7 +87,7 @@ impl<'instr> Visitor<'instr> for Emit<'_> { if let Some(map) = self.map.as_mut() { let pos = self.encoder.byte_len(); // Save the encoded_at position for the specified ExprId. - map.push((seq.end.clone(), pos)); + map.push((seq.end, pos)); } if let BlockKind::If = popped_kind.unwrap() { @@ -107,7 +107,7 @@ impl<'instr> Visitor<'instr> for Emit<'_> { if let Some(map) = self.map.as_mut() { let pos = self.encoder.byte_len(); // Save the encoded_at position for the specified ExprId. - map.push((instr_loc.clone(), pos)); + map.push((*instr_loc, pos)); } let is_block = match instr { diff --git a/src/module/functions/local_function/mod.rs b/src/module/functions/local_function/mod.rs index 9ff1d331..107ae266 100644 --- a/src/module/functions/local_function/mod.rs +++ b/src/module/functions/local_function/mod.rs @@ -44,6 +44,7 @@ impl LocalFunction { /// /// Validates the given function body and constructs the `Instr` IR at the /// same time. + #[allow(clippy::too_many_arguments)] pub(crate) fn parse( module: &Module, indices: &IndicesToIds, @@ -69,7 +70,7 @@ impl LocalFunction { }), }; - let result: Vec<_> = module.types.get(ty).results().iter().cloned().collect(); + let result: Vec<_> = module.types.get(ty).results().to_vec(); let result = result.into_boxed_slice(); let controls = &mut context::ControlStack::new(); @@ -206,6 +207,7 @@ impl LocalFunction { } /// Emit this function's compact locals declarations. + #[allow(clippy::type_complexity)] pub(crate) fn emit_locals( &self, module: &Module, @@ -299,11 +301,7 @@ fn block_param_tys(ctx: &ValidationContext, ty: wasmparser::BlockType) -> Result } } -fn append_instruction<'context>( - ctx: &'context mut ValidationContext, - inst: Operator, - loc: InstrLocId, -) { +fn append_instruction(ctx: &mut ValidationContext, inst: Operator, loc: InstrLocId) { // NB. there's a lot of `unwrap()` here in this function, and that's because // the `Operator` was validated above to already be valid, so everything // should succeed. @@ -963,10 +961,7 @@ fn append_instruction<'context>( } Operator::MemoryAtomicWait32 { ref memarg } | Operator::MemoryAtomicWait64 { ref memarg } => { - let sixty_four = match inst { - Operator::MemoryAtomicWait32 { .. } => false, - _ => true, - }; + let sixty_four = !matches!(inst, Operator::MemoryAtomicWait32 { .. }); let (memory, arg) = mem_arg(ctx, memarg); ctx.alloc_instr( AtomicWait { diff --git a/src/module/functions/mod.rs b/src/module/functions/mod.rs index 871bd5ba..42c02505 100644 --- a/src/module/functions/mod.rs +++ b/src/module/functions/mod.rs @@ -200,7 +200,7 @@ impl ModuleFunctions { /// return the first function in the module with the given name. pub fn by_name(&self, name: &str) -> Option { self.arena.iter().find_map(|(id, f)| { - if f.name.as_ref().map(|s| s.as_str()) == Some(name) { + if f.name.as_deref() == Some(name) { Some(id) } else { None @@ -292,7 +292,7 @@ impl ModuleFunctions { pub(crate) fn emit_func_section(&self, cx: &mut EmitContext) { log::debug!("emit function section"); let functions = used_local_functions(cx); - if functions.len() == 0 { + if functions.is_empty() { return; } let mut func_section = wasm_encoder::FunctionSection::new(); @@ -605,7 +605,7 @@ impl Emit for ModuleFunctions { fn emit(&self, cx: &mut EmitContext) { log::debug!("emit code section"); let functions = used_local_functions(cx); - if functions.len() == 0 { + if functions.is_empty() { return; } diff --git a/src/module/imports.rs b/src/module/imports.rs index b003584b..ebb7f301 100644 --- a/src/module/imports.rs +++ b/src/module/imports.rs @@ -219,6 +219,7 @@ impl Module { } /// Add an imported memory to this module + #[allow(clippy::too_many_arguments)] pub fn add_import_memory( &mut self, module: &str, @@ -310,8 +311,8 @@ impl Emit for ModuleImports { cx.indices.push_memory(id); let mem = cx.module.memories.get(id); wasm_encoder::EntityType::Memory(wasm_encoder::MemoryType { - minimum: mem.initial as u64, - maximum: mem.maximum.map(|v| v as u64), + minimum: mem.initial, + maximum: mem.maximum, memory64: false, shared: mem.shared, page_size_log2: None, diff --git a/src/module/memories.rs b/src/module/memories.rs index 1bd8b415..556d484d 100644 --- a/src/module/memories.rs +++ b/src/module/memories.rs @@ -141,10 +141,15 @@ impl ModuleMemories { self.arena.iter_mut().map(|(_, f)| f) } - /// Get the number of memories in this module + /// Get the number of memories in this module. pub fn len(&self) -> usize { self.arena.len() } + + /// Returns true if there are no memories in this module. + pub fn is_empty(&self) -> bool { + self.arena.len() == 0 + } } impl Module { diff --git a/src/module/mod.rs b/src/module/mod.rs index 471533c9..d7530335 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -130,8 +130,10 @@ impl Module { } fn parse(wasm: &[u8], config: &ModuleConfig) -> Result { - let mut ret = Module::default(); - ret.config = config.clone(); + let mut ret = Module { + config: config.clone(), + ..Default::default() + }; let mut indices = IndicesToIds::default(); // TODO: how should we handle config.only_stable_features? @@ -155,7 +157,7 @@ impl Module { validator .data_section(&s) .context("failed to parse data section")?; - ret.parse_data(s, &mut indices)?; + ret.parse_data(s, &indices)?; } Payload::TypeSection(s) => { validator @@ -191,7 +193,7 @@ impl Module { validator .export_section(&s) .context("failed to parse export section")?; - ret.parse_exports(s, &mut indices)?; + ret.parse_exports(s, &indices)?; } Payload::ElementSection(s) => { validator @@ -368,7 +370,7 @@ impl Module { log::debug!("skipping DWARF custom section"); } - let indices = mem::replace(cx.indices, Default::default()); + let indices = std::mem::take(cx.indices); for (_id, section) in customs.iter_mut() { if section.name().starts_with(".debug") { @@ -550,7 +552,7 @@ fn emit_name_section(cx: &mut EmitContext) { Some((*index, name)) }) .collect::>(); - if local_names.len() == 0 { + if local_names.is_empty() { None } else { Some((cx.indices.get_func_index(func.id()), local_names)) @@ -559,7 +561,7 @@ fn emit_name_section(cx: &mut EmitContext) { .collect::>(); locals.sort_by_key(|p| p.0); // sort by index - if cx.module.name.is_none() && funcs.len() == 0 && locals.len() == 0 { + if cx.module.name.is_none() && funcs.is_empty() && locals.is_empty() { return; } @@ -567,7 +569,7 @@ fn emit_name_section(cx: &mut EmitContext) { wasm_name_section.module(name); } - if funcs.len() > 0 { + if !funcs.is_empty() { let mut name_map = wasm_encoder::NameMap::new(); for (index, name) in funcs { name_map.append(index, name); @@ -575,7 +577,7 @@ fn emit_name_section(cx: &mut EmitContext) { wasm_name_section.functions(&name_map); } - if locals.len() > 0 { + if !locals.is_empty() { let mut indirect_name_map = wasm_encoder::IndirectNameMap::new(); for (index, mut map) in locals { let mut name_map = wasm_encoder::NameMap::new(); diff --git a/src/module/producers.rs b/src/module/producers.rs index 8dae0c96..882fc4a2 100644 --- a/src/module/producers.rs +++ b/src/module/producers.rs @@ -101,7 +101,7 @@ impl Module { impl Emit for ModuleProducers { fn emit(&self, cx: &mut EmitContext) { log::debug!("emit producers section"); - if self.fields.len() == 0 { + if self.fields.is_empty() { return; } let mut wasm_producers_section = wasm_encoder::ProducersSection::new(); diff --git a/src/module/types.rs b/src/module/types.rs index 23aec534..a7ad1f29 100644 --- a/src/module/types.rs +++ b/src/module/types.rs @@ -48,7 +48,7 @@ impl ModuleTypes { /// preserve type names from the WAT. pub fn by_name(&self, name: &str) -> Option { self.arena.iter().find_map(|(id, ty)| { - if ty.name.as_ref().map(|s| s.as_str()) == Some(name) { + if ty.name.as_deref() == Some(name) { Some(id) } else { None @@ -163,8 +163,12 @@ impl Emit for ModuleTypes { for (id, ty) in tys { cx.indices.push_type(id); wasm_type_section.function( - ty.params().iter().map(ValType::to_wasmencoder_type), - ty.results().iter().map(ValType::to_wasmencoder_type), + ty.params() + .iter() + .map(|ty| ValType::to_wasmencoder_type(*ty)), + ty.results() + .iter() + .map(|ty| ValType::to_wasmencoder_type(*ty)), ); } diff --git a/src/parse.rs b/src/parse.rs index 77195e38..2c23afd6 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -66,7 +66,7 @@ define_push_get!(push_data, get_data, DataId, data); impl IndicesToIds { /// Pushes a new local ID to map it to the next index internally pub(crate) fn push_local(&mut self, function: FunctionId, id: LocalId) -> u32 { - let list = self.locals.entry(function).or_insert(Vec::new()); + let list = self.locals.entry(function).or_default(); list.push(id); (list.len() as u32) - 1 } diff --git a/src/passes/used.rs b/src/passes/used.rs index f39a1174..eaf301ce 100644 --- a/src/passes/used.rs +++ b/src/passes/used.rs @@ -145,12 +145,12 @@ impl Used { } // Iteratively visit all items until our stack is empty - while stack.funcs.len() > 0 - || stack.tables.len() > 0 - || stack.memories.len() > 0 - || stack.globals.len() > 0 - || stack.datas.len() > 0 - || stack.elements.len() > 0 + while !stack.funcs.is_empty() + || !stack.tables.is_empty() + || !stack.memories.is_empty() + || !stack.globals.is_empty() + || !stack.datas.is_empty() + || !stack.elements.is_empty() { while let Some(f) = stack.funcs.pop() { let func = module.funcs.get(f); @@ -238,7 +238,7 @@ impl Used { // Let's keep `wabt` passing though and just say that if there are data // segments kept, but no memories, then we try to add the first memory, // if any, to the used set. - if stack.used.data.len() > 0 && stack.used.memories.len() == 0 { + if !stack.used.data.is_empty() && stack.used.memories.is_empty() { if let Some(mem) = module.memories.iter().next() { stack.used.memories.insert(mem.id()); } diff --git a/src/ty.rs b/src/ty.rs index 93f368ba..2f8ceca7 100644 --- a/src/ty.rs +++ b/src/ty.rs @@ -109,13 +109,13 @@ impl Type { /// Get the parameters to this function type. #[inline] pub fn params(&self) -> &[ValType] { - &*self.params + &self.params } /// Get the results of this function type. #[inline] pub fn results(&self) -> &[ValType] { - &*self.results + &self.results } pub(crate) fn is_for_function_entry(&self) -> bool { @@ -170,7 +170,7 @@ impl ValType { Ok(v.into_boxed_slice()) } - pub(crate) fn to_wasmencoder_type(&self) -> wasm_encoder::ValType { + pub(crate) fn to_wasmencoder_type(self) -> wasm_encoder::ValType { match self { ValType::I32 => wasm_encoder::ValType::I32, ValType::I64 => wasm_encoder::ValType::I64, From f455d796c8799b553231aa7286dcbc2e67bf538a Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Sat, 29 Jun 2024 14:33:00 -0400 Subject: [PATCH 22/31] Revert "apply clippy fix" This reverts commit 8272e87b7cf89969eaaa48060203432865f487f1. --- crates/fuzz-utils/src/lib.rs | 11 ++++---- crates/macro/src/lib.rs | 18 ++++++------- crates/tests-utils/src/lib.rs | 2 +- crates/tests/src/lib.rs | 26 ++++++++++--------- crates/tests/tests/spec-tests.rs | 9 ++++--- examples/round-trip.rs | 2 +- src/const_expr.rs | 18 ++++++------- src/dot.rs | 8 +++--- src/module/config.rs | 2 -- src/module/custom.rs | 25 +++++++++++++----- src/module/data.rs | 7 +++-- src/module/debug/dwarf.rs | 13 +++++----- src/module/debug/units.rs | 4 +-- src/module/elements.rs | 2 +- .../functions/local_function/context.rs | 4 +-- src/module/functions/local_function/emit.rs | 4 +-- src/module/functions/local_function/mod.rs | 15 +++++++---- src/module/functions/mod.rs | 6 ++--- src/module/imports.rs | 5 ++-- src/module/memories.rs | 7 +---- src/module/mod.rs | 20 +++++++------- src/module/producers.rs | 2 +- src/module/types.rs | 10 +++---- src/parse.rs | 2 +- src/passes/used.rs | 14 +++++----- src/ty.rs | 6 ++--- 26 files changed, 125 insertions(+), 117 deletions(-) diff --git a/crates/fuzz-utils/src/lib.rs b/crates/fuzz-utils/src/lib.rs index a86bc028..c595f25c 100755 --- a/crates/fuzz-utils/src/lib.rs +++ b/crates/fuzz-utils/src/lib.rs @@ -68,7 +68,7 @@ where .join("..") // pop `crates` .join("target") .join("walrus-fuzz"); - fs::create_dir_all(&dir).unwrap_or_else(|_| panic!("should create directory: {:?}", dir)); + fs::create_dir_all(&dir).expect(&format!("should create directory: {:?}", dir)); let scratch = tempfile::NamedTempFile::new_in(dir).expect("should create temp file OK"); @@ -99,20 +99,20 @@ where } fn interp(&self, wasm: &[u8]) -> Result { - fs::write(self.scratch.path(), wasm).context("failed to write to scratch file")?; + fs::write(self.scratch.path(), &wasm).context("failed to write to scratch file")?; wasm_interp(self.scratch.path()) } fn round_trip_through_walrus(&self, wasm: &[u8]) -> Result> { let mut module = - walrus::Module::from_buffer(wasm).context("walrus failed to parse the wasm buffer")?; + walrus::Module::from_buffer(&wasm).context("walrus failed to parse the wasm buffer")?; walrus::passes::gc::run(&mut module); let buf = module.emit_wasm(); Ok(buf) } fn test_wat(&self, wat: &str) -> Result<()> { - let wasm = self.wat2wasm(wat)?; + let wasm = self.wat2wasm(&wat)?; let expected = self.interp(&wasm)?; let walrus_wasm = self.round_trip_through_walrus(&wasm)?; @@ -160,7 +160,6 @@ where Ok(()) => { // We reduced fuel as far as we could, so return the last // failing test case. - #[allow(clippy::question_mark)] if failing.is_err() { return failing; } @@ -314,7 +313,7 @@ impl WatGen { self.wat.push_str(&operator.to_string()); for op in immediates.into_iter() { - self.wat.push(' '); + self.wat.push_str(" "); self.wat.push_str(op.as_ref()); } diff --git a/crates/macro/src/lib.rs b/crates/macro/src/lib.rs index 36731a7a..07029f2a 100755 --- a/crates/macro/src/lib.rs +++ b/crates/macro/src/lib.rs @@ -103,7 +103,7 @@ impl Parse for WalrusFieldOpts { if attr == "skip_visit" { return Ok(Attr::SkipVisit); } - Err(Error::new(attr.span(), "unexpected attribute")) + return Err(Error::new(attr.span(), "unexpected attribute")); } } } @@ -144,7 +144,7 @@ impl Parse for WalrusVariantOpts { if attr == "skip_builder" { return Ok(Attr::SkipBuilder); } - Err(Error::new(attr.span(), "unexpected attribute")) + return Err(Error::new(attr.span(), "unexpected attribute")); } } } @@ -166,7 +166,7 @@ fn walrus_attrs(attrs: &mut Vec) -> TokenStream { ret.extend(group); ret.extend(quote! { , }); } - ret.into() + return ret.into(); } fn create_types(attrs: &[syn::Attribute], variants: &[WalrusVariant]) -> impl quote::ToTokens { @@ -328,10 +328,10 @@ fn create_types(attrs: &[syn::Attribute], variants: &[WalrusVariant]) -> impl qu } } -fn visit_fields( - variant: &WalrusVariant, +fn visit_fields<'a>( + variant: &'a WalrusVariant, allow_skip: bool, -) -> impl Iterator + '_ { +) -> impl Iterator + 'a { return variant .syn .fields @@ -439,7 +439,7 @@ fn create_visit(variants: &[WalrusVariant]) -> impl quote::ToTokens { } }); - let doc = format!("Visit `{}`.", name); + let doc = format!("Visit `{}`.", name.to_string()); visitor_trait_methods.push(quote! { #[doc=#doc] #[inline] @@ -723,13 +723,13 @@ fn create_builder(variants: &[WalrusVariant]) -> impl quote::ToTokens { let doc = format!( "Push a new `{}` instruction onto this builder's block.", - name + name.to_string() ); let at_doc = format!( "Splice a new `{}` instruction into this builder's block at the given index.\n\n\ # Panics\n\n\ Panics if `position > self.instrs.len()`.", - name + name.to_string() ); let arg_names = &arg_names; diff --git a/crates/tests-utils/src/lib.rs b/crates/tests-utils/src/lib.rs index 4bad6b49..6840644f 100644 --- a/crates/tests-utils/src/lib.rs +++ b/crates/tests-utils/src/lib.rs @@ -67,7 +67,7 @@ where cmd.arg(input); cmd.arg("-o"); cmd.arg(tmp.path()); - cmd.args([ + cmd.args(&[ "--enable-threads", "--enable-bulk-memory", // "--enable-reference-types", diff --git a/crates/tests/src/lib.rs b/crates/tests/src/lib.rs index b167671a..63dd2584 100644 --- a/crates/tests/src/lib.rs +++ b/crates/tests/src/lib.rs @@ -79,11 +79,11 @@ impl FileCheck { let mut iter = contents.lines().map(str::trim); while let Some(line) = iter.next() { if line.starts_with("(; CHECK-ALL:") { - if !patterns.is_empty() { + if patterns.len() != 0 { panic!("CHECK cannot be used with CHECK-ALL"); } let mut pattern = Vec::new(); - for line in iter.by_ref() { + while let Some(line) = iter.next() { if line == ";)" { break; } @@ -94,17 +94,19 @@ impl FileCheck { } return FileCheck::Exhaustive(pattern, path.to_path_buf()); } - if let Some(p) = line.strip_prefix(";; CHECK:") { - patterns.push(vec![p.to_string()]); + + if line.starts_with(";; CHECK:") { + let p = line[";; CHECK:".len()..].to_string(); + patterns.push(vec![p]); } - if let Some(p) = line.strip_prefix(";; NEXT:") { - let v = patterns + if line.starts_with(";; NEXT:") { + let p = patterns .last_mut() .expect("NEXT should never come before CHECK"); - v.push(p.to_string()); + p.push(line[";; NEXT:".len()..].to_string()); } } - if patterns.is_empty() { + if patterns.len() == 0 { FileCheck::None(path.to_path_buf()) } else { FileCheck::Patterns(patterns) @@ -123,7 +125,7 @@ impl FileCheck { 'inner: while let Some(pos) = output_lines[start..] .iter() - .position(|l| matches(l, first_line)) + .position(|l| matches(*l, first_line)) { start = pos + 1; if output_lines[pos..].len() + 1 < pattern.len() { @@ -153,11 +155,11 @@ impl FileCheck { } } FileCheck::None(_) => { - println!(); + println!(""); println!("no test assertions were found in this file, but"); println!("you can rerun tests with `WALRUS_BLESS=1` to"); println!("automatically add assertions to this file"); - println!(); + println!(""); panic!("no tests to run") } } @@ -208,7 +210,7 @@ fn update_output(path: &Path, output: &str) { new_output.push_str(" "); new_output.push_str(line.trim_end()); } - new_output.push('\n'); + new_output.push_str("\n"); } let new = format!( "{}\n\n(; CHECK-ALL:\n{}\n;)\n", diff --git a/crates/tests/tests/spec-tests.rs b/crates/tests/tests/spec-tests.rs index 76913363..27e3ab89 100644 --- a/crates/tests/tests/spec-tests.rs +++ b/crates/tests/tests/spec-tests.rs @@ -19,7 +19,8 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { let proposal = wast .iter() .skip_while(|part| *part != "proposals") - .nth(1) + .skip(1) + .next() .map(|s| s.to_str().unwrap()); let extra_args: &[&str] = match proposal { @@ -53,7 +54,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { let mut files = Vec::new(); let mut config = walrus::ModuleConfig::new(); - if extra_args.is_empty() { + if extra_args.len() == 0 { config.only_stable_features(true); } @@ -153,7 +154,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { } let wasm = module.emit_wasm(); - fs::write(file, wasm)?; + fs::write(&file, wasm)?; } run_spectest_interp(tempdir.path(), extra_args)?; @@ -176,7 +177,7 @@ fn run_spectest_interp(cwd: &Path, extra_args: &[&str]) -> Result<(), anyhow::Er let stdout = String::from_utf8_lossy(&output.stdout); if let Some(line) = stdout.lines().find(|l| l.ends_with("tests passed.")) { let part = line.split_whitespace().next().unwrap(); - let mut parts = part.split('/'); + let mut parts = part.split("/"); let a = parts.next().unwrap().parse::(); let b = parts.next().unwrap().parse::(); if a == b { diff --git a/examples/round-trip.rs b/examples/round-trip.rs index 80f20c5d..716f22fc 100644 --- a/examples/round-trip.rs +++ b/examples/round-trip.rs @@ -5,7 +5,7 @@ fn main() -> anyhow::Result<()> { let a = std::env::args() .nth(1) .ok_or_else(|| anyhow::anyhow!("must provide the input wasm file as the first argument"))?; - let mut m = walrus::Module::from_file(a)?; + let mut m = walrus::Module::from_file(&a)?; let wasm = m.emit_wasm(); if let Some(destination) = std::env::args().nth(2) { std::fs::write(destination, wasm)?; diff --git a/src/const_expr.rs b/src/const_expr.rs index 98d03812..018c5361 100644 --- a/src/const_expr.rs +++ b/src/const_expr.rs @@ -58,17 +58,17 @@ impl ConstExpr { Ok(val) } - pub(crate) fn to_wasmencoder_type(self, cx: &EmitContext) -> wasm_encoder::ConstExpr { + pub(crate) fn to_wasmencoder_type(&self, cx: &EmitContext) -> wasm_encoder::ConstExpr { match self { ConstExpr::Value(v) => match v { - Value::I32(v) => wasm_encoder::ConstExpr::i32_const(v), - Value::I64(v) => wasm_encoder::ConstExpr::i64_const(v), - Value::F32(v) => wasm_encoder::ConstExpr::f32_const(v), - Value::F64(v) => wasm_encoder::ConstExpr::f64_const(v), - Value::V128(v) => wasm_encoder::ConstExpr::v128_const(v as i128), + Value::I32(v) => wasm_encoder::ConstExpr::i32_const(*v), + Value::I64(v) => wasm_encoder::ConstExpr::i64_const(*v), + Value::F32(v) => wasm_encoder::ConstExpr::f32_const(*v), + Value::F64(v) => wasm_encoder::ConstExpr::f64_const(*v), + Value::V128(v) => wasm_encoder::ConstExpr::v128_const(*v as i128), }, ConstExpr::Global(g) => { - wasm_encoder::ConstExpr::global_get(cx.indices.get_global_index(g)) + wasm_encoder::ConstExpr::global_get(cx.indices.get_global_index(*g)) } ConstExpr::RefNull(ty) => wasm_encoder::ConstExpr::ref_null(match ty { RefType::Externref => wasm_encoder::HeapType::Abstract { @@ -81,7 +81,7 @@ impl ConstExpr { }, }), ConstExpr::RefFunc(f) => { - wasm_encoder::ConstExpr::ref_func(cx.indices.get_func_index(f)) + wasm_encoder::ConstExpr::ref_func(cx.indices.get_func_index(*f)) } } } @@ -89,7 +89,7 @@ impl ConstExpr { pub(crate) fn v128_to_u128(value: &wasmparser::V128) -> u128 { let n = value.bytes(); - (n[0] as u128) + ((n[0] as u128) << 0) | ((n[1] as u128) << 8) | ((n[2] as u128) << 16) | ((n[3] as u128) << 24) diff --git a/src/dot.rs b/src/dot.rs index 4dfe4428..1729242c 100644 --- a/src/dot.rs +++ b/src/dot.rs @@ -103,7 +103,7 @@ impl Dot for T { impl FieldAggregator for AppendFields<'_> { fn add_field(&mut self, field: &[&str]) { - assert!(!field.is_empty()); + assert!(field.len() > 0); self.out.push_str("
"); @@ -114,7 +114,7 @@ impl Dot for T { } fn add_field_with_port(&mut self, port: &str, field: &str) { - assert!(!field.is_empty()); + assert!(field.len() > 0); self.out.push_str("
Dot for T { fn add_edge_from_port(&mut self, port: &str, to: &impl DotName) { self.out.push_str(" "); self.out.push_str(self.from); - self.out.push(':'); + self.out.push_str(":"); self.out.push_str(port); self.out.push_str(" -> "); self.out.push_str(&to.dot_name()); @@ -174,7 +174,7 @@ impl Dot for Module { // self.name.dot(out); // self.config.dot(out); - out.push('}'); + out.push_str("}"); } } diff --git a/src/module/config.rs b/src/module/config.rs index 9e9c3ce0..66470a65 100644 --- a/src/module/config.rs +++ b/src/module/config.rs @@ -15,10 +15,8 @@ pub struct ModuleConfig { pub(crate) skip_producers_section: bool, pub(crate) skip_name_section: bool, pub(crate) preserve_code_transform: bool, - #[allow(clippy::type_complexity)] pub(crate) on_parse: Option Result<()> + Sync + Send + 'static>>, - #[allow(clippy::type_complexity)] pub(crate) on_instr_loc: Option InstrLocId + Sync + Send + 'static>>, } diff --git a/src/module/custom.rs b/src/module/custom.rs index f1ade53f..89469ee7 100644 --- a/src/module/custom.rs +++ b/src/module/custom.rs @@ -193,7 +193,10 @@ where T: CustomSection, { fn clone(&self) -> Self { - *self + TypedCustomSectionId { + id: self.id, + _phantom: PhantomData, + } } } @@ -363,18 +366,26 @@ impl ModuleCustomSections { /// Iterate over shared references to custom sections and their ids. pub fn iter(&self) -> impl Iterator { - self.arena - .iter() - .flat_map(|(id, s)| s.as_ref().map(|s| (UntypedCustomSectionId(id), &**s))) + self.arena.iter().flat_map(|(id, s)| { + if let Some(s) = s.as_ref() { + Some((UntypedCustomSectionId(id), &**s)) + } else { + None + } + }) } /// Iterate over exclusive references to custom sections and their ids. pub fn iter_mut( &mut self, ) -> impl Iterator { - self.arena - .iter_mut() - .flat_map(|(id, s)| s.as_mut().map(|s| (UntypedCustomSectionId(id), &mut **s))) + self.arena.iter_mut().flat_map(|(id, s)| { + if let Some(s) = s.as_mut() { + Some((UntypedCustomSectionId(id), &mut **s)) + } else { + None + } + }) } /// Remove a custom section (by type) from the module. diff --git a/src/module/data.rs b/src/module/data.rs index 4fa02c3f..55a3c740 100644 --- a/src/module/data.rs +++ b/src/module/data.rs @@ -78,7 +78,10 @@ impl Data { /// Is this a passive data segment? pub fn is_passive(&self) -> bool { - matches!(self.kind, DataKind::Passive) + match self.kind { + DataKind::Passive => true, + _ => false, + } } } @@ -140,7 +143,7 @@ impl ModuleData { let mut any_passive = false; for data in self.iter() { - cx.indices.set_data_index(data.id(), count); + cx.indices.set_data_index(data.id(), count as u32); count += 1; any_passive |= data.is_passive(); } diff --git a/src/module/debug/dwarf.rs b/src/module/debug/dwarf.rs index dc9f3ea9..5d571e82 100644 --- a/src/module/debug/dwarf.rs +++ b/src/module/debug/dwarf.rs @@ -324,9 +324,9 @@ mod tests { use gimli::*; use std::cell::RefCell; - fn make_test_debug_line( - debug_line: &mut write::DebugLine>, - ) -> IncompleteLineProgram> { + fn make_test_debug_line<'a>( + debug_line: &'a mut write::DebugLine>, + ) -> IncompleteLineProgram> { let encoding = Encoding { format: Format::Dwarf32, version: 4, @@ -368,10 +368,11 @@ mod tests { .unwrap(); let debug_line = read::DebugLine::new(debug_line.slice(), LittleEndian); - - debug_line + let incomplete_debug_line = debug_line .program(DebugLineOffset(0), 4, None, None) - .unwrap() + .unwrap(); + + incomplete_debug_line } #[test] diff --git a/src/module/debug/units.rs b/src/module/debug/units.rs index 8905de68..10030d88 100644 --- a/src/module/debug/units.rs +++ b/src/module/debug/units.rs @@ -76,10 +76,10 @@ mod tests { let mut cursor = DebuggingInformationCursor::new(&mut unit1); - assert!(cursor.current().is_none()); + assert_eq!(cursor.current().is_none(), true); assert_eq!(cursor.next_dfs().unwrap().id(), root_id); assert_eq!(cursor.next_dfs().unwrap().id(), child1_id); assert_eq!(cursor.next_dfs().unwrap().id(), child2_id); - assert!(cursor.next_dfs().is_none()); + assert_eq!(cursor.next_dfs().is_none(), true); } } diff --git a/src/module/elements.rs b/src/module/elements.rs index cd4b3808..672defbe 100644 --- a/src/module/elements.rs +++ b/src/module/elements.rs @@ -237,7 +237,7 @@ impl Emit for ModuleElements { Some(cx.indices.get_table_index(*table)).filter(|&index| index != 0); wasm_element_section.active( table_index, - &offset.to_wasmencoder_type(cx), + &offset.to_wasmencoder_type(&cx), els, ); } diff --git a/src/module/functions/local_function/context.rs b/src/module/functions/local_function/context.rs index a29700c7..1902ae60 100644 --- a/src/module/functions/local_function/context.rs +++ b/src/module/functions/local_function/context.rs @@ -110,7 +110,7 @@ impl<'a> ValidationContext<'a> { } pub fn pop_control(&mut self) -> Result<(ControlFrame, InstrSeqId)> { - let frame = impl_pop_control(self.controls)?; + let frame = impl_pop_control(&mut self.controls)?; let block = frame.block; Ok((frame, block)) } @@ -214,7 +214,7 @@ fn impl_push_control_with_ty( fn impl_pop_control(controls: &mut ControlStack) -> Result { controls .last() - .ok_or(ErrorKind::InvalidWasm) + .ok_or_else(|| ErrorKind::InvalidWasm) .context("attempted to pop a frame from an empty control stack")?; let frame = controls.pop().unwrap(); Ok(frame) diff --git a/src/module/functions/local_function/emit.rs b/src/module/functions/local_function/emit.rs index 7f5da78d..41447bc8 100644 --- a/src/module/functions/local_function/emit.rs +++ b/src/module/functions/local_function/emit.rs @@ -87,7 +87,7 @@ impl<'instr> Visitor<'instr> for Emit<'_> { if let Some(map) = self.map.as_mut() { let pos = self.encoder.byte_len(); // Save the encoded_at position for the specified ExprId. - map.push((seq.end, pos)); + map.push((seq.end.clone(), pos)); } if let BlockKind::If = popped_kind.unwrap() { @@ -107,7 +107,7 @@ impl<'instr> Visitor<'instr> for Emit<'_> { if let Some(map) = self.map.as_mut() { let pos = self.encoder.byte_len(); // Save the encoded_at position for the specified ExprId. - map.push((*instr_loc, pos)); + map.push((instr_loc.clone(), pos)); } let is_block = match instr { diff --git a/src/module/functions/local_function/mod.rs b/src/module/functions/local_function/mod.rs index 107ae266..9ff1d331 100644 --- a/src/module/functions/local_function/mod.rs +++ b/src/module/functions/local_function/mod.rs @@ -44,7 +44,6 @@ impl LocalFunction { /// /// Validates the given function body and constructs the `Instr` IR at the /// same time. - #[allow(clippy::too_many_arguments)] pub(crate) fn parse( module: &Module, indices: &IndicesToIds, @@ -70,7 +69,7 @@ impl LocalFunction { }), }; - let result: Vec<_> = module.types.get(ty).results().to_vec(); + let result: Vec<_> = module.types.get(ty).results().iter().cloned().collect(); let result = result.into_boxed_slice(); let controls = &mut context::ControlStack::new(); @@ -207,7 +206,6 @@ impl LocalFunction { } /// Emit this function's compact locals declarations. - #[allow(clippy::type_complexity)] pub(crate) fn emit_locals( &self, module: &Module, @@ -301,7 +299,11 @@ fn block_param_tys(ctx: &ValidationContext, ty: wasmparser::BlockType) -> Result } } -fn append_instruction(ctx: &mut ValidationContext, inst: Operator, loc: InstrLocId) { +fn append_instruction<'context>( + ctx: &'context mut ValidationContext, + inst: Operator, + loc: InstrLocId, +) { // NB. there's a lot of `unwrap()` here in this function, and that's because // the `Operator` was validated above to already be valid, so everything // should succeed. @@ -961,7 +963,10 @@ fn append_instruction(ctx: &mut ValidationContext, inst: Operator, loc: InstrLoc } Operator::MemoryAtomicWait32 { ref memarg } | Operator::MemoryAtomicWait64 { ref memarg } => { - let sixty_four = !matches!(inst, Operator::MemoryAtomicWait32 { .. }); + let sixty_four = match inst { + Operator::MemoryAtomicWait32 { .. } => false, + _ => true, + }; let (memory, arg) = mem_arg(ctx, memarg); ctx.alloc_instr( AtomicWait { diff --git a/src/module/functions/mod.rs b/src/module/functions/mod.rs index 42c02505..871bd5ba 100644 --- a/src/module/functions/mod.rs +++ b/src/module/functions/mod.rs @@ -200,7 +200,7 @@ impl ModuleFunctions { /// return the first function in the module with the given name. pub fn by_name(&self, name: &str) -> Option { self.arena.iter().find_map(|(id, f)| { - if f.name.as_deref() == Some(name) { + if f.name.as_ref().map(|s| s.as_str()) == Some(name) { Some(id) } else { None @@ -292,7 +292,7 @@ impl ModuleFunctions { pub(crate) fn emit_func_section(&self, cx: &mut EmitContext) { log::debug!("emit function section"); let functions = used_local_functions(cx); - if functions.is_empty() { + if functions.len() == 0 { return; } let mut func_section = wasm_encoder::FunctionSection::new(); @@ -605,7 +605,7 @@ impl Emit for ModuleFunctions { fn emit(&self, cx: &mut EmitContext) { log::debug!("emit code section"); let functions = used_local_functions(cx); - if functions.is_empty() { + if functions.len() == 0 { return; } diff --git a/src/module/imports.rs b/src/module/imports.rs index ebb7f301..b003584b 100644 --- a/src/module/imports.rs +++ b/src/module/imports.rs @@ -219,7 +219,6 @@ impl Module { } /// Add an imported memory to this module - #[allow(clippy::too_many_arguments)] pub fn add_import_memory( &mut self, module: &str, @@ -311,8 +310,8 @@ impl Emit for ModuleImports { cx.indices.push_memory(id); let mem = cx.module.memories.get(id); wasm_encoder::EntityType::Memory(wasm_encoder::MemoryType { - minimum: mem.initial, - maximum: mem.maximum, + minimum: mem.initial as u64, + maximum: mem.maximum.map(|v| v as u64), memory64: false, shared: mem.shared, page_size_log2: None, diff --git a/src/module/memories.rs b/src/module/memories.rs index 556d484d..1bd8b415 100644 --- a/src/module/memories.rs +++ b/src/module/memories.rs @@ -141,15 +141,10 @@ impl ModuleMemories { self.arena.iter_mut().map(|(_, f)| f) } - /// Get the number of memories in this module. + /// Get the number of memories in this module pub fn len(&self) -> usize { self.arena.len() } - - /// Returns true if there are no memories in this module. - pub fn is_empty(&self) -> bool { - self.arena.len() == 0 - } } impl Module { diff --git a/src/module/mod.rs b/src/module/mod.rs index d7530335..471533c9 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -130,10 +130,8 @@ impl Module { } fn parse(wasm: &[u8], config: &ModuleConfig) -> Result { - let mut ret = Module { - config: config.clone(), - ..Default::default() - }; + let mut ret = Module::default(); + ret.config = config.clone(); let mut indices = IndicesToIds::default(); // TODO: how should we handle config.only_stable_features? @@ -157,7 +155,7 @@ impl Module { validator .data_section(&s) .context("failed to parse data section")?; - ret.parse_data(s, &indices)?; + ret.parse_data(s, &mut indices)?; } Payload::TypeSection(s) => { validator @@ -193,7 +191,7 @@ impl Module { validator .export_section(&s) .context("failed to parse export section")?; - ret.parse_exports(s, &indices)?; + ret.parse_exports(s, &mut indices)?; } Payload::ElementSection(s) => { validator @@ -370,7 +368,7 @@ impl Module { log::debug!("skipping DWARF custom section"); } - let indices = std::mem::take(cx.indices); + let indices = mem::replace(cx.indices, Default::default()); for (_id, section) in customs.iter_mut() { if section.name().starts_with(".debug") { @@ -552,7 +550,7 @@ fn emit_name_section(cx: &mut EmitContext) { Some((*index, name)) }) .collect::>(); - if local_names.is_empty() { + if local_names.len() == 0 { None } else { Some((cx.indices.get_func_index(func.id()), local_names)) @@ -561,7 +559,7 @@ fn emit_name_section(cx: &mut EmitContext) { .collect::>(); locals.sort_by_key(|p| p.0); // sort by index - if cx.module.name.is_none() && funcs.is_empty() && locals.is_empty() { + if cx.module.name.is_none() && funcs.len() == 0 && locals.len() == 0 { return; } @@ -569,7 +567,7 @@ fn emit_name_section(cx: &mut EmitContext) { wasm_name_section.module(name); } - if !funcs.is_empty() { + if funcs.len() > 0 { let mut name_map = wasm_encoder::NameMap::new(); for (index, name) in funcs { name_map.append(index, name); @@ -577,7 +575,7 @@ fn emit_name_section(cx: &mut EmitContext) { wasm_name_section.functions(&name_map); } - if !locals.is_empty() { + if locals.len() > 0 { let mut indirect_name_map = wasm_encoder::IndirectNameMap::new(); for (index, mut map) in locals { let mut name_map = wasm_encoder::NameMap::new(); diff --git a/src/module/producers.rs b/src/module/producers.rs index 882fc4a2..8dae0c96 100644 --- a/src/module/producers.rs +++ b/src/module/producers.rs @@ -101,7 +101,7 @@ impl Module { impl Emit for ModuleProducers { fn emit(&self, cx: &mut EmitContext) { log::debug!("emit producers section"); - if self.fields.is_empty() { + if self.fields.len() == 0 { return; } let mut wasm_producers_section = wasm_encoder::ProducersSection::new(); diff --git a/src/module/types.rs b/src/module/types.rs index a7ad1f29..23aec534 100644 --- a/src/module/types.rs +++ b/src/module/types.rs @@ -48,7 +48,7 @@ impl ModuleTypes { /// preserve type names from the WAT. pub fn by_name(&self, name: &str) -> Option { self.arena.iter().find_map(|(id, ty)| { - if ty.name.as_deref() == Some(name) { + if ty.name.as_ref().map(|s| s.as_str()) == Some(name) { Some(id) } else { None @@ -163,12 +163,8 @@ impl Emit for ModuleTypes { for (id, ty) in tys { cx.indices.push_type(id); wasm_type_section.function( - ty.params() - .iter() - .map(|ty| ValType::to_wasmencoder_type(*ty)), - ty.results() - .iter() - .map(|ty| ValType::to_wasmencoder_type(*ty)), + ty.params().iter().map(ValType::to_wasmencoder_type), + ty.results().iter().map(ValType::to_wasmencoder_type), ); } diff --git a/src/parse.rs b/src/parse.rs index 2c23afd6..77195e38 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -66,7 +66,7 @@ define_push_get!(push_data, get_data, DataId, data); impl IndicesToIds { /// Pushes a new local ID to map it to the next index internally pub(crate) fn push_local(&mut self, function: FunctionId, id: LocalId) -> u32 { - let list = self.locals.entry(function).or_default(); + let list = self.locals.entry(function).or_insert(Vec::new()); list.push(id); (list.len() as u32) - 1 } diff --git a/src/passes/used.rs b/src/passes/used.rs index eaf301ce..f39a1174 100644 --- a/src/passes/used.rs +++ b/src/passes/used.rs @@ -145,12 +145,12 @@ impl Used { } // Iteratively visit all items until our stack is empty - while !stack.funcs.is_empty() - || !stack.tables.is_empty() - || !stack.memories.is_empty() - || !stack.globals.is_empty() - || !stack.datas.is_empty() - || !stack.elements.is_empty() + while stack.funcs.len() > 0 + || stack.tables.len() > 0 + || stack.memories.len() > 0 + || stack.globals.len() > 0 + || stack.datas.len() > 0 + || stack.elements.len() > 0 { while let Some(f) = stack.funcs.pop() { let func = module.funcs.get(f); @@ -238,7 +238,7 @@ impl Used { // Let's keep `wabt` passing though and just say that if there are data // segments kept, but no memories, then we try to add the first memory, // if any, to the used set. - if !stack.used.data.is_empty() && stack.used.memories.is_empty() { + if stack.used.data.len() > 0 && stack.used.memories.len() == 0 { if let Some(mem) = module.memories.iter().next() { stack.used.memories.insert(mem.id()); } diff --git a/src/ty.rs b/src/ty.rs index 2f8ceca7..93f368ba 100644 --- a/src/ty.rs +++ b/src/ty.rs @@ -109,13 +109,13 @@ impl Type { /// Get the parameters to this function type. #[inline] pub fn params(&self) -> &[ValType] { - &self.params + &*self.params } /// Get the results of this function type. #[inline] pub fn results(&self) -> &[ValType] { - &self.results + &*self.results } pub(crate) fn is_for_function_entry(&self) -> bool { @@ -170,7 +170,7 @@ impl ValType { Ok(v.into_boxed_slice()) } - pub(crate) fn to_wasmencoder_type(self) -> wasm_encoder::ValType { + pub(crate) fn to_wasmencoder_type(&self) -> wasm_encoder::ValType { match self { ValType::I32 => wasm_encoder::ValType::I32, ValType::I64 => wasm_encoder::ValType::I64, From c1302d3d591acef12ce8bd08a1bb2c7829cbe6ab Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Sat, 29 Jun 2024 15:10:31 -0400 Subject: [PATCH 23/31] Reapply "apply clippy fix" This reverts commit f455d796c8799b553231aa7286dcbc2e67bf538a. --- crates/fuzz-utils/src/lib.rs | 11 ++++---- crates/macro/src/lib.rs | 18 ++++++------- crates/tests-utils/src/lib.rs | 2 +- crates/tests/src/lib.rs | 26 +++++++++---------- crates/tests/tests/spec-tests.rs | 9 +++---- examples/round-trip.rs | 2 +- src/const_expr.rs | 18 ++++++------- src/dot.rs | 8 +++--- src/module/config.rs | 2 ++ src/module/custom.rs | 25 +++++------------- src/module/data.rs | 7 ++--- src/module/debug/dwarf.rs | 13 +++++----- src/module/debug/units.rs | 4 +-- src/module/elements.rs | 2 +- .../functions/local_function/context.rs | 4 +-- src/module/functions/local_function/emit.rs | 4 +-- src/module/functions/local_function/mod.rs | 15 ++++------- src/module/functions/mod.rs | 6 ++--- src/module/imports.rs | 5 ++-- src/module/memories.rs | 7 ++++- src/module/mod.rs | 20 +++++++------- src/module/producers.rs | 2 +- src/module/types.rs | 10 ++++--- src/parse.rs | 2 +- src/passes/used.rs | 14 +++++----- src/ty.rs | 6 ++--- 26 files changed, 117 insertions(+), 125 deletions(-) diff --git a/crates/fuzz-utils/src/lib.rs b/crates/fuzz-utils/src/lib.rs index c595f25c..a86bc028 100755 --- a/crates/fuzz-utils/src/lib.rs +++ b/crates/fuzz-utils/src/lib.rs @@ -68,7 +68,7 @@ where .join("..") // pop `crates` .join("target") .join("walrus-fuzz"); - fs::create_dir_all(&dir).expect(&format!("should create directory: {:?}", dir)); + fs::create_dir_all(&dir).unwrap_or_else(|_| panic!("should create directory: {:?}", dir)); let scratch = tempfile::NamedTempFile::new_in(dir).expect("should create temp file OK"); @@ -99,20 +99,20 @@ where } fn interp(&self, wasm: &[u8]) -> Result { - fs::write(self.scratch.path(), &wasm).context("failed to write to scratch file")?; + fs::write(self.scratch.path(), wasm).context("failed to write to scratch file")?; wasm_interp(self.scratch.path()) } fn round_trip_through_walrus(&self, wasm: &[u8]) -> Result> { let mut module = - walrus::Module::from_buffer(&wasm).context("walrus failed to parse the wasm buffer")?; + walrus::Module::from_buffer(wasm).context("walrus failed to parse the wasm buffer")?; walrus::passes::gc::run(&mut module); let buf = module.emit_wasm(); Ok(buf) } fn test_wat(&self, wat: &str) -> Result<()> { - let wasm = self.wat2wasm(&wat)?; + let wasm = self.wat2wasm(wat)?; let expected = self.interp(&wasm)?; let walrus_wasm = self.round_trip_through_walrus(&wasm)?; @@ -160,6 +160,7 @@ where Ok(()) => { // We reduced fuel as far as we could, so return the last // failing test case. + #[allow(clippy::question_mark)] if failing.is_err() { return failing; } @@ -313,7 +314,7 @@ impl WatGen { self.wat.push_str(&operator.to_string()); for op in immediates.into_iter() { - self.wat.push_str(" "); + self.wat.push(' '); self.wat.push_str(op.as_ref()); } diff --git a/crates/macro/src/lib.rs b/crates/macro/src/lib.rs index 07029f2a..36731a7a 100755 --- a/crates/macro/src/lib.rs +++ b/crates/macro/src/lib.rs @@ -103,7 +103,7 @@ impl Parse for WalrusFieldOpts { if attr == "skip_visit" { return Ok(Attr::SkipVisit); } - return Err(Error::new(attr.span(), "unexpected attribute")); + Err(Error::new(attr.span(), "unexpected attribute")) } } } @@ -144,7 +144,7 @@ impl Parse for WalrusVariantOpts { if attr == "skip_builder" { return Ok(Attr::SkipBuilder); } - return Err(Error::new(attr.span(), "unexpected attribute")); + Err(Error::new(attr.span(), "unexpected attribute")) } } } @@ -166,7 +166,7 @@ fn walrus_attrs(attrs: &mut Vec) -> TokenStream { ret.extend(group); ret.extend(quote! { , }); } - return ret.into(); + ret.into() } fn create_types(attrs: &[syn::Attribute], variants: &[WalrusVariant]) -> impl quote::ToTokens { @@ -328,10 +328,10 @@ fn create_types(attrs: &[syn::Attribute], variants: &[WalrusVariant]) -> impl qu } } -fn visit_fields<'a>( - variant: &'a WalrusVariant, +fn visit_fields( + variant: &WalrusVariant, allow_skip: bool, -) -> impl Iterator + 'a { +) -> impl Iterator + '_ { return variant .syn .fields @@ -439,7 +439,7 @@ fn create_visit(variants: &[WalrusVariant]) -> impl quote::ToTokens { } }); - let doc = format!("Visit `{}`.", name.to_string()); + let doc = format!("Visit `{}`.", name); visitor_trait_methods.push(quote! { #[doc=#doc] #[inline] @@ -723,13 +723,13 @@ fn create_builder(variants: &[WalrusVariant]) -> impl quote::ToTokens { let doc = format!( "Push a new `{}` instruction onto this builder's block.", - name.to_string() + name ); let at_doc = format!( "Splice a new `{}` instruction into this builder's block at the given index.\n\n\ # Panics\n\n\ Panics if `position > self.instrs.len()`.", - name.to_string() + name ); let arg_names = &arg_names; diff --git a/crates/tests-utils/src/lib.rs b/crates/tests-utils/src/lib.rs index 6840644f..4bad6b49 100644 --- a/crates/tests-utils/src/lib.rs +++ b/crates/tests-utils/src/lib.rs @@ -67,7 +67,7 @@ where cmd.arg(input); cmd.arg("-o"); cmd.arg(tmp.path()); - cmd.args(&[ + cmd.args([ "--enable-threads", "--enable-bulk-memory", // "--enable-reference-types", diff --git a/crates/tests/src/lib.rs b/crates/tests/src/lib.rs index 63dd2584..b167671a 100644 --- a/crates/tests/src/lib.rs +++ b/crates/tests/src/lib.rs @@ -79,11 +79,11 @@ impl FileCheck { let mut iter = contents.lines().map(str::trim); while let Some(line) = iter.next() { if line.starts_with("(; CHECK-ALL:") { - if patterns.len() != 0 { + if !patterns.is_empty() { panic!("CHECK cannot be used with CHECK-ALL"); } let mut pattern = Vec::new(); - while let Some(line) = iter.next() { + for line in iter.by_ref() { if line == ";)" { break; } @@ -94,19 +94,17 @@ impl FileCheck { } return FileCheck::Exhaustive(pattern, path.to_path_buf()); } - - if line.starts_with(";; CHECK:") { - let p = line[";; CHECK:".len()..].to_string(); - patterns.push(vec![p]); + if let Some(p) = line.strip_prefix(";; CHECK:") { + patterns.push(vec![p.to_string()]); } - if line.starts_with(";; NEXT:") { - let p = patterns + if let Some(p) = line.strip_prefix(";; NEXT:") { + let v = patterns .last_mut() .expect("NEXT should never come before CHECK"); - p.push(line[";; NEXT:".len()..].to_string()); + v.push(p.to_string()); } } - if patterns.len() == 0 { + if patterns.is_empty() { FileCheck::None(path.to_path_buf()) } else { FileCheck::Patterns(patterns) @@ -125,7 +123,7 @@ impl FileCheck { 'inner: while let Some(pos) = output_lines[start..] .iter() - .position(|l| matches(*l, first_line)) + .position(|l| matches(l, first_line)) { start = pos + 1; if output_lines[pos..].len() + 1 < pattern.len() { @@ -155,11 +153,11 @@ impl FileCheck { } } FileCheck::None(_) => { - println!(""); + println!(); println!("no test assertions were found in this file, but"); println!("you can rerun tests with `WALRUS_BLESS=1` to"); println!("automatically add assertions to this file"); - println!(""); + println!(); panic!("no tests to run") } } @@ -210,7 +208,7 @@ fn update_output(path: &Path, output: &str) { new_output.push_str(" "); new_output.push_str(line.trim_end()); } - new_output.push_str("\n"); + new_output.push('\n'); } let new = format!( "{}\n\n(; CHECK-ALL:\n{}\n;)\n", diff --git a/crates/tests/tests/spec-tests.rs b/crates/tests/tests/spec-tests.rs index 27e3ab89..76913363 100644 --- a/crates/tests/tests/spec-tests.rs +++ b/crates/tests/tests/spec-tests.rs @@ -19,8 +19,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { let proposal = wast .iter() .skip_while(|part| *part != "proposals") - .skip(1) - .next() + .nth(1) .map(|s| s.to_str().unwrap()); let extra_args: &[&str] = match proposal { @@ -54,7 +53,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { let mut files = Vec::new(); let mut config = walrus::ModuleConfig::new(); - if extra_args.len() == 0 { + if extra_args.is_empty() { config.only_stable_features(true); } @@ -154,7 +153,7 @@ fn run(wast: &Path) -> Result<(), anyhow::Error> { } let wasm = module.emit_wasm(); - fs::write(&file, wasm)?; + fs::write(file, wasm)?; } run_spectest_interp(tempdir.path(), extra_args)?; @@ -177,7 +176,7 @@ fn run_spectest_interp(cwd: &Path, extra_args: &[&str]) -> Result<(), anyhow::Er let stdout = String::from_utf8_lossy(&output.stdout); if let Some(line) = stdout.lines().find(|l| l.ends_with("tests passed.")) { let part = line.split_whitespace().next().unwrap(); - let mut parts = part.split("/"); + let mut parts = part.split('/'); let a = parts.next().unwrap().parse::(); let b = parts.next().unwrap().parse::(); if a == b { diff --git a/examples/round-trip.rs b/examples/round-trip.rs index 716f22fc..80f20c5d 100644 --- a/examples/round-trip.rs +++ b/examples/round-trip.rs @@ -5,7 +5,7 @@ fn main() -> anyhow::Result<()> { let a = std::env::args() .nth(1) .ok_or_else(|| anyhow::anyhow!("must provide the input wasm file as the first argument"))?; - let mut m = walrus::Module::from_file(&a)?; + let mut m = walrus::Module::from_file(a)?; let wasm = m.emit_wasm(); if let Some(destination) = std::env::args().nth(2) { std::fs::write(destination, wasm)?; diff --git a/src/const_expr.rs b/src/const_expr.rs index 018c5361..98d03812 100644 --- a/src/const_expr.rs +++ b/src/const_expr.rs @@ -58,17 +58,17 @@ impl ConstExpr { Ok(val) } - pub(crate) fn to_wasmencoder_type(&self, cx: &EmitContext) -> wasm_encoder::ConstExpr { + pub(crate) fn to_wasmencoder_type(self, cx: &EmitContext) -> wasm_encoder::ConstExpr { match self { ConstExpr::Value(v) => match v { - Value::I32(v) => wasm_encoder::ConstExpr::i32_const(*v), - Value::I64(v) => wasm_encoder::ConstExpr::i64_const(*v), - Value::F32(v) => wasm_encoder::ConstExpr::f32_const(*v), - Value::F64(v) => wasm_encoder::ConstExpr::f64_const(*v), - Value::V128(v) => wasm_encoder::ConstExpr::v128_const(*v as i128), + Value::I32(v) => wasm_encoder::ConstExpr::i32_const(v), + Value::I64(v) => wasm_encoder::ConstExpr::i64_const(v), + Value::F32(v) => wasm_encoder::ConstExpr::f32_const(v), + Value::F64(v) => wasm_encoder::ConstExpr::f64_const(v), + Value::V128(v) => wasm_encoder::ConstExpr::v128_const(v as i128), }, ConstExpr::Global(g) => { - wasm_encoder::ConstExpr::global_get(cx.indices.get_global_index(*g)) + wasm_encoder::ConstExpr::global_get(cx.indices.get_global_index(g)) } ConstExpr::RefNull(ty) => wasm_encoder::ConstExpr::ref_null(match ty { RefType::Externref => wasm_encoder::HeapType::Abstract { @@ -81,7 +81,7 @@ impl ConstExpr { }, }), ConstExpr::RefFunc(f) => { - wasm_encoder::ConstExpr::ref_func(cx.indices.get_func_index(*f)) + wasm_encoder::ConstExpr::ref_func(cx.indices.get_func_index(f)) } } } @@ -89,7 +89,7 @@ impl ConstExpr { pub(crate) fn v128_to_u128(value: &wasmparser::V128) -> u128 { let n = value.bytes(); - ((n[0] as u128) << 0) + (n[0] as u128) | ((n[1] as u128) << 8) | ((n[2] as u128) << 16) | ((n[3] as u128) << 24) diff --git a/src/dot.rs b/src/dot.rs index 1729242c..4dfe4428 100644 --- a/src/dot.rs +++ b/src/dot.rs @@ -103,7 +103,7 @@ impl Dot for T { impl FieldAggregator for AppendFields<'_> { fn add_field(&mut self, field: &[&str]) { - assert!(field.len() > 0); + assert!(!field.is_empty()); self.out.push_str("
"); @@ -114,7 +114,7 @@ impl Dot for T { } fn add_field_with_port(&mut self, port: &str, field: &str) { - assert!(field.len() > 0); + assert!(!field.is_empty()); self.out.push_str("
Dot for T { fn add_edge_from_port(&mut self, port: &str, to: &impl DotName) { self.out.push_str(" "); self.out.push_str(self.from); - self.out.push_str(":"); + self.out.push(':'); self.out.push_str(port); self.out.push_str(" -> "); self.out.push_str(&to.dot_name()); @@ -174,7 +174,7 @@ impl Dot for Module { // self.name.dot(out); // self.config.dot(out); - out.push_str("}"); + out.push('}'); } } diff --git a/src/module/config.rs b/src/module/config.rs index 66470a65..9e9c3ce0 100644 --- a/src/module/config.rs +++ b/src/module/config.rs @@ -15,8 +15,10 @@ pub struct ModuleConfig { pub(crate) skip_producers_section: bool, pub(crate) skip_name_section: bool, pub(crate) preserve_code_transform: bool, + #[allow(clippy::type_complexity)] pub(crate) on_parse: Option Result<()> + Sync + Send + 'static>>, + #[allow(clippy::type_complexity)] pub(crate) on_instr_loc: Option InstrLocId + Sync + Send + 'static>>, } diff --git a/src/module/custom.rs b/src/module/custom.rs index 89469ee7..f1ade53f 100644 --- a/src/module/custom.rs +++ b/src/module/custom.rs @@ -193,10 +193,7 @@ where T: CustomSection, { fn clone(&self) -> Self { - TypedCustomSectionId { - id: self.id, - _phantom: PhantomData, - } + *self } } @@ -366,26 +363,18 @@ impl ModuleCustomSections { /// Iterate over shared references to custom sections and their ids. pub fn iter(&self) -> impl Iterator { - self.arena.iter().flat_map(|(id, s)| { - if let Some(s) = s.as_ref() { - Some((UntypedCustomSectionId(id), &**s)) - } else { - None - } - }) + self.arena + .iter() + .flat_map(|(id, s)| s.as_ref().map(|s| (UntypedCustomSectionId(id), &**s))) } /// Iterate over exclusive references to custom sections and their ids. pub fn iter_mut( &mut self, ) -> impl Iterator { - self.arena.iter_mut().flat_map(|(id, s)| { - if let Some(s) = s.as_mut() { - Some((UntypedCustomSectionId(id), &mut **s)) - } else { - None - } - }) + self.arena + .iter_mut() + .flat_map(|(id, s)| s.as_mut().map(|s| (UntypedCustomSectionId(id), &mut **s))) } /// Remove a custom section (by type) from the module. diff --git a/src/module/data.rs b/src/module/data.rs index 55a3c740..4fa02c3f 100644 --- a/src/module/data.rs +++ b/src/module/data.rs @@ -78,10 +78,7 @@ impl Data { /// Is this a passive data segment? pub fn is_passive(&self) -> bool { - match self.kind { - DataKind::Passive => true, - _ => false, - } + matches!(self.kind, DataKind::Passive) } } @@ -143,7 +140,7 @@ impl ModuleData { let mut any_passive = false; for data in self.iter() { - cx.indices.set_data_index(data.id(), count as u32); + cx.indices.set_data_index(data.id(), count); count += 1; any_passive |= data.is_passive(); } diff --git a/src/module/debug/dwarf.rs b/src/module/debug/dwarf.rs index 5d571e82..dc9f3ea9 100644 --- a/src/module/debug/dwarf.rs +++ b/src/module/debug/dwarf.rs @@ -324,9 +324,9 @@ mod tests { use gimli::*; use std::cell::RefCell; - fn make_test_debug_line<'a>( - debug_line: &'a mut write::DebugLine>, - ) -> IncompleteLineProgram> { + fn make_test_debug_line( + debug_line: &mut write::DebugLine>, + ) -> IncompleteLineProgram> { let encoding = Encoding { format: Format::Dwarf32, version: 4, @@ -368,11 +368,10 @@ mod tests { .unwrap(); let debug_line = read::DebugLine::new(debug_line.slice(), LittleEndian); - let incomplete_debug_line = debug_line - .program(DebugLineOffset(0), 4, None, None) - .unwrap(); - incomplete_debug_line + debug_line + .program(DebugLineOffset(0), 4, None, None) + .unwrap() } #[test] diff --git a/src/module/debug/units.rs b/src/module/debug/units.rs index 10030d88..8905de68 100644 --- a/src/module/debug/units.rs +++ b/src/module/debug/units.rs @@ -76,10 +76,10 @@ mod tests { let mut cursor = DebuggingInformationCursor::new(&mut unit1); - assert_eq!(cursor.current().is_none(), true); + assert!(cursor.current().is_none()); assert_eq!(cursor.next_dfs().unwrap().id(), root_id); assert_eq!(cursor.next_dfs().unwrap().id(), child1_id); assert_eq!(cursor.next_dfs().unwrap().id(), child2_id); - assert_eq!(cursor.next_dfs().is_none(), true); + assert!(cursor.next_dfs().is_none()); } } diff --git a/src/module/elements.rs b/src/module/elements.rs index 672defbe..cd4b3808 100644 --- a/src/module/elements.rs +++ b/src/module/elements.rs @@ -237,7 +237,7 @@ impl Emit for ModuleElements { Some(cx.indices.get_table_index(*table)).filter(|&index| index != 0); wasm_element_section.active( table_index, - &offset.to_wasmencoder_type(&cx), + &offset.to_wasmencoder_type(cx), els, ); } diff --git a/src/module/functions/local_function/context.rs b/src/module/functions/local_function/context.rs index 1902ae60..a29700c7 100644 --- a/src/module/functions/local_function/context.rs +++ b/src/module/functions/local_function/context.rs @@ -110,7 +110,7 @@ impl<'a> ValidationContext<'a> { } pub fn pop_control(&mut self) -> Result<(ControlFrame, InstrSeqId)> { - let frame = impl_pop_control(&mut self.controls)?; + let frame = impl_pop_control(self.controls)?; let block = frame.block; Ok((frame, block)) } @@ -214,7 +214,7 @@ fn impl_push_control_with_ty( fn impl_pop_control(controls: &mut ControlStack) -> Result { controls .last() - .ok_or_else(|| ErrorKind::InvalidWasm) + .ok_or(ErrorKind::InvalidWasm) .context("attempted to pop a frame from an empty control stack")?; let frame = controls.pop().unwrap(); Ok(frame) diff --git a/src/module/functions/local_function/emit.rs b/src/module/functions/local_function/emit.rs index 41447bc8..7f5da78d 100644 --- a/src/module/functions/local_function/emit.rs +++ b/src/module/functions/local_function/emit.rs @@ -87,7 +87,7 @@ impl<'instr> Visitor<'instr> for Emit<'_> { if let Some(map) = self.map.as_mut() { let pos = self.encoder.byte_len(); // Save the encoded_at position for the specified ExprId. - map.push((seq.end.clone(), pos)); + map.push((seq.end, pos)); } if let BlockKind::If = popped_kind.unwrap() { @@ -107,7 +107,7 @@ impl<'instr> Visitor<'instr> for Emit<'_> { if let Some(map) = self.map.as_mut() { let pos = self.encoder.byte_len(); // Save the encoded_at position for the specified ExprId. - map.push((instr_loc.clone(), pos)); + map.push((*instr_loc, pos)); } let is_block = match instr { diff --git a/src/module/functions/local_function/mod.rs b/src/module/functions/local_function/mod.rs index 9ff1d331..107ae266 100644 --- a/src/module/functions/local_function/mod.rs +++ b/src/module/functions/local_function/mod.rs @@ -44,6 +44,7 @@ impl LocalFunction { /// /// Validates the given function body and constructs the `Instr` IR at the /// same time. + #[allow(clippy::too_many_arguments)] pub(crate) fn parse( module: &Module, indices: &IndicesToIds, @@ -69,7 +70,7 @@ impl LocalFunction { }), }; - let result: Vec<_> = module.types.get(ty).results().iter().cloned().collect(); + let result: Vec<_> = module.types.get(ty).results().to_vec(); let result = result.into_boxed_slice(); let controls = &mut context::ControlStack::new(); @@ -206,6 +207,7 @@ impl LocalFunction { } /// Emit this function's compact locals declarations. + #[allow(clippy::type_complexity)] pub(crate) fn emit_locals( &self, module: &Module, @@ -299,11 +301,7 @@ fn block_param_tys(ctx: &ValidationContext, ty: wasmparser::BlockType) -> Result } } -fn append_instruction<'context>( - ctx: &'context mut ValidationContext, - inst: Operator, - loc: InstrLocId, -) { +fn append_instruction(ctx: &mut ValidationContext, inst: Operator, loc: InstrLocId) { // NB. there's a lot of `unwrap()` here in this function, and that's because // the `Operator` was validated above to already be valid, so everything // should succeed. @@ -963,10 +961,7 @@ fn append_instruction<'context>( } Operator::MemoryAtomicWait32 { ref memarg } | Operator::MemoryAtomicWait64 { ref memarg } => { - let sixty_four = match inst { - Operator::MemoryAtomicWait32 { .. } => false, - _ => true, - }; + let sixty_four = !matches!(inst, Operator::MemoryAtomicWait32 { .. }); let (memory, arg) = mem_arg(ctx, memarg); ctx.alloc_instr( AtomicWait { diff --git a/src/module/functions/mod.rs b/src/module/functions/mod.rs index 871bd5ba..42c02505 100644 --- a/src/module/functions/mod.rs +++ b/src/module/functions/mod.rs @@ -200,7 +200,7 @@ impl ModuleFunctions { /// return the first function in the module with the given name. pub fn by_name(&self, name: &str) -> Option { self.arena.iter().find_map(|(id, f)| { - if f.name.as_ref().map(|s| s.as_str()) == Some(name) { + if f.name.as_deref() == Some(name) { Some(id) } else { None @@ -292,7 +292,7 @@ impl ModuleFunctions { pub(crate) fn emit_func_section(&self, cx: &mut EmitContext) { log::debug!("emit function section"); let functions = used_local_functions(cx); - if functions.len() == 0 { + if functions.is_empty() { return; } let mut func_section = wasm_encoder::FunctionSection::new(); @@ -605,7 +605,7 @@ impl Emit for ModuleFunctions { fn emit(&self, cx: &mut EmitContext) { log::debug!("emit code section"); let functions = used_local_functions(cx); - if functions.len() == 0 { + if functions.is_empty() { return; } diff --git a/src/module/imports.rs b/src/module/imports.rs index b003584b..ebb7f301 100644 --- a/src/module/imports.rs +++ b/src/module/imports.rs @@ -219,6 +219,7 @@ impl Module { } /// Add an imported memory to this module + #[allow(clippy::too_many_arguments)] pub fn add_import_memory( &mut self, module: &str, @@ -310,8 +311,8 @@ impl Emit for ModuleImports { cx.indices.push_memory(id); let mem = cx.module.memories.get(id); wasm_encoder::EntityType::Memory(wasm_encoder::MemoryType { - minimum: mem.initial as u64, - maximum: mem.maximum.map(|v| v as u64), + minimum: mem.initial, + maximum: mem.maximum, memory64: false, shared: mem.shared, page_size_log2: None, diff --git a/src/module/memories.rs b/src/module/memories.rs index 1bd8b415..556d484d 100644 --- a/src/module/memories.rs +++ b/src/module/memories.rs @@ -141,10 +141,15 @@ impl ModuleMemories { self.arena.iter_mut().map(|(_, f)| f) } - /// Get the number of memories in this module + /// Get the number of memories in this module. pub fn len(&self) -> usize { self.arena.len() } + + /// Returns true if there are no memories in this module. + pub fn is_empty(&self) -> bool { + self.arena.len() == 0 + } } impl Module { diff --git a/src/module/mod.rs b/src/module/mod.rs index 471533c9..d7530335 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -130,8 +130,10 @@ impl Module { } fn parse(wasm: &[u8], config: &ModuleConfig) -> Result { - let mut ret = Module::default(); - ret.config = config.clone(); + let mut ret = Module { + config: config.clone(), + ..Default::default() + }; let mut indices = IndicesToIds::default(); // TODO: how should we handle config.only_stable_features? @@ -155,7 +157,7 @@ impl Module { validator .data_section(&s) .context("failed to parse data section")?; - ret.parse_data(s, &mut indices)?; + ret.parse_data(s, &indices)?; } Payload::TypeSection(s) => { validator @@ -191,7 +193,7 @@ impl Module { validator .export_section(&s) .context("failed to parse export section")?; - ret.parse_exports(s, &mut indices)?; + ret.parse_exports(s, &indices)?; } Payload::ElementSection(s) => { validator @@ -368,7 +370,7 @@ impl Module { log::debug!("skipping DWARF custom section"); } - let indices = mem::replace(cx.indices, Default::default()); + let indices = std::mem::take(cx.indices); for (_id, section) in customs.iter_mut() { if section.name().starts_with(".debug") { @@ -550,7 +552,7 @@ fn emit_name_section(cx: &mut EmitContext) { Some((*index, name)) }) .collect::>(); - if local_names.len() == 0 { + if local_names.is_empty() { None } else { Some((cx.indices.get_func_index(func.id()), local_names)) @@ -559,7 +561,7 @@ fn emit_name_section(cx: &mut EmitContext) { .collect::>(); locals.sort_by_key(|p| p.0); // sort by index - if cx.module.name.is_none() && funcs.len() == 0 && locals.len() == 0 { + if cx.module.name.is_none() && funcs.is_empty() && locals.is_empty() { return; } @@ -567,7 +569,7 @@ fn emit_name_section(cx: &mut EmitContext) { wasm_name_section.module(name); } - if funcs.len() > 0 { + if !funcs.is_empty() { let mut name_map = wasm_encoder::NameMap::new(); for (index, name) in funcs { name_map.append(index, name); @@ -575,7 +577,7 @@ fn emit_name_section(cx: &mut EmitContext) { wasm_name_section.functions(&name_map); } - if locals.len() > 0 { + if !locals.is_empty() { let mut indirect_name_map = wasm_encoder::IndirectNameMap::new(); for (index, mut map) in locals { let mut name_map = wasm_encoder::NameMap::new(); diff --git a/src/module/producers.rs b/src/module/producers.rs index 8dae0c96..882fc4a2 100644 --- a/src/module/producers.rs +++ b/src/module/producers.rs @@ -101,7 +101,7 @@ impl Module { impl Emit for ModuleProducers { fn emit(&self, cx: &mut EmitContext) { log::debug!("emit producers section"); - if self.fields.len() == 0 { + if self.fields.is_empty() { return; } let mut wasm_producers_section = wasm_encoder::ProducersSection::new(); diff --git a/src/module/types.rs b/src/module/types.rs index 23aec534..a7ad1f29 100644 --- a/src/module/types.rs +++ b/src/module/types.rs @@ -48,7 +48,7 @@ impl ModuleTypes { /// preserve type names from the WAT. pub fn by_name(&self, name: &str) -> Option { self.arena.iter().find_map(|(id, ty)| { - if ty.name.as_ref().map(|s| s.as_str()) == Some(name) { + if ty.name.as_deref() == Some(name) { Some(id) } else { None @@ -163,8 +163,12 @@ impl Emit for ModuleTypes { for (id, ty) in tys { cx.indices.push_type(id); wasm_type_section.function( - ty.params().iter().map(ValType::to_wasmencoder_type), - ty.results().iter().map(ValType::to_wasmencoder_type), + ty.params() + .iter() + .map(|ty| ValType::to_wasmencoder_type(*ty)), + ty.results() + .iter() + .map(|ty| ValType::to_wasmencoder_type(*ty)), ); } diff --git a/src/parse.rs b/src/parse.rs index 77195e38..2c23afd6 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -66,7 +66,7 @@ define_push_get!(push_data, get_data, DataId, data); impl IndicesToIds { /// Pushes a new local ID to map it to the next index internally pub(crate) fn push_local(&mut self, function: FunctionId, id: LocalId) -> u32 { - let list = self.locals.entry(function).or_insert(Vec::new()); + let list = self.locals.entry(function).or_default(); list.push(id); (list.len() as u32) - 1 } diff --git a/src/passes/used.rs b/src/passes/used.rs index f39a1174..eaf301ce 100644 --- a/src/passes/used.rs +++ b/src/passes/used.rs @@ -145,12 +145,12 @@ impl Used { } // Iteratively visit all items until our stack is empty - while stack.funcs.len() > 0 - || stack.tables.len() > 0 - || stack.memories.len() > 0 - || stack.globals.len() > 0 - || stack.datas.len() > 0 - || stack.elements.len() > 0 + while !stack.funcs.is_empty() + || !stack.tables.is_empty() + || !stack.memories.is_empty() + || !stack.globals.is_empty() + || !stack.datas.is_empty() + || !stack.elements.is_empty() { while let Some(f) = stack.funcs.pop() { let func = module.funcs.get(f); @@ -238,7 +238,7 @@ impl Used { // Let's keep `wabt` passing though and just say that if there are data // segments kept, but no memories, then we try to add the first memory, // if any, to the used set. - if stack.used.data.len() > 0 && stack.used.memories.len() == 0 { + if !stack.used.data.is_empty() && stack.used.memories.is_empty() { if let Some(mem) = module.memories.iter().next() { stack.used.memories.insert(mem.id()); } diff --git a/src/ty.rs b/src/ty.rs index 93f368ba..2f8ceca7 100644 --- a/src/ty.rs +++ b/src/ty.rs @@ -109,13 +109,13 @@ impl Type { /// Get the parameters to this function type. #[inline] pub fn params(&self) -> &[ValType] { - &*self.params + &self.params } /// Get the results of this function type. #[inline] pub fn results(&self) -> &[ValType] { - &*self.results + &self.results } pub(crate) fn is_for_function_entry(&self) -> bool { @@ -170,7 +170,7 @@ impl ValType { Ok(v.into_boxed_slice()) } - pub(crate) fn to_wasmencoder_type(&self) -> wasm_encoder::ValType { + pub(crate) fn to_wasmencoder_type(self) -> wasm_encoder::ValType { match self { ValType::I32 => wasm_encoder::ValType::I32, ValType::I64 => wasm_encoder::ValType::I64, From dac96ca17a2a3741e8dfe2455a586ad9cc15aa2c Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 2 Jul 2024 14:09:47 -0400 Subject: [PATCH 24/31] list all unimplemented operators --- src/module/functions/local_function/mod.rs | 169 ++++++++++++++++++++- 1 file changed, 167 insertions(+), 2 deletions(-) diff --git a/src/module/functions/local_function/mod.rs b/src/module/functions/local_function/mod.rs index 9ff1d331..bc8c327e 100644 --- a/src/module/functions/local_function/mod.rs +++ b/src/module/functions/local_function/mod.rs @@ -1323,7 +1323,172 @@ fn append_instruction<'context>( ctx.alloc_instr(ElemDrop { elem }, loc); } - // TODO: The remaining operators are for some on-going proposals - other => unimplemented!("unsupported operator: {:?}", other), + // List all unimplmented operators instead of have a catch-all arm. + // So that future upgrades won't miss additions to this list that may be important to know. + Operator::TryTable { try_table: _ } + | Operator::Throw { tag_index: _ } + | Operator::ThrowRef + | Operator::Try { blockty: _ } + | Operator::Catch { tag_index: _ } + | Operator::Rethrow { relative_depth: _ } + | Operator::Delegate { relative_depth: _ } + | Operator::CatchAll + | Operator::ReturnCall { function_index: _ } + | Operator::ReturnCallIndirect { + type_index: _, + table_index: _, + } + | Operator::RefEq + | Operator::StructNew { + struct_type_index: _, + } + | Operator::StructNewDefault { + struct_type_index: _, + } + | Operator::StructGet { + struct_type_index: _, + field_index: _, + } + | Operator::StructGetS { + struct_type_index: _, + field_index: _, + } + | Operator::StructGetU { + struct_type_index: _, + field_index: _, + } + | Operator::StructSet { + struct_type_index: _, + field_index: _, + } + | Operator::ArrayNew { + array_type_index: _, + } + | Operator::ArrayNewDefault { + array_type_index: _, + } + | Operator::ArrayNewFixed { + array_type_index: _, + array_size: _, + } + | Operator::ArrayNewData { + array_type_index: _, + array_data_index: _, + } + | Operator::ArrayNewElem { + array_type_index: _, + array_elem_index: _, + } + | Operator::ArrayGet { + array_type_index: _, + } + | Operator::ArrayGetS { + array_type_index: _, + } + | Operator::ArrayGetU { + array_type_index: _, + } + | Operator::ArraySet { + array_type_index: _, + } + | Operator::ArrayLen + | Operator::ArrayFill { + array_type_index: _, + } + | Operator::ArrayCopy { + array_type_index_dst: _, + array_type_index_src: _, + } + | Operator::ArrayInitData { + array_type_index: _, + array_data_index: _, + } + | Operator::ArrayInitElem { + array_type_index: _, + array_elem_index: _, + } + | Operator::RefTestNonNull { hty: _ } + | Operator::RefTestNullable { hty: _ } + | Operator::RefCastNonNull { hty: _ } + | Operator::RefCastNullable { hty: _ } + | Operator::BrOnCast { + relative_depth: _, + from_ref_type: _, + to_ref_type: _, + } + | Operator::BrOnCastFail { + relative_depth: _, + from_ref_type: _, + to_ref_type: _, + } + | Operator::AnyConvertExtern + | Operator::ExternConvertAny + | Operator::RefI31 + | Operator::I31GetS + | Operator::I31GetU + | Operator::MemoryDiscard { mem: _ } + | Operator::GlobalAtomicGet { + ordering: _, + global_index: _, + } + | Operator::GlobalAtomicSet { + ordering: _, + global_index: _, + } + | Operator::GlobalAtomicRmwAdd { + ordering: _, + global_index: _, + } + | Operator::GlobalAtomicRmwSub { + ordering: _, + global_index: _, + } + | Operator::GlobalAtomicRmwAnd { + ordering: _, + global_index: _, + } + | Operator::GlobalAtomicRmwOr { + ordering: _, + global_index: _, + } + | Operator::GlobalAtomicRmwXor { + ordering: _, + global_index: _, + } + | Operator::GlobalAtomicRmwXchg { + ordering: _, + global_index: _, + } + | Operator::GlobalAtomicRmwCmpxchg { + ordering: _, + global_index: _, + } + | Operator::I8x16RelaxedSwizzle + | Operator::I32x4RelaxedTruncF32x4S + | Operator::I32x4RelaxedTruncF32x4U + | Operator::I32x4RelaxedTruncF64x2SZero + | Operator::I32x4RelaxedTruncF64x2UZero + | Operator::F32x4RelaxedMadd + | Operator::F32x4RelaxedNmadd + | Operator::F64x2RelaxedMadd + | Operator::F64x2RelaxedNmadd + | Operator::I8x16RelaxedLaneselect + | Operator::I16x8RelaxedLaneselect + | Operator::I32x4RelaxedLaneselect + | Operator::I64x2RelaxedLaneselect + | Operator::F32x4RelaxedMin + | Operator::F32x4RelaxedMax + | Operator::F64x2RelaxedMin + | Operator::F64x2RelaxedMax + | Operator::I16x8RelaxedQ15mulrS + | Operator::I16x8RelaxedDotI8x16I7x16S + | Operator::I32x4RelaxedDotI8x16I7x16AddS + | Operator::CallRef { type_index: _ } + | Operator::ReturnCallRef { type_index: _ } + | Operator::RefAsNonNull + | Operator::BrOnNull { relative_depth: _ } + | Operator::BrOnNonNull { relative_depth: _ } => { + unimplemented!("unsupported operator: {:?}", inst) + } } } From e40af5fb9616afb110ac27701541c44fe6ef003f Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 2 Jul 2024 14:16:34 -0400 Subject: [PATCH 25/31] comment --- src/module/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/module/mod.rs b/src/module/mod.rs index 471533c9..484f7394 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -134,7 +134,8 @@ impl Module { ret.config = config.clone(); let mut indices = IndicesToIds::default(); - // TODO: how should we handle config.only_stable_features? + // For now we have the same set of wasm features + // regardless of config.only_stable_features. let wasm_features = WasmFeatures::default(); let mut validator = Validator::new_with_features(wasm_features); From a4f596519a23b6a9ccce99e74ad7216fee2e9716 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 2 Jul 2024 14:21:10 -0400 Subject: [PATCH 26/31] Update src/module/mod.rs Co-authored-by: Guy Bedford --- src/module/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/module/mod.rs b/src/module/mod.rs index 484f7394..7411d82b 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -135,7 +135,8 @@ impl Module { let mut indices = IndicesToIds::default(); // For now we have the same set of wasm features - // regardless of config.only_stable_features. + // regardless of config.only_stable_features. New unstable features + // may be enabled under `only_stable_features: false` in future. let wasm_features = WasmFeatures::default(); let mut validator = Validator::new_with_features(wasm_features); From f4709ac402208bf975a88c01b7a0c801f15e2330 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 2 Jul 2024 14:36:57 -0400 Subject: [PATCH 27/31] clippy cont. --- src/module/debug/dwarf.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/module/debug/dwarf.rs b/src/module/debug/dwarf.rs index dc9f3ea9..865c2188 100644 --- a/src/module/debug/dwarf.rs +++ b/src/module/debug/dwarf.rs @@ -556,6 +556,7 @@ mod tests { } #[test] + #[allow(clippy::field_reassign_with_default)] fn test_convert_high_pc() { let sections = make_test_debug_info(); let mut read_dwarf = Dwarf::default(); From 17ad0daa80f168d04223a5c98f53be4539a32138 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 2 Jul 2024 15:44:10 -0400 Subject: [PATCH 28/31] upgrade criterion and fix deprecated --- Cargo.toml | 2 +- benches/benches.rs | 26 ++++++++++++-------------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7296f785..f46260a2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,7 @@ parallel = ['rayon', 'id-arena/rayon'] [dev-dependencies] env_logger = "0.8.1" -criterion = "0.3.0" +criterion = "0.5.0" [workspace] members = [ diff --git a/benches/benches.rs b/benches/benches.rs index 3735a51d..99a6c060 100644 --- a/benches/benches.rs +++ b/benches/benches.rs @@ -1,20 +1,18 @@ -use criterion::{black_box, criterion_group, criterion_main, Benchmark, Criterion}; +use criterion::{black_box, criterion_group, criterion_main, Criterion}; use walrus::Module; fn criterion_benchmark(c: &mut Criterion) { - c.bench( - "round-trip-with-gc", - Benchmark::new("dodrio-todomvc.wasm", |b| { - let input_wasm = include_bytes!("./fixtures/dodrio-todomvc.wasm"); - b.iter(|| { - let input_wasm = black_box(input_wasm); - let mut module = Module::from_buffer(input_wasm).unwrap(); - walrus::passes::gc::run(&mut module); - let output_wasm = module.emit_wasm(); - black_box(output_wasm); - }); - }), - ); + let mut group = c.benchmark_group("round-trip-with-gc"); + group.bench_function("dodrio-todomvc.wasm", |b| { + let input_wasm = include_bytes!("./fixtures/dodrio-todomvc.wasm"); + b.iter(|| { + let input_wasm = black_box(input_wasm); + let mut module = Module::from_buffer(input_wasm).unwrap(); + walrus::passes::gc::run(&mut module); + let output_wasm = module.emit_wasm(); + black_box(output_wasm); + }); + }); } criterion_group!(benches, criterion_benchmark); From 08d335efbaf605d7c38bc75efa22322c713aa7fa Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 2 Jul 2024 16:04:20 -0400 Subject: [PATCH 29/31] upgrade env_logger --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index f46260a2..153e657a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,7 @@ gimli = "0.26.0" parallel = ['rayon', 'id-arena/rayon'] [dev-dependencies] -env_logger = "0.8.1" +env_logger = "0.11.0" criterion = "0.5.0" [workspace] From f51b828b60eff517ed38170869b86308d8a81795 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 2 Jul 2024 16:05:09 -0400 Subject: [PATCH 30/31] add clippy CI job --- .github/workflows/main.yml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 3a6c3482..fe708ef1 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -78,11 +78,20 @@ jobs: run: | cargo fuzz run ${{ matrix.test }} -- -max_total_time=300 -rss_limit_mb=4096 > fuzz.log 2>&1 || (tail -n 1000 fuzz.log && exit 1) - rustfmt: - name: Rustfmt + fmt: + name: fmt runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - name: Install Rust run: rustup update stable && rustup default stable && rustup component add rustfmt - run: cargo fmt -- --check + + clippy: + name: clippy + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install Rust + run: rustup update stable && rustup default stable && rustup component add clippy + - run: cargo clippy --tests --benches -- -D warnings From 96c57621eb32f4a456adef1fa600f2024ab9f58f Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 2 Jul 2024 16:05:23 -0400 Subject: [PATCH 31/31] format main.yml --- .github/workflows/main.yml | 126 ++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fe708ef1..b30928f7 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -9,49 +9,49 @@ jobs: matrix: rust: [stable, beta, nightly] steps: - - uses: actions/checkout@v4 - with: - submodules: true - - name: Install Rust - run: rustup update ${{ matrix.rust }} && rustup default ${{ matrix.rust }} - - name: Install wabt - run: | - set -e - curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - - echo "`pwd`/wabt-1.0.35/bin" > $GITHUB_PATH - - name: Install binaryen - run: | - set -e - curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - - echo "`pwd`/binaryen-version_117/bin" > $GITHUB_PATH - - run: cargo build --all - - run: cargo test --all - - run: cargo check --benches - - run: cargo test --features parallel - - run: cargo test --features parallel --manifest-path crates/tests/Cargo.toml + - uses: actions/checkout@v4 + with: + submodules: true + - name: Install Rust + run: rustup update ${{ matrix.rust }} && rustup default ${{ matrix.rust }} + - name: Install wabt + run: | + set -e + curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - + echo "`pwd`/wabt-1.0.35/bin" > $GITHUB_PATH + - name: Install binaryen + run: | + set -e + curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - + echo "`pwd`/binaryen-version_117/bin" > $GITHUB_PATH + - run: cargo build --all + - run: cargo test --all + - run: cargo check --benches + - run: cargo test --features parallel + - run: cargo test --features parallel --manifest-path crates/tests/Cargo.toml fuzz_crate: name: Fuzz Crate runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - name: Install Rust - run: rustup update stable && rustup default stable - - name: Install wabt - run: | - set -e - curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - - echo "`pwd`/wabt-1.0.35/bin" > $GITHUB_PATH - - name: Install binaryen - run: | - set -e - curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - - echo "`pwd`/binaryen-version_117/bin" > $GITHUB_PATH - - name: Run fuzzer - run: cargo test -p walrus-fuzz-utils > fuzz.log || (tail -n 1000 fuzz.log && exit 1) - env: - # 300 seconds = 5 minutes. - WALRUS_FUZZ_TIMEOUT: 300 + - uses: actions/checkout@v4 + - name: Install Rust + run: rustup update stable && rustup default stable + - name: Install wabt + run: | + set -e + curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - + echo "`pwd`/wabt-1.0.35/bin" > $GITHUB_PATH + - name: Install binaryen + run: | + set -e + curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - + echo "`pwd`/binaryen-version_117/bin" > $GITHUB_PATH + - name: Run fuzzer + run: cargo test -p walrus-fuzz-utils > fuzz.log || (tail -n 1000 fuzz.log && exit 1) + env: + # 300 seconds = 5 minutes. + WALRUS_FUZZ_TIMEOUT: 300 fuzz: name: Fuzz @@ -60,38 +60,38 @@ jobs: matrix: test: [watgen, wasm-opt-ttf, raw] steps: - - uses: actions/checkout@v4 - - name: Install Rust - run: rustup update nightly && rustup default nightly - - run: cargo install cargo-fuzz - - name: Install wabt - run: | - set -e - curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - - echo "`pwd`/wabt-1.0.35/bin" > $GITHUB_PATH - - name: Install binaryen - run: | - set -e - curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - - echo "`pwd`/binaryen-version_117/bin" > $GITHUB_PATH - - name: Run fuzzer - run: | - cargo fuzz run ${{ matrix.test }} -- -max_total_time=300 -rss_limit_mb=4096 > fuzz.log 2>&1 || (tail -n 1000 fuzz.log && exit 1) + - uses: actions/checkout@v4 + - name: Install Rust + run: rustup update nightly && rustup default nightly + - run: cargo install cargo-fuzz + - name: Install wabt + run: | + set -e + curl -L https://github.com/WebAssembly/wabt/releases/download/1.0.35/wabt-1.0.35-ubuntu-20.04.tar.gz | tar xzf - + echo "`pwd`/wabt-1.0.35/bin" > $GITHUB_PATH + - name: Install binaryen + run: | + set -e + curl -L https://github.com/WebAssembly/binaryen/releases/download/version_117/binaryen-version_117-x86_64-linux.tar.gz | tar xzf - + echo "`pwd`/binaryen-version_117/bin" > $GITHUB_PATH + - name: Run fuzzer + run: | + cargo fuzz run ${{ matrix.test }} -- -max_total_time=300 -rss_limit_mb=4096 > fuzz.log 2>&1 || (tail -n 1000 fuzz.log && exit 1) fmt: name: fmt runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - name: Install Rust - run: rustup update stable && rustup default stable && rustup component add rustfmt - - run: cargo fmt -- --check + - uses: actions/checkout@v4 + - name: Install Rust + run: rustup update stable && rustup default stable && rustup component add rustfmt + - run: cargo fmt -- --check clippy: name: clippy runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - name: Install Rust - run: rustup update stable && rustup default stable && rustup component add clippy - - run: cargo clippy --tests --benches -- -D warnings + - uses: actions/checkout@v4 + - name: Install Rust + run: rustup update stable && rustup default stable && rustup component add clippy + - run: cargo clippy --tests --benches -- -D warnings