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

std::slice::ChunkBy should implement Clone #137969

Open
nwoods-cimpress opened this issue Mar 3, 2025 · 7 comments
Open

std::slice::ChunkBy should implement Clone #137969

nwoods-cimpress opened this issue Mar 3, 2025 · 7 comments
Labels
C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@nwoods-cimpress
Copy link

I tried this code:

pub fn main() {
    let slice = &[1, 1, 1, 3, 3, 2, 2, 2];
    let mut iter = slice.chunk_by(|a, b| a == b);
    assert_eq!(iter.next(), Some(&[1, 1, 1][..]));
    assert_eq!(iter.next(), Some(&[3, 3][..]));

    let mut iter_clone = iter.clone();
    assert_eq!(iter_clone.next(), Some(&[2, 2, 2][..]));
    assert_eq!(iter_clone.next(), None);

    assert_eq!(iter.next(), Some(&[2, 2, 2][..]));
    assert_eq!(iter.next(), None);
}

I expected to see this happen: Code compiles and runs

Instead, this happened: Compilation error by virtue of std::slice::ChunkBy not implementing Clone

If there is a reason that Clone cannot be supported, I don't understand it.

@nwoods-cimpress nwoods-cimpress added the C-bug Category: This is a bug. label Mar 3, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 3, 2025
@gstjepan2
Copy link
Contributor

I assume you meant iter.cloned() instead of iter.clone() method, because .clone method doesn't exist on ChunkBy.

The reason .cloned() method exists on ChunkBy but can't be used in that example is because its implemented by default on Iterator trait, but it clause isn't satisfied.

Not sure if this is what you want, but you can try this:

pub fn main() {
    let slice = &[1, 1, 1, 3, 3, 2, 2, 2];
    let mut iter = slice
        .chunk_by(|a, b| a == b)
        .map(Vec::from)
        .collect::<Vec<_>>()
        .into_iter();

    assert_eq!(iter.next(), Some(vec![1, 1, 1]));
    assert_eq!(iter.next(), Some(vec![3, 3]));

    let mut iter_clone = iter.clone();
    assert_eq!(iter_clone.next(), Some(vec![2, 2, 2]));
    assert_eq!(iter_clone.next(), None);

    assert_eq!(iter.next(), Some(vec![2, 2, 2]));
    assert_eq!(iter.next(), None);
}

@compiler-errors
Copy link
Member

I assume you meant iter.cloned() instead of iter.clone() method, because .clone method doesn't exist on ChunkBy.

I think that's exactly what they're asking for, which is why ChunkBy doesn't implement the Clone trait.

@theemathas
Copy link
Contributor

I believe they actually meant clone. Some iterators in std implement Clone, for example std::vec::IntoIter and std::iter::Filter. The expected behavior of cloning iterators is that it produces an independent iterator that produces the same series of elements. I believe that the fact that ChunkBy doesn't implement Clone is an oversight.

@nwoods-cimpress
Copy link
Author

Correct; I want clone(), not cloned(). I don't want to duplicate the underlying data; just the state of the iterator.

Here is a permalink showing an equivalent to what I want, but using Itertools::coalesce(). This does indeed work, but it is much more verbose and less convenient than chunk_by().

@compiler-errors
Copy link
Member

@nwoods-cimpress: I think you can just add the clone impl in a PR and it will get FCP'd (team consensus voting) by T-libs-api, but if you want to be extra diligent you could 1. find the PR that added the ChunkBy iterator to find out if this was an oversight or intentional, and 2. use blame/history to find another PR that added a Clone impl to an iterator "after the fact" and compare what process they went through to add it.

@nwoods-cimpress
Copy link
Author

PR #138016 submitted. First PR to core Rust; hopefully I got the protocol correct 🤞

@lolbinarycat lolbinarycat added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 4, 2025
@thaliaarchi
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants