Add `PeekableIterator` trait #144935

wmstack · 2025-08-05T02:01:52Z

Tracking issue: #132973

Implementation

pub trait PeekableIterator: Iterator {
  // required method
  fn peek_with<T>(&mut self, func: impl for<'a> FnOnce(Option<&'a Self::Item>) -> T) -> T

  // provided methods
  fn peek_map<T>(&mut self, func: impl for<'a> FnOnce(&'a Self::Item) -> T) -> Option<T>
  fn next_if(&mut self, func: impl FnOnce(&Self::Item) -> bool) -> Option<Self::Item>
  fn next_if_eq<T>(&mut self, expected: &T) -> Option<Self::Item>
  where
    Self::Item: PartialEq<T>,
    T: ?Sized,
}

Todo: Determine which structures need to implement PeekableIterator:

slice::iter
str::Chars
vec::IntoIter
Peekable<T>

Unresolved Issues:

Add another trait for iterators that can peek() without mutating?

rustbot · 2025-08-05T02:01:56Z

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

theemathas · 2025-08-05T04:37:44Z

Maybe the correct function signature is this?

fn peek<T>(&mut self, callback: impl for<'a> FnOnce(Option<&'a Self::Item>) -> T) -> T

This would mean, for example, that str::Chars would be able to implement this without having a field inside the iterator that contains a char. Instead, it would be able to create a local variable in the peek method that stores the char.

library/core/src/iter/traits/peekable.rs

Improve documentation for `PeekableIterator` to reflect the changes in API Co-authored-by: +merlan #flirora <flirora@flirora.xyz>

library/core/src/array/iter/iter_inner.rs

theemathas · 2025-08-09T07:21:16Z

library/core/src/slice/iter/macros.rs

+                        // SAFETY: by type invariant, the `end_or_len` field is always
+                        // non-null for a non-ZST pointee.  (This transmute ensures we
+                        // get `!nonnull` metadata on the load of the field.)
+                        if ptr == crate::intrinsics::transmute::<$ptr, NonNull<T>>(end_or_len) {


Suggested change

// SAFETY: by type invariant, the `end_or_len` field is always

// non-null for a non-ZST pointee. (This transmute ensures we

// get `!nonnull` metadata on the load of the field.)

if ptr == crate::intrinsics::transmute::<$ptr, NonNull<T>>(end_or_len) {

if ptr.as_ptr() == end_or_len {

Hmm, I was not able to write it this way as Rust determines the types differ in their mutability. In general, this code was based on the next() method generic over both Iter and IterMut

What exactly was the error? Seems possible to resolve by adding a $as_ptr metavar that is either as_ptr or as_mut_ptr depending on which one is being implemented (like into_ref).

next is a bit magical, others should try to be simpler if possible. cloning and advancing should be fine, it's cheap.

Cloning actually wouldn't work for IterMut since it doesn't implement it. But that actually brings up a good point; the safety comments need to say why it is sound to hand out a&mut T.

library/core/src/slice/iter/macros.rs

Co-authored-by: Tim (Theemathas) Chirananthavat <theemathas@gmail.com>

tgross35

This doesn't seem to be tested at all. Please make sure that even unstable API gets helpful examples, aside from checking the implementation they serve as a very useful smoke test for whether or not the API makes sense in the first place.

Additionally, the type-specific impls need tests in library/coretests

View changes since this review

tgross35 · 2025-09-03T02:52:56Z

library/core/src/iter/traits/peekable.rs

+    /// Executes the closure on the `next()` element without advancing the iterator, or returns `None` if the iterator is exhausted.
+    fn peek_map<T>(&mut self, func: impl for<'a> FnOnce(&'a Self::Item) -> T) -> Option<T> {
+        self.peek_with(|x| x.map(|y| func(y)))
+    }


This doesn't seem to have been part of the ACP. Could you inline it into next_if, or make it a standalone private function?

tgross35 · 2025-09-03T03:02:03Z

library/core/src/slice/iter/macros.rs

+                // SAFETY: See inner comments.
+                unsafe {
+                    if T::IS_ZST {
+                        let len = end_or_len.addr();
+                        if len == 0 {
+                            return func(None);
+                        }
+                    } else {
+                        if self.ptr == crate::intrinsics::transmute::<$ptr, NonNull<T>>(end_or_len) {
+                            return func(None);
+                        }
+                    }
+                    // SAFETY: Now that we know it wasn't empty
+                    // we can give out a reference to it.
+                    func(Some(self.ptr.$into_ref()).as_ref())
+                }


Few requests:

Use mem::transmute rather than the intrinsic directly

Use NonNull methods if possible. If not, add a comment about why this is

Can this unsafe block be broken up?

#144935 (comment) actually covers this better

rustbot · 2025-09-04T04:27:39Z

⚠️ Warning ⚠️

This PR is based on an upstream commit that is older than 28 days.

It's recommended to update your branch according to the rustc-dev-guide.

rust-log-analyzer · 2025-09-04T08:01:05Z

The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)

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

   |
18 |     }
   |      ^ help: add `;` here
