Skip to content

Add reserve ergonomic wrappers for split_at_mut and array_mut_ref #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

burdges
Copy link
Contributor

@burdges burdges commented Apr 24, 2017

These are wrappers for combinations of slice::split_at or slice::split_at_mut with array_ref and array_mut_ref respectively, that simplify handling a variety of references into a slice. In essence, they use a call to mem::replace to keep the borrow checker happy while doing roughly this :

{ (r,heap) = heap.split_at_mut(len); array_mut_ref![r,0,len] }

These functions are handy if you need to borrow slices from binary data, put them into structs of mutable references, and manipulate them. I donno if they really belong in this crate, but it seemed like plausible place. There are only doc tests right now.

I donno if the name reserve is really ideal either. We could rename these to prefix_* and postfix_* or something. I do not use the reserve_tail forms anywhere yet myself, so I kept the name reserve.

These are convenient when you want to build a struct that contains
a variety of references into a slice.
@droundy
Copy link
Owner

droundy commented Apr 24, 2017

On looking at this more closely, I have a different proposal that might serve the same need while being closer in API and naming to the rest of arrayref. What you are aiming for with reserve seems to be very close to array_refs, with the caveat that you want a slice as input. The main difference being that you accept a &mut reference to a slice so you can modify the input slice. Why not just create two new macros similar to array_mut_refs! that can work on slices. Or (to be even fancier) it might be possible to make array_mut_refs! accept a .. in place of a number to match a slice of arbitrary size. I think that

let mut b = slice definition here;
let a = reserve!(b, 5);

could be more cleanly written as something like

let b = slice definition here;
let a, b = array_mut_refs!(b, 5, ..);

Now I don't have time for the work to get this macro doing this, but I think it would be way nicer as an API, and would avoid the distinction between tail and head. It would also enable taking several arrays off the top without having several bounds checks, so there is a potential efficiency gain as well.

This syntax would move arrayref a bit closer to a stable rust version of advanced slice patterns, which really ought to make arrayref obsolete once it stabilizes (assuming they do it right!).

(former notes below)

I would lean against exporting your functions that reserve slices, as the point of arrayref is to provide array references, and since the array references can always be used where a slice would be used. That would simplify the API which would seem a plus. I'd like to see some cleanup before pulling this: clearer docs, and removing the public functions in favor of the macros (which I believe can do anything the functions can do, with no overhead required).

But I must say that I find the documentation somewhat confusing: on a first glance I can't see what these functions do. The description involving heap.split_at_mut is not so entirely clear to me, and I expect a more clear description could be given in plain words.

If we don't export the functions, but only the macros, we would have two macros which would be used like:

let b = reserve(&mut sl, 5);

would take 5 elements out of the beginning of the slice, and make b an array reference to those 5 slices, while shortening sl itself by 5 elements. Similarly

let b = reserve_tail(&mut sl, 5);

would do the same for the last 5 elements of the slice.

I don't really care for the "reserve" name. This looks like a "popping" operation.

@burdges
Copy link
Contributor Author

burdges commented Apr 24, 2017

I've no opinion on the name myself. It originated from ad hoc arena allocators and makes a mess of taking postfixes. Just prefix and postfix work. Also clip, shave, etc.

I write things like let (a,b,c) = array_mut_refs![r, 16, 32, 16]; more often than I use these, so I'm certainly not criticizing that formulation.

I also like your idea of .. inside the current style. And macro_rules! does treat .. as a token albeit with some caveats. I suppose that combined with field puns covers like half my uses of reserve, etc.

Yet, there are situations where this mem::replace trick for modifying the slice and returning only the extracted part pays off though, especially binary protocols where some fields should be unpacked directly into conversion functions, tuple structs, and wrapper types, like say curve25519_dalek::CompressedEdwardsY(*reserve_fixed!(msg,32)).decompress(). A macro could do that as easily as a function of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants