Skip to content
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.

Sign up for GitHub

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

Match options directly in the Fuse implementation #70750

Merged
merged 2 commits into from
Apr 6, 2020
Merged

Match options directly in the Fuse implementation #70750

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f8d6fc1
Open-code Fuse's Option matches
cuviper Apr 3, 2020
6fdd4f3
Use a macro to expand the specialized Fuse
cuviper Apr 3, 2020
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
137 changes: 63 additions & 74 deletions src/libcore/iter/adapters/fuse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,22 @@ impl<I> Fuse<I> {
#[stable(feature = "fused", since = "1.26.0")]
impl<I> FusedIterator for Fuse<I> where I: Iterator {}

/// Fuse the iterator if the expression is `None`.
macro_rules! fuse {
($self:ident . iter . $($call:tt)+) => {
match $self.iter {
Some(ref mut iter) => match iter.$($call)+ {
None => {
$self.iter = None;
None
}
item => item,
},
None => None,
}
};
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<I> Iterator for Fuse<I>
where
Expand All @@ -37,35 +53,36 @@ where

#[inline]
default fn next(&mut self) -> Option<<I as Iterator>::Item> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to the tweaks in this PR... there's a default fn here and elsewhere that shouldn't be (Specialization should be negotiated through internal traits.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, it's been like that for a little while...
https://github.com/rust-lang/rust/pull/35656/files#diff-d93bf2799d551e25b17e877b23a37db3L1659-R1726

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, we discovered problematic usage of associated type defaults much older, still, it needed to be fixed however old. :)

Copy link
Member

Choose a reason for hiding this comment

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

@Centril Maybe open an E-Easy issue for it with instructions for a new contributor to pick up? I agree that it's "unrelated to the tweaks in this PR", so I don't think it needs to happen here.

Copy link
Contributor

Choose a reason for hiding this comment

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

@scottmcm Oh sure, I didn't mean it should be fixed in this PR. :)

I've filed #70796.

let next = self.iter.as_mut()?.next();
if next.is_none() {
self.iter = None;
}
next
fuse!(self.iter.next())
}

#[inline]
default fn nth(&mut self, n: usize) -> Option<I::Item> {
let nth = self.iter.as_mut()?.nth(n);
if nth.is_none() {
self.iter = None;
}
nth
fuse!(self.iter.nth(n))
Copy link
Member

Choose a reason for hiding this comment

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

This is beautiful. Nicely done.

}

#[inline]
default fn last(self) -> Option<I::Item> {
self.iter?.last()
match self.iter {
Some(iter) => iter.last(),
None => None,
}
}

#[inline]
default fn count(self) -> usize {
self.iter.map_or(0, I::count)
match self.iter {
Some(iter) => iter.count(),
None => 0,
}
}

#[inline]
default fn size_hint(&self) -> (usize, Option<usize>) {
self.iter.as_ref().map_or((0, Some(0)), I::size_hint)
match self.iter {
Some(ref iter) => iter.size_hint(),
None => (0, Some(0)),
}
}

#[inline]
Expand Down Expand Up @@ -98,11 +115,7 @@ where
where
P: FnMut(&Self::Item) -> bool,
{
let found = self.iter.as_mut()?.find(predicate);
if found.is_none() {
self.iter = None;
}
found
fuse!(self.iter.find(predicate))
}
}

Expand All @@ -113,20 +126,12 @@ where
{
#[inline]
default fn next_back(&mut self) -> Option<<I as Iterator>::Item> {
let next = self.iter.as_mut()?.next_back();
if next.is_none() {
self.iter = None;
}
next
fuse!(self.iter.next_back())
}

#[inline]
default fn nth_back(&mut self, n: usize) -> Option<<I as Iterator>::Item> {
let nth = self.iter.as_mut()?.nth_back(n);
if nth.is_none() {
self.iter = None;
}
nth
fuse!(self.iter.nth_back(n))
}

#[inline]
Expand Down Expand Up @@ -159,11 +164,7 @@ where
where
P: FnMut(&Self::Item) -> bool,
{
let found = self.iter.as_mut()?.rfind(predicate);
if found.is_none() {
self.iter = None;
}
found
fuse!(self.iter.rfind(predicate))
}
}

Expand All @@ -173,42 +174,30 @@ where
I: ExactSizeIterator,
{
default fn len(&self) -> usize {
self.iter.as_ref().map_or(0, I::len)
}

default fn is_empty(&self) -> bool {
self.iter.as_ref().map_or(true, I::is_empty)
}
}