19 |
20 |     u32::from_str_radix(c.as_str(), base).unwrap()
   |     --- unexpected token

error[E0308]: mismatched types
  --> library/core/src/iter/traits/peekable.rs:45:32
   |
10 |     let base = if c.next_if_eq(&"0").is_some() {
   |                     ---------- ^^^^ expected `&char`, found `&&str`
   |                     |
   |                     arguments to this method are incorrect
   |
   = note: expected reference `&char`
              found reference `&&'static str`
note: method defined here
  --> /checkout/library/core/src/iter/traits/peekable.rs:70:8
   |
70 |     fn next_if_eq<T>(&mut self, expected: &T) -> Option<Self::Item>
   |        ^^^^^^^^^^

error[E0277]: the trait bound `&char: Pattern` is not satisfied
    --> library/core/src/iter/traits/peekable.rs:46:44
     |
  11 |         match c.next_if(|c| "oxb".contains(c)) {
     |                                   -------- ^ the trait `Fn(char)` is not implemented for `char`
     |                                   |
     |                                   required by a bound introduced by this call
     |
     = note: required for `&char` to implement `FnOnce(char)`
     = note: required for `&char` to implement `Pattern`
note: required by a bound in `core::str::<impl str>::contains`
    --> /checkout/library/core/src/str/mod.rs:1340:24
     |
1340 |     pub fn contains<P: Pattern>(&self, pat: P) -> bool {
     |                        ^^^^^^^ required by this bound in `core::str::<impl str>::contains`
help: consider dereferencing here
     |
  11 |         match c.next_if(|c| "oxb".contains(*c)) {
     |                                            +

error[E0308]: mismatched types
  --> library/core/src/iter/traits/peekable.rs:47:18
   |
11 |         match c.next_if(|c| "oxb".contains(c)) {
   |               -------------------------------- this expression has type `Option<char>`
12 |             Some("x") => 16,
   |                  ^^^ expected `char`, found `&str`
   |
help: if you meant to write a `char` literal, use single quotes
   |
12 -             Some("x") => 16,
12 +             Some('x') => 16,
   |

error[E0308]: mismatched types
  --> library/core/src/iter/traits/peekable.rs:48:18
   |
11 |         match c.next_if(|c| "oxb".contains(c)) {
   |               -------------------------------- this expression has type `Option<char>`
12 |             Some("x") => 16,
13 |             Some("b") => 2,
   |                  ^^^ expected `char`, found `&str`
   |
help: if you meant to write a `char` literal, use single quotes
   |
13 -             Some("b") => 2,
13 +             Some('b') => 2,
   |

error: aborting due to 5 previous errors

Some errors have detailed explanations: E0277, E0308.
---
   0: __rustc::rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
   4: rust_out::main::_doctest_main_library_core_src_iter_traits_peekable_rs_9_0
   5: rust_out::main
   6: core::ops::function::FnOnce::call_once
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

---- library/core/src/iter/traits/peekable.rs - iter::traits::peekable::PeekableIterator::peek_with (line 9) stdout end ----

bluebear94 · 2025-10-09T19:09:56Z

@wmstack Are you still open to working on this PR? If not, then I’m willing to take over the work on this.

wmstack · 2025-10-09T19:55:24Z

@bluebear94 Feel free to pick this up as I’m tied up with coursework at the moment.

bors · 2025-10-14T10:15:35Z

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

Add peekable_iterator

38e446a

rustbot assigned tgross35 Aug 5, 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 Aug 5, 2025

This comment has been minimized.

Sign in to view

Implement peekable for slice::iter

8d2bdb0

This comment has been minimized.

Sign in to view

Implement new PeekableIterator for Iter

ecc7b86

This comment has been minimized.

Sign in to view

Replace T with U

3dd13bc

bluebear94 mentioned this pull request Aug 5, 2025

Tracking Issue for PeekableIterator #132973

Open

4 tasks

bluebear94 reviewed Aug 5, 2025

View reviewed changes

library/core/src/iter/traits/peekable.rs Outdated Show resolved Hide resolved

bluebear94 reviewed Aug 5, 2025

View reviewed changes

wmstack and others added 4 commits August 6, 2025 12:52

Apply suggestions from code review

bf0193c

Improve documentation for `PeekableIterator` to reflect the changes in API Co-authored-by: +merlan #flirora <flirora@flirora.xyz>

Update next_if to use the bool

dc33dd2

Update next_if logic to handle Some(false) correctly

5892463

Implement PeekableIterator for Chars

ec565f6

This comment has been minimized.

Sign in to view

Peek by cloning on Chars

ff668ec

This comment has been minimized.

Sign in to view

fix input parameter in Chars

56d74c6

theemathas mentioned this pull request Aug 6, 2025

Mismatched types causes ICE in compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs:2591:13 #144984

Open

wmstack added 4 commits August 6, 2025 16:35

Implement PeekableIterator for IntoIter

c552fc8

Forget temporary in iter_inner

6fc70f2

Implement PeekableIterator for Peekable

54ad568

Remove as_ref()

14a6ecd

theemathas reviewed Aug 9, 2025

View reviewed changes

library/core/src/array/iter/iter_inner.rs Show resolved Hide resolved

theemathas requested changes Aug 9, 2025

View reviewed changes

wmstack and others added 2 commits August 9, 2025 17:55

Remove unneeded transmute

746c699

Co-authored-by: Tim (Theemathas) Chirananthavat <theemathas@gmail.com>

Use assume_init_ref directly from slice

2fa7343

This comment has been minimized.

Sign in to view

fix peek_with in macros

a2ce876

tgross35 requested changes Sep 3, 2025

View reviewed changes

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 Sep 3, 2025

Reduce unsafe scope with Iter/IterMut

bf88550

remove peek_map, add examples

9e0f1ae

This comment has been minimized.

Sign in to view

Fix trailing whitespace, punctuation

495b775

Add PeekableIterator trait #144935

Are you sure you want to change the base?

Add PeekableIterator trait #144935

Uh oh!

Conversation

wmstack commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Aug 5, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

theemathas commented Aug 5, 2025

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

theemathas Aug 9, 2025

Choose a reason for hiding this comment

Uh oh!

wmstack Aug 9, 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!

tgross35 Sep 3, 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!

tgross35 Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

tgross35 left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

tgross35 Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

tgross35 Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

rustbot commented Sep 4, 2025

Uh oh!

This comment has been minimized.

rust-log-analyzer commented Sep 4, 2025

Uh oh!

bluebear94 commented Oct 9, 2025

Uh oh!

wmstack commented Oct 9, 2025

Uh oh!

bors commented Oct 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Add `PeekableIterator` trait #144935

Add `PeekableIterator` trait #144935

wmstack commented Aug 5, 2025 •

edited

Loading

wmstack Aug 9, 2025 •

edited

Loading

tgross35 Sep 3, 2025 •

edited

Loading

tgross35 left a comment •

edited by rustbot

Loading