Skip to content

Commit

Permalink
Auto merge of #56534 - xfix:copied, r=@SimonSapin
Browse files Browse the repository at this point in the history
Add unstable Iterator::copied()

Initially suggested at rust-itertools/itertools#289, however the maintainers of itertools suggested this may be better of in a standard library.

The intent of `copied` is to avoid accidentally cloning iterator elements after doing a code refactoring which causes a structure to be no longer `Copy`. This is a relatively common pattern, as it can be seen by calling `rg --pcre2 '[.]map[(][|](?:(\w+)[|] [*]\1|&(\w+)[|] \2)[)]'` on Rust main repository. Additionally, many uses of `cloned` actually want to simply `Copy`, and changing something to be no longer copyable may introduce unnoticeable performance penalty.

Also, this makes sense because the standard library includes `[T].copy_from_slice` to pair with `[T].clone_from_slice`.

This also adds `Option::copied`, because it makes sense to pair it with `Iterator::copied`. I don't think this feature is particularly important, but it makes sense to update `Option` along with `Iterator` for consistency.
  • Loading branch information
bors committed Dec 26, 2018
2 parents 79d8a0f + 315401d commit a7be40c
Show file tree
Hide file tree
Showing 6 changed files with 212 additions and 1 deletion.
31 changes: 30 additions & 1 deletion src/libcore/iter/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use cmp::Ordering;
use ops::Try;

use super::LoopState;
use super::{Chain, Cycle, Cloned, Enumerate, Filter, FilterMap, Fuse};
use super::{Chain, Cycle, Copied, Cloned, Enumerate, Filter, FilterMap, Fuse};
use super::{Flatten, FlatMap, flatten_compat};
use super::{Inspect, Map, Peekable, Scan, Skip, SkipWhile, StepBy, Take, TakeWhile, Rev};
use super::{Zip, Sum, Product};
Expand Down Expand Up @@ -2225,6 +2225,35 @@ pub trait Iterator {
(ts, us)
}

/// Creates an iterator which copies all of its elements.
///
/// This is useful when you have an iterator over `&T`, but you need an
/// iterator over `T`.
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// #![feature(iter_copied)]
///
/// let a = [1, 2, 3];
///
/// let v_cloned: Vec<_> = a.iter().copied().collect();
///
/// // copied is the same as .map(|&x| x)
/// let v_map: Vec<_> = a.iter().map(|&x| x).collect();
///
/// assert_eq!(v_cloned, vec![1, 2, 3]);
/// assert_eq!(v_map, vec![1, 2, 3]);
/// ```
#[unstable(feature = "iter_copied", issue = "57127")]
fn copied<'a, T: 'a>(self) -> Copied<Self>
where Self: Sized + Iterator<Item=&'a T>, T: Copy
{
Copied { it: self }
}

/// Creates an iterator which [`clone`]s all of its elements.
///
/// This is useful when you have an iterator over `&T`, but you need an
Expand Down
100 changes: 100 additions & 0 deletions src/libcore/iter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,106 @@ impl<I> FusedIterator for Rev<I>
unsafe impl<I> TrustedLen for Rev<I>
where I: TrustedLen + DoubleEndedIterator {}

/// An iterator that copies the elements of an underlying iterator.
///
/// This `struct` is created by the [`copied`] method on [`Iterator`]. See its
/// documentation for more.
///
/// [`copied`]: trait.Iterator.html#method.copied
/// [`Iterator`]: trait.Iterator.html
#[unstable(feature = "iter_copied", issue = "57127")]
#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
#[derive(Clone, Debug)]
pub struct Copied<I> {
it: I,
}

#[unstable(feature = "iter_copied", issue = "57127")]
impl<'a, I, T: 'a> Iterator for Copied<I>
where I: Iterator<Item=&'a T>, T: Copy
{
type Item = T;

fn next(&mut self) -> Option<T> {
self.it.next().copied()
}

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

fn try_fold<B, F, R>(&mut self, init: B, mut f: F) -> R where
Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try<Ok=B>
{
self.it.try_fold(init, move |acc, &elt| f(acc, elt))
}

fn fold<Acc, F>(self, init: Acc, mut f: F) -> Acc
where F: FnMut(Acc, Self::Item) -> Acc,
{
self.it.fold(init, move |acc, &elt| f(acc, elt))
}
}

#[unstable(feature = "iter_copied", issue = "57127")]
impl<'a, I, T: 'a> DoubleEndedIterator for Copied<I>
where I: DoubleEndedIterator<Item=&'a T>, T: Copy
{
fn next_back(&mut self) -> Option<T> {
self.it.next_back().copied()
}

fn try_rfold<B, F, R>(&mut self, init: B, mut f: F) -> R where
Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try<Ok=B>
{
self.it.try_rfold(init, move |acc, &elt| f(acc, elt))
}

fn rfold<Acc, F>(self, init: Acc, mut f: F) -> Acc
where F: FnMut(Acc, Self::Item) -> Acc,
{
self.it.rfold(init, move |acc, &elt| f(acc, elt))
}
}

