Adds heap based CircularBufferAlloc variant that allows for runtime chosen capacity

[Icxolu]

This PR adds a variant of CircularBuffer that is dynamically allocated on the heap called HeapCircularBuffer. By allocating on the heap this comes without a const generic parameter and allows construction with a capacity determined at runtime instead of compile time. It is internally backed by a Box<[MaybeUninit<T>]>. This behaves very similar to the [MaybeUninit<T>; N] thats used by CircularBuffer, which means implementations can easily shared or adapted.

The heap variant is located in the heap module. The implementation is mostly shared by moving all common implementation s onto a generic Backend type in the backend module. CircularBuffer and HeapCircularBuffer are just wrappter type arount the Backend. The methods are simple forwarded to their respective implementation on Backend via the impl_buffer macro in the crate root.

A similar method is used for the tests.

The borrowing iterators (Iter, IterMut) can be shared as they are. The owning iterators (IntoIter, Drain) are also implemented around the generic Backend. This way their implementation is easily shared. To prevent leaking implementation details around the Backend type, these are not exposed directly, but also using wrapper types.

Most trait impls are also implemented on Backend and the impl on the wrapper types simple forward to it.,

All tests (including doc tests) passed on my machine (windows and wsl) and under miri (except for randomized, but its the same for master) ✅

I think all the changes are fully backwards compatible (and cargo-semver-checks does not flag anything)

Open Questions:

• Do we expose the modules? Or do we only want to expose reexports?
• Is CircularBufferAlloc expressive enough? Is there something better? HeapCircularBuffer
• Are any (specific) tests missing?
• Should Drain share impl details via macro, or is duplicating it fine? Shared via common Backend
• Names for the IntoIter and Drain wrappers.

Todos:

• Documentation: either on the respective modules if we chose to expose those, or in the crate root.

Possible Followups

• make this available under alloc instead of std only

closes #6

[andreacorbellini]

Thanks for submitting this. I like the way the documentation is built to avoid duplication.

So... I know I said that I would be ok with using a macro to implement the two structs, however after thinking more carefully I think there's a better way:

struct Backend<T, B>
    where B: Deref<Target = [MaybeUninit<T>]> + DerefMut
{
    size: usize,
    start: usize,
    items: B,
}

pub struct CircularBuffer<const N: usize, T> {
    backend: Backend<T, [MaybeUninit<T>; T]>,
}

pub struct HeapCircularBuffer<T> {
    backend: Backend<T, Box<[MaybeUninit<T>]>,
}

With this approach, I believe that the iterator implementations could be easily shared. There would still be needed for two different sets of pub structs to avoid changing the type parameters.

A macro could still be used to implement the frontend methods, in particular to avoid duplicating the documentation.

Yet another approach could be to follow what the standard Vec, array and slice do: most of the implementation is in slice, and Vec/array are simply Deref<Target = [T]> so they get all the slice methods for free. Although I'm not sure if this is feasible.

Do we expose the modules? Or do we only want to expose reexports?

I would keep CircularBuffer (and related structs) into the root and avoid exposing any module for it. I would put the dynamic variant (and related structs) into a submodule and expose the submodule.

This is the module structure I would use:

• src/lib.rs -- contains CircularBuffer
• src/backend.rs -- contains Backend and macros (private submodule)
• src/heap.rs -- contains HeapCircularBuffer (public submodule)

Is CircularBufferAlloc expressive enough? Is there something better?

Some suggestions:

• HeapCircularBuffer
• DynCircularBuffer (may cause confusion with the dyn keyword)
• DynamicCircularBuffer (maybe too long)

Should Drain share impl details via macro, or is duplicating it fine?

I would definitely want a shared implementation. Drain is quite complicated, and it's going to get even more complicated once certain optimizations are put in place, so duplicating it would be quite hard to maintain.

Note that other trait and struct implementations will also get more complicated (see the various "TODO: optimize" entries to get an idea of the ones that will evolve in the near future).

[andreacorbellini]

By the way, I noticed that the style of some code has changed, perhaps by a (partial) application of rustfmt/cargo fmt. I do plan to use rustfmt in the future, once some nightly features become generally available, for for now I prefer to avoid it for this crate. It'd be great if you could revert the style changes to make the diff smaller and easier to review.

[Icxolu]

Thanks for the feedback! I can definitely remove the style changes. They originate indeed from rustfmt application of my ide. Because you do not want to use rustfmt right now. I would suggest to include the following in rustfmt.toml

disable_all_formatting = true

That way the code will not be automatically formatted until you decide to enable it again by removing the file.

I like the idea of abstracting the into a common backend. I agree that it should allow us to reuse all the iterators if we can implement the necessary methods on Backend. Maybe we can even implement all/most of the methods there and then simply call them from the wrapper structs (likely via a macro).

But I think for this to work we need our own trait, because array actually does not deref into a slice (probably because it is not a pointer type). I believe that conversion works via Unsize and CoerceUnsized which are both unstable.

Maybe something like:

pub(crate) trait AsSlice {
    type Item;
    fn as_slice(&self)-> &[Self::Item];
    fn as_slice_mut(&mut self) -> &mut [Self::Item];
}

I'll play with it a bit and see what i can come up with.

[Icxolu]

I updated with the styling changes removed and generic backend based implementation. I do like this version a lot more than my initial implementation. It feel a lot cleaner and not so much of the implementation is hidden in macros.

Also the sharing of the iterators works quite nicely, but they currently leak the backend storage type (and the private trait) in their public signature. I think we want to keep this an implementation detail, so we probably need some wrapper types to hide this.

[andreacorbellini]

Dropping a quick note to say that I haven't forgotten about this, but it might take some time before I have the capacity to look at it. This is a very busy month for me, sorry!

[Icxolu]

No worries. Take the time you need. And thanks for the update.

[andreacorbellini]

Another update: I started some work back in January to improve this branch even further and reduce a lot of code duplication. If I remember correctly, my changes are almost ready, there are just a few compile errors to fix. I'll try to find some time to complete those changes, push them on top of this branch (giving you credits for your work of course), and then merge them into master. It might take some more time but I'll do my best to get it done in a timely manner.