Skip to content

Add PeekableIterator trait as an experiment #95195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion library/core/src/iter/adapters/copied.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::iter::adapters::{
zip::try_get_unchecked, TrustedRandomAccess, TrustedRandomAccessNoCoerce,
};
use crate::iter::{FusedIterator, TrustedLen};
use crate::iter::{FusedIterator, PeekableIterator, TrustedLen};
use crate::ops::Try;

/// An iterator that copies the elements of an underlying iterator.
Expand Down Expand Up @@ -139,6 +139,17 @@ where
}
}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<'a, I, T: 'a> PeekableIterator for Copied<I>
where
I: PeekableIterator<Item = &'a T>,
T: Copy,
{
fn peek(&self) -> Option<T> {
self.it.peek().map(|x| *x)
}
}

#[stable(feature = "iter_copied", since = "1.36.0")]
impl<'a, I, T: 'a> FusedIterator for Copied<I>
where
Expand Down
15 changes: 14 additions & 1 deletion library/core/src/iter/adapters/cycle.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::{iter::FusedIterator, ops::Try};
use crate::{
iter::{FusedIterator, PeekableIterator},
ops::Try,
};

/// An iterator that repeats endlessly.
///
Expand Down Expand Up @@ -104,5 +107,15 @@ where
// and we can't do anything better than the default.
}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<I> PeekableIterator for Cycle<I>
where
I: Clone + PeekableIterator,
{
fn peek(&self) -> Option<I::Item> {
self.iter.peek().or_else(|| self.orig.peek())
}
}

#[stable(feature = "fused", since = "1.26.0")]
impl<I> FusedIterator for Cycle<I> where I: Clone + Iterator {}
12 changes: 11 additions & 1 deletion library/core/src/iter/adapters/enumerate.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::iter::adapters::{
zip::try_get_unchecked, SourceIter, TrustedRandomAccess, TrustedRandomAccessNoCoerce,
};
use crate::iter::{FusedIterator, InPlaceIterable, TrustedLen};
use crate::iter::{FusedIterator, InPlaceIterable, PeekableIterator, TrustedLen};
use crate::ops::Try;

/// An iterator that yields the current count and the element during iteration.
Expand Down Expand Up @@ -229,6 +229,16 @@ where
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<I> PeekableIterator for Enumerate<I>
where
I: PeekableIterator,
{
fn peek(&self) -> Option<(usize, I::Item)> {
self.iter.peek().map(|x| (self.count, x))
}
}

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<I> TrustedRandomAccess for Enumerate<I> where I: TrustedRandomAccess {}
Expand Down
14 changes: 12 additions & 2 deletions library/core/src/iter/adapters/fuse.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::intrinsics;
use crate::iter::adapters::zip::try_get_unchecked;
use crate::iter::{
DoubleEndedIterator, ExactSizeIterator, FusedIterator, TrustedLen, TrustedRandomAccess,
TrustedRandomAccessNoCoerce,
DoubleEndedIterator, ExactSizeIterator, FusedIterator, PeekableIterator, TrustedLen,
TrustedRandomAccess, TrustedRandomAccessNoCoerce,
};
use crate::ops::Try;

Expand All @@ -26,6 +26,16 @@ impl<I> Fuse<I> {
}
}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<I> PeekableIterator for Fuse<I>
where
I: PeekableIterator,
{
fn peek(&self) -> Option<I::Item> {
self.iter.as_ref().and_then(|iter| iter.peek())
}
}

#[stable(feature = "fused", since = "1.26.0")]
impl<I> FusedIterator for Fuse<I> where I: Iterator {}

Expand Down
12 changes: 11 additions & 1 deletion library/core/src/iter/adapters/inspect.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::fmt;
use crate::iter::{adapters::SourceIter, FusedIterator, InPlaceIterable};
use crate::iter::{adapters::SourceIter, FusedIterator, InPlaceIterable, PeekableIterator};
use crate::ops::Try;

/// An iterator that calls a function with a reference to each element before
Expand Down Expand Up @@ -145,6 +145,16 @@ where
}
}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<I: PeekableIterator, F> PeekableIterator for Inspect<I, F>
where
F: FnMut(&I::Item),
{
fn peek(&self) -> Option<I::Item> {
self.iter.peek()
}
}

#[stable(feature = "fused", since = "1.26.0")]
impl<I: FusedIterator, F> FusedIterator for Inspect<I, F> where F: FnMut(&I::Item) {}

Expand Down
14 changes: 13 additions & 1 deletion library/core/src/iter/adapters/take.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::cmp;
use crate::iter::{adapters::SourceIter, FusedIterator, InPlaceIterable, TrustedLen};
use crate::iter::{
adapters::SourceIter, FusedIterator, InPlaceIterable, PeekableIterator, TrustedLen,
};
use crate::ops::{ControlFlow, Try};

/// An iterator that only iterates over the first `n` iterations of `iter`.
Expand Down Expand Up @@ -237,6 +239,16 @@ where
#[stable(feature = "rust1", since = "1.0.0")]
impl<I> ExactSizeIterator for Take<I> where I: ExactSizeIterator {}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<I> PeekableIterator for Take<I>
where
I: PeekableIterator,
{
fn peek(&self) -> Option<I::Item> {
if self.n == 0 { None } else { self.iter.peek() }
}
}

#[stable(feature = "fused", since = "1.26.0")]
impl<I> FusedIterator for Take<I> where I: FusedIterator {}

Expand Down
2 changes: 2 additions & 0 deletions library/core/src/iter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,8 @@ pub use self::sources::{successors, Successors};
pub use self::traits::FusedIterator;
#[unstable(issue = "none", feature = "inplace_iteration")]
pub use self::traits::InPlaceIterable;
#[unstable(feature = "peekable_iterator", issue = "none")]
pub use self::traits::PeekableIterator;
#[unstable(feature = "trusted_len", issue = "37572")]
pub use self::traits::TrustedLen;
#[unstable(feature = "trusted_step", issue = "85731")]
Expand Down
24 changes: 23 additions & 1 deletion library/core/src/iter/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use crate::mem;
use crate::ops::{self, Try};

use super::{
FusedIterator, TrustedLen, TrustedRandomAccess, TrustedRandomAccessNoCoerce, TrustedStep,
FusedIterator, PeekableIterator, TrustedLen, TrustedRandomAccess, TrustedRandomAccessNoCoerce,
TrustedStep,
};

// Safety: All invariants are upheld.
Expand Down Expand Up @@ -765,6 +766,13 @@ impl<A: Step> Iterator for ops::Range<A> {
}
}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<A: Step> PeekableIterator for ops::Range<A> {
fn peek(&self) -> Option<A> {
if self.start < self.end { Some(self.start.clone()) } else { None }
}
}

// These macros generate `ExactSizeIterator` impls for various range types.
//
// * `ExactSizeIterator::len` is required to always return an exact `usize`,
Expand Down Expand Up @@ -879,6 +887,13 @@ impl<A: Step> Iterator for ops::RangeFrom<A> {
}
}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<A: Step> PeekableIterator for ops::RangeFrom<A> {
fn peek(&self) -> Option<A> {
Some(self.start.clone())
}
}

// Safety: See above implementation for `ops::Range<A>`
#[unstable(feature = "trusted_len", issue = "37572")]
unsafe impl<A: TrustedStep> TrustedLen for ops::RangeFrom<A> {}
Expand Down Expand Up @@ -1186,6 +1201,13 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
}
}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<A: Step> PeekableIterator for ops::RangeInclusive<A> {
fn peek(&self) -> Option<A> {
if self.is_empty() { None } else { Some(self.start.clone()) }
}
}

#[stable(feature = "inclusive_range", since = "1.26.0")]
impl<A: Step> DoubleEndedIterator for ops::RangeInclusive<A> {
#[inline]
Expand Down
9 changes: 8 additions & 1 deletion library/core/src/iter/sources/empty.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::fmt;
use crate::iter::{FusedIterator, TrustedLen};
use crate::iter::{FusedIterator, PeekableIterator, TrustedLen};
use crate::marker;

/// Creates an iterator that yields nothing.
Expand Down Expand Up @@ -68,6 +68,13 @@ impl<T> ExactSizeIterator for Empty<T> {
}
}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<T> PeekableIterator for Empty<T> {
fn peek(&self) -> Option<T> {
None
}
}

#[unstable(feature = "trusted_len", issue = "37572")]
unsafe impl<T> TrustedLen for Empty<T> {}

Expand Down
9 changes: 8 additions & 1 deletion library/core/src/iter/sources/once.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::iter::{FusedIterator, TrustedLen};
use crate::iter::{FusedIterator, PeekableIterator, TrustedLen};

/// Creates an iterator that yields an element exactly once.
///
Expand Down Expand Up @@ -92,6 +92,13 @@ impl<T> ExactSizeIterator for Once<T> {
}
}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<T: Clone> PeekableIterator for Once<T> {
fn peek(&self) -> Option<T> {
self.inner.peek()
}
}

#[unstable(feature = "trusted_len", issue = "37572")]
unsafe impl<T> TrustedLen for Once<T> {}

Expand Down
9 changes: 8 additions & 1 deletion library/core/src/iter/sources/repeat.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::iter::{FusedIterator, TrustedLen};
use crate::iter::{FusedIterator, PeekableIterator, TrustedLen};

/// Creates a new iterator that endlessly repeats a single element.
///
Expand Down Expand Up @@ -122,6 +122,13 @@ impl<A: Clone> DoubleEndedIterator for Repeat<A> {
}
}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<A: Copy> PeekableIterator for Repeat<A> {
fn peek(&self) -> Option<A> {
Some(self.element)
}
}

#[stable(feature = "fused", since = "1.26.0")]
impl<A: Clone> FusedIterator for Repeat<A> {}

