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

Lifetime constraints on e.g. ByteSlice::splitn_str are too restrictive #45

Closed
glandium opened this issue Mar 11, 2020 · 5 comments
Closed
Labels
bug Something isn't working
Milestone

Comments

@glandium
Copy link

They prevent things like:

use bstr::ByteSlice;

pub trait SliceExt<C> {
    fn split2(&self, c: C) -> Option<(&Self, &Self)>;
}

impl SliceExt<&[u8]> for [u8] {
    fn split2(&self, b: &[u8]) -> Option<(&[u8], &[u8])> {
        let mut iter = self.splitn_str(2, b);
        match (iter.next(), iter.next()) {
            (Some(a), Some(b)) => Some((a, b)),
            _ => None,
        }
    }
}

impl SliceExt<String> for [u8] {
    fn split2(&self, b: String) -> Option<(&[u8], &[u8])> {
        let mut iter = self.splitn_str(2, &b);
        match (iter.next(), iter.next()) {
            (Some(a), Some(b)) => Some((a, b)),
            _ => None,
        }
    }
}

The splitter doesn't need to have the same lifetime as the the slice that is being split.

@BurntSushi BurntSushi added the bug Something isn't working label Mar 11, 2020
@BurntSushi
Copy link
Owner

Fixing this is a breaking change, so I'm putting this in the 1.0 milestone.

@BurntSushi BurntSushi added this to the 1.0 release milestone May 10, 2020
@glandium
Copy link
Author

Out of educational interest, could you detail why that is so? Intuitively, that doesn't seem like so to me.

@BurntSushi
Copy link
Owner

Seems to me like it requires the iterator type to be parameterized over two lifetimes instead of the single one it has today. Adding a new non-default type parameter is a breaking change.

@glandium
Copy link
Author

Oh, right. Makes sense. Maybe it would be worth returning an impl Iterator for 1.0?

@BurntSushi
Copy link
Owner

No. Because then how do you impl things like DoubleEndedIterator? Or FusedIterator? You'd have to get everything right up front. Returning impl Trait in public APIs isn't a good idea unless you need it for unnamed types.

BurntSushi added a commit that referenced this issue Jul 5, 2022
In a few places I must have got lazy when defining the iterator types
and forced haystacks and needles/splitters to always have the same
lifetime. This works in most cases, but #45 shows a case where it breaks
down. To fix it, we just make sure we represent all of our lifetimes in
our types.

Note that #45 was reported against our split types, but we also have
this issue with our find types too. Indeed, our split types are built on
top of our find types, so we just fix everything.

This is a breaking change since we are adding a new lifetime parameter
to several public API types.

Fixes #45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants