make Option::cloned generic over Deref, make cloned's tests actually run, add Cloned iterator adaptor

[Gankra]

Edit: whoops, didn't mean to hit post.

Anyway, this is something I tried to do when I first implemented cloned, but couldn't figure out. Somewhere between then and the PR actually landing, we got Deref of references, so now this works! 🎉

Also turns out the test for the functionality was never marked as a #[test]. Oops!

Also added a Cloned iterator adaptor. If this isn't desirable, it can be taken out of the PR (seperate commits).

[Gankra]

r? @aturon

[aturon]

Remember that with DST, Deref<T> is actually a type (and in general Foo<Deref<T>> is a thing). This comment probably should be the more verbose "Maps Option<T> to Option<U> by derefencing and cloning".

[aturon]

Slick. r=me with minor nit addressed.

[aturon]

Slick. r=me with minor nit addressed.

[SimonSapin]

Add #[inline] to this and other one-line functions?

[Gankra]

It's generic, so it's already a candidate for cross-crate inlining. Otherwise I'd rather avoid adding more #[inline]s to std without any performance information.

[Gankra]

@aturon Updated description of Option::cloned, look good to you?

[aturon]

So, I was talking to @alexcrichton about this PR, and realized that I probably approved it a little too hastily; before we stabilize this API, I'd like to know what the concrete use cases are that need to cut through Deref. Use of generics like this always has a tradeoff in complexity/documentation/discoverability, so it's good to make sure there are clear, concrete motivations.

[Gankra]

This was primarily motivated by @SimonSapin's frustration with the ergonomics of getting a by-value iterator over a [u8]. They proposed an explicit by-value iterator, but I suggested this design as more useful and composable.

So now you can do let x = [1,2,3,4]; let y = x.iter().cloned() to get a by-value iterator of the refs. I didn't really dig into their usecase. If you control the iterator's usage, you can always do for &y in x.iter(), so the only place I see this useful is when you need to pass a by-value iterator somewhere else. One example I guess would be x.iter().cloned().collect().

I honestly expected this to be discussed more before being merged! (hence my note of "If this isn't desirable, it can be taken out of the PR")

[aturon]

@gankro Yeah, it was my bad for rubber-stamping it; that's what I get for reviewing in a hurry.

The idea of having cloned on Iterator itself is totally fine, I don't think that needs further discussion (since it matches the situation for Option). It's more a question of doing it generically over Deref, which is somewhat unusual. Let me give it some more thought/experimentation. Anyway, it's something we can tweak during API stabilization.

[Gankra]

Yeah I didn't mark it as unstable or anything, so it's experimental. The primary motivation for Deref is that it captures &T and &mut T. Everything else is just gravy.

[Gankra]

Oh durr I could have done this with IntoOwned, I guess. Which is basically Super Deref+Clone.

err wait, has that landed?

[aturon]

For posterity: we need Deref here to allow cloned to be an inherent method on Option but still support both mutable and immutable refs. See #19097