Skip to content

Commit c183d4a

Browse files
authored
Rollup merge of #94115 - scottmcm:iter-process-by-ref, r=yaahc
Let `try_collect` take advantage of `try_fold` overrides No public API changes. With this change, `try_collect` (#94047) is no longer going through the `impl Iterator for &mut impl Iterator`, and thus will be able to use `try_fold` overrides instead of being forced through `next` for every element. 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 This might as well go to the same person as my last `try_process` PR (#93572), so r? ``@yaahc``
2 parents 1bfe40d + 7ef74bc commit c183d4a

File tree

5 files changed

+72
-2
lines changed

5 files changed

+72
-2
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
use crate::ops::Try;
2+
3+
/// Like `Iterator::by_ref`, but requiring `Sized` so it can forward generics.
4+
///
5+
/// Ideally this will no longer be required, eventually, but as can be seen in
6+
/// the benchmarks (as of Feb 2022 at least) `by_ref` can have performance cost.
7+
pub(crate) struct ByRefSized<'a, I>(pub &'a mut I);
8+
9+
impl<I: Iterator> Iterator for ByRefSized<'_, I> {
10+
type Item = I::Item;
11+
12+
fn next(&mut self) -> Option<Self::Item> {
13+
self.0.next()
14+
}
15+
16+
fn size_hint(&self) -> (usize, Option<usize>) {
17+
self.0.size_hint()
18+
}
19+
20+
fn advance_by(&mut self, n: usize) -> Result<(), usize> {
21+
self.0.advance_by(n)
22+
}
23+
24+
fn nth(&mut self, n: usize) -> Option<Self::Item> {
25+
self.0.nth(n)
26+
}
27+
28+
fn fold<B, F>(self, init: B, f: F) -> B
29+
where
30+
F: FnMut(B, Self::Item) -> B,
31+
{
32+
self.0.fold(init, f)
33+
}
34+
35+
fn try_fold<B, F, R>(&mut self, init: B, f: F) -> R
36+
where
37+
F: FnMut(B, Self::Item) -> R,
38+
R: Try<Output = B>,
39+
{
40+
self.0.try_fold(init, f)
41+
}
42+
}

library/core/src/iter/adapters/mod.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::iter::{InPlaceIterable, Iterator};
22
use crate::ops::{ChangeOutputType, ControlFlow, FromResidual, NeverShortCircuit, Residual, Try};
33

4+
mod by_ref_sized;
45
mod chain;
56
mod cloned;
67
mod copied;
@@ -31,6 +32,8 @@ pub use self::{
3132
scan::Scan, skip::Skip, skip_while::SkipWhile, take::Take, take_while::TakeWhile, zip::Zip,
3233
};
3334

35+
pub(crate) use self::by_ref_sized::ByRefSized;
36+
3437
#[stable(feature = "iter_cloned", since = "1.1.0")]
3538
pub use self::cloned::Cloned;
3639

library/core/src/iter/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ pub use self::adapters::{
417417
#[unstable(feature = "iter_intersperse", reason = "recently added", issue = "79524")]
418418
pub use self::adapters::{Intersperse, IntersperseWith};
419419

420-
pub(crate) use self::adapters::try_process;
420+
pub(crate) use self::adapters::{try_process, ByRefSized};
421421

422422
mod adapters;
423423
mod range;

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::cmp::{self, Ordering};
22
use crate::ops::{ChangeOutputType, ControlFlow, FromResidual, Residual, Try};
33

44
use super::super::try_process;
5+
use super::super::ByRefSized;
56
use super::super::TrustedRandomAccessNoCoerce;
67
use super::super::{Chain, Cloned, Copied, Cycle, Enumerate, Filter, FilterMap, Fuse};
78
use super::super::{FlatMap, Flatten};
@@ -1861,7 +1862,7 @@ pub trait Iterator {
18611862
<<Self as Iterator>::Item as Try>::Residual: Residual<B>,
18621863
B: FromIterator<<Self::Item as Try>::Output>,
18631864
{
1864-
try_process(self, |i| i.collect())
1865+
try_process(ByRefSized(self), |i| i.collect())
18651866
}
18661867

18671868
/// Collects all the items from an iterator into a collection.

library/core/tests/iter/traits/iterator.rs

+24
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,30 @@ fn test_collect_into() {
551551
assert!(a == b);
552552
}
553553

554+
#[test]
555+
fn iter_try_collect_uses_try_fold_not_next() {
556+
// This makes sure it picks up optimizations, and doesn't use the `&mut I` impl.
557+
struct PanicOnNext<I>(I);
558+
impl<I: Iterator> Iterator for PanicOnNext<I> {
559+
type Item = I::Item;
560+
fn next(&mut self) -> Option<Self::Item> {
561+
panic!("Iterator::next should not be called!")
562+
}
563+
fn try_fold<B, F, R>(&mut self, init: B, f: F) -> R
564+
where
565+
Self: Sized,
566+
F: FnMut(B, Self::Item) -> R,
567+
R: std::ops::Try<Output = B>,
568+
{
569+
self.0.try_fold(init, f)
570+
}
571+
}
572+
573+
let it = (0..10).map(Some);
574+
let _ = PanicOnNext(it).try_collect::<Vec<_>>();
575+
// validation is just that it didn't panic.
576+
}
577+
554578
// just tests by whether or not this compiles
555579
fn _empty_impl_all_auto_traits<T>() {
556580
use std::panic::{RefUnwindSafe, UnwindSafe};

0 commit comments

Comments
 (0)