From 2870172e0d7c5a05026676a21372f01840398c00 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 21 Jul 2020 05:51:33 +0000 Subject: [PATCH] Deny warnings on CI (#1372) ## Issue Addressed Prevent CI from succeeding when there are warnings. Code can still be built and tested with warnings locally, but CI may fail during a Rust update (which is fine IMO). ## Proposed Changes * Deny warnings for all stable compiler jobs on CI (excludes `cargo udeps`, which runs under nightly) * Fix the warnings currently on `master` related to unnecessary `mem::replace` --- .github/workflows/test-suite.yml | 7 ++++++- consensus/merkle_proof/src/lib.rs | 9 ++++----- consensus/tree_hash/benches/benches.rs | 2 +- consensus/types/src/beacon_state/tree_hash_cache.rs | 2 +- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index 3cc127455..bb984565e 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -8,7 +8,9 @@ on: - trying - 'pr/*' pull_request: - +env: + # Deny warnings in CI + RUSTFLAGS: "-D warnings" jobs: cargo-fmt: name: cargo-fmt @@ -149,3 +151,6 @@ jobs: run: cargo install cargo-udeps --locked - name: Run cargo udeps to identify unused crates in the dependency graph run: make udeps + env: + # Allow warnings on Nightly + RUSTFLAGS: "" diff --git a/consensus/merkle_proof/src/lib.rs b/consensus/merkle_proof/src/lib.rs index f80ee2a2d..4f86cde1f 100644 --- a/consensus/merkle_proof/src/lib.rs +++ b/consensus/merkle_proof/src/lib.rs @@ -81,7 +81,6 @@ impl MerkleTree { /// Push an element in the MerkleTree. /// MerkleTree and depth must be correct, as the algorithm expects valid data. pub fn push_leaf(&mut self, elem: H256, depth: usize) -> Result<(), MerkleTreeError> { - use std::mem; use MerkleTree::*; if depth == 0 { @@ -91,7 +90,7 @@ impl MerkleTree { match self { Leaf(_) => return Err(MerkleTreeError::LeafReached), Zero(_) => { - mem::replace(self, MerkleTree::create(&[elem], depth)); + *self = MerkleTree::create(&[elem], depth); } Node(ref mut hash, ref mut left, ref mut right) => { let left: &mut MerkleTree = &mut *left; @@ -107,11 +106,11 @@ impl MerkleTree { } // Both branches are zero, insert in left one (Zero(_), Zero(_)) => { - mem::replace(left, MerkleTree::create(&[elem], depth - 1)); + *left = MerkleTree::create(&[elem], depth - 1); } // Leaf on left branch and zero on right branch, insert on right side (Leaf(_), Zero(_)) => { - mem::replace(right, MerkleTree::create(&[elem], depth - 1)); + *right = MerkleTree::create(&[elem], depth - 1); } // Try inserting on the left node -> if it fails because it is full, insert in right side. (Node(_, _, _), Zero(_)) => { @@ -119,7 +118,7 @@ impl MerkleTree { Ok(_) => (), // Left node is full, insert in right node Err(MerkleTreeError::MerkleTreeFull) => { - mem::replace(right, MerkleTree::create(&[elem], depth - 1)); + *right = MerkleTree::create(&[elem], depth - 1); } Err(e) => return Err(e), }; diff --git a/consensus/tree_hash/benches/benches.rs b/consensus/tree_hash/benches/benches.rs index 2e7b68952..0bde5f78d 100644 --- a/consensus/tree_hash/benches/benches.rs +++ b/consensus/tree_hash/benches/benches.rs @@ -5,7 +5,7 @@ use types::test_utils::{generate_deterministic_keypairs, TestingBeaconStateBuild use types::{BeaconState, EthSpec, Keypair, MainnetEthSpec, MinimalEthSpec}; lazy_static! { - static ref KEYPAIRS: Vec = { generate_deterministic_keypairs(300_000) }; + static ref KEYPAIRS: Vec = generate_deterministic_keypairs(300_000); } fn build_state(validator_count: usize) -> BeaconState { diff --git a/consensus/types/src/beacon_state/tree_hash_cache.rs b/consensus/types/src/beacon_state/tree_hash_cache.rs index d77987acd..a73e3326c 100644 --- a/consensus/types/src/beacon_state/tree_hash_cache.rs +++ b/consensus/types/src/beacon_state/tree_hash_cache.rs @@ -211,7 +211,7 @@ impl ValidatorsListTreeHashCache { .list_cache .recalculate_merkle_root(&mut list_arena, leaves.into_iter())?; - std::mem::replace(&mut self.list_arena, list_arena); + self.list_arena = list_arena; Ok(mix_in_length(&list_root, validators.len())) }