// NOTE: for `I: FusedIterator`, we assume that the iterator is always `Some`
impl<I: FusedIterator> Fuse<I> {
#[inline(always)]
fn as_inner(&self) -> &I {
match self.iter {
Some(ref iter) => iter,
// SAFETY: the specialized iterator never sets `None`
None => unsafe { intrinsics::unreachable() },
Some(ref iter) => iter.len(),
None => 0,
}
}

#[inline(always)]
fn as_inner_mut(&mut self) -> &mut I {
default fn is_empty(&self) -> bool {
match self.iter {
Some(ref mut iter) => iter,
// SAFETY: the specialized iterator never sets `None`
None => unsafe { intrinsics::unreachable() },
Some(ref iter) => iter.is_empty(),
None => true,
}
}
}

#[inline(always)]
fn into_inner(self) -> I {
match self.iter {
Some(iter) => iter,
// NOTE: for `I: FusedIterator`, we assume that the iterator is always `Some`.
// Implementing this as a directly-expanded macro helps codegen performance.
macro_rules! unchecked {
($self:ident) => {
match $self {
Fuse { iter: Some(iter) } => iter,
// SAFETY: the specialized iterator never sets `None`
None => unsafe { intrinsics::unreachable() },
Fuse { iter: None } => unsafe { intrinsics::unreachable() },
}
}
};
}

#[stable(feature = "fused", since = "1.26.0")]
Expand All @@ -218,27 +207,27 @@ where
{
#[inline]
fn next(&mut self) -> Option<<I as Iterator>::Item> {
self.as_inner_mut().next()
unchecked!(self).next()
}

#[inline]
fn nth(&mut self, n: usize) -> Option<I::Item> {
self.as_inner_mut().nth(n)
unchecked!(self).nth(n)
}

#[inline]
fn last(self) -> Option<I::Item> {
self.into_inner().last()
unchecked!(self).last()
}

#[inline]
fn count(self) -> usize {
self.into_inner().count()
unchecked!(self).count()
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.as_inner().size_hint()
unchecked!(self).size_hint()
}

#[inline]
Expand All @@ -248,23 +237,23 @@ where
Fold: FnMut(Acc, Self::Item) -> R,
R: Try<Ok = Acc>,
{
self.as_inner_mut().try_fold(init, fold)
unchecked!(self).try_fold(init, fold)
}

#[inline]
fn fold<Acc, Fold>(self, init: Acc, fold: Fold) -> Acc
where
Fold: FnMut(Acc, Self::Item) -> Acc,
{
self.into_inner().fold(init, fold)
unchecked!(self).fold(init, fold)
}

#[inline]
fn find<P>(&mut self, predicate: P) -> Option<Self::Item>
where
P: FnMut(&Self::Item) -> bool,
{
self.as_inner_mut().find(predicate)
unchecked!(self).find(predicate)
}
}

Expand All @@ -275,12 +264,12 @@ where
{
#[inline]
fn next_back(&mut self) -> Option<<I as Iterator>::Item> {
self.as_inner_mut().next_back()
unchecked!(self).next_back()
}

#[inline]
fn nth_back(&mut self, n: usize) -> Option<<I as Iterator>::Item> {
self.as_inner_mut().nth_back(n)
unchecked!(self).nth_back(n)
}

#[inline]
Expand All @@ -290,23 +279,23 @@ where
Fold: FnMut(Acc, Self::Item) -> R,
R: Try<Ok = Acc>,
{
self.as_inner_mut().try_rfold(init, fold)
unchecked!(self).try_rfold(init, fold)
}

#[inline]
fn rfold<Acc, Fold>(self, init: Acc, fold: Fold) -> Acc
where
Fold: FnMut(Acc, Self::Item) -> Acc,
{
self.into_inner().rfold(init, fold)
unchecked!(self).rfold(init, fold)
}

#[inline]
fn rfind<P>(&mut self, predicate: P) -> Option<Self::Item>
where
P: FnMut(&Self::Item) -> bool,
{
self.as_inner_mut().rfind(predicate)
unchecked!(self).rfind(predicate)
}
}

Expand All @@ -316,11 +305,11 @@ where
I: ExactSizeIterator + FusedIterator,
{
fn len(&self) -> usize {
self.as_inner().len()
unchecked!(self).len()
}

fn is_empty(&self) -> bool {
self.as_inner().is_empty()
unchecked!(self).is_empty()
}
}

Expand Down