-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Str -> String #15075
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
Str -> String #15075
Conversation
Updates #15046 |
What about StrAllocating::to_string? |
Note that #15046 was just a suggestion, no official decision has been made. It also looks like there's some travis errors. I'm saddened that our string conversions keep getting more and more wordier, but I agree that this is more consistent with today's terminology. |
@alexcrichton How about |
This approach has been generally agreed on and I think we can move forward. It is more verbose, following recent trends, but if it ends up being bad we will circle back around and think about ergonomics (we need some wholistic work on string ergonomics anyway). @richo can you rebase? |
Sure, I can do it tonight. |
I've started on a rebase of this. It's complicated by I'm going to try to just temporarily rename |
I'm just finding time to do this now. What's your branch looking like @brson? Should I do it or leave it for you? |
No it's not at all a problem. I'm just interested in seeing issues get fixed :) Just wanted to make sure we weren't duplicating effort, which would suck for all of us. |
Rebased in #15493. |
This updates #15075. Rename `ToStr::to_str` to `ToString::to_string`. The naive renaming ends up with two `to_string` functions defined on strings in the prelude (the other defined via `collections::str::StrAllocating`). To remedy this I removed `StrAllocating::to_string`, making all conversions from `&str` to `String` go through `Show`. This has a measurable impact on the speed of this conversion, but the sense I get from others is that it's best to go ahead and unify `to_string` and address performance for all `to_string` conversions in `core::fmt`. `String::from_str(...)` still works as a manual fast-path. Note that the patch was done with a script, and ended up renaming a number of other `*_to_str` functions, particularly inside of rustc. All the ones I saw looked correct, and I didn't notice any additional API breakage. Closes #15046.
- Renamed as of rust-lang/rust#15075
fix: Add binding definition for for-expr iterator desugared binding
This updates rust-lang/rust#15075. Rename `ToStr::to_str` to `ToString::to_string`. The naive renaming ends up with two `to_string` functions defined on strings in the prelude (the other defined via `collections::str::StrAllocating`). To remedy this I removed `StrAllocating::to_string`, making all conversions from `&str` to `String` go through `Show`. This has a measurable impact on the speed of this conversion, but the sense I get from others is that it's best to go ahead and unify `to_string` and address performance for all `to_string` conversions in `core::fmt`. `String::from_str(...)` still works as a manual fast-path. Note that the patch was done with a script, and ended up renaming a number of other `*_to_str` functions, particularly inside of rustc. All the ones I saw looked correct, and I didn't notice any additional API breakage. Closes #15046.
Opening a PR because my build environment is wedged on this machine.