Expand Down
3 changes: 3 additions & 0 deletions library/core/src/iter/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod double_ended;
mod exact_size;
mod iterator;
mod marker;
mod peekable;

#[stable(feature = "rust1", since = "1.0.0")]
pub use self::{
Expand All @@ -19,3 +20,5 @@ pub use self::{
pub use self::marker::InPlaceIterable;
#[unstable(feature = "trusted_step", issue = "85731")]
pub use self::marker::TrustedStep;
#[unstable(feature = "peekable_iterator", issue = "none")]
pub use self::peekable::PeekableIterator;
101 changes: 101 additions & 0 deletions library/core/src/iter/traits/peekable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/// An iterator whose elements may be checked without advancing the iterator.
///
/// In general, many [`Iterator`]s are able to "peek" their next element,
/// showing what would be returned by a call to `next` without advancing their
/// internal state. If these iterators do not offer such functionality, it can
/// be manually added to an iterator using the [`peekable`] method.
///
/// In most cases, calling [`peekable`] on a [`PeekableIterator`] shouldn't
/// noticeably affect functionality, however, it's worth pointing out a few
/// differences between [`peekable`] and [`PeekableIterator`]:
///
/// * Stateful iterators like those using [`inspect`](Iterator::inspect) will
/// eagerly evaluate when peeked by a [`peekable`] wrapper, but may do so
/// lazily with a custom [`PeekableIterator`] implementation.
/// * The [`peekable`] wrapper will incur a small performance penalty for
/// [`next`] and [`next_back`], but [`PeekableIterator`] implementations
/// incur no such penalty.
/// * The [`peekable`] wrapper will return a reference to its item, whereas
/// [`PeekableIterator`] will return the item directly.
///
/// Note that this trait is a safe trait and as such does *not* and *cannot*
/// guarantee that the peeked value will be returned in a subsequent call to
/// [`next`], no matter how soon the two are called together. A common
/// example of this is interior mutability; if the interior state of the
/// iterator is mutated between a call to [`peek`] and [`next`], then the
/// values may differ.
///
/// [`peek`]: Self::peek
/// [`peekable`]: Iterator::peekable
/// [`Peekable`]: super::Peekable
/// [`next`]: Iterator::next
/// [`next_back`]: DoubleEndedIterator::next_back
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// #![feature(peekable_iterator)]
/// use std::iter::PeekableIterator;
///
/// // a range knows its current state exactly
/// let five = 0..5;
///
/// assert_eq!(Some(0), five.peek());
/// ```
#[unstable(feature = "peekable_iterator", issue = "none")]
pub trait PeekableIterator: Iterator {
/// Returns a reference to the [`next`] value without advancing the iterator.
///
/// [`next`]: Iterator::next
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// #![feature(peekable_iterator)]
/// use std::iter::PeekableIterator;
///
/// // a finite range knows its current state exactly
/// let five = 0..5;
///
/// assert_eq!(Some(0), five.peek());
/// ```
#[unstable(feature = "peekable_iterator", issue = "none")]
fn peek(&self) -> Option<Self::Item>;
Copy link
Member

Choose a reason for hiding this comment

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

Can you say more about why you picked Option<Item> instead of Option<&Item> here? Especially since that's what peek does https://doc.rust-lang.org/std/iter/struct.Peekable.html#method.peek

And it'd make sense to me for things like Repeat to return a reference to the state, for example.

Copy link
Contributor Author

@clarfonthey clarfonthey Mar 22, 2022

Choose a reason for hiding this comment

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

The big reason is that a majority of iterators are completely impossible to do with that type signature. By-reference and by-mutable-reference iterators over collections would completely break, and that's loads of use cases right there.

At first I did that, then I realised how bad of an idea it would be to do that. Essentially, the use cases that would prefer just storing the next value and then referencing it are covered by peekable, and this covers the ones where that seems suboptimal.

Copy link
Member

@the8472 the8472 Mar 22, 2022

Choose a reason for hiding this comment

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

But that leads to some implementations using clone(), and has_next() just calls peek() which then clones. That calling has_next() might allocate is going to be surprising and certainly more costly than ExactSizeIterator's is_empty().

Edit: Ah, you mention this already in the PR comment.


/// Returns `true` if the [`next`] value is `None`.
///
/// [`next`]: Iterator::next
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// #![feature(peekable_iterator)]
/// use std::iter::PeekableIterator;
///
/// let mut one_element = std::iter::once(0);
/// assert!(one_element.has_next());
///
/// assert_eq!(one_element.next(), Some(0));
/// assert!(!one_element.has_next());
///
/// assert_eq!(one_element.next(), None);
/// ```
#[inline]
#[unstable(feature = "peekable_iterator", issue = "none")]
fn has_next(&self) -> bool {
self.peek().is_some()
}
}

#[unstable(feature = "peekable_iterator", issue = "none")]
impl<I: PeekableIterator + ?Sized> PeekableIterator for &mut I {
fn peek(&self) -> Option<I::Item> {
(**self).peek()
}
}
Loading