-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Provide more explanation for Deref in String docs #43721
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
src/liballoc/string.rs
Outdated
/// | ||
/// What would work in this case is changing the line | ||
/// `example_func(&example_string);` to | ||
/// `example_func(example_string.to_str());`. This works because we're doing |
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 the idiomatic solution is call it like this:
example_func(&*example_string);
You dereference a String
into a str
, and then reference the str
back to a &str
.
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 think that &*
is slightly more idiomatic, but it's also harder to understand. I think showing both is probably a good idea, because people will see &*
in the wild.
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.
Cool, I'll make the change. I like the idea of having both options in there, thanks!
src/liballoc/string.rs
Outdated
/// ``` | ||
/// | ||
/// What would work in this case is changing the line | ||
/// `example_func(&example_string);` to |
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 line has a trailing whitespace making CI to reject the commit.
[00:03:44] tidy error: /checkout/src/liballoc/string.rs:187: trailing whitespace
[00:03:45] some tidy checks failed
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.
Sorry, one last tiny nit! r=me after this is fixed
src/liballoc/string.rs
Outdated
/// | ||
/// In certain cases Rust doesn't have enough information to make this | ||
/// conversion, known as deref coercion. In the following example a string |
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 should be
`Deref`
since it's the name of a trait
@bors: r+ rollup |
📌 Commit fac6ce7 has been approved by |
Provide more explanation for Deref in String docs While working on a different project I encountered a point of confusion where using `&String` to dereference a `String` into `&str` did not compile. I found the explanation of [String Deref](https://doc.rust-lang.org/std/string/struct.String.html#deref), thought that it matched what I was trying to do, and was confused as to why my program did not compile when the docs stated that it would work with 'any function which takes a `&str`'. At the bottom it is mentioned that this will 'generally' work, unless `String` is needed, but I found this statement confusing based on the previous claim of 'any'. Looking further into the docs I was able to find the function `as_str()` that works instead. I thought it might be helpful to mention here deref coercion, an instance in which using `&String` does not work, to explain why it does not work, then direct users to a different option that should work in this instance. A user casually skimming the page will likely come to this explanation first, then find `as_str()` later, but be no the wiser as to what potentially went wrong. r? @steveklabnik
Provide more explanation for Deref in String docs While working on a different project I encountered a point of confusion where using `&String` to dereference a `String` into `&str` did not compile. I found the explanation of [String Deref](https://doc.rust-lang.org/std/string/struct.String.html#deref), thought that it matched what I was trying to do, and was confused as to why my program did not compile when the docs stated that it would work with 'any function which takes a `&str`'. At the bottom it is mentioned that this will 'generally' work, unless `String` is needed, but I found this statement confusing based on the previous claim of 'any'. Looking further into the docs I was able to find the function `as_str()` that works instead. I thought it might be helpful to mention here deref coercion, an instance in which using `&String` does not work, to explain why it does not work, then direct users to a different option that should work in this instance. A user casually skimming the page will likely come to this explanation first, then find `as_str()` later, but be no the wiser as to what potentially went wrong. r? @steveklabnik
While working on a different project I encountered a point of confusion where using
&String
to dereference aString
into&str
did not compile. I found the explanation of String Deref, thought that it matched what I was trying to do, and was confused as to why my program did not compile when the docs stated that it would work with 'any function which takes a&str
'. At the bottom it is mentioned that this will 'generally' work, unlessString
is needed, but I found this statement confusing based on the previous claim of 'any'. Looking further into the docs I was able to find the functionas_str()
that works instead.I thought it might be helpful to mention here deref coercion, an instance in which using
&String
does not work, to explain why it does not work, then direct users to a different option that should work in this instance. A user casually skimming the page will likely come to this explanation first, then findas_str()
later, but be no the wiser as to what potentially went wrong.r? @steveklabnik