Skip to content

Commit 928d94e

Browse files
committed
New take: use MaybeUninit
Here is a new take on fixing the soundness bugs with `ShallowCopy`: get rid of `ShallowCopy` entirely and instead store the aliased `T` in a `MaybeUninit` that we keep two copies of and access unsafely.
1 parent e758133 commit 928d94e

File tree

10 files changed

+399
-709
lines changed

10 files changed

+399
-709
lines changed

evmap/src/inner.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,10 @@ 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;
1110

1211
pub(crate) struct Inner<K, V, M, S>
1312
where
1413
K: Eq + Hash,
15-
V: ShallowCopy,
1614
S: BuildHasher,
1715
{
1816
pub(crate) data: MapImpl<K, Values<V, S>, S>,
@@ -24,8 +22,7 @@ impl<K, V, M, S> fmt::Debug for Inner<K, V, M, S>
2422
where
2523
K: Eq + Hash + fmt::Debug,
2624
S: BuildHasher,
27-
V: ShallowCopy,
28-
V::Target: fmt::Debug,
25+
V: fmt::Debug,
2926
M: fmt::Debug,
3027
{
3128
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -41,7 +38,6 @@ impl<K, V, M, S> Clone for Inner<K, V, M, S>
4138
where
4239
K: Eq + Hash + Clone,
4340
S: BuildHasher + Clone,
44-
V: ShallowCopy,
4541
M: Clone,
4642
{
4743
fn clone(&self) -> Self {
@@ -60,7 +56,6 @@ where
6056
impl<K, V, M, S> Inner<K, V, M, S>
6157
where
6258
K: Eq + Hash,
63-
V: ShallowCopy,
6459
S: BuildHasher,
6560
{
6661
pub fn with_hasher(m: M, hash_builder: S) -> Self {

evmap/src/lib.rs

+18-24
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,8 @@ pub use crate::write::WriteHandle;
214214
mod read;
215215
pub use crate::read::{MapReadRef, ReadGuardIter, ReadHandle, ReadHandleFactory};
216216

217-
pub mod shallow_copy;
218-
pub use crate::shallow_copy::ShallowCopy;
219-
use shallow_copy::MaybeShallowCopied;
217+
mod shallow_copy;
218+
use shallow_copy::ForwardThroughAliased;
220219

221220
// Expose `ReadGuard` since it has useful methods the user will likely care about.
222221
#[doc(inline)]
@@ -256,14 +255,11 @@ impl<V: ?Sized> fmt::Debug for Predicate<V> {
256255

257256
/// A pending map operation.
258257
#[non_exhaustive]
259-
pub(crate) enum Operation<K, V, M>
260-
where
261-
V: ShallowCopy,
262-
{
258+
pub(crate) enum Operation<K, V, M> {
263259
/// Replace the set of entries for this key with this value.
264-
Replace(K, MaybeShallowCopied<V>),
260+
Replace(K, ForwardThroughAliased<V>),
265261
/// Add this value to the set of entries for this key.
266-
Add(K, MaybeShallowCopied<V>),
262+
Add(K, ForwardThroughAliased<V>),
267263
/// Remove this value from the set of entries for this key.
268264
RemoveValue(K, V),
269265
/// Remove the value set for this key.
@@ -280,29 +276,30 @@ where
280276
/// Note that this will iterate once over all the keys internally.
281277
Purge,
282278
/// Retains all values matching the given predicate.
283-
Retain(K, Predicate<V::Target>),
284-
/// Shrinks [`MaybeShallowCopied<V>alues`] to their minimum necessary size, freeing memory
279+
Retain(K, Predicate<V>),
280+
/// Shrinks [`Values`] to their minimum necessary size, freeing memory
285281
/// and potentially improving cache locality.
286282
///
287-
/// If no key is given, all `MaybeShallowCopied<V>alues` will shrink to fit.
283+
/// If no key is given, all `Values` will shrink to fit.
288284
Fit(Option<K>),
289-
/// Reserves capacity for some number of additional elements in [`MaybeShallowCopied<V>alues`]
285+
/// Reserves capacity for some number of additional elements in [`Values`]
290286
/// for the given key. If the given key does not exist, allocate an empty
291-
/// `MaybeShallowCopied<V>alues` with the given capacity.
287+
/// `Values` with the given capacity.
292288
///
293289
/// This can improve performance by pre-allocating space for large bags of values.
294290
Reserve(K, usize),
295291
/// Mark the map as ready to be consumed for readers.
296292
MarkReady,
297293
/// Set the value of the map meta.
298294
SetMeta(M),
295+
/// Copy over the contents of the read map wholesale as the write map is empty.
296+
JustCloneRHandle,
299297
}
300298

301299
impl<K, V, M> fmt::Debug for Operation<K, V, M>
302300
where
303301
K: fmt::Debug,
304-
V: ShallowCopy + fmt::Debug,
305-
V::Target: fmt::Debug,
302+
V: fmt::Debug,
306303
M: fmt::Debug,
307304
{
308305
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -320,6 +317,7 @@ where
320317
Operation::Reserve(ref a, ref b) => f.debug_tuple("Reserve").field(a).field(b).finish(),
321318
Operation::MarkReady => f.debug_tuple("MarkReady").finish(),
322319
Operation::SetMeta(ref a) => f.debug_tuple("SetMeta").field(a).finish(),
320+
Operation::JustCloneRHandle => f.debug_tuple("JustCloneRHandle").finish(),
323321
}
324322
}
325323
}
@@ -400,8 +398,7 @@ where
400398
where
401399
K: Eq + Hash + Clone,
402400
S: BuildHasher + Clone,
403-
V: ShallowCopy,
404-
V::Target: Eq + Hash,
401+
V: Eq + Hash,
405402
M: 'static + Clone,
406403
{
407404
let inner = if let Some(cap) = self.capacity {
@@ -427,8 +424,7 @@ pub fn new<K, V>() -> (
427424
)
428425
where
429426
K: Eq + Hash + Clone,
430-
V: ShallowCopy,
431-
V::Target: Eq + Hash,
427+
V: Eq + Hash,
432428
{
433429
Options::default().construct()
434430
}
@@ -445,8 +441,7 @@ pub fn with_meta<K, V, M>(
445441
)
446442
where
447443
K: Eq + Hash + Clone,
448-
V: ShallowCopy,
449-
V::Target: Eq + Hash,
444+
V: Eq + Hash,
450445
M: 'static + Clone,
451446
{
452447
Options::default().with_meta(meta).construct()
@@ -462,8 +457,7 @@ pub fn with_hasher<K, V, M, S>(
462457
) -> (WriteHandle<K, V, M, S>, ReadHandle<K, V, M, S>)
463458
where
464459
K: Eq + Hash + Clone,
465-
V: ShallowCopy,
466-
V::Target: Eq + Hash,
460+
V: Eq + Hash,
467461
M: 'static + Clone,
468462
S: BuildHasher + Clone,
469463
{

evmap/src/read.rs

+8-10
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::inner::Inner;
2+
use crate::shallow_copy::ForwardThroughAliased;
23
use crate::values::Values;
3-
use crate::ShallowCopy;
44
use left_right::ReadGuard;
55

66
use std::borrow::Borrow;
@@ -23,7 +23,6 @@ pub use factory::ReadHandleFactory;
2323
pub struct ReadHandle<K, V, M = (), S = RandomState>
2424
where
2525
K: Eq + Hash,
26-
V: ShallowCopy,
2726
S: BuildHasher,
2827
{
2928
pub(crate) handle: left_right::ReadHandle<Inner<K, V, M, S>>,
@@ -32,7 +31,6 @@ where
3231
impl<K, V, M, S> fmt::Debug for ReadHandle<K, V, M, S>
3332
where
3433
K: Eq + Hash + fmt::Debug,
35-
V: ShallowCopy,
3634
S: BuildHasher,
3735
M: fmt::Debug,
3836
{
@@ -46,7 +44,6 @@ where
4644
impl<K, V, M, S> Clone for ReadHandle<K, V, M, S>
4745
where
4846
K: Eq + Hash,
49-
V: ShallowCopy,
5047
S: BuildHasher,
5148
{
5249
fn clone(&self) -> Self {
@@ -59,7 +56,6 @@ where
5956
impl<K, V, M, S> ReadHandle<K, V, M, S>
6057
where
6158
K: Eq + Hash,
62-
V: ShallowCopy,
6359
S: BuildHasher,
6460
{
6561
pub(crate) fn new(handle: left_right::ReadHandle<Inner<K, V, M, S>>) -> Self {
@@ -70,8 +66,7 @@ where
7066
impl<K, V, M, S> ReadHandle<K, V, M, S>
7167
where
7268
K: Eq + Hash,
73-
V: ShallowCopy,
74-
V::Target: Eq + Hash,
69+
V: Eq + Hash,
7570
S: BuildHasher,
7671
M: Clone,
7772
{
@@ -156,7 +151,7 @@ where
156151
/// refreshed by the writer. If no refresh has happened, or the map has been destroyed, this
157152
/// function returns `None`.
158153
#[inline]
159-
pub fn get_one<'rh, Q: ?Sized>(&'rh self, key: &'_ Q) -> Option<ReadGuard<'rh, V::Target>>
154+
pub fn get_one<'rh, Q: ?Sized>(&'rh self, key: &'_ Q) -> Option<ReadGuard<'rh, V>>
160155
where
161156
K: Borrow<Q>,
162157
Q: Hash + Eq,
@@ -210,12 +205,15 @@ where
210205

211206
/// Returns true if the map contains the specified value for the specified key.
212207
///
213-
/// The key may be any borrowed form of the map's key type, but `Hash` and
208+
/// The key and value may be any borrowed form of the map's respective types, but `Hash` and
214209
/// `Eq` on the borrowed form *must* match.
215-
pub fn contains_value<Q: ?Sized>(&self, key: &Q, value: &V::Target) -> bool
210+
pub fn contains_value<Q: ?Sized, W: ?Sized>(&self, key: &Q, value: &W) -> bool
216211
where
217212
K: Borrow<Q>,
213+
ForwardThroughAliased<V>: Borrow<W>,
218214
Q: Hash + Eq,
215+
W: Hash + Eq,
216+
V: Hash + Eq,
219217
{
220218
self.get_raw(key.borrow())
221219
.map(|x| x.contains(value))

evmap/src/read/factory.rs

-5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use super::ReadHandle;
22
use crate::inner::Inner;
3-
use crate::ShallowCopy;
43
use std::collections::hash_map::RandomState;
54
use std::fmt;
65
use std::hash::{BuildHasher, Hash};
@@ -14,7 +13,6 @@ use std::hash::{BuildHasher, Hash};
1413
pub struct ReadHandleFactory<K, V, M, S = RandomState>
1514
where
1615
K: Eq + Hash,
17-
V: ShallowCopy,
1816
S: BuildHasher,
1917
{
2018
pub(super) factory: left_right::ReadHandleFactory<Inner<K, V, M, S>>,
@@ -23,7 +21,6 @@ where
2321
impl<K, V, M, S> fmt::Debug for ReadHandleFactory<K, V, M, S>
2422
where
2523
K: Eq + Hash,
26-
V: ShallowCopy,
2724
S: BuildHasher,
2825
{
2926
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -36,7 +33,6 @@ where
3633
impl<K, V, M, S> Clone for ReadHandleFactory<K, V, M, S>
3734
where
3835
K: Eq + Hash,
39-
V: ShallowCopy,
4036
S: BuildHasher,
4137
{
4238
fn clone(&self) -> Self {
@@ -49,7 +45,6 @@ where
4945
impl<K, V, M, S> ReadHandleFactory<K, V, M, S>
5046
where
5147
K: Eq + Hash,
52-
V: ShallowCopy,
5348
S: BuildHasher,
5449
{
5550
/// Produce a new [`ReadHandle`] to the same left-right data structure as this factory was

0 commit comments

Comments
 (0)