Skip to content

Commit

Permalink
Auto merge of #546 - Amanieu:remove-raw, r=Amanieu
Browse files Browse the repository at this point in the history 
Remove the `raw` feature and make `RawTable` private

This will give more freedom for the internal implementation details of hashbrown to evolve without the need for regular releases with breaking changes.

All existing users of `RawTable` should migrate to the `HashTable` API which is entirely safe while providing the same flexibility as `RawTable`.

This also removes the following features which were only exposed under `RawTable`:
- `RawTable::iter_hash`
- `RawIter::reflect_insert` and `RawIter::reflect_remove`
- `RawTable::clone_from_with_hasher`
- `RawTable::insert_no_grow` and `RawTable::try_insert_no_grow`
- `RawTable::allocation_info`
- `RawTable::try_with_capacity(_in)`
- `HashMap::raw_table(_mut)` and `HashSet::raw_table(_mut)`

If anyone was previously relying on this functionaly, please raise a comment. It may be possible to re-introduce it as a safe API in `HashTable` and/or `HashMap`.
  • Loading branch information
@bors
bors committed Aug 23, 2024
2 parents f1ba3b4 + 26ef4a1 commit aa1411b
Show file tree
Hide file tree
Showing 10 changed files with 4 additions and 807 deletions.
5 changes: 1 addition & 4 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ rustc-dep-of-std = [
"raw-entry",
]

# Enables the experimental and unsafe RawTable API.
raw = []

# Enables the deprecated RawEntry API.
raw-entry = []

Expand All @@ -84,5 +81,5 @@ default-hasher = ["dep:ahash"]
inline-more = []

[package.metadata.docs.rs]
features = ["nightly", "rayon", "serde", "raw"]
features = ["nightly", "rayon", "serde", "raw-entry"]
rustdoc-args = ["--generate-link-to-definition"]
1 change: 0 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ This crate has the following Cargo features:
- `rkyv`: Enables rkyv serialization support.
- `rayon`: Enables rayon parallel iterator support.
- `equivalent`: Allows comparisons to be customized with the `Equivalent` trait.
- `raw`: Enables access to the experimental and unsafe `RawTable` API.
- `raw-entry`: Enables access to the deprecated `RawEntry` API.
- `inline-more`: Adds inline hints to most functions, improving run-time performance at the cost
of compilation time. (enabled by default)
Expand Down
2 changes: 1 addition & 1 deletion ci/run.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ if [ "${NO_STD}" = "1" ]; then
FEATURES="rustc-internal-api"
OP="build"
else
FEATURES="rustc-internal-api,serde,rayon,raw"
FEATURES="rustc-internal-api,serde,rayon"
OP="test"
fi

Expand Down
1 change: 0 additions & 1 deletion ci/tools.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ fi

if retry rustup component add clippy ; then
cargo clippy --all --tests --features serde,rayon -- -D clippy::all
cargo clippy --all --tests --features raw -- -D clippy::all
fi

if command -v shellcheck ; then
Expand Down
21 changes: 0 additions & 21 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,27 +61,6 @@ doc_comment::doctest!("../README.md");
#[macro_use]
mod macros;

#[cfg(feature = "raw")]
/// Experimental and unsafe `RawTable` API. This module is only available if the
/// `raw` feature is enabled.
pub mod raw {
// The RawTable API is still experimental and is not properly documented yet.
#[allow(missing_docs)]
#[path = "mod.rs"]
mod inner;
pub use inner::*;

#[cfg(feature = "rayon")]
/// [rayon]-based parallel iterator types for hash maps.
/// You will rarely need to interact with it directly unless you have need
/// to name one of the iterator types.
///
/// [rayon]: https://docs.rs/rayon/1.0/rayon
pub mod rayon {
pub use crate::external_trait_impls::rayon::raw::*;
}
}
#[cfg(not(feature = "raw"))]
mod raw;

mod external_trait_impls;
Expand Down
143 changes: 0 additions & 143 deletions src/map.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1931,81 +1931,6 @@ where
}
}