#[unstable(feature = "iter_copied", issue = "57127")]
impl<'a, I, T: 'a> ExactSizeIterator for Copied<I>
where I: ExactSizeIterator<Item=&'a T>, T: Copy
{
fn len(&self) -> usize {
self.it.len()
}

fn is_empty(&self) -> bool {
self.it.is_empty()
}
}

#[unstable(feature = "iter_copied", issue = "57127")]
impl<'a, I, T: 'a> FusedIterator for Copied<I>
where I: FusedIterator<Item=&'a T>, T: Copy
{}

#[doc(hidden)]
unsafe impl<'a, I, T: 'a> TrustedRandomAccess for Copied<I>
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 {
I::may_have_side_effect()
}
}

#[unstable(feature = "iter_copied", issue = "57127")]
unsafe impl<'a, I, T: 'a> TrustedLen for Copied<I>
where I: TrustedLen<Item=&'a T>,
T: Copy
{}

/// An iterator that clones the elements of an underlying iterator.
///
/// This `struct` is created by the [`cloned`] method on [`Iterator`]. See its
Expand Down
42 changes: 42 additions & 0 deletions src/libcore/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,48 @@ impl<T> Option<T> {
}
}

impl<'a, T: Copy> Option<&'a T> {
/// Maps an `Option<&T>` to an `Option<T>` by copying the contents of the
/// option.
///
/// # Examples
///
/// ```
/// #![feature(copied)]
///
/// let x = 12;
/// let opt_x = Some(&x);
/// assert_eq!(opt_x, Some(&12));
/// let copied = opt_x.copied();
/// assert_eq!(copied, Some(12));
/// ```
#[unstable(feature = "copied", issue = "57126")]
pub fn copied(self) -> Option<T> {
self.map(|&t| t)
}
}

impl<'a, T: Copy> Option<&'a mut T> {
/// Maps an `Option<&mut T>` to an `Option<T>` by copying the contents of the
/// option.
///
/// # Examples
///
/// ```
/// #![feature(copied)]
///
/// let mut x = 12;
/// let opt_x = Some(&mut x);
/// assert_eq!(opt_x, Some(&mut 12));
/// let copied = opt_x.copied();
/// assert_eq!(copied, Some(12));
/// ```
#[unstable(feature = "copied", issue = "57126")]
pub fn copied(self) -> Option<T> {
self.map(|&mut t| t)
}
}

impl<'a, T: Clone> Option<&'a T> {
/// Maps an `Option<&T>` to an `Option<T>` by cloning the contents of the
/// option.
Expand Down
17 changes: 17 additions & 0 deletions src/libcore/tests/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1253,6 +1253,23 @@ fn test_rev() {
vec![16, 14, 12, 10, 8, 6]);
}

#[test]
fn test_copied() {
let xs = [2, 4, 6, 8];

let mut it = xs.iter().copied();
assert_eq!(it.len(), 4);
assert_eq!(it.next(), Some(2));
assert_eq!(it.len(), 3);
assert_eq!(it.next(), Some(4));
assert_eq!(it.len(), 2);
assert_eq!(it.next_back(), Some(8));
assert_eq!(it.len(), 1);
assert_eq!(it.next_back(), Some(6));
assert_eq!(it.len(), 0);
assert_eq!(it.next_back(), None);
}

#[test]
fn test_cloned() {
let xs = [2, 4, 6, 8];
Expand Down
2 changes: 2 additions & 0 deletions src/libcore/tests/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![feature(box_syntax)]
#![feature(cell_update)]
#![feature(copied)]
#![feature(core_private_bignum)]
#![feature(core_private_diy_float)]
#![feature(dec2flt)]
Expand All @@ -9,6 +10,7 @@
#![feature(flt2dec)]
#![feature(fmt_internals)]
#![feature(hashmap_internals)]
#![feature(iter_copied)]
#![feature(iter_nth_back)]
#![feature(iter_unfold)]
#![feature(pattern)]
Expand Down
21 changes: 21 additions & 0 deletions src/libcore/tests/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,27 @@ fn test_collect() {
assert!(v == None);
}

#[test]
fn test_copied() {
let val = 1;
let val_ref = &val;
let opt_none: Option<&'static u32> = None;
let opt_ref = Some(&val);
let opt_ref_ref = Some(&val_ref);

// None works
assert_eq!(opt_none.clone(), None);
assert_eq!(opt_none.copied(), None);

// Immutable ref works
assert_eq!(opt_ref.clone(), Some(&val));
assert_eq!(opt_ref.copied(), Some(1));

// Double Immutable ref works
assert_eq!(opt_ref_ref.clone(), Some(&val_ref));
assert_eq!(opt_ref_ref.clone().copied(), Some(&val));
assert_eq!(opt_ref_ref.copied().copied(), Some(1));
}

#[test]
fn test_cloned() {
Expand Down

0 comments on commit a7be40c

Please sign in to comment.