Skip to content

Commit

Permalink
Add implementation of Clone for StoreBox
Browse files Browse the repository at this point in the history
* Changes:

- Add implementation of Clone for StoreBox.
- Add unresolved question to RFC with regard to whether this should
  require a Clone, Default, or other trait on Store.

* Motivation:

Demonstrate that cloning is possible, and raise the issue as to which
trait is best used for that.
  • Loading branch information
matthieu-m committed May 21, 2023
1 parent b4235a8 commit e80e318
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 14 deletions.
26 changes: 26 additions & 0 deletions etc/rfc.md
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,32 @@ the companion repository showcases how building upon this `StoreBox` gains the a
a topic for another RFC, however.


## (Medium) What trait should the store have, for `Clone`?

In the standard library, a `Box` or a `Vec` are clone-able if the allocator implements `Clone`. It is certainly possible
to copy this bound for `Store`, but the semantics feel _wrong_ to me.

If one clones a `Vec`, the expectation is that the new `Vec` is indistinguishable from the old `Vec` value-wise. On the
other hand, in cloning a `Store` or an `Allocator` the expectation is that the clone be _empty_, and _independent_ from
the original. Most notably, there is not guarantee that a handle or a pointer allocated by the original can be used
safely with the clone. This... doesn't sound like a clone to me.

`Default` feels like a more appropriate bound, in this sense. It is expected that a `Default` instance of a type be
empty and independent from other instances.

`Default` does have the issue that it may not mesh well with `dyn Store` or `dyn Allocator` for that matter, however.
While `Clone` can reasonably be implemented for `&dyn Store`, or `Rc<dyn Store>`, such is not the case for `Default`.

This leaves 4 possibilities:

- Use `Clone` despite the poor semantics match.
- Use `Default` despite it being at odds with `dyn Store` use.
- Add a new `SpawningStore` trait to create an independent instance, though mixing several non-empty traits in a `dyn`
context is not supported yet.
- Add a method to `Store` to create an independent instance, fixing the semantics of `Clone`. Possibly a faillible
one.


## (Minor) What should the capabilities of `Handle` be?

