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

ArrayChunks::into_remainder does not return None when there is no remainder #116000

Open
hkBst opened this issue Sep 20, 2023 · 3 comments
Open
Labels
C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@hkBst
Copy link
Contributor

hkBst commented Sep 20, 2023

I tried this code:

#![feature(iter_array_chunks)]

fn main() {
    let line = "abc";
    let mut chunks = line.chars().array_chunks::<3>();
    let _result: Vec<_> = chunks.by_ref().collect();
    if let Some(rem) = chunks.into_remainder() {
        assert!(rem.count() != 0);
    }
}

I expected to see this happen: into_remainder returns None when there is no remainder

Instead, this happened: into_remainder returns Some of an iterator with zero elements

@hkBst hkBst added the C-bug Category: This is a bug. label Sep 20, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 20, 2023
@zirconium-n
Copy link
Contributor

zirconium-n commented Sep 20, 2023

Should we even return an Option here? Maybe we can just scrap that so returning an iterator with zero elements is the expected output?

Similiar API does not return an Option.

@saethlin saethlin added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 20, 2023
@rossmacarthur
Copy link
Contributor

rossmacarthur commented Feb 21, 2024

Should we even return an Option here? Maybe we can just scrap that so returning an iterator with zero elements is the expected output?

I think it is worth returning Option because it forces the user to consider that the remainder might not be known yet. Because we can't know the remainder until the iterator has been consumed.

In the slice case, the remainder can always be calculated because it has a length.

@hkBst
Copy link
Contributor Author

hkBst commented Feb 25, 2024

@rossmacarthur Perhaps I am misunderstanding you here, but it seems that you are saying that None is used as an error value for the case where the iterator was not exhausted when into_remainder was called. Perhaps naively, I assumed that into_remainder would return whatever remained of the original iterator after however many chunks were consumed (and I can see now how the "remaining elements of the original iterator that are not going to be returned by this iterator" is supposed to be interpreted). With this interpretation the remainder is always obvious and there is no need for an Option. For slices it is possible to determine the remainder-after-consuming-all-chunks even when all chunks have not yet been consumed and this has been exposed in the API, but perhaps that was a mistake? Or perhaps it was a mistake for into_remainder to mimic remainder in this case.

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. T-libs Relevant to the library team, which will review and decide on the PR/issue. 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

5 participants