impl<K, V, S, A: Allocator> HashMap<K, V, S, A> {
/// Returns a reference to the [`RawTable`] used underneath [`HashMap`].
/// This function is only available if the `raw` feature of the crate is enabled.
///
/// See [`raw_table_mut`] for more.
///
/// [`raw_table_mut`]: Self::raw_table_mut
#[cfg(feature = "raw")]
#[cfg_attr(feature = "inline-more", inline)]
pub fn raw_table(&self) -> &RawTable<(K, V), A> {
&self.table
}

/// Returns a mutable reference to the [`RawTable`] used underneath [`HashMap`].
/// This function is only available if the `raw` feature of the crate is enabled.
///
/// # Note
///
/// Calling this function is safe, but using the raw hash table API may require
/// unsafe functions or blocks.
///
/// `RawTable` API gives the lowest level of control under the map that can be useful
/// for extending the HashMap's API, but may lead to *[undefined behavior]*.
///
/// [`HashMap`]: struct.HashMap.html
/// [`RawTable`]: crate::raw::RawTable
/// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
///
/// # Examples
///
/// ```
/// use core::hash::{BuildHasher, Hash};
/// use hashbrown::HashMap;
///
/// let mut map = HashMap::new();
/// map.extend([("a", 10), ("b", 20), ("c", 30)]);
/// assert_eq!(map.len(), 3);
///
/// // Let's imagine that we have a value and a hash of the key, but not the key itself.
/// // However, if you want to remove the value from the map by hash and value, and you
/// // know exactly that the value is unique, then you can create a function like this:
/// fn remove_by_hash<K, V, S, F>(
/// map: &mut HashMap<K, V, S>,
/// hash: u64,
/// is_match: F,
/// ) -> Option<(K, V)>
/// where
/// F: Fn(&(K, V)) -> bool,
/// {
/// let raw_table = map.raw_table_mut();
/// match raw_table.find(hash, is_match) {
/// Some(bucket) => Some(unsafe { raw_table.remove(bucket).0 }),
/// None => None,
/// }
/// }
///
/// fn compute_hash<K: Hash + ?Sized, S: BuildHasher>(hash_builder: &S, key: &K) -> u64 {
/// use core::hash::Hasher;
/// let mut state = hash_builder.build_hasher();
/// key.hash(&mut state);
/// state.finish()
/// }
///
/// let hash = compute_hash(map.hasher(), "a");
/// assert_eq!(remove_by_hash(&mut map, hash, |(_, v)| *v == 10), Some(("a", 10)));
/// assert_eq!(map.get(&"a"), None);
/// assert_eq!(map.len(), 2);
/// ```
#[cfg(feature = "raw")]
#[cfg_attr(feature = "inline-more", inline)]
pub fn raw_table_mut(&mut self) -> &mut RawTable<(K, V), A> {
&mut self.table
}
}

impl<K, V, S, A> PartialEq for HashMap<K, V, S, A>
where
K: Eq + Hash,
Expand Down Expand Up @@ -5958,74 +5883,6 @@ mod test_map {
}
}

#[test]
#[cfg(feature = "raw")]
fn test_into_iter_refresh() {
#[cfg(miri)]
const N: usize = 32;
#[cfg(not(miri))]
const N: usize = 128;

let mut rng = rand::thread_rng();
for n in 0..N {
let mut map = HashMap::new();
for i in 0..n {
assert!(map.insert(i, 2 * i).is_none());
}
let hash_builder = map.hasher().clone();

let mut it = unsafe { map.table.iter() };
assert_eq!(it.len(), n);

let mut i = 0;
let mut left = n;
let mut removed = Vec::new();
loop {
// occasionally remove some elements
if i < n && rng.gen_bool(0.1) {
let hash_value = super::make_hash(&hash_builder, &i);

unsafe {
let e = map.table.find(hash_value, |q| q.0.eq(&i));
if let Some(e) = e {
it.reflect_remove(&e);
let t = map.table.remove(e).0;
removed.push(t);
left -= 1;
} else {
assert!(removed.contains(&(i, 2 * i)), "{i} not in {removed:?}");
let e = map.table.insert(
hash_value,
(i, 2 * i),
super::make_hasher::<_, usize, _>(&hash_builder),
);
it.reflect_insert(&e);
if let Some(p) = removed.iter().position(|e| e == &(i, 2 * i)) {
removed.swap_remove(p);
}
left += 1;
}
}
}

let e = it.next();
if e.is_none() {
break;
}
assert!(i < n);
let t = unsafe { e.unwrap().as_ref() };
assert!(!removed.contains(t));
let (key, value) = t;
assert_eq!(*value, 2 * key);
i += 1;
}
assert!(i <= n);

// just for safety:
assert_eq!(map.table.len(), left);
}
}

#[test]
fn test_const_with_hasher() {
use core::hash::BuildHasher;
Expand Down
16 changes: 0 additions & 16 deletions src/raw/bitmask.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,22 +105,6 @@ impl IntoIterator for BitMask {
#[derive(Copy, Clone)]
pub(crate) struct BitMaskIter(pub(crate) BitMask);

impl BitMaskIter {
/// Flip the bit in the mask for the entry at the given index.
///
/// Returns the bit's previous state.
#[inline]
#[allow(clippy::cast_ptr_alignment)]
#[cfg(feature = "raw")]
pub(crate) unsafe fn flip(&mut self, index: usize) -> bool {
// NOTE: The + BITMASK_STRIDE - 1 is to set the high bit.
let mask = 1 << (index * BITMASK_STRIDE + BITMASK_STRIDE - 1);
self.0 .0 ^= mask;
// The bit was set if the bit is now 0.
self.0 .0 & mask == 0
}
}

impl Iterator for BitMaskIter {
type Item = usize;

Expand Down
Loading

0 comments on commit aa1411b

Please sign in to comment.