Skip to content

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

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

Implement IntoIterator for [&[mut]] Box<[T; N], A>#134021
WaffleLapkin wants to merge 8 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.

Specifically make it public + unstable under `std_internals` + `doc(hidden)`.
This is honestly Not Great, but I do not know a better solution :(
…>` and `[T; N]`

They'll be needed for `IntoIterator` impls.
@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.

Note: this removes warnings, as this breakage was deemed acceptable, see
<rust-lang#124108 (comment)>
i forgot that `#![doc(hidden)]` applies to the module, not everything in
it... again. (this caused linkchecker to fail, because of a debug impl)
@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

// The iterator indeed reports the correct length. The number of "alive"
// elements (that will still be yielded) is the length of the range `alive`.
// This range is decremented in length in either `next` or `next_back`. It is
// always decremented by 1 in those methods, but only if `Some(_)` is returned.
Copy link
Member

Choose a reason for hiding this comment

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

"It is always decremented[...], but only if" seems like odd phrasing. I think we can just delete this sentence?


let mut new = Self {
data: Box::new_in(
[const { MaybeUninit::uninit() }; N],
Copy link
Member

Choose a reason for hiding this comment

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

nit: should this use the uninit constructors to avoid the memcpy given large N?


#[doc(hidden)]
#[unstable(issue = "none", feature = "std_internals")]
unsafe impl<T, const N: usize, A: Allocator> TrustedRandomAccessNoCoerce
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be good for this to have a SAFETY comment.

Copy link
Member

Choose a reason for hiding this comment

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

Should this get the same treatment as array IntoIterator did? Maybe starting with edition next (~2026/2027)?

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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