Skip to content

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

Open
WaffleLapkin wants to merge 5 commits intorust-lang:mainfrom
WaffleLapkin:box-arr-into-iter2
Open

Implement IntoIterator for [&[mut]] Box<[T; N], A>#134021
WaffleLapkin wants to merge 5 commits intorust-lang:mainfrom
WaffleLapkin:box-arr-into-iter2

Conversation

@WaffleLapkin
Copy link
Member

Revival of #124108

I copied the <[T; N] as IntoIterator>::Iter's impl, but this does not seem satisfying:

  1. I had to make IndexRange public
  2. this duplicates a lot of code
  3. it's unclear if the copied unsafe and spec code is worth it

r? @scottmcm
maybe you have better implementation ideas.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 8, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@tgross35 tgross35 added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Dec 13, 2024
@WaffleLapkin WaffleLapkin marked this pull request as ready for review December 16, 2024 19:24
@tgross35 tgross35 added the needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. label Dec 19, 2024
@bors
Copy link
Collaborator

bors commented Feb 6, 2025

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

@Dylan-DPC Dylan-DPC added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 6, 2025
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 14, 2025
@WaffleLapkin
Copy link
Member Author

@scottmcm anything I can do to move this forward?

@rust-bors

This comment was marked as resolved.

@scottmcm

This comment was marked as resolved.

@rustbot rustbot assigned Mark-Simulacrum and unassigned scottmcm Feb 8, 2026
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 14, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 10, 2026

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.

@rust-log-analyzer

This comment has been minimized.

@WaffleLapkin WaffleLapkin force-pushed the box-arr-into-iter2 branch 2 times, most recently from 33ce55c to a26c96f Compare March 10, 2026 15:10
@WaffleLapkin WaffleLapkin added relnotes Marks issues that should be documented in the release notes of the next release. and removed needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. labels Mar 10, 2026
@WaffleLapkin
Copy link
Member Author

@Mark-Simulacrum I don't think this requires edition handling or FCP, as per the FCP on the previous attempt at doing this:

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 10, 2026
@WaffleLapkin
Copy link
Member Author

I've also removed the complicated unsafe impl for now, making the BoxedArrayIntoIter a wrapper around vec::IntoIter. I'll add back the more complicated impl in a follow-up.

@rust-log-analyzer

This comment has been minimized.

Note: this removes warnings, as this breakage was deemed acceptable, see
<rust-lang#124108 (comment)>
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this requires edition handling or FCP, as per the FCP on the previous attempt at doing this:

I think we do need FCP or at least libs-api meeting nomination to re-confirm that the >1 year old FCP from 2024 is still valid. Maybe a Crater run. This is a breaking change and there was some post-initial-FCP discussion on whether we should do the unsizing impl (#124108 (comment)). Making two breaking changes here feels unfortunate so we should decide upfront which we want, even if we think the change is otherwise reasonable.

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.

View changes since this review

@Mark-Simulacrum Mark-Simulacrum added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

I-libs-api-nominated Nominated for discussion during a libs-api team meeting. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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

Successfully merging this pull request may close these issues.

8 participants