-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
RFC: Add pub fn identity<T>(x: T) -> T { x }
to core::convert
#2306
Conversation
text/0000-convert-id.md
Outdated
|
||
### Using `id` to concatenate an iterator of iterators | ||
|
||
Given the law `join = (>>= id)`, we use the identity function to perform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence needlessly complicates a simple operation; I don't see any use of including the monad reference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The operation .flat_map(id)
is equivalent to join and may have some use for Haskllers who are reading. I can make the used syntax more Rusty tho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, don't mention monad at all. Rust doesn't typically use that term. There is no reason to include it here. You're writing for Rustaceans, not Haskellers, just say "flatten" and "flat_map"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to choose my battles here =) I tried to come up with a compromise solution... But I don't like the "don't mention the M word" attitude in general.. Rustaceans say "and_then", "flat_map", etc... but they may also say bind, "monad", and "join" at times since Rustaceans are a diverse group and some of them come from Haskell, Scala, F# and other such places. Also, I don't think that shying away from the word Monad helps learnability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that's the point. You aren't optimizing for Haskellers Rustaceans by using "monad". Haskeller Rustaceans know the Rust terminology perfectly well. You only end up disoptimizing for non-FP Rustaceans.
Rustaceans are a diverse group, but there's an option here that is adequate for all Rustaceans.
This is literally the kind of thing that made folks confused by the pi types RFC. Now, this is at a much smaller scale, but I'm trying to stomp out the attitude of using unfamiliar terminology where not necessary in RFCs as much as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a fundamental disagreement here.
Haskeller Rustaceans know the Rust terminology perfectly well.
Haskeller Rustaceans might not know that particular part of Rust... And nonetheless, there might be non-Rustacean functional programmers that might be interested in reading our RFCs or other documentation at some point. Besides, the current text of the RFC explains it in a way that is adequate for all Rustaceans since it includes the following language as well:
In other words we are concatenating an iterator of iterators into a single iterator,
However, in an effort to not make this RFC block on this disagreement I'll agree to remove the language from the RFC.
text/0000-convert-id.md
Outdated
# Summary | ||
[summary]: #summary | ||
|
||
Adds an identity function `pub fn id<T>(x: T) -> T { x }` as `core::convert::id`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel it should be called identity
, id is really short and ambiguous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id
has pecedent in other languages like Haskell. I could go either way, but it being short and known as the identity function elsewhere is nice to have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Identity is a better name in the prelude, if we keep that possibility open. I think we go for not overly abbreviated names of functions in Rust
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also called identity in Java, which is probably more used than Haskell. Though that function may be used less in Java.
Familiarity arguments work if there's uniformity; there isn't, and it feels like plenty of folks won't have ever consciously noticed/used it, so I'd prefer to optimize for folks who don't know what it is. identity
is clear and also should be obvious to those used to id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Manishearth I've provided this motivation for the naming:
Naming the function
identity
instead ofid
is a possibility. However, to make theid
function more appetizing than using a|x| x
, it is preferrable for theidentity
function to have a shorter but still clear name.
Why is id
ambiguous? Are you referring to a potential ambiguity with "identifier"?
And why is this potential ambiguity an overriding concern to the one I've listed?
I think id
is clear from context - If there are other functions around in the context that look like T -> T
, you know it's not going to be "identifier"...
Yes - I suspect it is used way less in Java than in Haskell. I prefer to optimize for folks who are most likely to use the operation, which are probably on balance more functional programmers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not find length to be that pressing a concern when it comes to ergonomics.
Just to clarify: it is the relative length I am concerned about, not the absolute. I'd like to encourage people to use id
instead of |x| x
, but that might be harder with identity
... Does that make sense to you?
[..] more short things [..]
Out of curiosity - since you say more short things - which ones are you referring to?
[..] (but I can't articulate why :/ ) [..]
But if you can, that will make your concern more actionable for me =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point still stands. identity
is more ergonomic than the closure even if it is longer, by virtue of being just an identifier. It also can be used in more contexts because it has the same type as other functions (almost).
I do not think that I'd being shorter than the closure has any benefit. We are rarely trying to optimize for less typing. See also
We already have short functions like drop in the prelude. drop is fine and well known, but adding more feels iffy to me. I feel this is going to clash with locals quite often. Clashes in the prelude are fine, but make things murkier because someone reading the code would expect it to be a local.
(Actually, for that matter there's a general argument to be made against adding functions to the prelude -- there are very few and it's going to be surprising for folks to see a different function that doesn't seem to be imported anywhere. Unless it becomes ubiquitous. Which I suspect it won't.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I'll change it to identity
then by popular demand. What tipped me over was this note by @nox:
88,618 lines with 'id' in Servo.
I think it is necessary and essential that this be in the prelude however and there's certainly precedent for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How embarrassing.
For the record, I forgot to filter the grep command to only *.rs
files and thus it also included our local clone of web-platform-tests
, a massive repository of Web tests running in browsers. There are actually only 1,533 lines with the word id
in Servo.
That's still a huge number to me though, but nowhere near the one I mistakenly reported at first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the function shall henceforth be known as identity
.
pub fn id<T>(x: T) -> T { x }
to core::convertpub fn identity<T>(x: T) -> T { x }
to core::convert
text/0000-convert-id.md
Outdated
|
||
It is already possible to do this in user code by: | ||
|
||
+ using an identity closure: `|x| x`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What specific advantages do you see of identity
over |x| x
? identity
is longer, and I think its meaning would be less obvious to most users, as they'd then have to go looking for the definition of a function called identity
. I suppose there are potential codegen advantages, but those are small at best.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A previous version of the RFC used id
, but it was changed because id
is ambiguous and for those who haven't heard of identity functions very non obvious (it's easy to guess what it means if you see "identity", but not "id")
I assert that length is not the defining factor of ergonomics. identity
is still easier to read and type over the closure, but I agree that the closure just easier to grok for those who haven't seen this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
identity
is still easier to read and type
I guess that's what I'm disputing, but it's totally a matter of opinion. I'd expect people familiar with Rust's syntax to quickly grok |x| x
, while identity
could require looking around for a function called identity
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't speak for everyone, but I always double take when I see |x| x
and the way I grok it is by going "oh right it's just the identity function"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[..] I think its meaning would be less obvious to most users [..]
What users are you referring to specifically here? Current Rust users or all programmers in general? I think I believe you are correct in either case, but still - I have no evidence to offer myself that your assertion is correct, which is less than satisfactory.
What specific advantages do you see of identity over |x| x? identity is longer [..]
Now that the length "advantage" (if we believe it is that) of id
has been removed with the renaming to identity
, I can only offer these thoughts:
-
A lot of functional languages and others have this function, so functional programmers may expect the function to exist while they may be new to the language and don't understand what
|x| x
means without reading about it. Consistency with those (quite many) languages is an argument on its own. -
Understanding
identity
for those that don't already understand (I think functional programmers will) it is a one time cost - this can also be said of closure syntax or any other identifier in the standard library, you have to learn it and that too is a one time setup cost. If you previously didn't know aboutidentity
, reading about it can also be a useful experience learning-wise. -
I believe that using
identity
is more clearly showing that the identity-conversion was intentional compared to|x| x
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Centril Thanks for explaining! Those arguments seem reasonable to me. I personally still think that identity
is less clear, but I'll leave it to others to voice their opinions one way or another, and I won't object to the feature if general consensus is that it would be beneficial.
Another argument for the function over the closure is auto-completion, as tools are getting better. This might may also be an argument for not including it in the prelude, though I haven't seen good support for auto-import in any tools yet. |
@mfarrugi I like your argument in general, however wrt. this:
I think that tools should take into account that there is a prelude, and if a user starts writing |
Strong 👎 👎 👎 for adding it to prelude. From #503,
I don't see how any of these "would otherwise occur in nearly all Rust modules". Furthermore, the rationale for including
Yes including it to prelude is shorter, so what? We are not trying to optimize the language for code-golfing. No one is going to avoid
|
So I agree that this does not hold for It is not about code golfing - it is about eliminating paper cuts for reaching for a trivial operation that you want to use for clarity. Length matters... I think Furthermore, you could add to the rationale for inclusion into the prelude that this choice has precedent among other programming languages. However, if there is consensus around not including it in the prelude, I'll agree to that consensus and change the RFC accordingly. |
@Centril Furthermore, just because
Prelude should not be an ad-space 😞. |
For historical context, Rust used to have a |
The semantic difference was more a later development =) The original frustration was with the longer syntax (that you easily can forget...) =)
I guess that's a kind of "two wrongs don't make a right" argument that I accept. But I am not saying that |
text/0000-convert-id.md
Outdated
Comparing the length of these lines, we see that there's not much difference in | ||
length when defining the function yourself or when importing or using an absolute | ||
path. But the prelude-using variant is considerably shorter. To encourage the | ||
use of the function, exporting to the prelude is therefore a good idea. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the prelude-using variant is considerably shorter.
This is true for literally every possible library addition, so I don't think its a good argument.
text/0000-convert-id.md
Outdated
### Using `identity` to concatenate an iterator of iterators | ||
|
||
We can use the identity function to concatenate an iterator of iterators | ||
into a single iterator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section argues to me more that we should have a flatten
or concat
or something, since I don't find flat_map(identity)
any clearer than flat_map(|x| x)
—neither conveys the intent, to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, let's do .flatten()
=)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And now .flatten()
is in your nightly compiler in a day or so. Tracking issue: rust-lang/rust#48213
I want to bring up a point where the @bluss has a good blog post where they describe the effects of passing ownership into and immediately out of the |
I think that @KiChjang That's mostly trivia and fun. Here's another as a bonus, we already have an identity function in the prelude -- for any type |
Sounds similar to |
As someone who has less experience with functional language jargon, if I see |
Additional bikeshed datum: To me "identity" first means the identity function (I don't think this has much to do with functional languages, it seems to come up everywhere for me), and second means identity elements in math such as 0 being the additive identity and 1 the multiplicative identity and so on. I would only start thinking about identification of a person or object in the physical world if it was abbreviated to "id", or if we were talking about philosophy for some reason. Personally I can't come up with a better name for this function than |
This is an identity conversion =) Move semantics does not change this. |
error[E0382]: use of moved value: `x`
--> src/main.rs:6:16
|
6 | identity(x) == x
| - ^ value used here after move
| |
| value moved here
|
= note: move occurs because `x` has type `std::string::String`, which does not implement the `Copy` trait I rather like bluss's |
I meant If and when Haskell adds linear types, they'd just change the definition of id :: forall a. a ->. a -- ->. is the linear arrow
id x = x and that is fine, since it would still be the identity function.
While I sympathize with that goal, I think I believe the identity function is also most useful when passed to higher order functions or when stored among other functions in maps, etc. and not when applied directly as |
I believe at this stage consensus for inclusion into the prelude does not exist and is unlikely to develop. I also do not have more arguments to present in favor of prelude inclusion. Is this an accurate description of where we're at? If so; I move to strike inclusion into the prelude and propose that we simply have |
I'd rather write |
I agree that |
For that I normally use |
The libs team discussed this RFC today and those of us present decided we like We are not sold on including it in the prelude at this point. @Centril please update that part of the RFC per #2306 (comment). Thanks! While this bakes in nightly we are open to suggestions of a better module to expose this function in (please follow up in the tracking issue if you have ideas), but @rfcbot fcp merge |
Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged teams: 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. |
@dtolnay Done :) |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period, with a disposition to merge, as per the review above, is now complete. |
Huzzah! This RFC has been merged! Tracking issue: rust-lang/rust#53500 |
Add the identity function as core::convert::identity ## New notes This implements rust-lang/rfcs#2306 (see #53500). ## Old notes (ignore this in new reviews) Adds the identity function `fn id<T>(x: T) -> T { x }` to core::convert and the prelude. Some motivations for why this is useful are explained in the doc tests. Another is that using the identity function instead of `{ x }` or `|x| x` makes it clear that you intended to use an identity conversion on purpose. The reasoning: + behind adding this to `convert` and not `mem` is that this is an identity *conversion*. + for adding this to the prelude is that it should be easy enough to use that the ease of writing your own identity function or using a closure `|x| x` doesn't overtake that. I've separated this out into two feature gates so that the addition to the prelude can be considered and stabilized separately. cc @bluss
Do we need a clippy lint that will point out to |
@pravic It would need to be very carefully written to be helpful, I think. It's not uncommon for |
That's easy to avoid (we do this for a bunch of lints already), but a more pressing concern is that unless |
@scottmcm Hmm, any examples? |
fn f(q: Option<!>) -> Option<u8> {
q.map(|x| x)
} |
More generally: https://bluss.github.io/rust/fun/2015/10/11/stuff-the-identity-function-does/ |
Add the identity function as core::convert::identity ## New notes This implements rust-lang/rfcs#2306 (see rust-lang/rust#53500). ## Old notes (ignore this in new reviews) Adds the identity function `fn id<T>(x: T) -> T { x }` to core::convert and the prelude. Some motivations for why this is useful are explained in the doc tests. Another is that using the identity function instead of `{ x }` or `|x| x` makes it clear that you intended to use an identity conversion on purpose. The reasoning: + behind adding this to `convert` and not `mem` is that this is an identity *conversion*. + for adding this to the prelude is that it should be easy enough to use that the ease of writing your own identity function or using a closure `|x| x` doesn't overtake that. I've separated this out into two feature gates so that the addition to the prelude can be considered and stabilized separately. cc @bluss
RENDERED
TRACKING ISSUE
Adds an identity function
pub fn identity<T>(x: T) -> T { x }
ascore::convert::identity
.The function is also re-exported to
std::convert::identity
as well as the prelude ofboth libcore and libstd.
Linking rust-lang/rust#47562
cc @bluss @Manishearth