Implement IntoIterator for [&[mut]] Box<[T; N], A>

[WaffleLapkin]

View all comments

Revival of #124108

[bors]

☔ The latest upstream changes (presumably #136572) made this pull request unmergeable. Please resolve the merge conflicts.

[WaffleLapkin]

@scottmcm anything I can do to move this forward?

[Mark-Simulacrum]

I guess this also needs rebasing + FCP, maybe starting with the FCP makes sense since it's not obvious to me this is worth it (especially if it needs an edition change...)

View changes since this review

[WaffleLapkin]

I guess the current version of this PR proposes neither option and instead has a dedicated new iterator type (BoxedArrayIntoIter). I think that's entirely net-new surface area, right? It also means we're missing a chunk of impls (e.g., DoubleEndedIterator) that we'd get 'for free' with vec::IntoIter.

No, we aren't missing any impls, since I added all of them :)

Also, this PR is compatible with the unsizing idea, with that you would have:

struct AllocatedContinuousIntoIter<T: ?Sized, A: Allocator = Global> { ... } // name bikeshedable

type vec::IntoIter<T, A: Allocator> = AllocatedContinuousIntoIter<[T], A>;
type box::BoxedArrayIntoIter<T, const N: usize, A: Allocator> = AllocatedContinuousIntoIter<[T; N], A>;

Note that vec::IntoIter and box::BoxedArrayIntoIter still normalize to different types, just like in this PR. So we will be able to do this in a follow up.

I.e. one of two follow ups is possible:

1. Making BoxedArrayIntoIter use custom iterator impls, rather than wrapping vec::IntoIter as it does in this PR
2. Making vec::IntoIter more generic, so it can support both use cases (unsizing idea)

[Amanieu]

@rfcbot merge libs-api

[rust-rfcbot]

@Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @the8472

Concerns:

• crater (Implement IntoIterator for [&[mut]] Box<[T; N], A> #134021 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[Mark-Simulacrum]

r=me with FCP done

View changes since this review

[Mark-Simulacrum]

It looks like we're missing overrides for next_chunk, try_fold, and probably __iterator_get_unchecked? Those are 'just' perf optimizations though.

I think the best way to fix this long-term is the change to have a common iterator type rather than duplicating all the surface area.

[programmerjake]

can you add Default for BoxedArrayIntoIter<...> that gives an empty iterator just like vec::IntoIter and array::IntoIter?

View changes since the review

[WaffleLapkin]

I don't want to change what the FCP is about, so maybe this should be left as a follow up?

[BurntSushi]

Given the potential for breaking changes here, it seems prudent to do a crater run. I don't see that one has been done. Thoughts on doing so?

@rfcbot concern crater

[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.

[WaffleLapkin]

@BurntSushi doing a crater run seems reasonable to me. I'll start it once the try build completes.

@bors try

[rust-log-analyzer]

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

...............................F.................. (50/144)
.................................................. (100/144)
............................................       (144/144)

======== tests/rustdoc-gui/go-to-collapsed-elem.goml ========

[ERROR] line 40
    at `tests/rustdoc-gui/go-to-collapsed-elem.goml` line 21: Error: Node is detached from document: for command `click: "//*[@id='search']//a[@href='../test_docs/struct.Foo.html#method.must_use']"`
    at <file:///checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/doc/test_docs/struct.Foo.html?search=t_use>


<= doc-ui tests done: 143 succeeded, 1 failed, 0 filtered out

Error: ()

[rust-bors]

☀️ Try build successful (CI)
Build commit: c37f0ee (c37f0eebe7e1f4c42e88bffc7961c6a7aa9c4d00, parent: 057ed8ba87418a77d5758f2d5e13f84b89b197e9)

[WaffleLapkin]

@craterbot check

[craterbot]

👌 Experiment pr-134021 created and queued.
🤖 Automatically detected try build c37f0ee
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more