Skip to content

Commit 6b542fc

Browse files
committed
First draft of a sound ShallowCopy
This is trying to address the unsoundness that arises from the current version of `ShallowCopy` (see #74). In the process, it also deals with the fact that casting between `Inner<.., T, ..>` and `Inner<.., ManuallyDrop<T>, ..>` is likely not sound (rust-lang/unsafe-code-guidelines#35 (comment)). It does not yet work.
1 parent 81d84ee commit 6b542fc

File tree

12 files changed

+793
-423
lines changed

12 files changed

+793
-423
lines changed

Cargo.toml

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
[workspace]
2-
members = ["evmap", "evmap-derive", "left-right"]
2+
members = ["evmap", "left-right"]
3+
exclude = ["evmap-derive"]

evmap/src/inner.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ pub(crate) use indexmap::IndexMap as MapImpl;
77
pub(crate) use std::collections::HashMap as MapImpl;
88

99
use crate::values::Values;
10+
use crate::ShallowCopy;
1011

1112
pub(crate) struct Inner<K, V, M, S>
1213
where
1314
K: Eq + Hash,
15+
V: ShallowCopy,
1416
S: BuildHasher,
1517
{
1618
pub(crate) data: MapImpl<K, Values<V, S>, S>,
@@ -22,7 +24,8 @@ impl<K, V, M, S> fmt::Debug for Inner<K, V, M, S>
2224
where
2325
K: Eq + Hash + fmt::Debug,
2426
S: BuildHasher,
25-
V: fmt::Debug,
27+
V: ShallowCopy,
28+
V::Target: fmt::Debug,
2629
M: fmt::Debug,
2730
{
2831
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -38,6 +41,7 @@ impl<K, V, M, S> Clone for Inner<K, V, M, S>
3841
where
3942
K: Eq + Hash + Clone,
4043
S: BuildHasher + Clone,
44+
V: ShallowCopy,
4145
M: Clone,
4246
{
4347
fn clone(&self) -> Self {
@@ -56,6 +60,7 @@ where
5660
impl<K, V, M, S> Inner<K, V, M, S>
5761
where
5862
K: Eq + Hash,
63+
V: ShallowCopy,
5964
S: BuildHasher,
6065
{
6166
pub fn with_hasher(m: M, hash_builder: S) -> Self {

evmap/src/lib.rs

+51-20
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ pub use crate::read::{MapReadRef, ReadGuardIter, ReadHandle, ReadHandleFactory};
216216

217217
pub mod shallow_copy;
218218
pub use crate::shallow_copy::ShallowCopy;
219+
use shallow_copy::MaybeShallowCopied;
219220

220221
// Expose `ReadGuard` since it has useful methods the user will likely care about.
221222
#[doc(inline)]
@@ -225,27 +226,27 @@ pub use left_right::ReadGuard;
225226
///
226227
/// The predicate function is called once for each distinct value, and `true` if this is the
227228
/// _first_ call to the predicate on the _second_ application of the operation.
228-
pub struct Predicate<V>(pub(crate) Box<dyn FnMut(&V, bool) -> bool + Send>);
229+
pub struct Predicate<V: ?Sized>(pub(crate) Box<dyn FnMut(&V, bool) -> bool + Send>);
229230

230-
impl<V> Predicate<V> {
231+
impl<V: ?Sized> Predicate<V> {
231232
/// Evaluate the predicate for the given element
232233
#[inline]
233234
pub fn eval(&mut self, value: &V, reset: bool) -> bool {
234235
(*self.0)(value, reset)
235236
}
236237
}
237238

238-
impl<V> PartialEq for Predicate<V> {
239+
impl<V: ?Sized> PartialEq for Predicate<V> {
239240
#[inline]
240241
fn eq(&self, other: &Self) -> bool {
241242
// only compare data, not vtable: https://stackoverflow.com/q/47489449/472927
242243
&*self.0 as *const _ as *const () == &*other.0 as *const _ as *const ()
243244
}
244245
}
245246

246-
impl<V> Eq for Predicate<V> {}
247+
impl<V: ?Sized> Eq for Predicate<V> {}
247248

248-
impl<V> fmt::Debug for Predicate<V> {
249+
impl<V: ?Sized> fmt::Debug for Predicate<V> {
249250
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
250251
f.debug_tuple("Predicate")
251252
.field(&format_args!("{:p}", &*self.0 as *const _))
@@ -255,12 +256,14 @@ impl<V> fmt::Debug for Predicate<V> {
255256

256257
/// A pending map operation.
257258
#[non_exhaustive]
258-
#[derive(PartialEq, Eq, Debug)]
259-
pub(crate) enum Operation<K, V, M> {
259+
pub(crate) enum Operation<K, V, M>
260+
where
261+
V: ShallowCopy,
262+
{
260263
/// Replace the set of entries for this key with this value.
261-
Replace(K, V),
264+
Replace(K, MaybeShallowCopied<V>),
262265
/// Add this value to the set of entries for this key.
263-
Add(K, V),
266+
Add(K, MaybeShallowCopied<V>),
264267
/// Remove this value from the set of entries for this key.
265268
RemoveValue(K, V),
266269
/// Remove the value set for this key.
@@ -277,24 +280,48 @@ pub(crate) enum Operation<K, V, M> {
277280
/// Note that this will iterate once over all the keys internally.
278281
Purge,
279282
/// Retains all values matching the given predicate.
280-
Retain(K, Predicate<V>),
281-
/// Shrinks [`Values`] to their minimum necessary size, freeing memory
283+
Retain(K, Predicate<V::Target>),
284+
/// Shrinks [`MaybeShallowCopied<V>alues`] to their minimum necessary size, freeing memory
282285
/// and potentially improving cache locality.
283286
///
284-
/// If no key is given, all `Values` will shrink to fit.
287+
/// If no key is given, all `MaybeShallowCopied<V>alues` will shrink to fit.
285288
Fit(Option<K>),
286-
/// Reserves capacity for some number of additional elements in [`Values`]
289+
/// Reserves capacity for some number of additional elements in [`MaybeShallowCopied<V>alues`]
287290
/// for the given key. If the given key does not exist, allocate an empty
288-
/// `Values` with the given capacity.
291+
/// `MaybeShallowCopied<V>alues` with the given capacity.
289292
///
290293
/// This can improve performance by pre-allocating space for large bags of values.
291294
Reserve(K, usize),
292295
/// Mark the map as ready to be consumed for readers.
293296
MarkReady,
294297
/// Set the value of the map meta.
295298
SetMeta(M),
296-
/// Copy over the contents of the read map wholesale as the write map is empty.
297-
JustCloneRHandle,
299+
}
300+
301+
impl<K, V, M> fmt::Debug for Operation<K, V, M>
302+
where
303+
K: fmt::Debug,
304+
V: ShallowCopy + fmt::Debug,
305+
V::Target: fmt::Debug,
306+
M: fmt::Debug,
307+
{
308+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
309+
match *self {
310+
Operation::Replace(ref a, ref b) => f.debug_tuple("Replace").field(a).field(b).finish(),
311+
Operation::Add(ref a, ref b) => f.debug_tuple("Add").field(a).field(b).finish(),
312+
Operation::RemoveValue(ref a, ref b) => {
313+
f.debug_tuple("RemoveValue").field(a).field(b).finish()
314+
}
315+
Operation::RemoveEntry(ref a) => f.debug_tuple("RemoveEntry").field(a).finish(),
316+
Operation::Clear(ref a) => f.debug_tuple("Clear").field(a).finish(),
317+
Operation::Purge => f.debug_tuple("Purge").finish(),
318+
Operation::Retain(ref a, ref b) => f.debug_tuple("Retain").field(a).field(b).finish(),
319+
Operation::Fit(ref a) => f.debug_tuple("Fit").field(a).finish(),
320+
Operation::Reserve(ref a, ref b) => f.debug_tuple("Reserve").field(a).field(b).finish(),
321+
Operation::MarkReady => f.debug_tuple("MarkReady").finish(),
322+
Operation::SetMeta(ref a) => f.debug_tuple("SetMeta").field(a).finish(),
323+
}
324+
}
298325
}
299326

300327
/// Options for how to initialize the map.
@@ -373,7 +400,8 @@ where
373400
where
374401
K: Eq + Hash + Clone,
375402
S: BuildHasher + Clone,
376-
V: Eq + Hash + ShallowCopy,
403+
V: ShallowCopy,
404+
V::Target: Eq + Hash,
377405
M: 'static + Clone,
378406
{
379407
let inner = if let Some(cap) = self.capacity {
@@ -399,7 +427,8 @@ pub fn new<K, V>() -> (
399427
)
400428
where
401429
K: Eq + Hash + Clone,
402-
V: Eq + Hash + ShallowCopy,
430+
V: ShallowCopy,
431+
V::Target: Eq + Hash,
403432
{
404433
Options::default().construct()
405434
}
@@ -416,7 +445,8 @@ pub fn with_meta<K, V, M>(
416445
)
417446
where
418447
K: Eq + Hash + Clone,
419-
V: Eq + Hash + ShallowCopy,
448+
V: ShallowCopy,
449+
V::Target: Eq + Hash,
420450
M: 'static + Clone,
421451
{
422452
Options::default().with_meta(meta).construct()
@@ -432,7 +462,8 @@ pub fn with_hasher<K, V, M, S>(
432462
) -> (WriteHandle<K, V, M, S>, ReadHandle<K, V, M, S>)
433463
where
434464
K: Eq + Hash + Clone,
435-
V: Eq + Hash + ShallowCopy,
465+
V: ShallowCopy,
466+
V::Target: Eq + Hash,
436467
M: 'static + Clone,
437468
S: BuildHasher + Clone,
438469
{

evmap/src/read.rs

+10-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::inner::Inner;
22
use crate::values::Values;
3+
use crate::ShallowCopy;
34
use left_right::ReadGuard;
45

56
use std::borrow::Borrow;
@@ -22,6 +23,7 @@ pub use factory::ReadHandleFactory;
2223
pub struct ReadHandle<K, V, M = (), S = RandomState>
2324
where
2425
K: Eq + Hash,
26+
V: ShallowCopy,
2527
S: BuildHasher,
2628
{
2729
pub(crate) handle: left_right::ReadHandle<Inner<K, V, M, S>>,
@@ -30,6 +32,7 @@ where
3032
impl<K, V, M, S> fmt::Debug for ReadHandle<K, V, M, S>
3133
where
3234
K: Eq + Hash + fmt::Debug,
35+
V: ShallowCopy,
3336
S: BuildHasher,
3437
M: fmt::Debug,
3538
{
@@ -43,6 +46,7 @@ where
4346
impl<K, V, M, S> Clone for ReadHandle<K, V, M, S>
4447
where
4548
K: Eq + Hash,
49+
V: ShallowCopy,
4650
S: BuildHasher,
4751
{
4852
fn clone(&self) -> Self {
@@ -55,6 +59,7 @@ where
5559
impl<K, V, M, S> ReadHandle<K, V, M, S>
5660
where
5761
K: Eq + Hash,
62+
V: ShallowCopy,
5863
S: BuildHasher,
5964
{
6065
pub(crate) fn new(handle: left_right::ReadHandle<Inner<K, V, M, S>>) -> Self {
@@ -65,7 +70,8 @@ where
6570
impl<K, V, M, S> ReadHandle<K, V, M, S>
6671
where
6772
K: Eq + Hash,
68-
V: Eq + Hash,
73+
V: ShallowCopy,
74+
V::Target: Eq + Hash,
6975
S: BuildHasher,
7076
M: Clone,
7177
{
@@ -150,7 +156,7 @@ where
150156
/// refreshed by the writer. If no refresh has happened, or the map has been destroyed, this
151157
/// function returns `None`.
152158
#[inline]
153-
pub fn get_one<'rh, Q: ?Sized>(&'rh self, key: &'_ Q) -> Option<ReadGuard<'rh, V>>
159+
pub fn get_one<'rh, Q: ?Sized>(&'rh self, key: &'_ Q) -> Option<ReadGuard<'rh, V::Target>>
154160
where
155161
K: Borrow<Q>,
156162
Q: Hash + Eq,
@@ -204,14 +210,12 @@ where
204210

205211
/// Returns true if the map contains the specified value for the specified key.
206212
///
207-
/// The key and value may be any borrowed form of the map's respective types, but `Hash` and
213+
/// The key may be any borrowed form of the map's key type, but `Hash` and
208214
/// `Eq` on the borrowed form *must* match.
209-
pub fn contains_value<Q: ?Sized, W: ?Sized>(&self, key: &Q, value: &W) -> bool
215+
pub fn contains_value<Q: ?Sized>(&self, key: &Q, value: &V::Target) -> bool
210216
where
211217
K: Borrow<Q>,
212-
V: Borrow<W>,
213218
Q: Hash + Eq,
214-
W: Hash + Eq,
215219
{
216220
self.get_raw(key.borrow())
217221
.map(|x| x.contains(value))

evmap/src/read/factory.rs

+5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use super::ReadHandle;
22
use crate::inner::Inner;
3+
use crate::ShallowCopy;
34
use std::collections::hash_map::RandomState;
45
use std::fmt;
56
use std::hash::{BuildHasher, Hash};
@@ -13,6 +14,7 @@ use std::hash::{BuildHasher, Hash};
1314
pub struct ReadHandleFactory<K, V, M, S = RandomState>
1415
where
1516
K: Eq + Hash,
17+
V: ShallowCopy,
1618
S: BuildHasher,
1719
{
1820
pub(super) factory: left_right::ReadHandleFactory<Inner<K, V, M, S>>,
@@ -21,6 +23,7 @@ where
2123
impl<K, V, M, S> fmt::Debug for ReadHandleFactory<K, V, M, S>
2224
where
2325
K: Eq + Hash,
26+
V: ShallowCopy,
2427
S: BuildHasher,
2528
{
2629
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -33,6 +36,7 @@ where
3336
impl<K, V, M, S> Clone for ReadHandleFactory<K, V, M, S>
3437
where
3538
K: Eq + Hash,
39+
V: ShallowCopy,
3640
S: BuildHasher,
3741
{
3842
fn clone(&self) -> Self {
@@ -45,6 +49,7 @@ where
4549
impl<K, V, M, S> ReadHandleFactory<K, V, M, S>
4650
where
4751
K: Eq + Hash,
52+
V: ShallowCopy,
4853
S: BuildHasher,
4954
{
5055
/// Produce a new [`ReadHandle`] to the same left-right data structure as this factory was

0 commit comments

Comments
 (0)