Skip to content

TrustedRandomAccess specialisation for Iterator::cloned when Item: Copy. #44790

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

Merged
merged 1 commit into from
Sep 28, 2017
Merged

TrustedRandomAccess specialisation for Iterator::cloned when Item: Copy. #44790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 13 additions & 1 deletion src/libcore/iter/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ impl<'a, I, T: 'a> FusedIterator for Cloned<I>
{}

#[doc(hidden)]
unsafe impl<'a, I, T: 'a> TrustedRandomAccess for Cloned<I>
default unsafe impl<'a, I, T: 'a> TrustedRandomAccess for Cloned<I>
where I: TrustedRandomAccess<Item=&'a T>, T: Clone
{
unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
Expand All @@ -499,6 +499,18 @@ unsafe impl<'a, I, T: 'a> TrustedRandomAccess for Cloned<I>
fn may_have_side_effect() -> bool { true }
}

#[doc(hidden)]
unsafe impl<'a, I, T: 'a> TrustedRandomAccess for Cloned<I>
Copy link
Member

Choose a reason for hiding this comment

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

Why is this specialization required? It seems totally equivalent to the impl above.

Copy link
Contributor

Choose a reason for hiding this comment

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

It returns false for may_have_side_effect.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe may_have_side_effect could be removed and it just assumes everything has side effects. Since some .map().zip() invocations optimize ok it could be suspected that the compiler can remove dead code itself. Unfortunately a closer examination of benchmarks is needed.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right.

Copy link
Member

Choose a reason for hiding this comment

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

This does assume that Clone is simply *self, but it could do weird side-effectful things. I think it's probably reasonable to state that we're allowed to assume that Clone behaves sanely for Copy types, but I don't think that's explicitly documented currently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears to be in fact documented: https://doc.rust-lang.org/std/clone/trait.Clone.html#how-can-i-implement-clone

Copy link
Member

Choose a reason for hiding this comment

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

Ah great, I missed that.

where I: TrustedRandomAccess<Item=&'a T>, T: Copy
{
unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
*self.it.get_unchecked(i)
}

#[inline]
fn may_have_side_effect() -> bool { false }
}

#[unstable(feature = "trusted_len", issue = "37572")]
unsafe impl<'a, I, T: 'a> TrustedLen for Cloned<I>
where I: TrustedLen<Item=&'a T>,
Expand Down
14 changes: 13 additions & 1 deletion src/libcore/str/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ use self::pattern::{Searcher, ReverseSearcher, DoubleEndedSearcher};
use char;
use convert::TryFrom;
use fmt;
use iter::{Map, Cloned, FusedIterator};
use iter::{Map, Cloned, FusedIterator, TrustedLen};
use iter_private::TrustedRandomAccess;
use slice::{self, SliceIndex};
use mem;

Expand Down Expand Up @@ -818,6 +819,17 @@ impl<'a> ExactSizeIterator for Bytes<'a> {
#[unstable(feature = "fused", issue = "35602")]
impl<'a> FusedIterator for Bytes<'a> {}

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

#[doc(hidden)]
unsafe impl<'a> TrustedRandomAccess for Bytes<'a> {
unsafe fn get_unchecked(&mut self, i: usize) -> u8 {
self.0.get_unchecked(i)
}
fn may_have_side_effect() -> bool { false }
}

/// This macro generates a Clone impl for string pattern API
/// wrapper types of the form X<'a, P>
macro_rules! derive_pattern_clone {
Expand Down