Implement DoubleEndedIterator::next_chunk_back

[asder8215]

View all comments

This PR builds off on @vinDelphini's PR on introducing DoubleEndedIterator::next_chunk_back (which will be used in a follow up PR to optimize ArrayChunks::try_rfold once merged). This was in the works since late Jan and I thought to pick it up and get this over the finish line as vinDelphini has been away from the PR for a couple of months. I've made sure to pull from vinDelphini's branch to keep him authored on the commits he made.

Other things introduced from this PR is specialization like done for Iterator::next_chunk, constifying some stuff, making a custom DoubleEndedIterator::next_chunk_back for IntoIter (since it also has a custom impl of Iterator::next_chunk), and a bunch of tests mirroring that of Iterator::next_chunk.

There's no tracking issue for this yet, but the ACP for this introduction is here. I'll create the tracking issue for this soon.

---

Actually, can we have DoubleEndedIterator::next_chunk_back underneath Iterator::next_chunk tracking issue?

[rustbot]

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: libs
• libs expanded to 7 candidates
• Random selection from Mark-Simulacrum, tgross35

[asder8215]

r? libs

[aapoalas]

Some minor nits to pick and one case of "I don't think that's what we want to indicate to the compiler".

I'm also a little worried about adding const stuff as this has recently been a topic of discussion - this is in line with the fn next_chunk and doesn't seem like a problem in that sense, but I figured it's best to mention it as well.

View changes since this review

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[asder8215]

I'm also a little worried about adding const stuff as #155816 - this is in line with the fn next_chunk and doesn't seem like a problem in that sense, but I figured it's best to mention it as well.

Yeah, I'm aware that the Rust libs team are cautious with constifying functions/methods. I figured in this case it was fine for consistency with Iterator::next_chunk, which seemed to have been constified from this PR.

[asder8215]

Did they change how tidy work on const impl trait blocks recently? My tidy --bless automatically puts the const right before the trait name rather than how it recommends me to put it here before the impl.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[asder8215]

Just rebased main onto this branch, so the tidy script works appropriately in where to put the const keyword now.

[asder8215]

Just in case, I'm going to mark this as libs-api nominated on whether or not constifying certain functions here for DoubleEndedIterator::next_chunk_back is okay.

@rustbot label +I-libs-api-nominated

[aapoalas]

LGTM :) I'll let the nomination run its course.

View changes since this review

[nia-e]

Per today's libs-api meeting, we'd rather keep this non-const for now unless there's a concrete usecase stronger than just consistency. Thanks ^^

[asder8215]

@aapoalas @nia-e actually, these functions need const implementations. I just noticed that the DoubleEndedIterator trait has been marked const from this PR 4 months ago.

When @vinDelphini started working on this, that DoubleEndedIterator wasn't marked as a const trait, which I can see why his implementation didn't have const markings.

I think there's no other changes I can make to this PR.

Edit: Actually nevermind. We can use the #[rustc_non_const_trait_method] to create a non-const trait method within a const trait. However, I also feel like making DoubleEndedIterator::next_chunk_back non-const defeats the purpose with the DoubleEndedIterator trait being marked const and the implementation for this method is already possible to be made const.