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.

Sign up for GitHub

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

Jump to bottom

Clean up librustc_typeck error_codes file #65965

Merged
merged 1 commit into from
Nov 12, 2019
Merged

Clean up librustc_typeck error_codes file #65965

merged 1 commit into from
Nov 12, 2019

Conversation

GuillaumeGomez
Copy link
Member

@rust-highfive
Copy link
Collaborator

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 30, 2019
@GuillaumeGomez
Copy link
Member Author

r? @Mark-Simulacrum

Mark-Simulacrum
Mark-Simulacrum reviewed Nov 2, 2019
View reviewed changes
src/librustc_typeck/error_codes.rs Outdated
compile-time, and is unable to evaluate arbitrary comparison functions. If you
want to capture values of an orderable type between two end-points, you can use
a guard.
Something else than numbers and characters has been used for a range.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Something else than numbers and characters has been used for a range.
Something other than numbers and characters has been used for a range.

@Mark-Simulacrum
Copy link
Member

I am personally feeling rather against these changes. They seem to move the main description down below the code example, which would at least for me be harmful rather than helpful (the code example isn't usually all that helpful, as I expect it to be essentially the code I've already written!). As such, I would personally not want to r+ this PR. I'm not sure -- but I suspect it'd be good to codify some guidelines before we go through all of these files and change them around. cc @rust-lang/wg-diagnostics -- seems like you'd be interested and would want to get involved.

@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Nov 2, 2019 •
edited
Loading

We did make such rules a few years ago. The structure is supposed to be as follows:

[short description]

Erroneous code example(s):

[failing code example]

[error explanation]

[fixed code example]

cc @steveklabnik

@Mark-Simulacrum
Copy link
Member

Do you have a pointer to somewhere where that's documented? If not, would you be willing to make a PR to the forge? I personally think the previously existing "short descriptions" were better than the one-sentence examples this modifies them to -- and would consider them to be pretty short.

@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Nov 2, 2019 •
edited
Loading

That's why I added @steveklabnik: I don't remember if such resource exists or not and I recall talking about this with him at the time. If it does exist, we need to make it more easily accessible.

@JohnCSimon
Copy link
Member

Ping from triage - this PR has sat idle for a week.
@steveklabnik can you comment on this?
cc: @GuillaumeGomez @Dylan-DPC @Mark-Simulacrum

Thanks

@GuillaumeGomez
Copy link
Member Author

Actually, it has a merged RFC: rust-lang/rfcs#1567

So I guess it's good to go? Unless if we want to change the RFC?

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

Thanks for citing the RFC! It'd be good to perhaps add comments to the top of all the error code files linking to the RFC (http://rust-lang.github.io/rfcs/1567-long-error-codes-explanation-normalization.html)?

@bors
Copy link
Contributor

bors commented Nov 10, 2019

📌 Commit 4a43812049d7e1a5678d71949bdbbd1022f088ef has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 10, 2019
@Mark-Simulacrum
Copy link
Member

@bors r-

Actually, it'd be great to squash this first.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 10, 2019
@GuillaumeGomez GuillaumeGomez force-pushed the clean-up-librustc_typeck-error-codes branch from 4a43812 to db1dd8f Compare November 10, 2019 13:57
@GuillaumeGomez
Copy link
Member Author

Squashed!

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 10, 2019

📌 Commit db1dd8f has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 10, 2019
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 12, 2019
@JohnTitor
Rollup merge of rust-lang#65965 - GuillaumeGomez:clean-up-librustc_ty…
b850620 
…peck-error-codes, r=Mark-Simulacrum

Clean up librustc_typeck error_codes file

r? @Dylan-DPC
@JohnTitor JohnTitor mentioned this pull request Nov 12, 2019
Merged
bors added a commit that referenced this pull request Nov 12, 2019
@bors
Auto merge of #66323 - JohnTitor:rollup-jl8xdk4, r=JohnTitor
a19f934 
Rollup of 11 pull requests

Successful merges:

 - #65965 (Clean up librustc_typeck error_codes file)
 - #66230 (remove vestigial comments referring to defunct numeric trait hierarchy)
 - #66241 (bump openssl version)
 - #66257 (Drop long-section-names linker workaround for windows-gnu)
 - #66263 (make the error message more readable)
 - #66267 (Add rustdoc doc)
 - #66276 (Move lock into CodeStats)
 - #66278 (Fix error message about exported symbols from proc-macro crates)
 - #66280 (Fix HashSet::union performance)
 - #66299 (support issue = "none" in unstable attributes )
 - #66309 (Tiny cleanup to size assertions)

Failed merges:

r? @ghost
@bors bors merged commit db1dd8f into rust-lang:master Nov 12, 2019
@GuillaumeGomez GuillaumeGomez deleted the clean-up-librustc_typeck-error-codes branch November 12, 2019 22:41
@GuillaumeGomez GuillaumeGomez mentioned this pull request Nov 15, 2019
Merged
Centril added a commit to Centril/rust that referenced this pull request Nov 15, 2019
@Centril
Rollup merge of rust-lang#66443 - GuillaumeGomez:port-erased-cleanup,…
3294f41 
… r=Mark-Simulacrum

Port erased cleanup

Just realised that the changes I made in rust-lang#65965 were removed after the move of all error codes so here it is. I made them into separate commits to make the history look better this time.

r? @Mark-Simulacrum
tmandry added a commit to tmandry/rust that referenced this pull request Nov 15, 2019
@tmandry
Rollup merge of rust-lang#66443 - GuillaumeGomez:port-erased-cleanup,…
0f2f03f 
… r=Mark-Simulacrum

Port erased cleanup

Just realised that the changes I made in rust-lang#65965 were removed after the move of all error codes so here it is. I made them into separate commits to make the history look better this time.

r? @Mark-Simulacrum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@GuillaumeGomez @rust-highfive @Mark-Simulacrum @JohnCSimon @bors @Dylan-DPC-zz