Skip to content
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

Make T: Copy for &[T] streams #44

Merged
merged 1 commit into from
Aug 27, 2015
Merged

Make T: Copy for &[T] streams #44

merged 1 commit into from
Aug 27, 2015

Conversation

Marwes
Copy link
Owner

@Marwes Marwes commented Aug 21, 2015

As was pointed out to me by @m4rw3r &[T] streams are usually on &[u8] or other types T which are copyable. This would allow the Item type of such streams to be T rather than &T but has the downside that you can use &[T] where T != Copy. A newtype could be added which wraps such &[T] slices allowing for &T items again.

Alternatives:

  • Introduce a newtype which can be used to wrap slices with T: Copy instead.
#[derive(Debug, PartialEq, Clone)]
struct CopySlice<'a, T: 'a>(&'a [T]);

impl <'a, T: Positioner + Copy + 'a> Stream for CopySlice<'a, T> {
    type Item = T;
    type Range = &'a [T];
    fn uncons(self) -> Result<(T, CopySlice<'a, T>), Error<T, CopySlice<'a, T>> {
        ...
    }
}

I don't have much of any opinion either way but it needs to be decided before 1.0. Wrapping the slice in a newtype is only necessary for the first call to parse so its only a minor ergonomic win for the case which does not need wrapping, it's more of deciding which way is the least surprising for the majority of users.

@Marwes Marwes self-assigned this Aug 21, 2015
@Marwes Marwes added this to the 1.0 milestone Aug 21, 2015
As &[T] is likely to be used mainly with &[u8] it would be a gain in usability to avoid &u8 references etc.
@Marwes
Copy link
Owner Author

Marwes commented Aug 27, 2015

Given how much a usability boost this gives for &[u8] which I would expect is the most common slice to parse I am going to merge this. For non copy-able slices just wrap the input slice with the added SliceStream type.

Marwes added a commit that referenced this pull request Aug 27, 2015
Make T: Copy for &[T] streams
@Marwes Marwes merged commit fbd22b1 into master Aug 27, 2015
@Marwes Marwes deleted the copy_slice branch August 28, 2015 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant