Skip to content

Implement diagnostic translation for rustc-errors #113281

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

Merged
merged 2 commits into from
Jul 27, 2023
Merged

Conversation

dayo05
Copy link
Contributor

@dayo05 dayo05 commented Jul 3, 2023

This is my first PR to rustc yeah~

I'm going to implement diagnostic translation on rustc-errors crate.

This PR is WIP, the reason of opening this as draft, I want to show my code to prevent the issue caused by misunderstanding and also I have few questions.

Some error messages are processed by pluralize! macro which determines to use plural word or not. From now, I make two kinds of keys and combine with enum but I'm not sure is this best method to do it.

Is there any prefered method to do this? => This resolved on conversation on PR.

I'll remain to perform force-push until my first implementation looks good to me

@rustbot
Copy link
Collaborator

rustbot commented Jul 3, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @b-naber (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 3, 2023
@dayo05
Copy link
Contributor Author

dayo05 commented Jul 3, 2023

@rustbot label +A-translation

@rustbot rustbot added the A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic label Jul 3, 2023
@dayo05
Copy link
Contributor Author

dayo05 commented Jul 3, 2023

r? rust-lang/diagnostics

@rustbot rustbot assigned compiler-errors and unassigned b-naber Jul 3, 2023
@rust-log-analyzer

This comment has been minimized.

@dayo05 dayo05 force-pushed the master branch 3 times, most recently from b8e808d to 38e2794 Compare July 3, 2023 12:37
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

@rust-log-analyzer

This comment has been minimized.

@dayo05 dayo05 force-pushed the master branch 4 times, most recently from 6f6be6c to 8c389b1 Compare July 7, 2023 06:40
@dayo05 dayo05 marked this pull request as ready for review July 7, 2023 06:57
@compiler-errors
Copy link
Member

Please try applying what I suggested (specifically the code = "{suggestion}" part) and if that does/doesn't work, mark this as @rustbot review.

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 16, 2023
@dayo05
Copy link
Contributor Author

dayo05 commented Jul 17, 2023

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 17, 2023
@dayo05
Copy link
Contributor Author

dayo05 commented Jul 23, 2023

@compiler-errors Any progress in here?

@compiler-errors
Copy link
Member

r? @davidtwco

@rustbot rustbot assigned davidtwco and unassigned compiler-errors Jul 26, 2023
@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 27, 2023

📌 Commit 05c856a has been approved by davidtwco

It is now in the queue for this repository.

@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 Jul 27, 2023
@bors
Copy link
Collaborator

bors commented Jul 27, 2023

⌛ Testing commit 05c856a with merge f239bb6...

@bors
Copy link
Collaborator

bors commented Jul 27, 2023

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing f239bb6 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 27, 2023
@bors bors merged commit f239bb6 into rust-lang:master Jul 27, 2023
@rustbot rustbot added this to the 1.73.0 milestone Jul 27, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (f239bb6): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
69.5% [2.1%, 136.8%] 2
Improvements ✅
(primary)
-0.4% [-0.4%, -0.3%] 2
Improvements ✅
(secondary)
-0.4% [-0.5%, -0.3%] 3
All ❌✅ (primary) -0.4% [-0.4%, -0.3%] 2

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.9% [0.8%, 3.1%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.9% [0.8%, 3.1%] 2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
176.3% [176.3%, 176.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 652.317s -> 655.772s (0.53%)

@nnethercote
Copy link
Contributor

The huge unused-warnings check incr-unchanged regression here was just a blip and immediately disappeared in #114080. Interesting that it happened on the benchmark that issues a huge number of warnings, given that this was a diagnostic PR.

@dayo05
Copy link
Contributor Author

dayo05 commented Jul 30, 2023

I have no idea about this performance issue, I thought that there are any logical reasons for the cause, because my PR is not related to unused check. I think its just blip.
But, it occured on other PR, so I'll migrate this to other issue soon

@nnethercote
Copy link
Contributor

No need for further perf investigation here, in my opinion.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Jul 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants