From 2c4700f4c1aa97fbfb5b57a01523553f12ee0063 Mon Sep 17 00:00:00 2001 From: Tomusdrw Date: Mon, 15 Feb 2016 00:51:50 +0100 Subject: [PATCH] Fixing clippy warnings --- ethcore/src/basic_types.rs | 1 + ethcore/src/block_queue.rs | 1 + ethcore/src/evm/interpreter.rs | 55 +++++++++++++++----------------- ethcore/src/evm/tests.rs | 23 ++++++------- ethcore/src/externalities.rs | 2 +- ethcore/src/lib.rs | 15 ++++++--- parity/main.rs | 2 +- rpc/src/v1/types/block_number.rs | 1 + sync/src/chain.rs | 14 ++++---- sync/src/lib.rs | 5 ++- sync/src/range_collection.rs | 3 +- util/src/lib.rs | 12 +++++-- util/src/panics.rs | 2 +- 13 files changed, 78 insertions(+), 58 deletions(-) diff --git a/ethcore/src/basic_types.rs b/ethcore/src/basic_types.rs index 51e05500c..2e9c5d7b9 100644 --- a/ethcore/src/basic_types.rs +++ b/ethcore/src/basic_types.rs @@ -24,6 +24,7 @@ pub type LogBloom = H2048; /// Constant 2048-bit datum for 0. Often used as a default. pub static ZERO_LOGBLOOM: LogBloom = H2048([0x00; 256]); +#[allow(enum_variant_names)] /// Semantic boolean for when a seal/signature is included. pub enum Seal { /// The seal/signature is included. diff --git a/ethcore/src/block_queue.rs b/ethcore/src/block_queue.rs index dcfcec1e4..1a1dee48e 100644 --- a/ethcore/src/block_queue.rs +++ b/ethcore/src/block_queue.rs @@ -87,6 +87,7 @@ struct QueueSignal { } impl QueueSignal { + #[allow(bool_comparison)] fn set(&self) { if self.signalled.compare_and_swap(false, true, AtomicOrdering::Relaxed) == false { self.message_channel.send(UserMessage(SyncMessage::BlockVerified)).expect("Error sending BlockVerified message"); diff --git a/ethcore/src/evm/interpreter.rs b/ethcore/src/evm/interpreter.rs index 92d5434d0..50c0377ac 100644 --- a/ethcore/src/evm/interpreter.rs +++ b/ethcore/src/evm/interpreter.rs @@ -212,7 +212,7 @@ impl Memory for Vec { fn write(&mut self, offset: U256, value: U256) { let off = offset.low_u64() as usize; let mut val = value; - + let end = off + 32; for pos in 0..32 { self[end - pos - 1] = val.low_u64() as u8; @@ -229,7 +229,7 @@ impl Memory for Vec { fn resize(&mut self, new_size: usize) { self.resize(new_size, 0); } - + fn expand(&mut self, size: usize) { if size > self.len() { Memory::resize(self, size) @@ -258,6 +258,7 @@ impl<'a> CodeReader<'a> { } } +#[allow(enum_variant_names)] enum InstructionCost { Gas(U256), GasMem(U256, U256), @@ -282,7 +283,7 @@ impl evm::Evm for Interpreter { let code = ¶ms.code.as_ref().unwrap(); let valid_jump_destinations = self.find_jump_destinations(&code); - let mut current_gas = params.gas.clone(); + let mut current_gas = params.gas; let mut stack = VecStack::with_capacity(ext.schedule().stack_limit, U256::zero()); let mut mem = vec![]; let mut reader = CodeReader { @@ -331,7 +332,7 @@ impl evm::Evm for Interpreter { let pos = try!(self.verify_jump(position, &valid_jump_destinations)); reader.position = pos; }, - InstructionResult::StopExecutionWithGasLeft(gas_left) => { + InstructionResult::StopExecutionWithGasLeft(gas_left) => { current_gas = gas_left; reader.position = code.len(); }, @@ -380,10 +381,9 @@ impl Interpreter { let gas = if self.is_zero(&val) && !self.is_zero(newval) { schedule.sstore_set_gas - } else if !self.is_zero(&val) && self.is_zero(newval) { - // Refund is added when actually executing sstore - schedule.sstore_reset_gas } else { + // Refund for below case is added when actually executing sstore + // !self.is_zero(&val) && self.is_zero(newval) schedule.sstore_reset_gas }; InstructionCost::Gas(U256::from(gas)) @@ -406,10 +406,7 @@ impl Interpreter { let gas = U256::from(schedule.sha3_gas) + (U256::from(schedule.sha3_word_gas) * words); InstructionCost::GasMem(gas, try!(self.mem_needed(stack.peek(0), stack.peek(1)))) }, - instructions::CALLDATACOPY => { - InstructionCost::GasMemCopy(default_gas, try!(self.mem_needed(stack.peek(0), stack.peek(2))), stack.peek(2).clone()) - }, - instructions::CODECOPY => { + instructions::CALLDATACOPY | instructions::CODECOPY => { InstructionCost::GasMemCopy(default_gas, try!(self.mem_needed(stack.peek(0), stack.peek(2))), stack.peek(2).clone()) }, instructions::EXTCODECOPY => { @@ -432,7 +429,7 @@ impl Interpreter { try!(self.mem_needed(stack.peek(5), stack.peek(6))), try!(self.mem_needed(stack.peek(3), stack.peek(4))) ); - + let address = u256_to_address(stack.peek(1)); if instruction == instructions::CALL && !ext.exists(&address) { @@ -529,8 +526,8 @@ impl Interpreter { params: &ActionParams, ext: &mut evm::Ext, instruction: Instruction, - code: &mut CodeReader, - mem: &mut Memory, + code: &mut CodeReader, + mem: &mut Memory, stack: &mut Stack ) -> Result { match instruction { @@ -559,7 +556,7 @@ impl Interpreter { let contract_code = mem.read_slice(init_off, init_size); let can_create = ext.balance(¶ms.address) >= endowment && ext.depth() < ext.schedule().max_depth; - + if !can_create { stack.push(U256::zero()); return Ok(InstructionResult::Ok); @@ -638,7 +635,7 @@ impl Interpreter { Ok(InstructionResult::Ok) } }; - }, + }, instructions::RETURN => { let init_off = stack.pop_back(); let init_size = stack.pop_back(); @@ -832,20 +829,20 @@ impl Interpreter { } } - fn verify_instructions_requirements(&self, - info: &instructions::InstructionInfo, - stack_limit: usize, + fn verify_instructions_requirements(&self, + info: &instructions::InstructionInfo, + stack_limit: usize, stack: &Stack) -> Result<(), evm::Error> { if !stack.has(info.args) { Err(evm::Error::StackUnderflow { instruction: info.name, - wanted: info.args, + wanted: info.args, on_stack: stack.size() }) } else if stack.size() - info.args + info.ret > stack_limit { Err(evm::Error::OutOfStack { instruction: info.name, - wanted: info.ret - info.args, + wanted: info.ret - info.args, limit: stack_limit }) } else { @@ -919,7 +916,7 @@ impl Interpreter { stack.push(if !self.is_zero(&b) { a.overflowing_div(b).0 } else { - U256::zero() + U256::zero() }); }, instructions::MOD => { @@ -978,9 +975,9 @@ impl Interpreter { let (a, neg_a) = get_and_reset_sign(stack.pop_back()); let (b, neg_b) = get_and_reset_sign(stack.pop_back()); - let is_positive_lt = a < b && (neg_a | neg_b) == false; - let is_negative_lt = a > b && (neg_a & neg_b) == true; - let has_different_signs = neg_a == true && neg_b == false; + let is_positive_lt = a < b && !(neg_a | neg_b); + let is_negative_lt = a > b && (neg_a & neg_b); + let has_different_signs = neg_a && !neg_b; stack.push(self.bool_to_u256(is_positive_lt | is_negative_lt | has_different_signs)); }, @@ -993,9 +990,9 @@ impl Interpreter { let (a, neg_a) = get_and_reset_sign(stack.pop_back()); let (b, neg_b) = get_and_reset_sign(stack.pop_back()); - let is_positive_gt = a > b && (neg_a | neg_b) == false; - let is_negative_gt = a < b && (neg_a & neg_b) == true; - let has_different_signs = neg_a == false && neg_b == true; + let is_positive_gt = a > b && !(neg_a | neg_b); + let is_negative_gt = a < b && (neg_a & neg_b); + let has_different_signs = !neg_a && neg_b; stack.push(self.bool_to_u256(is_positive_gt | is_negative_gt | has_different_signs)); }, @@ -1175,7 +1172,7 @@ mod tests { let schedule = evm::Schedule::default(); let current_mem_size = 0; let mem_size = U256::from(5); - + // when let (mem_cost, mem_size) = interpreter.mem_gas_cost(&schedule, current_mem_size, &mem_size).unwrap(); diff --git a/ethcore/src/evm/tests.rs b/ethcore/src/evm/tests.rs index 3eadef15a..02f929192 100644 --- a/ethcore/src/evm/tests.rs +++ b/ethcore/src/evm/tests.rs @@ -25,6 +25,7 @@ struct FakeLogEntry { } #[derive(PartialEq, Eq, Hash, Debug)] +#[allow(enum_variant_names)] // Common prefix is C ;) enum FakeCallType { CALL, CREATE } @@ -59,7 +60,7 @@ struct FakeExt { } impl FakeExt { - fn new() -> Self { + fn new() -> Self { FakeExt::default() } } @@ -104,13 +105,13 @@ impl Ext for FakeExt { ContractCreateResult::Failed } - fn call(&mut self, - gas: &U256, - sender_address: &Address, - receive_address: &Address, + fn call(&mut self, + gas: &U256, + sender_address: &Address, + receive_address: &Address, value: Option, - data: &[u8], - code_address: &Address, + data: &[u8], + code_address: &Address, _output: &mut [u8]) -> MessageCallResult { self.calls.insert(FakeCall { @@ -176,7 +177,7 @@ fn test_stack_underflow() { let vm : Box = Box::new(super::interpreter::Interpreter); vm.exec(params, &mut ext).unwrap_err() }; - + match err { evm::Error::StackUnderflow {wanted, on_stack, ..} => { assert_eq!(wanted, 2); @@ -353,7 +354,7 @@ evm_test!{test_log_sender: test_log_sender_jit, test_log_sender_int} fn test_log_sender(factory: super::Factory) { // 60 ff - push ff // 60 00 - push 00 - // 53 - mstore + // 53 - mstore // 33 - sender // 60 20 - push 20 // 60 00 - push 0 @@ -449,7 +450,7 @@ fn test_author(factory: super::Factory) { evm_test!{test_timestamp: test_timestamp_jit, test_timestamp_int} fn test_timestamp(factory: super::Factory) { - let timestamp = 0x1234; + let timestamp = 0x1234; let code = "42600055".from_hex().unwrap(); let mut params = ActionParams::default(); @@ -469,7 +470,7 @@ fn test_timestamp(factory: super::Factory) { evm_test!{test_number: test_number_jit, test_number_int} fn test_number(factory: super::Factory) { - let number = 0x1234; + let number = 0x1234; let code = "43600055".from_hex().unwrap(); let mut params = ActionParams::default(); diff --git a/ethcore/src/externalities.rs b/ethcore/src/externalities.rs index 4ad84497f..360bd9738 100644 --- a/ethcore/src/externalities.rs +++ b/ethcore/src/externalities.rs @@ -363,7 +363,7 @@ mod tests { &Address::new(), &Address::new(), Some(U256::from_str("0000000000000000000000000000000000000000000000000000000000150000").unwrap()), - &vec![], + &[], &Address::new(), &mut output); } diff --git a/ethcore/src/lib.rs b/ethcore/src/lib.rs index 6c4535339..4cca74319 100644 --- a/ethcore/src/lib.rs +++ b/ethcore/src/lib.rs @@ -18,8 +18,15 @@ #![feature(cell_extras)] #![feature(augmented_assignments)] #![feature(plugin)] +// Clippy #![plugin(clippy)] -#![allow(needless_range_loop, match_bool)] +// TODO [todr] not really sure +#![allow(needless_range_loop)] +// Shorter than if-else +#![allow(match_bool)] +// Keeps consistency (all lines with `.clone()`) and helpful when changing ref to non-ref. +#![allow(clone_on_copy)] + //! Ethcore library //! @@ -54,7 +61,7 @@ //! cd parity //! cargo build --release //! ``` -//! +//! //! - OSX: //! //! ```bash @@ -124,8 +131,8 @@ mod executive; mod externalities; mod verification; -#[cfg(test)] +#[cfg(test)] mod tests; #[cfg(test)] -#[cfg(feature="json-tests")] +#[cfg(feature="json-tests")] mod json_tests; diff --git a/parity/main.rs b/parity/main.rs index 62b73ca47..460922b64 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -165,7 +165,7 @@ impl Configuration { } Some(ref a) => { public_address = SocketAddr::from_str(a.as_ref()).expect("Invalid listen/public address given with --address"); - listen_address = public_address.clone(); + listen_address = public_address; } }; diff --git a/rpc/src/v1/types/block_number.rs b/rpc/src/v1/types/block_number.rs index b524d8450..546816eba 100644 --- a/rpc/src/v1/types/block_number.rs +++ b/rpc/src/v1/types/block_number.rs @@ -55,6 +55,7 @@ impl Visitor for BlockNumberVisitor { } impl Into for BlockNumber { + #[allow(match_same_arms)] fn into(self) -> BlockId { match self { BlockNumber::Num(n) => BlockId::Number(n), diff --git a/sync/src/chain.rs b/sync/src/chain.rs index f0c0347a9..7a1643477 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -14,17 +14,17 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . -/// +/// /// BlockChain synchronization strategy. -/// Syncs to peers and keeps up to date. +/// Syncs to peers and keeps up to date. /// This implementation uses ethereum protocol v63 /// /// Syncing strategy. /// /// 1. A peer arrives with a total difficulty better than ours -/// 2. Find a common best block between our an peer chain. +/// 2. Find a common best block between our an peer chain. /// Start with out best block and request headers from peer backwards until a common block is found -/// 3. Download headers and block bodies from peers in parallel. +/// 3. Download headers and block bodies from peers in parallel. /// As soon as a set of the blocks is fully downloaded at the head of the queue it is fed to the blockchain /// 4. Maintain sync by handling NewBlocks/NewHashes messages /// @@ -240,6 +240,8 @@ impl ChainSync { self.peers.clear(); } + + #[allow(for_kv_map)] // Because it's not possible to get `values_mut()` /// Rest sync. Clear all downloaded data but keep the queue fn reset(&mut self) { self.downloading_headers.clear(); @@ -1023,7 +1025,7 @@ impl ChainSync { GET_NODE_DATA_PACKET => self.return_rlp(io, &rlp, ChainSync::return_node_data, |e| format!("Error sending nodes: {:?}", e)), - + _ => { debug!(target: "sync", "Unknown packet {}", packet_id); Ok(()) @@ -1061,7 +1063,7 @@ impl ChainSync { for block_hash in route.blocks { let mut hash_rlp = RlpStream::new_list(2); let difficulty = chain.block_total_difficulty(BlockId::Hash(block_hash.clone())).expect("Mallformed block without a difficulty on the chain!"); - + hash_rlp.append(&block_hash); hash_rlp.append(&difficulty); rlp_stream.append_raw(&hash_rlp.out(), 1); diff --git a/sync/src/lib.rs b/sync/src/lib.rs index 522062778..74edab4a5 100644 --- a/sync/src/lib.rs +++ b/sync/src/lib.rs @@ -16,8 +16,11 @@ #![warn(missing_docs)] #![feature(plugin)] -#![plugin(clippy)] #![feature(augmented_assignments)] +#![plugin(clippy)] +// Keeps consistency (all lines with `.clone()`) and helpful when changing ref to non-ref. +#![allow(clone_on_copy)] + //! Blockchain sync module //! Implements ethereum protocol version 63 as specified here: //! https://github.com/ethereum/wiki/wiki/Ethereum-Wire-Protocol diff --git a/sync/src/range_collection.rs b/sync/src/range_collection.rs index a07f85a7f..c3333ab63 100644 --- a/sync/src/range_collection.rs +++ b/sync/src/range_collection.rs @@ -170,8 +170,7 @@ impl RangeCollection for Vec<(K, Vec)> where K: Ord + PartialEq + // todo: fix warning let lower = match self.binary_search_by(|&(k, _)| k.cmp(&key).reverse()) { - Ok(index) => index, - Err(index) => index, + Ok(index) | Err(index) => index }; let mut to_remove: Option = None; diff --git a/util/src/lib.rs b/util/src/lib.rs index bdd595014..59e9b966c 100644 --- a/util/src/lib.rs +++ b/util/src/lib.rs @@ -19,9 +19,17 @@ #![feature(augmented_assignments)] #![feature(associated_consts)] #![feature(plugin)] -#![plugin(clippy)] -#![allow(needless_range_loop, match_bool)] #![feature(catch_panic)] +// Clippy settings +#![plugin(clippy)] +// TODO [todr] not really sure +#![allow(needless_range_loop)] +// Shorter than if-else +#![allow(match_bool)] +// We use that to be more explicit about handled cases +#![allow(match_same_arms)] +// Keeps consistency (all lines with `.clone()`) and helpful when changing ref to non-ref. +#![allow(clone_on_copy)] //! Ethcore-util library //! diff --git a/util/src/panics.rs b/util/src/panics.rs index 72718db58..27dd605f0 100644 --- a/util/src/panics.rs +++ b/util/src/panics.rs @@ -104,7 +104,7 @@ impl OnPanicListener for F } fn convert_to_string(t: &Box) -> Option { - let as_str = t.downcast_ref::<&'static str>().map(|t| t.clone().to_owned()); + let as_str = t.downcast_ref::<&'static str>().cloned().map(|t| t.to_owned()); let as_string = t.downcast_ref::().cloned(); as_str.or(as_string)