Since any capability specified in the associated type definition is "mandatory", I am of the opinion that it should be
Expand Down
67 changes: 53 additions & 14 deletions src/collection/store_box.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Proof-of-Concept implementation of a `Box` atop a `Store`.
use core::{
alloc::AllocError,
fmt,
marker::Unsize,
mem::{self, ManuallyDrop},
Expand All @@ -18,9 +19,16 @@ pub struct StoreBox<T: ?Sized, S: Store> {
handle: UniqueHandle<T, S::Handle>,
}

impl<T, S: Store + Default> StoreBox<T, S> {
/// Creates a new instance.
pub fn new(value: T) -> Result<Self, (T, S)> {
Self::new_in(value, S::default())
}
}

impl<T, S: Store> StoreBox<T, S> {
/// Creates a new instance.
pub fn new(value: T, store: S) -> Result<Self, (T, S)> {
pub fn new_in(value: T, store: S) -> Result<Self, (T, S)> {
let Ok(handle) = UniqueHandle::allocate(&store) else {
return Err((value, store))
};
Expand All @@ -40,6 +48,23 @@ impl<T, S: Store> StoreBox<T, S> {
}
}

impl<T: Clone, S: Store + Default> Clone for StoreBox<T, S> {
fn clone(&self) -> Self {
let value: &T = self;

Self::new(value.clone())
.map_err(|_| AllocError)
.expect("Clone would have sufficient store space")
}

fn clone_from(&mut self, source: &StoreBox<T, S>) {
let dest: &mut T = self;
let source: &T = &source;

dest.clone_from(source);
}
}

impl<T: ?Sized, S: Store> Drop for StoreBox<T, S> {
fn drop(&mut self) {
let value: &mut T = &mut *self;
Expand Down Expand Up @@ -136,19 +161,26 @@ mod test_inline {
#[test]
fn sized_store() {
let store = InlineSingleStore::<u8>::default();
let mut boxed = StoreBox::new(1u8, store).unwrap();
let mut boxed = StoreBox::new_in(1u8, store).unwrap();

assert_eq!(1u8, *boxed);

*boxed = 2;

assert_eq!(2u8, *boxed);

let mut clone = boxed.clone();

*clone = 3;

assert_eq!(2u8, *boxed);
assert_eq!(3u8, *clone);
}

#[test]
fn slice_store() {
let store = InlineSingleStore::<[u8; 4]>::default();
let boxed = StoreBox::new([1u8, 2, 3], store).unwrap();
let boxed = StoreBox::new_in([1u8, 2, 3], store).unwrap();
let mut boxed: StoreBox<[u8], _> = StoreBox::coerce(boxed);

assert_eq!([1u8, 2, 3], &*boxed);
Expand All @@ -162,7 +194,7 @@ mod test_inline {
#[test]
fn slice_coercion() {
let store = InlineSingleStore::<[u8; 4]>::default();
let boxed = StoreBox::new([1u8, 2, 3], store).unwrap();
let boxed = StoreBox::new_in([1u8, 2, 3], store).unwrap();
let mut boxed: StoreBox<[u8], _> = boxed;

assert_eq!([1u8, 2, 3], &*boxed);
Expand All @@ -175,7 +207,7 @@ mod test_inline {
#[test]
fn trait_store() {
let store = InlineSingleStore::<[u8; 4]>::default();
let boxed = StoreBox::new([1u8, 2, 3], store).unwrap();
let boxed = StoreBox::new_in([1u8, 2, 3], store).unwrap();
let boxed: StoreBox<dyn fmt::Debug, _> = StoreBox::coerce(boxed);

assert_eq!("StoreBox([1, 2, 3])", format!("{:?}", boxed));
Expand All @@ -185,7 +217,7 @@ mod test_inline {
#[test]
fn trait_coercion() {
let store = InlineSingleStore::<[u8; 4]>::default();
let boxed = StoreBox::new([1u8, 2, 3], store).unwrap();
let boxed = StoreBox::new_in([1u8, 2, 3], store).unwrap();
let boxed: StoreBox<dyn fmt::Debug, _> = boxed;

assert_eq!("StoreBox([1, 2, 3])", format!("{:?}", boxed));
Expand All @@ -202,28 +234,35 @@ mod test_allocator {

#[test]
fn sized_failure() {
StoreBox::new(1, NonAllocator::default()).unwrap_err();
StoreBox::new_in(1, NonAllocator::default()).unwrap_err();
}

#[test]
fn sized_allocated() {
let mut boxed = StoreBox::new(1, System::default()).unwrap();
let mut boxed = StoreBox::new_in(1, System::default()).unwrap();

assert_eq!(1u32, *boxed);

*boxed = 2;

assert_eq!(2u32, *boxed);

let mut clone = boxed.clone();

*clone = 3;

assert_eq!(2u32, *boxed);
assert_eq!(3u32, *clone);
}

#[test]
fn slice_failure() {
StoreBox::new([1u8, 2, 3], NonAllocator::default()).unwrap_err();
StoreBox::new_in([1u8, 2, 3], NonAllocator::default()).unwrap_err();
}

#[test]
fn slice_allocated() {
let boxed = StoreBox::new([1u8, 2, 3], System::default()).unwrap();
let boxed = StoreBox::new_in([1u8, 2, 3], System::default()).unwrap();
let mut boxed: StoreBox<[u8], _> = StoreBox::coerce(boxed);

assert_eq!([1u8, 2, 3], &*boxed);
Expand All @@ -236,7 +275,7 @@ mod test_allocator {
#[cfg(feature = "coercible-metadata")]
#[test]
fn slice_coercion() {
let boxed = StoreBox::new([1u8, 2, 3], System::default()).unwrap();
let boxed = StoreBox::new_in([1u8, 2, 3], System::default()).unwrap();
let mut boxed: StoreBox<[u8], _> = boxed;

assert_eq!([1u8, 2, 3], &*boxed);
Expand All @@ -248,12 +287,12 @@ mod test_allocator {

#[test]
fn trait_failure() {
StoreBox::new([1u8, 2, 3], NonAllocator::default()).unwrap_err();
StoreBox::new_in([1u8, 2, 3], NonAllocator::default()).unwrap_err();
}

#[test]
fn trait_allocated() {
let boxed = StoreBox::new([1u8, 2, 3], System::default()).unwrap();
let boxed = StoreBox::new_in([1u8, 2, 3], System::default()).unwrap();
let boxed: StoreBox<dyn fmt::Debug, _> = StoreBox::coerce(boxed);

assert_eq!("StoreBox([1, 2, 3])", format!("{:?}", boxed));
Expand All @@ -262,7 +301,7 @@ mod test_allocator {
#[cfg(feature = "coercible-metadata")]
#[test]
fn trait_coercion() {
let boxed = StoreBox::new([1u8, 2, 3], System::default()).unwrap();
let boxed = StoreBox::new_in([1u8, 2, 3], System::default()).unwrap();
let boxed: StoreBox<dyn fmt::Debug, _> = boxed;

assert_eq!("StoreBox([1, 2, 3])", format!("{:?}", boxed));
Expand Down

0 comments on commit e80e318

Please sign in to comment.