Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upRFC: Make the `as` keyword consider `Into` Trait implementations #2308
Conversation
Kerollmops
changed the title
RFC: Add the `as` keyword consider `Into` Trait implementations
RFC: Make the `as` keyword consider `Into` Trait implementations
Jan 21, 2018
Kerollmops
force-pushed the
Kerollmops:as-keyword-consider-into-trait
branch
3 times, most recently
from
94fe115
to
486cbcc
Jan 21, 2018
Kerollmops
force-pushed the
Kerollmops:as-keyword-consider-into-trait
branch
from
486cbcc
to
1285b70
Jan 21, 2018
jonas-schievink
reviewed
Jan 21, 2018
| // is a syntax sugar for | ||
| let x = Foo(42); | ||
| let y = Into::<Bar>::into(x); | ||
| ``` |
This comment has been minimized.
This comment has been minimized.
jonas-schievink
Jan 21, 2018
•
Member
This won't work since as is not equivalent to a call to .into() - for example, as can convert from f32 to u32, while .into() can't (this is a lossy conversion, so )..into() shouldn't work here
This comment has been minimized.
This comment has been minimized.
Kerollmops
Jan 21, 2018
•
Author
Contributor
So what is the rule of as conversion and into conversion ?
I think this is related to this thread about lossy conversions between as and into (I linked it in the Rendered).
This comment has been minimized.
This comment has been minimized.
|
Here is more informations on the actual |
Centril
added
the
T-lang
label
Jan 21, 2018
This comment has been minimized.
This comment has been minimized.
|
I would rather deprecate |
This comment has been minimized.
This comment has been minimized.
|
Not sure I want this, since there’s a difference between a cast/coercion and a conversion. |
This comment has been minimized.
This comment has been minimized.
|
Exactly, and `as` unhelpfully conflates them.. e.g. floating point to
integer conversion and truncating integers.
…On Sun, Jan 21, 2018 at 2:43 PM, Dimitri Sabadie ***@***.***> wrote:
Not sure I want this, since there’s a differenc between a cast/coercion
and a conversion.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2308 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAC3nwkHC6ZdpcLu5F6lT5eiKKVzcUTVks5tM5NtgaJpZM4Rl3p0>
.
|
This comment has been minimized.
This comment has been minimized.
|
@durka isn’t that unsafe anyway? |
This comment has been minimized.
This comment has been minimized.
|
Oh yeah, no warning nor error. It should be! |
This comment has been minimized.
This comment has been minimized.
|
Here is more information on the error emmited by the compiler on lossy conversions, TLDR; no error nor warnings with the |
This comment has been minimized.
This comment has been minimized.
|
The In this RFC I propose to keep the actual warnings and errors that the |
This comment has been minimized.
This comment has been minimized.
|
@durka Why not keeping both good parts of these features ? Keeping the simple |
This comment has been minimized.
This comment has been minimized.
|
My beef is that `as` isn't currently simple. It does both casts and
coercions. I'd be fine merging `as` and Into as long as that means it would
work with generics such as `fn conv<A: Into<B>>(a: A) -> B { a as B }`.
However, currently, `as` supports conversions that Into doesn't, like lossy
integer truncation and unsizing coercions. So I don't see it as simple as
"just combine them".
…On Mon, Jan 22, 2018 at 5:11 PM, Clément Renault ***@***.***> wrote:
@durka <https://github.com/durka> Why not keeping both good parts of
these features ? Keeping the simple as keyword and keeping the warnings
and errors of the trait-based conversions like Into ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2308 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAC3n4GY7aHKbuZ-mkODXpEezKUM5D0Tks5tNQeLgaJpZM4Rl3p0>
.
|
This comment has been minimized.
This comment has been minimized.
|
That was actually just a little idea in my head, just wanted to have some more discussion about it. I think you are right that’s not as simple as « just combine them » but you talked about deprecating it, what about reusing it ? |
This comment has been minimized.
This comment has been minimized.
|
Well that might present backcompat issues, but if there's a way to
integrate `as` better with the trait system (like a std::ops trait so you
can extend it etc) without horribly complicating things then I'm all for it!
…On Mon, Jan 22, 2018 at 5:37 PM, Clément Renault ***@***.***> wrote:
That was actually just a little idea in my head, just wanted to have some
more discution about it.
I think you are right that’s not as simple as « just combine them » but
you talked about deprecating it, what about reusing it ?
|
This comment has been minimized.
This comment has been minimized.
synek317
commented
Jan 23, 2018
|
I think that if we unify cheap
|
This comment has been minimized.
This comment has been minimized.
|
In clippy we actually recommend folks use |
This comment has been minimized.
This comment has been minimized.
|
Yeah, that’s actually what we do in Haskell: we use typeclasses’ methods, casts / coercions are not directly accessible via an operator baked in the language. |
This comment has been minimized.
This comment has been minimized.
alekratz
commented
Jan 23, 2018
|
My understanding of I haven't hacked on the compiler and looked at how |
This comment has been minimized.
This comment has been minimized.
Kixunil
commented
Jan 23, 2018
|
Slightly different idea: make a |
This comment has been minimized.
This comment has been minimized.
I've found this to be a useful guarantee, and the existing impls in libstd certainly comply with this, but the |
This comment has been minimized.
This comment has been minimized.
|
I forgot about another usage of |
This comment has been minimized.
This comment has been minimized.
|
There is an interresting comment about the compiler not emiting enough warnings/errors about unsafe casts/coercions. I propose to use the mem::transmute function to do unsafe conversions like EDIT: transmuting here is probably not possible because of the two possible different memory sizes. |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
What about an |
This comment has been minimized.
This comment has been minimized.
|
True, but that's very untypical (and still safe) |
This comment has been minimized.
This comment has been minimized.
ssokolow
commented
Jan 23, 2018
•
It never occurred to me that Rust would allow such a thing, given past experience with things like DOS game installers that refuse to complete within a typical DOSBox setup because their signed integer free space variable wrapped around to the negative when encountering DOSBox's representation of the free space on a modern-sized hard drive. Is there a warning (core or clippy) that I can |
This comment has been minimized.
This comment has been minimized.
|
Clippy has a pretty nice overview for all the Lints. It has lints for possible truncation, wrap, sign and even precision loss. |
This comment has been minimized.
This comment has been minimized.
ssokolow
commented
Jan 23, 2018
•
|
Ahh, so it's handled like any other int-to-int cast would be with regards to lints? (I think of enums as being distinct from regular ints when it comes to the kinds of guarantees you want to enforce on them, so that possibility didn't occur to me.) In that case, I'll have to think about whether I want to bump those lints up to |
This comment has been minimized.
This comment has been minimized.
|
Please do not use sarcasm in a RFC’s discussion section; it conveys no useful meaning nor material to step forwards. Casting such a big enum to a
|
This comment has been minimized.
This comment has been minimized.
|
My message is not appropriate I have to admit. I just removed it and add a playground to show the actual behavior of casting a big enum to a smaller/un-adapted primitive. This is not unsafe, it's more an unspecified behavior or something like that I would say. I will repeat myself but we must add more warnings in the compiler. Using The |
scottmcm
added
the
T-libs
label
Feb 7, 2018
This comment has been minimized.
This comment has been minimized.
|
Tagging T-libs as well, since the most- |
This comment has been minimized.
This comment has been minimized.
|
@rfcbot fcp close I'm going to propose closing this RFC -- though not because I'm opposed to the general idea. I just don't think the specifics of this proposal feel right. I've not ready the comment thread in detail, and I'm sure the following points have been made, but I'll repeat:
This seems to violate both of those rules, since the (I would perhaps be in favor of some other proposals -- presuming they could be made to work technically -- such as |
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
Mar 1, 2018
•
|
Team member @nikomatsakis has proposed to close 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. |
rfcbot
added
the
proposed-final-comment-period
label
Mar 1, 2018
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
Mar 15, 2018
|
|
rfcbot
added
final-comment-period
and removed
proposed-final-comment-period
labels
Mar 15, 2018
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
Mar 25, 2018
|
The final comment period is now complete. |
This comment has been minimized.
This comment has been minimized.
|
Closing as nothing new has happened during FCP. Thanks for the RFC, @Kerollmops! |
Kerollmops commentedJan 21, 2018
•
edited
Permit to use the common
askeyword with any type that implement theIntoTrait,allowing explicit conversions whose primitives already benefit, more visible than simple function calls.
Rendered
#2308 (comment) In this RFC I propose to keep the actual warnings and errors that the
Intotrait already emit on possible lossy conversions and apply these to the actualaskeyword, combining the two features and removing possible confusions.This keyword is used in the disambiguation of function calls and also in the renaming imports. But it seems those two usage are unrelated to the one I want to change, this is like another keyword with the same name.