Add Itertools::copied

[KamilaBorowska]

This is to pair with cloned. There are other pairs like that in Rust, like copy_from_slice and clone_from_slice. This allows to avoid accidentally calling clone while using cloned method, by allowing to use a specialization that only accepts Copy types.

[bluss]

I'd vote to leave this out, it is a bit trivial and niche.

[joshtriplett]

I'd really like to have this as well. I often find myself writing .cloned() for something that implements Copy, and I'd like to avoid accidentally doing an expensive clone if I refactor somewhere. I feel like .copied() would make my code more self-documenting, and enforce invariants I care about.

[scottmcm]

There are other pairs like that in Rust, like copy_from_slice and clone_from_slice

That's a case where the Copy bound allows a different, probably-more-efficient implementation, though. That wouldn't be the case with an Itertools::copied.

[KamilaBorowska]

Something worth noting is that cloned() doesn't really do more than map(|x| x.clone()) (other than being able to name type of an iterator struct). It's not a more-efficient implementation, considering clone method can run arbitrary code.

[rkarp]

This can actually be implemented much more simply, though I'm not sure if deref or copied would be the best name:

pub trait IteratorDerefExt: Iterator {
    fn deref<'a, T>(self) -> Cloned<Self>
        where Self: Sized + Iterator<Item = &'a T>,
              T: 'a + Clone + Copy
    {
        self.cloned()
    }
}

impl<T> IteratorDerefExt for T
    where T: Iterator {}

And yes, I'm in favor of adding this functionality as well.

[bluss]

Ok this sounds good to me

1. Can we check if this is something std wants to take before we do it?
2. If we take it, a free function copied would be a good addition too (like cloned).

I have a related problem that still doesn't have a good solution. It's a simple one: Declare a function that takes an iterator of f32 values. Make the function generic so that it accepts both iterators of &f32 and f32 in practice. What's the best solution for this?

fn sum_the_values<I>(values: I) -> f32
    where I: IntoIteratorOf<Item=f32>  // New trait?
{
     unimplemented!()
}

The reason that .copied() is not the full answer is that the distance between sum_the_values(&v) and sum_the_values(v.iter().copied()) where v is a slice or Vec is two method calls. With a free function we are down to one function: sum_the_values(copied(&v)), it's a bit better. sum_the_values(&v) would be ideal 🙂

[joshtriplett]

@bluss

Can we check if this is something std wants to take before we do it?

I'm all for it; I'd happily support a patch adding .copied() to core.

I have a related problem that still doesn't have a good solution. It's a simple one: Declare a function that takes an iterator of f32 values. Make the function generic so that it accepts both iterators of &f32 and f32 in practice.

Here's one solution:

https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=0b1520dcda103320ca0aeb0331157f58

fn sum_the_values<B, I>(values: I) -> f32
where
    B: std::borrow::Borrow<f32>,
    I: IntoIterator<Item = B>,
{
    let mut total = 0.0;
    for x in values {
        total += x.borrow();
    }
    total
}

fn main() {
    println!(
        "{} {}",
        sum_the_values(std::iter::repeat(1.1).take(5)),
        sum_the_values(std::iter::repeat(&1.1).take(5))
    );
}

[bluss]

Neat. I think this (playground) is the expression I'd like, to make it easy to reuse. That is, to hide the Borrow magic behind a new trait and iterator adaptor that makes it simple to use.

[bluss]

@joshtriplett .cloned() is already in libcore, and I guess there's a chance we can make a proposal for .copied() go through?

[joshtriplett]

@bluss Seems entirely reasonable.

Anyone interested in writing a patch?

(Skip the copied standalone function, and just add a new iterator adapter .copied(), to make the proposal more likely to go through.)

[KamilaBorowska]

I'm interested in writing a patch.

[joshtriplett]

Given that nightly has this available now, I don't think itertools should add it. Given that, I think this can be closed.