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
This commit is contained in:
Boris-Chengbiao Zhou
2018-10-23 06:52:35 +02:00
committed by Dan Gohman
parent 586a8835e9
commit b288c6001a
17 changed files with 100 additions and 98 deletions

View File

@@ -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 /// Returns a vector of clap value options and changes these options into a vector of strings
fn get_vec<'a>(argument_vec: Option<clap::Values<'a>>) -> Vec<String> { fn get_vec(argument_vec: Option<clap::Values>) -> Vec<String> {
let mut ret_vec: Vec<String> = Vec::new(); let mut ret_vec: Vec<String> = Vec::new();
if let Some(clap_vec) = argument_vec { if let Some(clap_vec) = argument_vec {
for val in clap_vec { for val in clap_vec {
@@ -199,12 +199,11 @@ fn main() {
} }
("test", Some(rest_cmd)) => { ("test", Some(rest_cmd)) => {
handle_debug_flag(rest_cmd.is_present("debug")); 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("verbose"),
rest_cmd.is_present("time-passes"), rest_cmd.is_present("time-passes"),
&get_vec(rest_cmd.values_of("file")), &get_vec(rest_cmd.values_of("file")),
).map(|_time| ()); ).map(|_time| ())
result
} }
("pass", Some(rest_cmd)) => { ("pass", Some(rest_cmd)) => {
handle_debug_flag(rest_cmd.is_present("debug")); handle_debug_flag(rest_cmd.is_present("debug"));

View File

@@ -151,7 +151,7 @@ fn handle_module(
} }
fn print_readonly_data(mem: &[u8]) { fn print_readonly_data(mem: &[u8]) {
if mem.len() == 0 { if mem.is_empty() {
return; return;
} }

View File

@@ -284,6 +284,7 @@ where
/// ///
/// If the cursor reaches the end, return `None` and leave the cursor at the off-the-end /// If the cursor reaches the end, return `None` and leave the cursor at the off-the-end
/// position. /// position.
#[cfg_attr(feature = "cargo-clippy", allow(should_implement_trait))]
pub fn next(&mut self) -> Option<(K, V)> { pub fn next(&mut self) -> Option<(K, V)> {
self.path.next(self.pool) self.path.next(self.pool)
} }

View File

@@ -225,6 +225,7 @@ where
/// ///
/// If the cursor reaches the end, return `None` and leave the cursor at the off-the-end /// If the cursor reaches the end, return `None` and leave the cursor at the off-the-end
/// position. /// position.
#[cfg_attr(feature = "cargo-clippy", allow(should_implement_trait))]
pub fn next(&mut self) -> Option<K> { pub fn next(&mut self) -> Option<K> {
self.path.next(self.pool).map(|(k, _)| k) self.path.next(self.pool).map(|(k, _)| k)
} }

View File

@@ -161,8 +161,8 @@ pub enum LaneType {
impl LaneType { impl LaneType {
/// Return a string containing the documentation comment for this lane type. /// Return a string containing the documentation comment for this lane type.
pub fn doc(&self) -> String { pub fn doc(self) -> String {
match *self { match self {
LaneType::BoolType(_) => format!("A boolean type with {} bits.", self.lane_bits()), LaneType::BoolType(_) => format!("A boolean type with {} bits.", self.lane_bits()),
LaneType::FloatType(base_types::Float::F32) => String::from( LaneType::FloatType(base_types::Float::F32) => String::from(
"A 32-bit floating point type represented in the IEEE 754-2008 "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. /// Return the number of bits in a lane.
pub fn lane_bits(&self) -> u64 { pub fn lane_bits(self) -> u64 {
match *self { match self {
LaneType::BoolType(ref b) => *b as u64, LaneType::BoolType(ref b) => *b as u64,
LaneType::FloatType(ref f) => *f as u64, LaneType::FloatType(ref f) => *f as u64,
LaneType::IntType(ref i) => *i as u64, LaneType::IntType(ref i) => *i as u64,
@@ -194,8 +194,8 @@ impl LaneType {
} }
/// Get the name of this lane type. /// Get the name of this lane type.
pub fn name(&self) -> String { pub fn name(self) -> String {
match *self { match self {
LaneType::BoolType(_) => format!("b{}", self.lane_bits()), LaneType::BoolType(_) => format!("b{}", self.lane_bits()),
LaneType::FloatType(_) => format!("f{}", self.lane_bits()), LaneType::FloatType(_) => format!("f{}", self.lane_bits()),
LaneType::IntType(_) => format!("i{}", 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. /// Find the unique number associated with this lane type.
pub fn number(&self) -> u8 { pub fn number(self) -> u8 {
LANE_BASE + match *self { LANE_BASE + match self {
LaneType::BoolType(base_types::Bool::B1) => 0, LaneType::BoolType(base_types::Bool::B1) => 0,
LaneType::BoolType(base_types::Bool::B8) => 1, LaneType::BoolType(base_types::Bool::B8) => 1,
LaneType::BoolType(base_types::Bool::B16) => 2, LaneType::BoolType(base_types::Bool::B16) => 2,
@@ -394,8 +394,8 @@ pub enum SpecialType {
impl SpecialType { impl SpecialType {
/// Return a string containing the documentation comment for this special type. /// Return a string containing the documentation comment for this special type.
pub fn doc(&self) -> String { pub fn doc(self) -> String {
match *self { match self {
SpecialType::Flag(base_types::Flag::IFlags) => String::from( SpecialType::Flag(base_types::Flag::IFlags) => String::from(
"CPU flags representing the result of an integer comparison. These flags "CPU flags representing the result of an integer comparison. These flags
can be tested with an :type:`intcc` condition code.", can be tested with an :type:`intcc` condition code.",
@@ -408,23 +408,23 @@ impl SpecialType {
} }
/// Return the number of bits in a lane. /// Return the number of bits in a lane.
pub fn lane_bits(&self) -> u64 { pub fn lane_bits(self) -> u64 {
match *self { match self {
SpecialType::Flag(_) => 0, SpecialType::Flag(_) => 0,
} }
} }
/// Get the name of this special type. /// Get the name of this special type.
pub fn name(&self) -> String { pub fn name(self) -> String {
match *self { match self {
SpecialType::Flag(base_types::Flag::IFlags) => "iflags".to_string(), SpecialType::Flag(base_types::Flag::IFlags) => "iflags".to_string(),
SpecialType::Flag(base_types::Flag::FFlags) => "fflags".to_string(), SpecialType::Flag(base_types::Flag::FFlags) => "fflags".to_string(),
} }
} }
/// Find the unique number associated with this special type. /// Find the unique number associated with this special type.
pub fn number(&self) -> u8 { pub fn number(self) -> u8 {
match *self { match self {
SpecialType::Flag(base_types::Flag::IFlags) => 1, SpecialType::Flag(base_types::Flag::IFlags) => 1,
SpecialType::Flag(base_types::Flag::FFlags) => 2, SpecialType::Flag(base_types::Flag::FFlags) => 2,
} }

View File

@@ -47,8 +47,7 @@ fn emit_vectors(bits: u64, fmt: &mut srcgen::Formatter) -> Result<(), error::Err
/// Emit types using the given formatter object. /// Emit types using the given formatter object.
fn emit_types(fmt: &mut srcgen::Formatter) -> Result<(), error::Error> { fn emit_types(fmt: &mut srcgen::Formatter) -> Result<(), error::Error> {
// Emit all of the special types, such as types for CPU flags. // 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)?; emit_type(&spec, fmt)?;
} }

View File

@@ -127,7 +127,7 @@ fn maybe_resolve_aliases(values: &PrimaryMap<Value, ValueData>, value: Value) ->
let mut v = value; let mut v = value;
// Note that values may be empty here. // 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] { if let ValueData::Alias { original, .. } = values[v] {
v = original; v = original;
} else { } else {
@@ -174,8 +174,7 @@ impl<'a> Iterator for Values<'a> {
fn next(&mut self) -> Option<Self::Item> { fn next(&mut self) -> Option<Self::Item> {
self.inner self.inner
.by_ref() .by_ref()
.filter(|kv| valid_valuedata(kv.1)) .find(|kv| valid_valuedata(kv.1))
.next()
.map(|kv| kv.0) .map(|kv| kv.0)
} }
} }

View File

@@ -178,19 +178,14 @@ impl Function {
/// ///
/// This function can only be used after the code layout has been computed by the /// This function can only be used after the code layout has been computed by the
/// `binemit::relax_branches()` function. /// `binemit::relax_branches()` function.
pub fn inst_offsets<'a>( pub fn inst_offsets<'a>(&'a self, ebb: Ebb, encinfo: &EncInfo) -> InstOffsetIter<'a> {
&'a self,
func: &'a Function,
ebb: Ebb,
encinfo: &EncInfo,
) -> InstOffsetIter<'a> {
assert!( assert!(
!self.offsets.is_empty(), !self.offsets.is_empty(),
"Code layout must be computed first" "Code layout must be computed first"
); );
InstOffsetIter { InstOffsetIter {
encinfo: encinfo.clone(), encinfo: encinfo.clone(),
func, func: self,
divert: RegDiversions::new(), divert: RegDiversions::new(),
encodings: &self.encodings, encodings: &self.encodings,
offset: self.offsets[ebb], offset: self.offsets[ebb],

View File

@@ -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 /// 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 /// 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. /// 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 sizing.base_size
} }

View File

@@ -25,7 +25,7 @@ pub fn pretty_verifier_error<'a>(
let mut w = String::new(); let mut w = String::new();
decorate_function( 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, &mut w,
func, func,
isa, isa,

View File

@@ -234,7 +234,8 @@ pub fn verify_function<'a, FOI: Into<FlagsOrIsa<'a>>>(
let verifier = Verifier::new(func, fisa.into()); let verifier = Verifier::new(func, fisa.into());
let result = verifier.run(&mut errors); let result = verifier.run(&mut errors);
if errors.is_empty() { if errors.is_empty() {
Ok(result.unwrap()) result.unwrap();
Ok(())
} else { } else {
Err(errors) Err(errors)
} }
@@ -392,15 +393,9 @@ impl<'a> Verifier<'a> {
); );
} }
match heap_data.style { if let ir::HeapStyle::Dynamic { bound_gv, .. } = heap_data.style {
ir::HeapStyle::Dynamic { bound_gv, .. } => {
if !self.func.global_values.is_valid(bound_gv) { if !self.func.global_values.is_valid(bound_gv) {
return nonfatal!( return nonfatal!(errors, heap, "invalid bound global value {}", bound_gv);
errors,
heap,
"invalid bound global value {}",
bound_gv
);
} }
let index_type = heap_data.index_type; let index_type = heap_data.index_type;
@@ -415,8 +410,6 @@ impl<'a> Verifier<'a> {
); );
} }
} }
_ => {}
}
} }
} }

View File

@@ -31,7 +31,7 @@ enum State {
Done(TestResult), Done(TestResult),
} }
#[derive(PartialEq, Eq, Debug)] #[derive(PartialEq, Eq, Debug, Clone, Copy)]
pub enum IsPass { pub enum IsPass {
Pass, Pass,
NotPass, NotPass,

View File

@@ -205,7 +205,7 @@ impl SubTest for TestBinEmit {
"Inconsistent {} header offset", "Inconsistent {} header offset",
ebb 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); assert_eq!(sink.offset, offset);
sink.text.clear(); sink.text.clear();
let enc = func.encodings[inst]; let enc = func.encodings[inst];

View File

@@ -8,7 +8,7 @@ type EntryIndex = u64;
/// Unlike with `br_table`, `Switch` cases may be sparse or non-0-based. /// 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. /// They emit efficient code using branches, jump tables, or a combination of both.
#[derive(Debug)] #[derive(Debug, Default)]
pub struct Switch { pub struct Switch {
cases: HashMap<EntryIndex, Ebb>, cases: HashMap<EntryIndex, Ebb>,
} }
@@ -16,7 +16,7 @@ pub struct Switch {
impl Switch { impl Switch {
/// Create a new empty switch /// Create a new empty switch
pub fn new() -> Self { pub fn new() -> Self {
Switch { Self {
cases: HashMap::new(), cases: HashMap::new(),
} }
} }
@@ -31,23 +31,23 @@ impl Switch {
); );
} }
fn collect_contiguous_case_ranges(self) -> Vec<(EntryIndex, Vec<Ebb>)> { fn collect_contiguous_case_ranges(self) -> Vec<ContiguousCaseRange> {
debug!("build_contiguous_case_ranges before: {:#?}", self.cases); debug!("build_contiguous_case_ranges before: {:#?}", self.cases);
let mut cases = self.cases.into_iter().collect::<Vec<(_, _)>>(); let mut cases = self.cases.into_iter().collect::<Vec<(_, _)>>();
cases.sort_by_key(|&(index, _)| index); cases.sort_by_key(|&(index, _)| index);
let mut contiguous_case_ranges: Vec<(EntryIndex, Vec<Ebb>)> = vec![]; let mut contiguous_case_ranges: Vec<ContiguousCaseRange> = vec![];
let mut last_index = None; let mut last_index = None;
for (index, ebb) in cases { for (index, ebb) in cases {
match last_index { match last_index {
None => contiguous_case_ranges.push((index, vec![])), None => contiguous_case_ranges.push(ContiguousCaseRange::new(index)),
Some(last_index) => { Some(last_index) => {
if index > last_index + 1 { 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); last_index = Some(index);
} }
@@ -63,7 +63,7 @@ impl Switch {
bx: &mut FunctionBuilder, bx: &mut FunctionBuilder,
val: Value, val: Value,
otherwise: Ebb, otherwise: Ebb,
contiguous_case_ranges: Vec<(EntryIndex, Vec<Ebb>)>, contiguous_case_ranges: Vec<ContiguousCaseRange>,
) -> Vec<(EntryIndex, Ebb, Vec<Ebb>)> { ) -> Vec<(EntryIndex, Ebb, Vec<Ebb>)> {
let mut cases_and_jt_ebbs = Vec::new(); let mut cases_and_jt_ebbs = Vec::new();
@@ -79,7 +79,7 @@ impl Switch {
return cases_and_jt_ebbs; return cases_and_jt_ebbs;
} }
let mut stack: Vec<(Option<Ebb>, Vec<(EntryIndex, Vec<Ebb>)>)> = Vec::new(); let mut stack: Vec<(Option<Ebb>, Vec<ContiguousCaseRange>)> = Vec::new();
stack.push((None, contiguous_case_ranges)); stack.push((None, contiguous_case_ranges));
while let Some((ebb, contiguous_case_ranges)) = stack.pop() { while let Some((ebb, contiguous_case_ranges)) = stack.pop() {
@@ -103,9 +103,11 @@ impl Switch {
let left_ebb = bx.create_ebb(); let left_ebb = bx.create_ebb();
let right_ebb = bx.create_ebb(); let right_ebb = bx.create_ebb();
let should_take_right_side = let should_take_right_side = bx.ins().icmp_imm(
bx.ins() IntCC::UnsignedGreaterThanOrEqual,
.icmp_imm(IntCC::UnsignedGreaterThanOrEqual, val, right[0].0 as i64); val,
right[0].first_index as i64,
);
bx.ins().brnz(should_take_right_side, right_ebb, &[]); bx.ins().brnz(should_take_right_side, right_ebb, &[]);
bx.ins().jump(left_ebb, &[]); bx.ins().jump(left_ebb, &[]);
@@ -121,10 +123,10 @@ impl Switch {
bx: &mut FunctionBuilder, bx: &mut FunctionBuilder,
val: Value, val: Value,
otherwise: Ebb, otherwise: Ebb,
contiguous_case_ranges: Vec<(EntryIndex, Vec<Ebb>)>, contiguous_case_ranges: Vec<ContiguousCaseRange>,
cases_and_jt_ebbs: &mut Vec<(EntryIndex, Ebb, Vec<Ebb>)>, cases_and_jt_ebbs: &mut Vec<(EntryIndex, Ebb, Vec<Ebb>)>,
) { ) {
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 { if ebbs.len() == 1 {
let is_good_val = bx.ins().icmp_imm(IntCC::Equal, val, first_index as i64); let is_good_val = bx.ins().icmp_imm(IntCC::Equal, val, first_index as i64);
bx.ins().brnz(is_good_val, ebbs[0], &[]); bx.ins().brnz(is_good_val, ebbs[0], &[]);
@@ -180,6 +182,21 @@ impl Switch {
} }
} }
#[derive(Debug)]
struct ContiguousCaseRange {
first_index: EntryIndex,
ebbs: Vec<Ebb>,
}
impl ContiguousCaseRange {
fn new(first_index: EntryIndex) -> Self {
Self {
first_index,
ebbs: Vec::new(),
}
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;

View File

@@ -775,8 +775,7 @@ impl<'a> Parser<'a> {
let mut targets = Vec::new(); let mut targets = Vec::new();
let flag_builder = settings::builder(); let flag_builder = settings::builder();
match target_pass { if let Some(targ) = target_pass {
Some(targ) => {
let loc = self.loc; let loc = self.loc;
let triple = match Triple::from_str(targ) { let triple = match Triple::from_str(targ) {
Ok(triple) => triple, Ok(triple) => triple,
@@ -796,8 +795,6 @@ impl<'a> Parser<'a> {
// Construct a trait object with the aggregate settings. // Construct a trait object with the aggregate settings.
targets.push(isa_builder.finish(settings::Flags::new(flag_builder.clone()))); targets.push(isa_builder.finish(settings::Flags::new(flag_builder.clone())));
} }
None => (),
};
if !specified_target { if !specified_target {
// No `target` commands. // No `target` commands.

View File

@@ -411,7 +411,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend {
/// SimpleJIT emits code and data into memory as it processes them, so it /// SimpleJIT emits code and data into memory as it processes them, so it
/// doesn't need to provide anything after the `Module` is complete. /// doesn't need to provide anything after the `Module` is complete.
fn finish(self) -> () {} fn finish(self) {}
} }
#[cfg(not(windows))] #[cfg(not(windows))]

View File

@@ -161,6 +161,7 @@ pub trait FuncEnvironment {
/// The signature `sig_ref` was previously created by `make_indirect_sig()`. /// The signature `sig_ref` was previously created by `make_indirect_sig()`.
/// ///
/// Return the call instruction whose results are the WebAssembly return values. /// Return the call instruction whose results are the WebAssembly return values.
#[cfg_attr(feature = "cargo-clippy", allow(too_many_arguments))]
fn translate_call_indirect( fn translate_call_indirect(
&mut self, &mut self,
pos: FuncCursor, pos: FuncCursor,