Skip to content

Commit

Permalink
Applies all clippy suggestions (wasmi-labs#244)
Browse files Browse the repository at this point in the history
* Applies all clippy suggestions

In a few cases a clippy annotation was provided to ignore the clippy warning.

* apply rustfmt

* make wasmi compiler under no_std again

* use rustfmt +stable
  • Loading branch information
Robbepop authored Jan 31, 2021
1 parent f1f95f5 commit dd87384
Show file tree
Hide file tree
Showing 23 changed files with 343 additions and 388 deletions.
28 changes: 13 additions & 15 deletions examples/invoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ fn main() {
.entries()
.iter()
.find(|entry| func_name == entry.field())
.expect(&format!("No export with name {} found", func_name));
.unwrap_or_else(|| panic!("No export with name {} found", func_name));

// Function index in the function index space (internally-defined + imported)
let function_index: usize = match found_entry.internal() {
&Internal::Function(index) => index as usize,
Internal::Function(index) => *index as usize,
_ => panic!("Founded export is not a function"),
};

Expand All @@ -48,10 +48,7 @@ fn main() {
Some(import) => import
.entries()
.iter()
.filter(|entry| match entry.external() {
&External::Function(_) => true,
_ => false,
})
.filter(|entry| matches!(entry.external(), External::Function(_)))
.count(),
None => 0,
};
Expand All @@ -64,8 +61,9 @@ fn main() {
function_section.entries()[function_index_in_section].type_ref() as usize;

// Use the reference to get an actual function type
#[allow(clippy::infallible_destructuring_match)]
let function_type: &FunctionType = match &type_section.types()[func_type_ref] {
&Type::Function(ref func_type) => func_type,
Type::Function(ref func_type) => func_type,
};

// Parses arguments and constructs runtime values in correspondence of their types
Expand All @@ -74,26 +72,26 @@ fn main() {
.iter()
.enumerate()
.map(|(i, value)| match value {
&ValueType::I32 => RuntimeValue::I32(
ValueType::I32 => RuntimeValue::I32(
program_args[i]
.parse::<i32>()
.expect(&format!("Can't parse arg #{} as i32", program_args[i])),
.unwrap_or_else(|_| panic!("Can't parse arg #{} as i32", program_args[i])),
),
&ValueType::I64 => RuntimeValue::I64(
ValueType::I64 => RuntimeValue::I64(
program_args[i]
.parse::<i64>()
.expect(&format!("Can't parse arg #{} as i64", program_args[i])),
.unwrap_or_else(|_| panic!("Can't parse arg #{} as i64", program_args[i])),
),
&ValueType::F32 => RuntimeValue::F32(
ValueType::F32 => RuntimeValue::F32(
program_args[i]
.parse::<f32>()
.expect(&format!("Can't parse arg #{} as f32", program_args[i]))
.unwrap_or_else(|_| panic!("Can't parse arg #{} as f32", program_args[i]))
.into(),
),
&ValueType::F64 => RuntimeValue::F64(
ValueType::F64 => RuntimeValue::F64(
program_args[i]
.parse::<f64>()
.expect(&format!("Can't parse arg #{} as f64", program_args[i]))
.unwrap_or_else(|_| panic!("Can't parse arg #{} as f64", program_args[i]))
.into(),
),
})
Expand Down
14 changes: 8 additions & 6 deletions examples/tictactoe.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::wrong_self_convention)]

extern crate parity_wasm;
extern crate wasmi;

Expand Down Expand Up @@ -67,7 +69,7 @@ mod tictactoe {
}

pub fn set(&mut self, idx: i32, player: Player) -> Result<(), Error> {
if idx < 0 || idx > 9 {
if !(0..9).contains(&idx) {
return Err(Error::OutOfRange);
}
if self.board[idx as usize] != None {
Expand All @@ -78,7 +80,7 @@ mod tictactoe {
}

pub fn get(&self, idx: i32) -> Result<Option<Player>, Error> {
if idx < 0 || idx > 9 {
if !(0..9).contains(&idx) {
return Err(Error::OutOfRange);
}
Ok(self.board[idx as usize])
Expand All @@ -103,6 +105,7 @@ mod tictactoe {
];

// Returns Some(player) if all cells contain same Player.
#[allow(clippy::question_mark)]
let all_same = |i1: usize, i2: usize, i3: usize| -> Option<Player> {
if self.board[i1].is_none() {
return None;
Expand Down Expand Up @@ -221,14 +224,13 @@ fn play(
{
let mut runtime = Runtime {
player: turn_of,
game: game,
game,
};
let _ = instance.invoke_export("mk_turn", &[], &mut runtime)?;
}

match game.game_result() {
Some(game_result) => break game_result,
None => {}
if let Some(game_result) = game.game_result() {
break game_result;
}

turn_of = next_turn_of;
Expand Down
16 changes: 8 additions & 8 deletions src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ pub(crate) enum FuncInstanceInternal {
impl fmt::Debug for FuncInstance {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self.as_internal() {
&FuncInstanceInternal::Internal { ref signature, .. } => {
FuncInstanceInternal::Internal { ref signature, .. } => {
// We can't write description of self.module here, because it generate
// debug string for function instances and this will lead to infinite loop.
write!(f, "Internal {{ signature={:?} }}", signature,)
}
&FuncInstanceInternal::Host { ref signature, .. } => {
FuncInstanceInternal::Host { ref signature, .. } => {
write!(f, "Host {{ signature={:?} }}", signature)
}
}
Expand Down Expand Up @@ -112,7 +112,7 @@ impl FuncInstance {
) -> FuncRef {
let func = FuncInstanceInternal::Internal {
signature,
module: module,
module,
body: Rc::new(body),
};
FuncRef(Rc::new(FuncInstance(func)))
Expand Down Expand Up @@ -269,19 +269,19 @@ impl<'args> FuncInvocation<'args> {
/// Whether this invocation is currently resumable.
pub fn is_resumable(&self) -> bool {
match &self.kind {
&FuncInvocationKind::Internal(ref interpreter) => interpreter.state().is_resumable(),
&FuncInvocationKind::Host { .. } => false,
FuncInvocationKind::Internal(ref interpreter) => interpreter.state().is_resumable(),
FuncInvocationKind::Host { .. } => false,
}
}

/// If the invocation is resumable, the expected return value type to be feed back in.
pub fn resumable_value_type(&self) -> Option<ValueType> {
match &self.kind {
&FuncInvocationKind::Internal(ref interpreter) => match interpreter.state() {
&InterpreterState::Resumable(ref value_type) => value_type.clone(),
FuncInvocationKind::Internal(ref interpreter) => match interpreter.state() {
InterpreterState::Resumable(ref value_type) => *value_type,
_ => None,
},
&FuncInvocationKind::Host { .. } => None,
FuncInvocationKind::Host { .. } => None,
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ impl<'a> RuntimeArgs<'a> {
where
T: FromRuntimeValue,
{
Ok(self
.nth_value_checked(idx)?
self.nth_value_checked(idx)?
.try_into()
.ok_or_else(|| TrapKind::UnexpectedSignature)?)
.ok_or(TrapKind::UnexpectedSignature)
.map_err(Into::into)
}

/// Extract argument as a [`RuntimeValue`] by index `idx`.
Expand Down
26 changes: 8 additions & 18 deletions src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,56 +255,46 @@ pub trait ModuleImportResolver {

impl ModuleImportResolver for ModuleRef {
fn resolve_func(&self, field_name: &str, _signature: &Signature) -> Result<FuncRef, Error> {
Ok(self
.export_by_name(field_name)
self.export_by_name(field_name)
.ok_or_else(|| Error::Instantiation(format!("Export {} not found", field_name)))?
.as_func()
.cloned()
.ok_or_else(|| {
Error::Instantiation(format!("Export {} is not a function", field_name))
})?)
.ok_or_else(|| Error::Instantiation(format!("Export {} is not a function", field_name)))
}

fn resolve_global(
&self,
field_name: &str,
_global_type: &GlobalDescriptor,
) -> Result<GlobalRef, Error> {
Ok(self
.export_by_name(field_name)
self.export_by_name(field_name)
.ok_or_else(|| Error::Instantiation(format!("Export {} not found", field_name)))?
.as_global()
.cloned()
.ok_or_else(|| {
Error::Instantiation(format!("Export {} is not a global", field_name))
})?)
.ok_or_else(|| Error::Instantiation(format!("Export {} is not a global", field_name)))
}

fn resolve_memory(
&self,
field_name: &str,
_memory_type: &MemoryDescriptor,
) -> Result<MemoryRef, Error> {
Ok(self
.export_by_name(field_name)
self.export_by_name(field_name)
.ok_or_else(|| Error::Instantiation(format!("Export {} not found", field_name)))?
.as_memory()
.cloned()
.ok_or_else(|| {
Error::Instantiation(format!("Export {} is not a memory", field_name))
})?)
.ok_or_else(|| Error::Instantiation(format!("Export {} is not a memory", field_name)))
}

fn resolve_table(
&self,
field_name: &str,
_table_type: &TableDescriptor,
) -> Result<TableRef, Error> {
Ok(self
.export_by_name(field_name)
self.export_by_name(field_name)
.ok_or_else(|| Error::Instantiation(format!("Export {} not found", field_name)))?
.as_table()
.cloned()
.ok_or_else(|| Error::Instantiation(format!("Export {} is not a table", field_name)))?)
.ok_or_else(|| Error::Instantiation(format!("Export {} is not a table", field_name)))
}
}
6 changes: 1 addition & 5 deletions src/isa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,11 +595,7 @@ impl<'a> Iterator for InstructionIter<'a> {

#[inline]
fn next(&mut self) -> Option<Self::Item> {
let internal = if let Some(i) = self.instructions.get(self.position as usize) {
i
} else {
return None;
};
let internal = self.instructions.get(self.position as usize)?;

let out = match *internal {
InstructionInternal::GetLocal(x) => Instruction::GetLocal(x),
Expand Down
8 changes: 4 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@
#![warn(missing_docs)]
#![cfg_attr(not(feature = "std"), no_std)]
#![allow(clippy::len_without_is_empty)]
#![allow(clippy::new_ret_no_self)]

#[cfg(not(feature = "std"))]
#[macro_use]
Expand Down Expand Up @@ -246,10 +248,7 @@ pub enum TrapKind {
impl TrapKind {
/// Whether this trap is specified by the host.
pub fn is_host(&self) -> bool {
match self {
&TrapKind::Host(_) => true,
_ => false,
}
matches!(self, TrapKind::Host(_))
}
}

Expand Down Expand Up @@ -333,6 +332,7 @@ impl Error {
}
}

#[allow(clippy::from_over_into)]
impl Into<String> for Error {
fn into(self) -> String {
match self {
Expand Down
36 changes: 14 additions & 22 deletions src/memory/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,11 @@ impl MemoryInstance {

let initial_size: Bytes = initial.into();
Ok(MemoryInstance {
limits: limits,
buffer: RefCell::new(
ByteBuf::new(initial_size.0).map_err(|err| Error::Memory(err.to_string()))?,
),
initial: initial,
limits,
buffer: RefCell::new(ByteBuf::new(initial_size.0).map_err(Error::Memory)?),
initial,
current_size: Cell::new(initial_size.0),
maximum: maximum,
maximum,
})
}

Expand Down Expand Up @@ -267,9 +265,9 @@ impl MemoryInstance {
return Ok(size_before_grow);
}
if additional > Pages(65536) {
return Err(Error::Memory(format!(
"Trying to grow memory by more than 65536 pages"
)));
return Err(Error::Memory(
"Trying to grow memory by more than 65536 pages".to_string(),
));
}

let new_size: Pages = size_before_grow + additional;
Expand All @@ -287,7 +285,7 @@ impl MemoryInstance {
self.buffer
.borrow_mut()
.realloc(new_buffer_length.0)
.map_err(|err| Error::Memory(err.to_string()))?;
.map_err(Error::Memory)?;

self.current_size.set(new_buffer_length.0);

Expand Down Expand Up @@ -316,10 +314,7 @@ impl MemoryInstance {
)));
}

Ok(CheckedRegion {
offset: offset,
size: size,
})
Ok(CheckedRegion { offset, size })
}

fn checked_region_pair(
Expand Down Expand Up @@ -421,9 +416,9 @@ impl MemoryInstance {
self.checked_region_pair(&mut buffer, src_offset, len, dst_offset, len)?;

if read_region.intersects(&write_region) {
return Err(Error::Memory(format!(
"non-overlapping copy is used for overlapping regions"
)));
return Err(Error::Memory(
"non-overlapping copy is used for overlapping regions".to_string(),
));
}

unsafe {
Expand Down Expand Up @@ -501,10 +496,7 @@ impl MemoryInstance {
///
/// Might be useful for some optimization shenanigans.
pub fn erase(&self) -> Result<(), Error> {
self.buffer
.borrow_mut()
.erase()
.map_err(|err| Error::Memory(err))
self.buffer.borrow_mut().erase().map_err(Error::Memory)
}

/// Provides direct access to the underlying memory buffer.
Expand Down Expand Up @@ -564,7 +556,7 @@ mod tests {

for (index, &(initial, maybe_max, expected_ok)) in fixtures.iter().enumerate() {
let initial: Pages = Pages(initial);
let maximum: Option<Pages> = maybe_max.map(|m| Pages(m));
let maximum: Option<Pages> = maybe_max.map(Pages);
let result = MemoryInstance::alloc(initial, maximum);
if result.is_ok() != expected_ok {
panic!(
Expand Down
Loading

0 comments on commit dd87384

Please sign in to comment.