You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
pnkfelix opened this issue
Jun 20, 2019
· 1 comment
Labels
A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.T-langRelevant to the language team, which will review and decide on the PR/issue.
While perusing the rustc source, I am now seeing instances of String values being constructed and then passed via-reference into a function that wants a String. These would not have compiled without the change added by PR #59825.
It might be worthwhile to try to make a lint that detects the last use of a String that is then being used as an argument to & which is then itself passed to this conversion method (which effectively hides the clone taking place.
(You can see discussion of the hidden clone on #59827; I'm not seeking debate on the merits of PR #59825. I just am wondering if we could easily highlight the unnecessary clones via a lint.)
The text was updated successfully, but these errors were encountered:
Yeah, I'm not sure I like that change. I always liked that potentially expensive operations like clone() were explicit and searchable. Also, I usually think of .into() and ::from as a move, not a copy, since it consumes the callee.
In C++, it's extremely easy to make copies accidentally and while impl From<&String> for String might be reasonably harmless, I feel like an impl <T: Clone> From<&T> for T kinda pushes Rust too far in that direction.
If that feature is here to stay, then I would probably always enable this lint as an error in my projects
A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.T-langRelevant to the language team, which will review and decide on the PR/issue.
PR #59825 added a
impl From<&String> for String
.While perusing the
rustc
source, I am now seeing instances ofString
values being constructed and then passed via-reference into a function that wants aString
. These would not have compiled without the change added by PR #59825.It might be worthwhile to try to make a lint that detects the last use of a
String
that is then being used as an argument to&
which is then itself passed to this conversion method (which effectively hides the clone taking place.(You can see discussion of the hidden clone on #59827; I'm not seeking debate on the merits of PR #59825. I just am wondering if we could easily highlight the unnecessary clones via a lint.)
The text was updated successfully, but these errors were encountered: