Rust: Tracking issue for RFC 2306, "Add core::convert::identity"

[Centril]

This is a tracking issue for the RFC "Add fn identity<T>(x: T) -> T { x } to core::convert" (rust-lang/rfcs#2306).

Steps:

• [x] Implement the RFC: https://github.com/rust-lang/rust/pull/47562
• [x] Stabilization PR: https://github.com/rust-lang/rust/pull/57322

Unresolved questions:

None.

[shepmaster]

The documentation suggests:

#![feature(convert_id)]
use std::convert::identity;

let iter = vec![Some(1), None, Some(3)].into_iter();
let filtered = iter.filter_map(identity).collect::<Vec<_>>();
assert_eq!(vec![1, 3], filtered);

I'd argue this is actively non-idiomatic Rust. It is better to use flatten

let iter = vec![Some(1), None, Some(3)].into_iter();
let filtered = iter.flatten().collect::<Vec<_>>();
assert_eq!(vec![1, 3], filtered);

I'm bringing this up here because if we can't have good examples, perhaps the feature doesn't carry it's own weight.

[shepmaster]

The documentation suggests using identity to "do nothing among other interesting functions" and "that changes nothing in a conditional" (which are really the same example). I think this is very weak motivation because it cannot be used with a function of different arity:

#![feature(convert_id)]
use std::convert::identity;

fn manipulation(x: u32, y: u32) -> u32 {
    x + y
}

fn main() {
    let do_stuff = if true { manipulation } else { identity };
    let _results = do_stuff(1, 2);
}

error[E0308]: if and else have incompatible types
 --> src/main.rs:9:20
  |
9 |     let do_stuff = if true { manipulation } else { identity };
  |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ incorrect number of function parameters
  |
  = note: expected type `fn(u32, u32) -> u32 {manipulation}`
             found type `fn(_) -> _ {std::convert::identity::<_>}`

[Centril]

@shepmaster

So .flatten() was added to libcore as a direct consequence of discussing identity.

However, I think that:

let filtered = iter.filter_map(identity).collect::<Vec<_>>();

is more clear with respect to intent.

To know that .flatten() is equivalent to .filter_map(identity) one has to remember the behavior of Option<T> as IntoIterator so I would write the latter rather than the former. Note however that .flatten() is Iterator specific and may not exist for your random type. For example, there is no prop_flatten in Strategy. In such a case .flatten() can't be idiomatic because it does not exist.

I think this is very weak motivation because it cannot be used with a function of different arity:

Given that Rust is a typed language I am unsure as to why one would expect this to work with functions of different arity. This would be the same as expecting impl Fn(A) -> A to also support impl Fn(A, A) -> A. However, if you partially apply manipulation you can use identityand manipulation in a list of boxed closures.

[Centril]

@SimonSapin How do you feel about stabilizing this?

[SimonSapin]

Let’s.

@rfcbot fcp merge

[rfcbot]

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

• [ ] @Kimundi
• [x] @SimonSapin
• [x] @alexcrichton
• [x] @dtolnay
• [x] @sfackler
• [ ] @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

:bell: This is now entering its final comment period, as per the review above. :bell:

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.