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

Remove need for &format!(...) or &&"" dances in span_label calls #41745

Merged
merged 1 commit into from
May 8, 2017

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented May 4, 2017

These were always a thorn in my eye. Note that this will monomorphize to two impls, one for String and one for &str. But I think that cost is worth the ergonomics at the call sites that can be seen throughout this PR.

@oli-obk
Copy link
Contributor Author

oli-obk commented May 4, 2017

r? @jonathandturner

@alexcrichton alexcrichton added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 4, 2017
@sophiajt
Copy link
Contributor

sophiajt commented May 7, 2017

Thanks! I always had a mental note to do that but then forgot.

@sophiajt
Copy link
Contributor

sophiajt commented May 7, 2017

@bors r+

@bors
Copy link
Contributor

bors commented May 7, 2017

📌 Commit b18117a has been approved by jonathandturner

@bors
Copy link
Contributor

bors commented May 7, 2017

⌛ Testing commit b18117a with merge 0c85b2b...

@bors
Copy link
Contributor

bors commented May 7, 2017

💔 Test failed - status-travis

@sophiajt
Copy link
Contributor

sophiajt commented May 7, 2017

Looking at Travis, seems like a few places might have been missed:

[00:03:00] error[E0277]: the trait bound `std::string::String: std::convert::From<&&str>` is not satisfied

[00:03:00]     --> /checkout/src/libsyntax/parse/parser.rs:2711:21

[00:03:00]      |

[00:03:00] 2711 |                 err.span_label(span_of_tilde, &"did you mean `!`?");

[00:03:00]      |                     ^^^^^^^^^^ the trait `std::convert::From<&&str>` is not implemented for `std::string::String`

[00:03:00]      |

[00:03:00]      = help: the following implementations were found:

[00:03:00]                <std::string::String as std::convert::From<&'a str>>

[00:03:00]                <std::string::String as std::convert::From<std::boxed::Box<str>>>

[00:03:00]                <std::string::String as std::convert::From<std::borrow::Cow<'a, str>>>

[00:03:00]      = note: required because of the requirements on the impl of `std::convert::Into<std::string::String>` for `&&str`

[00:03:00] 

@oli-obk
Copy link
Contributor Author

oli-obk commented May 8, 2017

Nothing missed. It just bitrots very fast since it touches every single span_labelb. Will fix later

@GuillaumeGomez
Copy link
Member

Well, since @jonathandturner already r+ed and you didn't change much, I'll just r+ as well.

@bors: r=jonathandturner

@bors
Copy link
Contributor

bors commented May 8, 2017

📌 Commit dd87eab has been approved by jonathandturner

@bors
Copy link
Contributor

bors commented May 8, 2017

⌛ Testing commit dd87eab with merge 198917b...

bors added a commit that referenced this pull request May 8, 2017
Remove need for &format!(...) or &&"" dances in `span_label` calls

These were always a thorn in my eye. Note that this will monomorphize to two impls, one for `String` and one for `&str`. But I think that cost is worth the ergonomics at the call sites that can be seen throughout this PR.
@bors
Copy link
Contributor

bors commented May 8, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: jonathandturner
Pushing 198917b to master...

@bors bors merged commit dd87eab into rust-lang:master May 8, 2017
@oli-obk oli-obk deleted the diagnostics branch June 15, 2020 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants