spec next chunk for trustedlen by bend-n · Pull Request #150230 · rust-lang/rust

bend-n · 2025-12-21T18:19:18Z

relevant to #98326

rustbot · 2025-12-21T18:19:23Z

rustbot has assigned @scottmcm.
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

scottmcm · 2025-12-21T20:43:58Z

library/core/src/array/mod.rs

+            return Err(unsafe { IntoIter::new_unchecked(array, 0..initialized) });
+        } else {
+            // SAFETY: must be at least N elements; safe to unwrap N elements.
+            return Ok(from_fn(|_| unsafe { self.next().unwrap_unchecked() }));


unsure: Since you're specializing anyway, maybe https://std.noratrieb.dev/x86_64-unknown-linux-gnu/core/iter/traits/unchecked_iterator/trait.UncheckedIterator.html#method.next_unchecked instead?

Suggested change

return Ok(from_fn(|_| unsafe { self.next().unwrap_unchecked() }));

return Ok(from_fn(|_| unsafe { self.next_unchecked() }));

oh yeah i was looking for that but i couldnt find it 😔
idk why it isnt in my docs

but theres only like six things that are uncheckediterator?
unless theres a blanket impl im missing

uncheckediterator is also not actually spec-on-able, i'd have to add the marker thing

scottmcm · 2025-12-21T20:46:19Z

r? the8472

the8472 · 2025-12-22T12:44:19Z

library/core/src/array/mod.rs

+) -> Result<[T; N], IntoIter<T, N>> {
+    iter.spec_next_chunk()
+}
+#[inline]


the attribute should go after the doc comment

the8472 · 2025-12-22T14:02:41Z

library/core/src/array/mod.rs

+                // SAFETY: Has to error out; trusted len means that
+                // SAFETY: there may only be less than N elements


Suggested change

// SAFETY: Has to error out; trusted len means that

// SAFETY: there may only be less than N elements

// SAFETY: Has to error out; trusted len means that

// there may only be less than N elements

it doesn't need to be repeated for every line

yeah i thought so, and was confused, so i put that in, but it turned out it was mad because the unsafe block was on the next line due to formatting, mb

the8472 · 2025-12-22T14:11:00Z

library/core/src/array/mod.rs

+            Err(unsafe { IntoIter::new_unchecked(array, 0..initialized) })
+        } else {
+            // SAFETY: must be at least N elements; safe to unwrap N elements.
+            Ok(from_fn(|_| unsafe { self.next().unwrap_unchecked() }))


The default impl uses an uninit array for either case and moves that into the result. If the ~~stars~~ struct layouts align maybe that results in fewer moves. Have you checked in godbolt whether this is fine?

let me see if using the same array makes any difference.

doesn't seem to make a difference for my test case

the8472 · 2025-12-22T14:12:12Z

library/core/src/array/mod.rs

 /// If `iter.next()` panics, all items already yielded by the iterator are
 /// dropped.
 ///
 /// Used for [`Iterator::next_chunk`].


The documentation should go in the pub(crate) fn since that's what gets called on other modules and someone might want to get docs for.

the8472 · 2025-12-22T14:12:43Z

library/core/src/array/mod.rs

+}
+impl<I: Iterator<Item = T>, T, const N: usize> SpecNextChunk<T, N> for I {
+    default fn spec_next_chunk(&mut self) -> Result<[T; N], IntoIter<T, N>> {
+        generic_iter_next_chunk(self)


we could just inline the code here, there doesn't seem to be any reason for the indirection.

well i reuse it

the8472 · 2025-12-22T14:15:15Z

library/core/src/array/mod.rs

+            let initialized =
+                // SAFETY: Has to error out; trusted len means that
+                // SAFETY: there may only be less than N elements
+                unsafe { iter_next_chunk_erased(&mut array, self).unwrap_err_unchecked() };


I suspect that going through that method doesn't give us the benefit of TrustedLen, if it doesn't see that next can never fail.

on the error case? the error case wasnt very different when i used the next().unwrap_unchecked(), i looked at the asm a little and it was identical versus reusing that method. im not completely sure why, however its the slow path anyway.

rustbot assigned scottmcm Dec 21, 2025

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 21, 2025

This comment has been minimized.

Sign in to view

bend-n force-pushed the trusted_len_spec_for_iter_next_chunk branch from eccda83 to 4c4a61b Compare December 21, 2025 18:31

This comment has been minimized.

Sign in to view

bend-n force-pushed the trusted_len_spec_for_iter_next_chunk branch from 4c4a61b to 4a0dbad Compare December 21, 2025 18:42

scottmcm reviewed Dec 21, 2025

View reviewed changes

rustbot assigned the8472 and unassigned scottmcm Dec 21, 2025

bend-n force-pushed the trusted_len_spec_for_iter_next_chunk branch from 4a0dbad to 18a71c8 Compare December 22, 2025 10:48

the8472 reviewed Dec 22, 2025

View reviewed changes

bend-n force-pushed the trusted_len_spec_for_iter_next_chunk branch 2 times, most recently from 18391e2 to 65349e7 Compare December 22, 2025 21:08

spec next chunk for trustedlen

54a49a3

bend-n force-pushed the trusted_len_spec_for_iter_next_chunk branch from 65349e7 to 54a49a3 Compare December 22, 2025 21:08

bend-n requested a review from the8472 January 8, 2026 16:59

This comment was marked as resolved.

Sign in to view

rustbot assigned jhpratt and unassigned the8472 Jan 28, 2026

	return Ok(from_fn(\|_\| unsafe { self.next().unwrap_unchecked() }));
	return Ok(from_fn(\|_\| unsafe { self.next_unchecked() }));

		// SAFETY: Has to error out; trusted len means that
		// SAFETY: there may only be less than N elements

Uh oh!

Conversation

bend-n commented Dec 21, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Dec 21, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

bend-n Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bend-n Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottmcm commented Dec 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bend-n commented Dec 21, 2025 •

edited by rustbot

Loading

bend-n Dec 22, 2025 •

edited

Loading

bend-n Dec 22, 2025 •

edited

Loading