From b288c6001a8e963739519114d6d69ec18e53b14c Mon Sep 17 00:00:00 2001 From: Boris-Chengbiao Zhou Date: Tue, 23 Oct 2018 06:52:35 +0200 Subject: [PATCH] Fix all clippy warnings (#564) * Fix all clippy warnings * Revert usage of inclusive ranges * Remove redundant function argument * Revert use of unavailable pointer methods * Introduce ContiguousCaseRange --- cranelift/src/clif-util.rs | 7 ++--- cranelift/src/compile.rs | 2 +- lib/bforest/src/map.rs | 1 + lib/bforest/src/set.rs | 1 + lib/codegen/meta/src/cdsl/types.rs | 32 ++++++++++----------- lib/codegen/meta/src/gen_types.rs | 3 +- lib/codegen/src/ir/dfg.rs | 5 ++-- lib/codegen/src/ir/function.rs | 9 ++---- lib/codegen/src/isa/encoding.rs | 2 +- lib/codegen/src/print_errors.rs | 2 +- lib/codegen/src/verifier/mod.rs | 41 +++++++++++---------------- lib/filetests/src/runner.rs | 2 +- lib/filetests/src/test_binemit.rs | 2 +- lib/frontend/src/switch.rs | 45 ++++++++++++++++++++---------- lib/reader/src/parser.rs | 41 +++++++++++++-------------- lib/simplejit/src/backend.rs | 2 +- lib/wasm/src/environ/spec.rs | 1 + 17 files changed, 100 insertions(+), 98 deletions(-) diff --git a/cranelift/src/clif-util.rs b/cranelift/src/clif-util.rs index 13a6ba7d06..5d5ca2b5d4 100755 --- a/cranelift/src/clif-util.rs +++ b/cranelift/src/clif-util.rs @@ -115,7 +115,7 @@ fn add_debug_flag<'a>() -> clap::Arg<'a, 'a> { } /// Returns a vector of clap value options and changes these options into a vector of strings -fn get_vec<'a>(argument_vec: Option>) -> Vec { +fn get_vec(argument_vec: Option) -> Vec { let mut ret_vec: Vec = Vec::new(); if let Some(clap_vec) = argument_vec { for val in clap_vec { @@ -199,12 +199,11 @@ fn main() { } ("test", Some(rest_cmd)) => { handle_debug_flag(rest_cmd.is_present("debug")); - let result = cranelift_filetests::run( + cranelift_filetests::run( rest_cmd.is_present("verbose"), rest_cmd.is_present("time-passes"), &get_vec(rest_cmd.values_of("file")), - ).map(|_time| ()); - result + ).map(|_time| ()) } ("pass", Some(rest_cmd)) => { handle_debug_flag(rest_cmd.is_present("debug")); diff --git a/cranelift/src/compile.rs b/cranelift/src/compile.rs index 59402ff97f..0babf6f83e 100644 --- a/cranelift/src/compile.rs +++ b/cranelift/src/compile.rs @@ -151,7 +151,7 @@ fn handle_module( } fn print_readonly_data(mem: &[u8]) { - if mem.len() == 0 { + if mem.is_empty() { return; } diff --git a/lib/bforest/src/map.rs b/lib/bforest/src/map.rs index 97ab7e6d90..9b1c78f906 100644 --- a/lib/bforest/src/map.rs +++ b/lib/bforest/src/map.rs @@ -284,6 +284,7 @@ where /// /// If the cursor reaches the end, return `None` and leave the cursor at the off-the-end /// position. + #[cfg_attr(feature = "cargo-clippy", allow(should_implement_trait))] pub fn next(&mut self) -> Option<(K, V)> { self.path.next(self.pool) } diff --git a/lib/bforest/src/set.rs b/lib/bforest/src/set.rs index f950e1e3cb..f73a6b06d3 100644 --- a/lib/bforest/src/set.rs +++ b/lib/bforest/src/set.rs @@ -225,6 +225,7 @@ where /// /// If the cursor reaches the end, return `None` and leave the cursor at the off-the-end /// position. + #[cfg_attr(feature = "cargo-clippy", allow(should_implement_trait))] pub fn next(&mut self) -> Option { self.path.next(self.pool).map(|(k, _)| k) } diff --git a/lib/codegen/meta/src/cdsl/types.rs b/lib/codegen/meta/src/cdsl/types.rs index 9d2e9a16dc..6f191a10a9 100644 --- a/lib/codegen/meta/src/cdsl/types.rs +++ b/lib/codegen/meta/src/cdsl/types.rs @@ -161,8 +161,8 @@ pub enum LaneType { impl LaneType { /// Return a string containing the documentation comment for this lane type. - pub fn doc(&self) -> String { - match *self { + pub fn doc(self) -> String { + match self { LaneType::BoolType(_) => format!("A boolean type with {} bits.", self.lane_bits()), LaneType::FloatType(base_types::Float::F32) => String::from( "A 32-bit floating point type represented in the IEEE 754-2008 @@ -185,8 +185,8 @@ impl LaneType { } /// Return the number of bits in a lane. - pub fn lane_bits(&self) -> u64 { - match *self { + pub fn lane_bits(self) -> u64 { + match self { LaneType::BoolType(ref b) => *b as u64, LaneType::FloatType(ref f) => *f as u64, LaneType::IntType(ref i) => *i as u64, @@ -194,8 +194,8 @@ impl LaneType { } /// Get the name of this lane type. - pub fn name(&self) -> String { - match *self { + pub fn name(self) -> String { + match self { LaneType::BoolType(_) => format!("b{}", self.lane_bits()), LaneType::FloatType(_) => format!("f{}", self.lane_bits()), LaneType::IntType(_) => format!("i{}", self.lane_bits()), @@ -203,8 +203,8 @@ impl LaneType { } /// Find the unique number associated with this lane type. - pub fn number(&self) -> u8 { - LANE_BASE + match *self { + pub fn number(self) -> u8 { + LANE_BASE + match self { LaneType::BoolType(base_types::Bool::B1) => 0, LaneType::BoolType(base_types::Bool::B8) => 1, LaneType::BoolType(base_types::Bool::B16) => 2, @@ -394,8 +394,8 @@ pub enum SpecialType { impl SpecialType { /// Return a string containing the documentation comment for this special type. - pub fn doc(&self) -> String { - match *self { + pub fn doc(self) -> String { + match self { SpecialType::Flag(base_types::Flag::IFlags) => String::from( "CPU flags representing the result of an integer comparison. These flags can be tested with an :type:`intcc` condition code.", @@ -408,23 +408,23 @@ impl SpecialType { } /// Return the number of bits in a lane. - pub fn lane_bits(&self) -> u64 { - match *self { + pub fn lane_bits(self) -> u64 { + match self { SpecialType::Flag(_) => 0, } } /// Get the name of this special type. - pub fn name(&self) -> String { - match *self { + pub fn name(self) -> String { + match self { SpecialType::Flag(base_types::Flag::IFlags) => "iflags".to_string(), SpecialType::Flag(base_types::Flag::FFlags) => "fflags".to_string(), } } /// Find the unique number associated with this special type. - pub fn number(&self) -> u8 { - match *self { + pub fn number(self) -> u8 { + match self { SpecialType::Flag(base_types::Flag::IFlags) => 1, SpecialType::Flag(base_types::Flag::FFlags) => 2, } diff --git a/lib/codegen/meta/src/gen_types.rs b/lib/codegen/meta/src/gen_types.rs index 1055f0de0e..1bbcbfe687 100644 --- a/lib/codegen/meta/src/gen_types.rs +++ b/lib/codegen/meta/src/gen_types.rs @@ -47,8 +47,7 @@ fn emit_vectors(bits: u64, fmt: &mut srcgen::Formatter) -> Result<(), error::Err /// Emit types using the given formatter object. fn emit_types(fmt: &mut srcgen::Formatter) -> Result<(), error::Error> { // Emit all of the special types, such as types for CPU flags. - for spec in cdsl_types::ValueType::all_special_types().map(|ty| cdsl_types::ValueType::from(ty)) - { + for spec in cdsl_types::ValueType::all_special_types().map(cdsl_types::ValueType::from) { emit_type(&spec, fmt)?; } diff --git a/lib/codegen/src/ir/dfg.rs b/lib/codegen/src/ir/dfg.rs index 7bcf17e949..6d92f70667 100644 --- a/lib/codegen/src/ir/dfg.rs +++ b/lib/codegen/src/ir/dfg.rs @@ -127,7 +127,7 @@ fn maybe_resolve_aliases(values: &PrimaryMap, value: Value) -> let mut v = value; // Note that values may be empty here. - for _ in 0..1 + values.len() { + for _ in 0..values.len() + 1 { if let ValueData::Alias { original, .. } = values[v] { v = original; } else { @@ -174,8 +174,7 @@ impl<'a> Iterator for Values<'a> { fn next(&mut self) -> Option { self.inner .by_ref() - .filter(|kv| valid_valuedata(kv.1)) - .next() + .find(|kv| valid_valuedata(kv.1)) .map(|kv| kv.0) } } diff --git a/lib/codegen/src/ir/function.rs b/lib/codegen/src/ir/function.rs index 800c9f2e2f..f2db834eab 100644 --- a/lib/codegen/src/ir/function.rs +++ b/lib/codegen/src/ir/function.rs @@ -178,19 +178,14 @@ impl Function { /// /// This function can only be used after the code layout has been computed by the /// `binemit::relax_branches()` function. - pub fn inst_offsets<'a>( - &'a self, - func: &'a Function, - ebb: Ebb, - encinfo: &EncInfo, - ) -> InstOffsetIter<'a> { + pub fn inst_offsets<'a>(&'a self, ebb: Ebb, encinfo: &EncInfo) -> InstOffsetIter<'a> { assert!( !self.offsets.is_empty(), "Code layout must be computed first" ); InstOffsetIter { encinfo: encinfo.clone(), - func, + func: self, divert: RegDiversions::new(), encodings: &self.encodings, offset: self.offsets[ebb], diff --git a/lib/codegen/src/isa/encoding.rs b/lib/codegen/src/isa/encoding.rs index 589069b311..ee643c508d 100644 --- a/lib/codegen/src/isa/encoding.rs +++ b/lib/codegen/src/isa/encoding.rs @@ -85,7 +85,7 @@ type SizeCalculatorFn = fn(&RecipeSizing, Inst, &RegDiversions, &Function) -> u8 /// Returns the base size of the Recipe, assuming it's fixed. This is the default for most /// encodings; others can be variable and longer than this base size, depending on the registers /// they're using and use a different function, specific per platform. -pub fn base_size(sizing: &RecipeSizing, _: Inst, _2: &RegDiversions, _3: &Function) -> u8 { +pub fn base_size(sizing: &RecipeSizing, _: Inst, _: &RegDiversions, _: &Function) -> u8 { sizing.base_size } diff --git a/lib/codegen/src/print_errors.rs b/lib/codegen/src/print_errors.rs index 217d09f902..5944e407ff 100644 --- a/lib/codegen/src/print_errors.rs +++ b/lib/codegen/src/print_errors.rs @@ -25,7 +25,7 @@ pub fn pretty_verifier_error<'a>( let mut w = String::new(); decorate_function( - &mut PrettyVerifierError(func_w.unwrap_or(Box::new(PlainWriter)), &mut errors), + &mut PrettyVerifierError(func_w.unwrap_or_else(|| Box::new(PlainWriter)), &mut errors), &mut w, func, isa, diff --git a/lib/codegen/src/verifier/mod.rs b/lib/codegen/src/verifier/mod.rs index 1e4fd631d1..6fedcc5af2 100644 --- a/lib/codegen/src/verifier/mod.rs +++ b/lib/codegen/src/verifier/mod.rs @@ -234,7 +234,8 @@ pub fn verify_function<'a, FOI: Into>>( let verifier = Verifier::new(func, fisa.into()); let result = verifier.run(&mut errors); if errors.is_empty() { - Ok(result.unwrap()) + result.unwrap(); + Ok(()) } else { Err(errors) } @@ -392,30 +393,22 @@ impl<'a> Verifier<'a> { ); } - match heap_data.style { - ir::HeapStyle::Dynamic { bound_gv, .. } => { - if !self.func.global_values.is_valid(bound_gv) { - return nonfatal!( - errors, - heap, - "invalid bound global value {}", - bound_gv - ); - } - - let index_type = heap_data.index_type; - let bound_type = self.func.global_values[bound_gv].global_type(isa); - if index_type != bound_type { - report!( - errors, - heap, - "heap index type {} differs from the type of its bound, {}", - index_type, - bound_type - ); - } + if let ir::HeapStyle::Dynamic { bound_gv, .. } = heap_data.style { + if !self.func.global_values.is_valid(bound_gv) { + return nonfatal!(errors, heap, "invalid bound global value {}", bound_gv); + } + + let index_type = heap_data.index_type; + let bound_type = self.func.global_values[bound_gv].global_type(isa); + if index_type != bound_type { + report!( + errors, + heap, + "heap index type {} differs from the type of its bound, {}", + index_type, + bound_type + ); } - _ => {} } } } diff --git a/lib/filetests/src/runner.rs b/lib/filetests/src/runner.rs index 7840c0dfff..fb2a0d4177 100644 --- a/lib/filetests/src/runner.rs +++ b/lib/filetests/src/runner.rs @@ -31,7 +31,7 @@ enum State { Done(TestResult), } -#[derive(PartialEq, Eq, Debug)] +#[derive(PartialEq, Eq, Debug, Clone, Copy)] pub enum IsPass { Pass, NotPass, diff --git a/lib/filetests/src/test_binemit.rs b/lib/filetests/src/test_binemit.rs index da0f0eec09..da8310bfe6 100644 --- a/lib/filetests/src/test_binemit.rs +++ b/lib/filetests/src/test_binemit.rs @@ -205,7 +205,7 @@ impl SubTest for TestBinEmit { "Inconsistent {} header offset", ebb ); - for (offset, inst, enc_bytes) in func.inst_offsets(&func, ebb, &encinfo) { + for (offset, inst, enc_bytes) in func.inst_offsets(ebb, &encinfo) { assert_eq!(sink.offset, offset); sink.text.clear(); let enc = func.encodings[inst]; diff --git a/lib/frontend/src/switch.rs b/lib/frontend/src/switch.rs index f01a9f620b..ce3a2d943f 100644 --- a/lib/frontend/src/switch.rs +++ b/lib/frontend/src/switch.rs @@ -8,7 +8,7 @@ type EntryIndex = u64; /// Unlike with `br_table`, `Switch` cases may be sparse or non-0-based. /// They emit efficient code using branches, jump tables, or a combination of both. -#[derive(Debug)] +#[derive(Debug, Default)] pub struct Switch { cases: HashMap, } @@ -16,7 +16,7 @@ pub struct Switch { impl Switch { /// Create a new empty switch pub fn new() -> Self { - Switch { + Self { cases: HashMap::new(), } } @@ -31,23 +31,23 @@ impl Switch { ); } - fn collect_contiguous_case_ranges(self) -> Vec<(EntryIndex, Vec)> { + fn collect_contiguous_case_ranges(self) -> Vec { debug!("build_contiguous_case_ranges before: {:#?}", self.cases); let mut cases = self.cases.into_iter().collect::>(); cases.sort_by_key(|&(index, _)| index); - let mut contiguous_case_ranges: Vec<(EntryIndex, Vec)> = vec![]; + let mut contiguous_case_ranges: Vec = vec![]; let mut last_index = None; for (index, ebb) in cases { match last_index { - None => contiguous_case_ranges.push((index, vec![])), + None => contiguous_case_ranges.push(ContiguousCaseRange::new(index)), Some(last_index) => { if index > last_index + 1 { - contiguous_case_ranges.push((index, vec![])); + contiguous_case_ranges.push(ContiguousCaseRange::new(index)); } } } - contiguous_case_ranges.last_mut().unwrap().1.push(ebb); + contiguous_case_ranges.last_mut().unwrap().ebbs.push(ebb); last_index = Some(index); } @@ -63,7 +63,7 @@ impl Switch { bx: &mut FunctionBuilder, val: Value, otherwise: Ebb, - contiguous_case_ranges: Vec<(EntryIndex, Vec)>, + contiguous_case_ranges: Vec, ) -> Vec<(EntryIndex, Ebb, Vec)> { let mut cases_and_jt_ebbs = Vec::new(); @@ -79,7 +79,7 @@ impl Switch { return cases_and_jt_ebbs; } - let mut stack: Vec<(Option, Vec<(EntryIndex, Vec)>)> = Vec::new(); + let mut stack: Vec<(Option, Vec)> = Vec::new(); stack.push((None, contiguous_case_ranges)); while let Some((ebb, contiguous_case_ranges)) = stack.pop() { @@ -103,9 +103,11 @@ impl Switch { let left_ebb = bx.create_ebb(); let right_ebb = bx.create_ebb(); - let should_take_right_side = - bx.ins() - .icmp_imm(IntCC::UnsignedGreaterThanOrEqual, val, right[0].0 as i64); + let should_take_right_side = bx.ins().icmp_imm( + IntCC::UnsignedGreaterThanOrEqual, + val, + right[0].first_index as i64, + ); bx.ins().brnz(should_take_right_side, right_ebb, &[]); bx.ins().jump(left_ebb, &[]); @@ -121,10 +123,10 @@ impl Switch { bx: &mut FunctionBuilder, val: Value, otherwise: Ebb, - contiguous_case_ranges: Vec<(EntryIndex, Vec)>, + contiguous_case_ranges: Vec, cases_and_jt_ebbs: &mut Vec<(EntryIndex, Ebb, Vec)>, ) { - for (first_index, ebbs) in contiguous_case_ranges.into_iter().rev() { + for ContiguousCaseRange { first_index, ebbs } in contiguous_case_ranges.into_iter().rev() { if ebbs.len() == 1 { let is_good_val = bx.ins().icmp_imm(IntCC::Equal, val, first_index as i64); bx.ins().brnz(is_good_val, ebbs[0], &[]); @@ -180,6 +182,21 @@ impl Switch { } } +#[derive(Debug)] +struct ContiguousCaseRange { + first_index: EntryIndex, + ebbs: Vec, +} + +impl ContiguousCaseRange { + fn new(first_index: EntryIndex) -> Self { + Self { + first_index, + ebbs: Vec::new(), + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/lib/reader/src/parser.rs b/lib/reader/src/parser.rs index c129b76e64..937edbee02 100644 --- a/lib/reader/src/parser.rs +++ b/lib/reader/src/parser.rs @@ -775,29 +775,26 @@ impl<'a> Parser<'a> { let mut targets = Vec::new(); let flag_builder = settings::builder(); - match target_pass { - Some(targ) => { - let loc = self.loc; - let triple = match Triple::from_str(targ) { - Ok(triple) => triple, - Err(err) => return err!(loc, err), - }; - let mut isa_builder = match isa::lookup(triple) { - Err(isa::LookupError::SupportDisabled) => { - return err!(loc, "support disabled target '{}'", targ) - } - Err(isa::LookupError::Unsupported) => { - return err!(loc, "unsupported target '{}'", targ) - } - Ok(b) => b, - }; - specified_target = true; + if let Some(targ) = target_pass { + let loc = self.loc; + let triple = match Triple::from_str(targ) { + Ok(triple) => triple, + Err(err) => return err!(loc, err), + }; + let mut isa_builder = match isa::lookup(triple) { + Err(isa::LookupError::SupportDisabled) => { + return err!(loc, "support disabled target '{}'", targ) + } + Err(isa::LookupError::Unsupported) => { + return err!(loc, "unsupported target '{}'", targ) + } + Ok(b) => b, + }; + specified_target = true; - // Construct a trait object with the aggregate settings. - targets.push(isa_builder.finish(settings::Flags::new(flag_builder.clone()))); - } - None => (), - }; + // Construct a trait object with the aggregate settings. + targets.push(isa_builder.finish(settings::Flags::new(flag_builder.clone()))); + } if !specified_target { // No `target` commands. diff --git a/lib/simplejit/src/backend.rs b/lib/simplejit/src/backend.rs index 3fc46cd993..68cc7e8e92 100644 --- a/lib/simplejit/src/backend.rs +++ b/lib/simplejit/src/backend.rs @@ -411,7 +411,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { /// SimpleJIT emits code and data into memory as it processes them, so it /// doesn't need to provide anything after the `Module` is complete. - fn finish(self) -> () {} + fn finish(self) {} } #[cfg(not(windows))] diff --git a/lib/wasm/src/environ/spec.rs b/lib/wasm/src/environ/spec.rs index 872ec28475..9f0dc14964 100644 --- a/lib/wasm/src/environ/spec.rs +++ b/lib/wasm/src/environ/spec.rs @@ -161,6 +161,7 @@ pub trait FuncEnvironment { /// The signature `sig_ref` was previously created by `make_indirect_sig()`. /// /// Return the call instruction whose results are the WebAssembly return values. + #[cfg_attr(feature = "cargo-clippy", allow(too_many_arguments))] fn translate_call_indirect( &mut self, pos: FuncCursor,