Skip to content

Commit

Permalink
Let try_collect take advantage of try_fold overrides
Browse files Browse the repository at this point in the history
Without this change it's going through `&mut impl Iterator`, which handles `?Sized` and thus currently can't forward generics.

Here's the test added, to see that it fails before this PR (once a new enough nightly is out): https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=462f2896f2fed2c238ee63ca1a7e7c56
  • Loading branch information
scottmcm committed Mar 10, 2022
1 parent 01ad0ad commit 7ef74bc
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 2 deletions.
42 changes: 42 additions & 0 deletions library/core/src/iter/adapters/by_ref_sized.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
use crate::ops::Try;

/// Like `Iterator::by_ref`, but requiring `Sized` so it can forward generics.
///
/// Ideally this will no longer be required, eventually, but as can be seen in
/// the benchmarks (as of Feb 2022 at least) `by_ref` can have performance cost.
pub(crate) struct ByRefSized<'a, I>(pub &'a mut I);

impl<I: Iterator> Iterator for ByRefSized<'_, I> {
type Item = I::Item;

fn next(&mut self) -> Option<Self::Item> {
self.0.next()
}

fn size_hint(&self) -> (usize, Option<usize>) {
self.0.size_hint()
}

fn advance_by(&mut self, n: usize) -> Result<(), usize> {
self.0.advance_by(n)
}

fn nth(&mut self, n: usize) -> Option<Self::Item> {
self.0.nth(n)
}

fn fold<B, F>(self, init: B, f: F) -> B
where
F: FnMut(B, Self::Item) -> B,
{
self.0.fold(init, f)
}

fn try_fold<B, F, R>(&mut self, init: B, f: F) -> R
where
F: FnMut(B, Self::Item) -> R,
R: Try<Output = B>,
{
self.0.try_fold(init, f)
}
}
3 changes: 3 additions & 0 deletions library/core/src/iter/adapters/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::iter::{InPlaceIterable, Iterator};
use crate::ops::{ChangeOutputType, ControlFlow, FromResidual, NeverShortCircuit, Residual, Try};

mod by_ref_sized;
mod chain;
mod cloned;
mod copied;
Expand Down Expand Up @@ -31,6 +32,8 @@ pub use self::{
scan::Scan, skip::Skip, skip_while::SkipWhile, take::Take, take_while::TakeWhile, zip::Zip,
};

pub(crate) use self::by_ref_sized::ByRefSized;

#[stable(feature = "iter_cloned", since = "1.1.0")]
pub use self::cloned::Cloned;

Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ pub use self::adapters::{
#[unstable(feature = "iter_intersperse", reason = "recently added", issue = "79524")]
pub use self::adapters::{Intersperse, IntersperseWith};

pub(crate) use self::adapters::try_process;
pub(crate) use self::adapters::{try_process, ByRefSized};

mod adapters;
mod range;
Expand Down
3 changes: 2 additions & 1 deletion library/core/src/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::cmp::{self, Ordering};
use crate::ops::{ChangeOutputType, ControlFlow, FromResidual, Residual, Try};

use super::super::try_process;
use super::super::ByRefSized;
use super::super::TrustedRandomAccessNoCoerce;
use super::super::{Chain, Cloned, Copied, Cycle, Enumerate, Filter, FilterMap, Fuse};
use super::super::{FlatMap, Flatten};
Expand Down Expand Up @@ -1856,7 +1857,7 @@ pub trait Iterator {
<<Self as Iterator>::Item as Try>::Residual: Residual<B>,
B: FromIterator<<Self::Item as Try>::Output>,
{
try_process(self, |i| i.collect())
try_process(ByRefSized(self), |i| i.collect())
}

/// Collects all the items from an iterator into a collection.
Expand Down
24 changes: 24 additions & 0 deletions library/core/tests/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,30 @@ fn test_collect_into() {
assert!(a == b);
}

#[test]
fn iter_try_collect_uses_try_fold_not_next() {
// This makes sure it picks up optimizations, and doesn't use the `&mut I` impl.
struct PanicOnNext<I>(I);
impl<I: Iterator> Iterator for PanicOnNext<I> {
type Item = I::Item;
fn next(&mut self) -> Option<Self::Item> {
panic!("Iterator::next should not be called!")
}
fn try_fold<B, F, R>(&mut self, init: B, f: F) -> R
where
Self: Sized,
F: FnMut(B, Self::Item) -> R,
R: std::ops::Try<Output = B>,
{
self.0.try_fold(init, f)
}
}

let it = (0..10).map(Some);
let _ = PanicOnNext(it).try_collect::<Vec<_>>();
// validation is just that it didn't panic.
}

// just tests by whether or not this compiles
fn _empty_impl_all_auto_traits<T>() {
use std::panic::{RefUnwindSafe, UnwindSafe};
Expand Down

0 comments on commit 7ef74bc

Please sign in to comment.