diff --git a/.travis.yml b/.travis.yml index 4571ece13d..5d85b235bf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,38 +1,67 @@ language: rust sudo: false +rust: nightly matrix: include: - name: "miri" - rust: nightly - install: - - rustup component add rust-src - - cargo +nightly install --force --git https://github.com/rust-lang/miri miri - - cargo install xargo - script: - - cargo clean - - cargo +nightly miri test - - rust: 1.29.0 - - rust: stable - - rust: beta - - rust: nightly - - rust: nightly - env: - - FEATURES='nightly' + env: TARGET=x86_64-unknown-linux-gnu + script: sh ci/miri.sh + - name: "rustfmt/clippy" + env: TARGET=i586-unknown-linux-gnu + script: sh ci/tools.sh + - name: "docs" + env: TARGET=x86_64-unknown-linux-gnu + script: cargo -vv doc --features nightly,serde,rayon + deploy: + provider: pages + skip-cleanup: true + github-token: $GITHUB_TOKEN + local-dir: target/doc + keep-history: false + on: + branch: master - # test a target without sse2 for raw/generic.rs - - rust: stable - env: - - TARGET=i586-unknown-linux-gnu - addons: - apt: - packages: - - gcc-multilib - install: - - rustup target add $TARGET - script: - - cargo build --verbose --target $TARGET - - cargo test --verbose --target $TARGET + # Tier 1 targets: + - name: "x86_64-unknown-linux-gnu" + env: TARGET=x86_64-unknown-linux-gnu + - name: "x86_64-unknown-linux-gnu (beta)" + rust: beta + env: TARGET=x86_64-unknown-linux-gnu + - name: "x86_64-unknown-linux-gnu (stable)" + rust: stable + env: TARGET=x86_64-unknown-linux-gnu + - name: "x86_64-unknown-linux-gnu (Rust 1.29.0)" + rust: 1.31.0 + env: TARGET=x86_64-unknown-linux-gnu + - name: "i686-unknown-linux-gnu" + env: TARGET=i686-unknown-linux-gnu CROSS=1 + - name: "x86_64-apple-darwin" + env: TARGET=x86_64-apple-darwin + os: osx + osx_image: xcode10 + - name: "i686-apple-darwin" + env: TARGET=i686-apple-darwin + os: osx + osx_image: xcode10 + - name: "x86_64-pc-windows-msvc" + env: TARGET=x86_64-pc-windows-msvc + os: windows + - name: "x86_64-pc-windows-gnu" + env: TARGET=x86_64-pc-windows-gnu CROSS=1 + - name: "i686-pc-windows-gnu" + env: TARGET=i686-pc-windows-gnu CROSS=1 + + # Tier 2/3 targets: + - name: "i586-unknown-linux-gnu (no SSE2)" + env: TARGET=i586-unknown-linux-gnu CROSS=1 + - name: "armv7-unknown-linux-gnueabihf" + env: TARGET=armv7-unknown-linux-gnueabihf CROSS=1 + - name: "aarch64-unknown-linux-gnu" + env: TARGET=aarch64-unknown-linux-gnu CROSS=1 + +install: travis_retry rustup target add "${TARGET}" +script: sh ci/run.sh branches: # Don't build these branches @@ -40,19 +69,3 @@ branches: # Used by bors - trying.tmp - staging.tmp - -before_script: - - if [ "$TRAVIS_RUST_VERSION" == "stable" ]; then rustup component add rustfmt; fi - -script: - - if [ "$TRAVIS_RUST_VERSION" == "stable" ]; then cargo fmt -- --check; fi - - - cargo build --verbose --features "$FEATURES" - - cargo test --verbose --features "$FEATURES" - - if [ "$TRAVIS_RUST_VERSION" == "nightly" ]; then cargo bench --verbose --features "$FEATURES"; fi - - cargo doc --verbose --features "$FEATURES" - - - cargo build --verbose --features "$FEATURES serde" - - cargo test --verbose --features "$FEATURES serde" - - if [ "$TRAVIS_RUST_VERSION" == "nightly" ]; then cargo bench --verbose --features "$FEATURES serde"; fi - - cargo doc --verbose --features "$FEATURES serde" diff --git a/Cargo.toml b/Cargo.toml index b835dd3bc1..82f303a335 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,4 +26,5 @@ rustc-hash = "1.0" serde_test = "1.0" [features] +default = [] nightly = [] diff --git a/ci/miri.sh b/ci/miri.sh new file mode 100644 index 0000000000..a948f69f59 --- /dev/null +++ b/ci/miri.sh @@ -0,0 +1,11 @@ +#!/usr/bin/env sh + +set -ex + +export CARGO_NET_RETRY=5 +export CARGO_NET_TIMEOUT=10 + +if cargo install --force --git https://github.com/rust-lang/miri miri ; then + cargo miri setup + cargo miri test +fi diff --git a/ci/run.sh b/ci/run.sh new file mode 100644 index 0000000000..99ed4276d6 --- /dev/null +++ b/ci/run.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env sh + +set -ex + +: "${TARGET?The TARGET environment variable must be set.}" + +FEATURES="rayon,serde" +if [ "${TRAVIS_RUST_VERSION}" = "nightly" ]; then + FEATURES="${FEATURES},nightly" + export RUSTFLAGS="$RUSTFLAGS -D warnings" +fi + +CARGO=cargo +if [ "${CROSS}" = "1" ]; then + export CARGO_NET_RETRY=5 + export CARGO_NET_TIMEOUT=10 + + cargo install cross + CARGO=cross +fi + +export RUSTFLAGS="$RUSTFLAGS --cfg hashbrown_deny_warnings" + +"${CARGO}" -vv test --target="${TARGET}" +"${CARGO}" -vv test --target="${TARGET}" --features "${FEATURES}" + +"${CARGO}" -vv test --target="${TARGET}" --release +"${CARGO}" -vv test --target="${TARGET}" --release --features "${FEATURES}" + +if [ "${TRAVIS_RUST_VERSION}" = "nightly" ]; then + # Run benchmark on native targets, build them on non-native ones: + NO_RUN="" + if [ "${CROSS}" = "1" ]; then + NO_RUN="--no-run" + fi + + "${CARGO}" -vv bench "${NO_RUN}" --features "${FEATURES}" +fi diff --git a/ci/tools.sh b/ci/tools.sh new file mode 100644 index 0000000000..fe2ba52f08 --- /dev/null +++ b/ci/tools.sh @@ -0,0 +1,45 @@ +#!/usr/bin/env sh + +set -ex + +retry() { + result=0 + count=1 + max=5 + while [ "$count" -le 3 ]; do + [ "$result" -ne 0 ] && { + printf "\nRetrying, %d of %d\n" $count $max >&2 + } + "$@" + result=$? + [ $result -eq 0 ] && break + count=$(count + 1) + sleep 1 + done + + [ "$count" -gt 3 ] && { + printf "\nFailed %d times.\n" $max >&2 + } + + return $result +} + + +if retry rustup component add rustfmt ; then + cargo fmt --all -- --check +fi + +if retry rustup component add clippy ; then + cargo clippy --all -- -D clippy::pedantic +fi + +if [ "${TRAVIS_OS_NAME}" = "linux" ]; then + if retry rustup component add clippy ; then + cargo clippy --all --target=i586-unknown-linux-gnu -- -D clippy::pedantic + fi +fi + +if command -v shellcheck ; then + shellcheck --version + shellcheck ci/*.sh +fi diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 0000000000..d98bf2c09b --- /dev/null +++ b/clippy.toml @@ -0,0 +1 @@ +doc-valid-idents = [ "CppCon", "SwissTable", "SipHash", "HashDoS" ] diff --git a/src/fx.rs b/src/fx.rs index f45399fdc6..11574f9122 100644 --- a/src/fx.rs +++ b/src/fx.rs @@ -26,14 +26,14 @@ pub struct FxHasher { } #[cfg(target_pointer_width = "32")] -const K: usize = 0x9e3779b9; +const K: usize = 0x9e37_79b9; #[cfg(target_pointer_width = "64")] -const K: usize = 0x517cc1b727220a95; +const K: usize = 0x517c_c1b7_2722_0a95; impl Default for FxHasher { #[inline] - fn default() -> FxHasher { - FxHasher { hash: 0 } + fn default() -> Self { + Self { hash: 0 } } } @@ -48,14 +48,16 @@ impl Hasher for FxHasher { #[inline] fn write(&mut self, mut bytes: &[u8]) { #[cfg(target_pointer_width = "32")] - let read_usize = |bytes| NativeEndian::read_u32(bytes); + #[allow(clippy::cast_possible_truncation)] + let read_usize = |bytes| NativeEndian::read_u32(bytes) as usize; #[cfg(target_pointer_width = "64")] - let read_usize = |bytes| NativeEndian::read_u64(bytes); + #[allow(clippy::cast_possible_truncation)] + let read_usize = |bytes| NativeEndian::read_u64(bytes) as usize; - let mut hash = FxHasher { hash: self.hash }; + let mut hash = Self { hash: self.hash }; assert!(size_of::() <= 8); while bytes.len() >= size_of::() { - hash.add_to_hash(read_usize(bytes) as usize); + hash.add_to_hash(read_usize(bytes)); bytes = &bytes[size_of::()..]; } if (size_of::() > 4) && (bytes.len() >= 4) { @@ -66,7 +68,7 @@ impl Hasher for FxHasher { hash.add_to_hash(NativeEndian::read_u16(bytes) as usize); bytes = &bytes[2..]; } - if (size_of::() > 1) && bytes.len() >= 1 { + if (size_of::() > 1) && !bytes.is_empty() { hash.add_to_hash(bytes[0] as usize); } self.hash = hash.hash; @@ -89,6 +91,7 @@ impl Hasher for FxHasher { #[cfg(target_pointer_width = "32")] #[inline] + #[allow(clippy::cast_possible_truncation)] fn write_u64(&mut self, i: u64) { self.add_to_hash(i as usize); self.add_to_hash((i >> 32) as usize); @@ -96,6 +99,7 @@ impl Hasher for FxHasher { #[cfg(target_pointer_width = "64")] #[inline] + #[allow(clippy::cast_possible_truncation)] fn write_u64(&mut self, i: u64) { self.add_to_hash(i as usize); } diff --git a/src/lib.rs b/src/lib.rs index 3bd972e7ac..1c0723bf11 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,7 +2,7 @@ //! map, adapted to make it a drop-in replacement for Rust's standard `HashMap` //! and `HashSet` types. //! -//! The original C++ version of SwissTable can be found [here], and this +//! The original C++ version of [SwissTable] can be found [here], and this //! [CppCon talk] gives an overview of how the algorithm works. //! //! [SwissTable]: https://abseil.io/blog/20180927-swisstables @@ -23,9 +23,11 @@ ) )] #![warn(missing_docs)] +#![allow(clippy::module_name_repetitions)] #[cfg(test)] #[macro_use] +#[allow(unused_imports)] extern crate std; #[cfg(test)] extern crate rand; @@ -84,7 +86,7 @@ pub mod hash_set { pub use map::HashMap; pub use set::HashSet; -/// Augments `AllocErr` with a CapacityOverflow variant. +/// Augments `AllocErr` with a `CapacityOverflow` variant. #[derive(Clone, PartialEq, Eq, Debug)] pub enum CollectionAllocErr { /// Error due to the computed capacity exceeding the collection's maximum diff --git a/src/map.rs b/src/map.rs index 9265888f1e..665c1e2856 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1,5 +1,3 @@ -use self::Entry::*; - use core::borrow::Borrow; use core::fmt::{self, Debug}; use core::hash::{BuildHasher, Hash, Hasher}; @@ -212,8 +210,8 @@ impl HashMap { /// let mut map: HashMap<&str, i32> = HashMap::new(); /// ``` #[inline] - pub fn new() -> HashMap { - Default::default() + pub fn new() -> Self { + Self::default() } /// Creates an empty `HashMap` with the specified capacity. @@ -228,8 +226,8 @@ impl HashMap { /// let mut map: HashMap<&str, i32> = HashMap::with_capacity(10); /// ``` #[inline] - pub fn with_capacity(capacity: usize) -> HashMap { - HashMap::with_capacity_and_hasher(capacity, Default::default()) + pub fn with_capacity(capacity: usize) -> Self { + Self::with_capacity_and_hasher(capacity, DefaultHashBuilder::default()) } } @@ -259,8 +257,8 @@ where /// map.insert(1, 2); /// ``` #[inline] - pub fn with_hasher(hash_builder: S) -> HashMap { - HashMap { + pub fn with_hasher(hash_builder: S) -> Self { + Self { hash_builder, table: RawTable::new(), } @@ -288,8 +286,8 @@ where /// map.insert(1, 2); /// ``` #[inline] - pub fn with_capacity_and_hasher(capacity: usize, hash_builder: S) -> HashMap { - HashMap { + pub fn with_capacity_and_hasher(capacity: usize, hash_builder: S) -> Self { + Self { hash_builder, table: RawTable::with_capacity(capacity), } @@ -1023,7 +1021,7 @@ where V: PartialEq, S: BuildHasher, { - fn eq(&self, other: &HashMap) -> bool { + fn eq(&self, other: &Self) -> bool { if self.len() != other.len() { return false; } @@ -1059,8 +1057,8 @@ where { /// Creates an empty `HashMap`, with the `Default` value for the hasher. #[inline] - fn default() -> HashMap { - HashMap::with_hasher(Default::default()) + fn default() -> Self { + Self::with_hasher(Default::default()) } } @@ -1245,7 +1243,7 @@ pub struct ValuesMut<'a, K: 'a, V: 'a> { inner: IterMut<'a, K, V>, } -/// A builder for computing where in a HashMap a key-value pair would be stored. +/// A builder for computing where in a [`HashMap`] a key-value pair would be stored. /// /// See the [`HashMap::raw_entry_mut`] docs for usage examples. /// @@ -1288,7 +1286,7 @@ pub struct RawVacantEntryMut<'a, K: 'a, V: 'a, S: 'a> { hash_builder: &'a S, } -/// A builder for computing where in a HashMap a key-value pair would be stored. +/// A builder for computing where in a [`HashMap`] a key-value pair would be stored. /// /// See the [`HashMap::raw_entry`] docs for usage examples. /// @@ -1304,6 +1302,7 @@ where { /// Create a `RawEntryMut` from the given key. #[inline] + #[allow(clippy::wrong_self_convention)] pub fn from_key(self, k: &Q) -> RawEntryMut<'a, K, V, S> where K: Borrow, @@ -1316,6 +1315,7 @@ where /// Create a `RawEntryMut` from the given key and its hash. #[inline] + #[allow(clippy::wrong_self_convention)] pub fn from_key_hashed_nocheck(self, hash: u64, k: &Q) -> RawEntryMut<'a, K, V, S> where K: Borrow, @@ -1343,6 +1343,7 @@ where /// Create a `RawEntryMut` from the given hash. #[inline] + #[allow(clippy::wrong_self_convention)] pub fn from_hash(self, hash: u64, is_match: F) -> RawEntryMut<'a, K, V, S> where for<'b> F: FnMut(&'b K) -> bool, @@ -1357,6 +1358,7 @@ where { /// Access an entry by key. #[inline] + #[allow(clippy::wrong_self_convention)] pub fn from_key(self, k: &Q) -> Option<(&'a K, &'a V)> where K: Borrow, @@ -1369,6 +1371,7 @@ where /// Access an entry by a key and its hash. #[inline] + #[allow(clippy::wrong_self_convention)] pub fn from_key_hashed_nocheck(self, hash: u64, k: &Q) -> Option<(&'a K, &'a V)> where K: Borrow, @@ -1393,6 +1396,7 @@ where /// Access an entry by hash. #[inline] + #[allow(clippy::wrong_self_convention)] pub fn from_hash(self, hash: u64, is_match: F) -> Option<(&'a K, &'a V)> where F: FnMut(&K) -> bool, @@ -1614,6 +1618,7 @@ impl<'a, K, V, S> RawVacantEntryMut<'a, K, V, S> { /// Sets the value of the entry with the VacantEntry's key, /// and returns a mutable reference to it. #[inline] + #[allow(clippy::shadow_unrelated)] pub fn insert_hashed_nocheck(self, hash: u64, key: K, value: V) -> (&'a mut K, &'a mut V) where K: Hash, @@ -1683,8 +1688,8 @@ pub enum Entry<'a, K: 'a, V: 'a, S: 'a> { impl<'a, K: 'a + Debug + Eq + Hash, V: 'a + Debug, S: 'a> Debug for Entry<'a, K, V, S> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { - Vacant(ref v) => f.debug_tuple("Entry").field(v).finish(), - Occupied(ref o) => f.debug_tuple("Entry").field(o).finish(), + Entry::Vacant(ref v) => f.debug_tuple("Entry").field(v).finish(), + Entry::Occupied(ref o) => f.debug_tuple("Entry").field(o).finish(), } } } @@ -2007,8 +2012,8 @@ impl<'a, K, V, S> Entry<'a, K, V, S> { S: BuildHasher, { match self { - Occupied(entry) => entry.into_mut(), - Vacant(entry) => entry.insert(default), + Entry::Occupied(entry) => entry.into_mut(), + Entry::Vacant(entry) => entry.insert(default), } } @@ -2034,8 +2039,8 @@ impl<'a, K, V, S> Entry<'a, K, V, S> { S: BuildHasher, { match self { - Occupied(entry) => entry.into_mut(), - Vacant(entry) => entry.insert(default()), + Entry::Occupied(entry) => entry.into_mut(), + Entry::Vacant(entry) => entry.insert(default()), } } @@ -2052,8 +2057,8 @@ impl<'a, K, V, S> Entry<'a, K, V, S> { #[inline] pub fn key(&self) -> &K { match *self { - Occupied(ref entry) => entry.key(), - Vacant(ref entry) => entry.key(), + Entry::Occupied(ref entry) => entry.key(), + Entry::Vacant(ref entry) => entry.key(), } } @@ -2083,11 +2088,11 @@ impl<'a, K, V, S> Entry<'a, K, V, S> { F: FnOnce(&mut V), { match self { - Occupied(mut entry) => { + Entry::Occupied(mut entry) => { f(entry.get_mut()); - Occupied(entry) + Entry::Occupied(entry) } - Vacant(entry) => Vacant(entry), + Entry::Vacant(entry) => Entry::Vacant(entry), } } } @@ -2115,8 +2120,8 @@ impl<'a, K, V: Default, S> Entry<'a, K, V, S> { S: BuildHasher, { match self { - Occupied(entry) => entry.into_mut(), - Vacant(entry) => entry.insert(Default::default()), + Entry::Occupied(entry) => entry.into_mut(), + Entry::Vacant(entry) => entry.insert(Default::default()), } } } @@ -2423,9 +2428,9 @@ where S: BuildHasher + Default, { #[inline] - fn from_iter>(iter: T) -> HashMap { + fn from_iter>(iter: T) -> Self { let iter = iter.into_iter(); - let mut map = HashMap::with_capacity_and_hasher(iter.size_hint().0, Default::default()); + let mut map = Self::with_capacity_and_hasher(iter.size_hint().0, S::default()); for (k, v) in iter { map.insert(k, v); } @@ -3393,7 +3398,13 @@ mod test_map { if let Err(AllocErr) = empty_bytes.try_reserve(MAX_USIZE / 8) { } else { - panic!("usize::MAX / 8 should trigger an OOM!") + // This may succeed if there is enough free memory. Attempt to + // allocate a second hashmap to ensure the allocation will fail. + let mut empty_bytes2: HashMap = HashMap::new(); + if let Err(AllocErr) = empty_bytes2.try_reserve(MAX_USIZE / 8) { + } else { + panic!("usize::MAX / 8 should trigger an OOM!"); + } } } diff --git a/src/raw/bitmask.rs b/src/raw/bitmask.rs index bc6f427451..d8238feffa 100644 --- a/src/raw/bitmask.rs +++ b/src/raw/bitmask.rs @@ -16,18 +16,19 @@ use core::intrinsics; #[derive(Copy, Clone)] pub struct BitMask(pub BitMaskWord); +#[allow(clippy::use_self)] impl BitMask { /// Returns a new `BitMask` with all bits inverted. #[inline] #[must_use] - pub fn invert(self) -> BitMask { + pub fn invert(self) -> Self { BitMask(self.0 ^ BITMASK_MASK) } /// Returns a new `BitMask` with the lowest bit removed. #[inline] #[must_use] - pub fn remove_lowest_bit(self) -> BitMask { + pub fn remove_lowest_bit(self) -> Self { BitMask(self.0 & (self.0 - 1)) } /// Returns whether the `BitMask` has at least one set bit. diff --git a/src/raw/generic.rs b/src/raw/generic.rs index 0e144fb332..797654149b 100644 --- a/src/raw/generic.rs +++ b/src/raw/generic.rs @@ -21,12 +21,16 @@ type GroupWord = u32; pub type BitMaskWord = GroupWord; pub const BITMASK_STRIDE: usize = 8; // We only care about the highest bit of each byte for the mask. -pub const BITMASK_MASK: BitMaskWord = 0x8080_8080_8080_8080u64 as GroupWord; +#[allow( + clippy::cast_possible_truncation, + clippy::unnecessary_cast, +)] +pub const BITMASK_MASK: BitMaskWord = 0x8080_8080_8080_8080_u64 as GroupWord; /// Helper function to replicate a byte across a `GroupWord`. #[inline] fn repeat(byte: u8) -> GroupWord { - let repeat = byte as GroupWord; + let repeat = GroupWord::from(byte); let repeat = repeat | repeat.wrapping_shl(8); let repeat = repeat | repeat.wrapping_shl(16); // This last line is a no-op with a 32-bit GroupWord @@ -44,6 +48,7 @@ pub struct Group(GroupWord); // little-endian just before creating a BitMask. The can potentially // enable the compiler to eliminate unnecessary byte swaps if we are // only checking whether a BitMask is empty. +#[allow(clippy::use_self)] impl Group { /// Number of bytes in the group. pub const WIDTH: usize = mem::size_of::(); @@ -66,23 +71,28 @@ impl Group { /// Loads a group of bytes starting at the given address. #[inline] - pub unsafe fn load(ptr: *const u8) -> Group { + #[allow(clippy::cast_ptr_alignment)] // unaligned load + pub unsafe fn load(ptr: *const u8) -> Self { Group(ptr::read_unaligned(ptr as *const _)) } /// Loads a group of bytes starting at the given address, which must be /// aligned to `mem::align_of::()`. #[inline] - pub unsafe fn load_aligned(ptr: *const u8) -> Group { - debug_assert_eq!(ptr as usize & (mem::align_of::() - 1), 0); + #[allow(clippy::cast_ptr_alignment)] + pub unsafe fn load_aligned(ptr: *const u8) -> Self { + // FIXME: use align_offset once it stabilizes + debug_assert_eq!(ptr as usize & (mem::align_of::() - 1), 0); Group(ptr::read(ptr as *const _)) } /// Stores the group of bytes to the given address, which must be /// aligned to `mem::align_of::()`. #[inline] - pub unsafe fn store_aligned(&self, ptr: *mut u8) { - debug_assert_eq!(ptr as usize & (mem::align_of::() - 1), 0); + #[allow(clippy::cast_ptr_alignment)] + pub unsafe fn store_aligned(self, ptr: *mut u8) { + // FIXME: use align_offset once it stabilizes + debug_assert_eq!(ptr as usize & (mem::align_of::() - 1), 0); ptr::write(ptr as *mut _, self.0); } @@ -97,7 +107,7 @@ impl Group { /// - This only happens if there is at least 1 true match. /// - The chance of this happening is very low (< 1% chance per byte). #[inline] - pub fn match_byte(&self, byte: u8) -> BitMask { + pub fn match_byte(self, byte: u8) -> BitMask { // This algorithm is derived from // http://graphics.stanford.edu/~seander/bithacks.html##ValueInWord let cmp = self.0 ^ repeat(byte); @@ -107,7 +117,7 @@ impl Group { /// Returns a `BitMask` indicating all bytes in the group which are /// `EMPTY`. #[inline] - pub fn match_empty(&self) -> BitMask { + pub fn match_empty(self) -> BitMask { // If the high bit is set, then the byte must be either: // 1111_1111 (EMPTY) or 1000_0000 (DELETED). // So we can just check if the top two bits are 1 by ANDing them. @@ -117,7 +127,7 @@ impl Group { /// Returns a `BitMask` indicating all bytes in the group which are /// `EMPTY` or `DELETED`. #[inline] - pub fn match_empty_or_deleted(&self) -> BitMask { + pub fn match_empty_or_deleted(self) -> BitMask { // A byte is EMPTY or DELETED iff the high bit is set BitMask((self.0 & repeat(0x80)).to_le()) } @@ -127,7 +137,7 @@ impl Group { /// - `DELETED => EMPTY` /// - `FULL => DELETED` #[inline] - pub fn convert_special_to_empty_and_full_to_deleted(&self) -> Group { + pub fn convert_special_to_empty_and_full_to_deleted(self) -> Self { // Map high_bit = 1 (EMPTY or DELETED) to 1111_1111 // and high_bit = 0 (FULL) to 1000_0000 // diff --git a/src/raw/mod.rs b/src/raw/mod.rs index 31a59e779a..b36dfa20d3 100644 --- a/src/raw/mod.rs +++ b/src/raw/mod.rs @@ -64,6 +64,7 @@ use self::bitmask::BitMask; use self::imp::Group; /// Whether memory allocation errors should return an error or abort. +#[derive(Copy, Clone)] enum Fallibility { Fallible, Infallible, @@ -72,8 +73,8 @@ enum Fallibility { impl Fallibility { /// Error to return on capacity overflow. #[inline] - fn capacity_overflow(&self) -> CollectionAllocErr { - match *self { + fn capacity_overflow(self) -> CollectionAllocErr { + match self { Fallibility::Fallible => CollectionAllocErr::CapacityOverflow, Fallibility::Infallible => panic!("Hash table capacity overflow"), } @@ -81,8 +82,8 @@ impl Fallibility { /// Error to return on allocation error. #[inline] - fn alloc_err(&self, layout: Layout) -> CollectionAllocErr { - match *self { + fn alloc_err(self, layout: Layout) -> CollectionAllocErr { + match self { Fallibility::Fallible => CollectionAllocErr::AllocErr, Fallibility::Infallible => handle_alloc_error(layout), } @@ -90,10 +91,10 @@ impl Fallibility { } /// Control byte value for an empty bucket. -const EMPTY: u8 = 0b11111111; +const EMPTY: u8 = 0b1111_1111; /// Control byte value for a deleted bucket. -const DELETED: u8 = 0b10000000; +const DELETED: u8 = 0b1000_0000; /// Checks whether a control byte represents a full bucket (top bit is clear). #[inline] @@ -116,19 +117,25 @@ fn special_is_empty(ctrl: u8) -> bool { /// Primary hash function, used to select the initial bucket to probe from. #[inline] +#[allow(clippy::cast_possible_truncation)] fn h1(hash: u64) -> usize { - hash as usize + #[cfg(target_pointer_width = "32")] + { + debug_assert!(hash <= u64::from(u32::max_value())); + } + hash as usize // truncation } /// Secondary hash function, saved in the low 7 bits of the control byte. #[inline] +#[allow(clippy::cast_possible_truncation)] fn h2(hash: u64) -> u8 { // Grab the top 7 bits of the hash. While the hash is normally a full 64-bit // value, some hash functions (such as FxHash) produce a usize result // instead, which means that the top 32 bits are 0 on 32-bit platforms. let hash_len = usize::min(mem::size_of::(), mem::size_of::()); let top7 = hash >> (hash_len * 8 - 7); - (top7 & 0x7f) as u8 + (top7 & 0x7f) as u8 // truncation } /// Probe sequence based on triangular numbers, which is guaranteed (since our @@ -242,14 +249,14 @@ unsafe impl Send for Bucket {} impl Clone for Bucket { #[inline] fn clone(&self) -> Self { - Bucket { ptr: self.ptr } + Self { ptr: self.ptr } } } impl Bucket { #[inline] unsafe fn from_ptr(ptr: *const T) -> Self { - Bucket { + Self { ptr: NonNull::new_unchecked(ptr as *mut T), } } @@ -291,8 +298,8 @@ impl RawTable { /// leave the data pointer dangling since that bucket is never written to /// due to our load factor forcing us to always have at least 1 free bucket. #[inline] - pub fn new() -> RawTable { - RawTable { + pub fn new() -> Self { + Self { data: NonNull::dangling(), ctrl: NonNull::from(&Group::static_empty()[0]), bucket_mask: 0, @@ -308,12 +315,12 @@ impl RawTable { unsafe fn new_uninitialized( buckets: usize, fallability: Fallibility, - ) -> Result, CollectionAllocErr> { + ) -> Result { let (layout, data_offset) = calculate_layout::(buckets).ok_or_else(|| fallability.capacity_overflow())?; let ctrl = NonNull::new(alloc(layout)).ok_or_else(|| fallability.alloc_err(layout))?; let data = NonNull::new_unchecked(ctrl.as_ptr().add(data_offset) as *mut T); - Ok(RawTable { + Ok(Self { data, ctrl, bucket_mask: buckets - 1, @@ -327,14 +334,14 @@ impl RawTable { fn try_with_capacity( capacity: usize, fallability: Fallibility, - ) -> Result, CollectionAllocErr> { + ) -> Result { if capacity == 0 { - Ok(RawTable::new()) + Ok(Self::new()) } else { unsafe { let buckets = capacity_to_buckets(capacity).ok_or_else(|| fallability.capacity_overflow())?; - let result = RawTable::new_uninitialized(buckets, fallability)?; + let result = Self::new_uninitialized(buckets, fallability)?; result .ctrl(0) .write_bytes(EMPTY, result.buckets() + Group::WIDTH); @@ -355,8 +362,8 @@ impl RawTable { /// Allocates a new hash table with at least enough capacity for inserting /// the given number of elements without reallocating. - pub fn with_capacity(capacity: usize) -> RawTable { - RawTable::try_with_capacity(capacity, Fallibility::Infallible) + pub fn with_capacity(capacity: usize) -> Self { + Self::try_with_capacity(capacity, Fallibility::Infallible) .unwrap_or_else(|_| unsafe { hint::unreachable_unchecked() }) } @@ -689,7 +696,7 @@ impl RawTable { debug_assert!(self.items <= capacity); // Allocate and initialize the new table. - let mut new_table = RawTable::try_with_capacity(capacity, fallability)?; + let mut new_table = Self::try_with_capacity(capacity, fallability)?; new_table.growth_left -= self.items; new_table.items = self.items; @@ -820,12 +827,12 @@ impl RawTable { /// should be dropped using a `RawIter` before freeing the allocation. #[inline] pub fn into_alloc(self) -> Option<(NonNull, Layout)> { - let alloc = if self.bucket_mask != 0 { + let alloc = if self.bucket_mask == 0 { + None + } else { let (layout, _) = calculate_layout::(self.buckets()) .unwrap_or_else(|| unsafe { hint::unreachable_unchecked() }); Some((self.ctrl.cast(), layout)) - } else { - None }; mem::forget(self); alloc @@ -949,18 +956,14 @@ impl RawIterRange { /// /// The start offset must be aligned to the group width. #[inline] - unsafe fn new( - input_ctrl: *const u8, - input_data: *const T, - range: Range, - ) -> RawIterRange { + unsafe fn new(input_ctrl: *const u8, input_data: *const T, range: Range) -> Self { debug_assert_eq!(range.start % Group::WIDTH, 0); let ctrl = input_ctrl.add(range.start); let data = input_data.add(range.start); let end = input_ctrl.add(range.end); debug_assert_eq!(offset_from(end, ctrl), range.end - range.start); let current_group = Group::load_aligned(ctrl).match_empty_or_deleted().invert(); - RawIterRange { + Self { data, ctrl, current_group, @@ -973,7 +976,7 @@ impl RawIterRange { /// This will fail if the total range is smaller than the group width. #[inline] #[cfg(feature = "rayon")] - pub fn split(mut self) -> (RawIterRange, Option>) { + pub fn split(mut self) -> (Self, Option>) { unsafe { let len = offset_from(self.end, self.ctrl); debug_assert!(len.is_power_of_two()); @@ -982,7 +985,7 @@ impl RawIterRange { } else { debug_assert_eq!(len % (Group::WIDTH * 2), 0); let mid = len / 2; - let tail = RawIterRange::new(self.ctrl, self.data, mid..len); + let tail = Self::new(self.ctrl, self.data, mid..len); debug_assert_eq!(self.data.add(mid), tail.data); debug_assert_eq!(self.end, tail.end); self.end = self.ctrl.add(mid); @@ -999,7 +1002,7 @@ unsafe impl Sync for RawIterRange where T: Sync {} impl Clone for RawIterRange { #[inline] fn clone(&self) -> Self { - RawIterRange { + Self { data: self.data, ctrl: self.ctrl, current_group: self.current_group, @@ -1051,7 +1054,7 @@ pub struct RawIter { impl Clone for RawIter { #[inline] fn clone(&self) -> Self { - RawIter { + Self { iter: self.iter.clone(), items: self.items, } @@ -1063,18 +1066,15 @@ impl Iterator for RawIter { #[inline] fn next(&mut self) -> Option> { - match self.iter.next() { - Some(b) => { - self.items -= 1; - Some(b) - } - None => { - // We don't check against items == 0 here to allow the - // compiler to optimize away the item count entirely if the - // iterator length is never queried. - debug_assert_eq!(self.items, 0); - None - } + if let Some(b) = self.iter.next() { + self.items -= 1; + Some(b) + } else { + // We don't check against items == 0 here to allow the + // compiler to optimize away the item count entirely if the + // iterator length is never queried. + debug_assert_eq!(self.items, 0); + None } } diff --git a/src/raw/sse2.rs b/src/raw/sse2.rs index ecb238f24d..8e1abdfe71 100644 --- a/src/raw/sse2.rs +++ b/src/raw/sse2.rs @@ -18,6 +18,8 @@ pub const BITMASK_MASK: BitMaskWord = 0xffff; #[derive(Copy, Clone)] pub struct Group(x86::__m128i); +// FIXME: https://github.com/rust-lang/rust-clippy/issues/3859 +#[allow(clippy::use_self)] impl Group { /// Number of bytes in the group. pub const WIDTH: usize = mem::size_of::(); @@ -40,30 +42,43 @@ impl Group { /// Loads a group of bytes starting at the given address. #[inline] - pub unsafe fn load(ptr: *const u8) -> Group { + #[allow(clippy::cast_ptr_alignment)] // unaligned load + pub unsafe fn load(ptr: *const u8) -> Self { Group(x86::_mm_loadu_si128(ptr as *const _)) } /// Loads a group of bytes starting at the given address, which must be /// aligned to `mem::align_of::()`. #[inline] - pub unsafe fn load_aligned(ptr: *const u8) -> Group { - debug_assert_eq!(ptr as usize & (mem::align_of::() - 1), 0); + #[allow(clippy::cast_ptr_alignment)] + pub unsafe fn load_aligned(ptr: *const u8) -> Self { + // FIXME: use align_offset once it stabilizes + debug_assert_eq!(ptr as usize & (mem::align_of::() - 1), 0); Group(x86::_mm_load_si128(ptr as *const _)) } /// Stores the group of bytes to the given address, which must be /// aligned to `mem::align_of::()`. #[inline] - pub unsafe fn store_aligned(&self, ptr: *mut u8) { - debug_assert_eq!(ptr as usize & (mem::align_of::() - 1), 0); + #[allow(clippy::cast_ptr_alignment)] + pub unsafe fn store_aligned(self, ptr: *mut u8) { + // FIXME: use align_offset once it stabilizes + debug_assert_eq!(ptr as usize & (mem::align_of::() - 1), 0); x86::_mm_store_si128(ptr as *mut _, self.0); } /// Returns a `BitMask` indicating all bytes in the group which have /// the given value. #[inline] - pub fn match_byte(&self, byte: u8) -> BitMask { + pub fn match_byte(self, byte: u8) -> BitMask { + #[allow( + clippy::cast_possible_wrap, // byte: u8 as i8 + // byte: i32 as u16 + // note: _mm_movemask_epi8 returns a 16-bit mask in a i32, the + // upper 16-bits of the i32 are zeroed: + clippy::cast_sign_loss, + clippy::cast_possible_truncation + )] unsafe { let cmp = x86::_mm_cmpeq_epi8(self.0, x86::_mm_set1_epi8(byte as i8)); BitMask(x86::_mm_movemask_epi8(cmp) as u16) @@ -73,16 +88,25 @@ impl Group { /// Returns a `BitMask` indicating all bytes in the group which are /// `EMPTY`. #[inline] - pub fn match_empty(&self) -> BitMask { + pub fn match_empty(self) -> BitMask { self.match_byte(EMPTY) } /// Returns a `BitMask` indicating all bytes in the group which are /// `EMPTY` or `DELETED`. #[inline] - pub fn match_empty_or_deleted(&self) -> BitMask { - // A byte is EMPTY or DELETED iff the high bit is set - unsafe { BitMask(x86::_mm_movemask_epi8(self.0) as u16) } + pub fn match_empty_or_deleted(self) -> BitMask { + #[allow( + // byte: i32 as u16 + // note: _mm_movemask_epi8 returns a 16-bit mask in a i32, the + // upper 16-bits of the i32 are zeroed: + clippy::cast_sign_loss, + clippy::cast_possible_truncation + )] + unsafe { + // A byte is EMPTY or DELETED iff the high bit is set + BitMask(x86::_mm_movemask_epi8(self.0) as u16) + } } /// Performs the following transformation on all bytes in the group: @@ -90,7 +114,7 @@ impl Group { /// - `DELETED => EMPTY` /// - `FULL => DELETED` #[inline] - pub fn convert_special_to_empty_and_full_to_deleted(&self) -> Group { + pub fn convert_special_to_empty_and_full_to_deleted(self) -> Self { // Map high_bit = 1 (EMPTY or DELETED) to 1111_1111 // and high_bit = 0 (FULL) to 1000_0000 // @@ -98,10 +122,13 @@ impl Group { // let special = 0 > byte = 1111_1111 (true) or 0000_0000 (false) // 1111_1111 | 1000_0000 = 1111_1111 // 0000_0000 | 1000_0000 = 1000_0000 + #[allow( + clippy::cast_possible_wrap, // byte: 0x80_u8 as i8 + )] unsafe { let zero = x86::_mm_setzero_si128(); let special = x86::_mm_cmpgt_epi8(zero, self.0); - Group(x86::_mm_or_si128(special, x86::_mm_set1_epi8(0x80u8 as i8))) + Group(x86::_mm_or_si128(special, x86::_mm_set1_epi8(0x80_u8 as i8))) } } } diff --git a/src/set.rs b/src/set.rs index 7c81fcb915..923ae42f2e 100644 --- a/src/set.rs +++ b/src/set.rs @@ -129,8 +129,8 @@ impl HashSet { /// let set: HashSet = HashSet::new(); /// ``` #[inline] - pub fn new() -> HashSet { - HashSet { + pub fn new() -> Self { + Self { map: HashMap::new(), } } @@ -148,8 +148,8 @@ impl HashSet { /// assert!(set.capacity() >= 10); /// ``` #[inline] - pub fn with_capacity(capacity: usize) -> HashSet { - HashSet { + pub fn with_capacity(capacity: usize) -> Self { + Self { map: HashMap::with_capacity(capacity), } } @@ -181,8 +181,8 @@ where /// set.insert(2); /// ``` #[inline] - pub fn with_hasher(hasher: S) -> HashSet { - HashSet { + pub fn with_hasher(hasher: S) -> Self { + Self { map: HashMap::with_hasher(hasher), } } @@ -209,8 +209,8 @@ where /// set.insert(1); /// ``` #[inline] - pub fn with_capacity_and_hasher(capacity: usize, hasher: S) -> HashSet { - HashSet { + pub fn with_capacity_and_hasher(capacity: usize, hasher: S) -> Self { + Self { map: HashMap::with_capacity_and_hasher(capacity, hasher), } } @@ -384,7 +384,7 @@ where /// assert_eq!(diff, [4].iter().collect()); /// ``` #[inline] - pub fn difference<'a>(&'a self, other: &'a HashSet) -> Difference<'a, T, S> { + pub fn difference<'a>(&'a self, other: &'a Self) -> Difference<'a, T, S> { Difference { iter: self.iter(), other, @@ -413,10 +413,7 @@ where /// assert_eq!(diff1, [1, 4].iter().collect()); /// ``` #[inline] - pub fn symmetric_difference<'a>( - &'a self, - other: &'a HashSet, - ) -> SymmetricDifference<'a, T, S> { + pub fn symmetric_difference<'a>(&'a self, other: &'a Self) -> SymmetricDifference<'a, T, S> { SymmetricDifference { iter: self.difference(other).chain(other.difference(self)), } @@ -441,7 +438,7 @@ where /// assert_eq!(intersection, [2, 3].iter().collect()); /// ``` #[inline] - pub fn intersection<'a>(&'a self, other: &'a HashSet) -> Intersection<'a, T, S> { + pub fn intersection<'a>(&'a self, other: &'a Self) -> Intersection<'a, T, S> { Intersection { iter: self.iter(), other, @@ -467,7 +464,7 @@ where /// assert_eq!(union, [1, 2, 3, 4].iter().collect()); /// ``` #[inline] - pub fn union<'a>(&'a self, other: &'a HashSet) -> Union<'a, T, S> { + pub fn union<'a>(&'a self, other: &'a Self) -> Union<'a, T, S> { Union { iter: self.iter().chain(other.difference(self)), } @@ -619,7 +616,7 @@ where /// b.insert(1); /// assert_eq!(a.is_disjoint(&b), false); /// ``` - pub fn is_disjoint(&self, other: &HashSet) -> bool { + pub fn is_disjoint(&self, other: &Self) -> bool { self.iter().all(|v| !other.contains(v)) } @@ -640,7 +637,7 @@ where /// set.insert(4); /// assert_eq!(set.is_subset(&sup), false); /// ``` - pub fn is_subset(&self, other: &HashSet) -> bool { + pub fn is_subset(&self, other: &Self) -> bool { self.iter().all(|v| other.contains(v)) } @@ -665,7 +662,7 @@ where /// assert_eq!(set.is_superset(&sub), true); /// ``` #[inline] - pub fn is_superset(&self, other: &HashSet) -> bool { + pub fn is_superset(&self, other: &Self) -> bool { other.is_subset(self) } @@ -801,7 +798,7 @@ where T: Eq + Hash, S: BuildHasher, { - fn eq(&self, other: &HashSet) -> bool { + fn eq(&self, other: &Self) -> bool { if self.len() != other.len() { return false; } @@ -833,8 +830,8 @@ where S: BuildHasher + Default, { #[inline] - fn from_iter>(iter: I) -> HashSet { - let mut set = HashSet::with_hasher(Default::default()); + fn from_iter>(iter: I) -> Self { + let mut set = Self::with_hasher(Default::default()); set.extend(iter); set } @@ -869,8 +866,8 @@ where { /// Creates an empty `HashSet` with the `Default` value for the hasher. #[inline] - fn default() -> HashSet { - HashSet { + fn default() -> Self { + Self { map: HashMap::default(), } }