Skip to content
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

Add impl From<&String> for String #59827

Closed
jsgf opened this issue Apr 9, 2019 · 6 comments · Fixed by #59825
Closed

Add impl From<&String> for String #59827

jsgf opened this issue Apr 9, 2019 · 6 comments · Fixed by #59825
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@jsgf
Copy link
Contributor

jsgf commented Apr 9, 2019

If a function takes impl Into<String> it would be nice to pass it a &String (say, from pattern matching) without additional friction.

#59825 implements this.

@jonas-schievink jonas-schievink added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 9, 2019
@Stargateur
Copy link
Contributor

Stargateur commented Apr 9, 2019

that could hide unnecessary clone

@jsgf
Copy link
Contributor Author

jsgf commented Apr 9, 2019

This is functionally identical to the existing From<&str> for String impl.

The clone would be necessary - if the receiving function needs to own the String, and all you have is a &String, then someone has to do a clone. If it didn't need to own it then it could take &str (or impl AsRef<str>). And if you had a String to pass in, then it would hit the no-op impl From<T> for T blanket impl.

@hellow554
Copy link
Contributor

hellow554 commented Apr 10, 2019

Do you have an example why this is useful? &String coerces to &str so there is no need to implement this (also https://stackoverflow.com/questions/40006219)

@jsgf
Copy link
Contributor Author

jsgf commented Apr 10, 2019

@hellow554 > &String coerces to &str

No it doesn't, but it Derefs to it. The specific case I had was:

fn somefunction(s: impl Into<String>) {
    println!("a String: {}", s.into())
}

match &someenum {
SomeCase { string_field } => somefunction(string_field), // string_field: &String
...
}

Currently the nicest way to handle this is somefunction(string_field.as_str()), since there's a From<&str> for String.

@hellow554
Copy link
Contributor

Okay, that's true. But you will have the same issue with Vec, &Vec and &[_]. What about that?

@jsgf
Copy link
Contributor Author

jsgf commented Apr 10, 2019

To quote me: #59825 (comment)

Centril added a commit to Centril/rust that referenced this issue May 15, 2019
string: implement From<&String> for String

Allow Strings to be created from borrowed Strings. This is mostly
to make things like passing `&String` to an `impl Into<String>`
parameter frictionless.

Fixes rust-lang#59827.
Centril added a commit to Centril/rust that referenced this issue May 15, 2019
string: implement From<&String> for String

Allow Strings to be created from borrowed Strings. This is mostly
to make things like passing `&String` to an `impl Into<String>`
parameter frictionless.

Fixes rust-lang#59827.
Centril added a commit to Centril/rust that referenced this issue May 16, 2019
string: implement From<&String> for String

Allow Strings to be created from borrowed Strings. This is mostly
to make things like passing `&String` to an `impl Into<String>`
parameter frictionless.

Fixes rust-lang#59827.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants