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

Migrate rustc_ast_passes diagnostics to SessionDiagnostic and translatable messages (first part) #100694

Merged
merged 16 commits into from
Aug 22, 2022
Merged

Migrate rustc_ast_passes diagnostics to SessionDiagnostic and translatable messages (first part) #100694

merged 16 commits into from
Aug 22, 2022

Conversation

finalchild
Copy link
Contributor

@finalchild finalchild commented Aug 17, 2022 •
edited
Loading

Doing a full migration of the rustc_ast_passes crate.
Making a draft here since there's not yet a tracking issue for the migrations going on.

@rustbot label +A-translation

@rustbot rustbot added A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 17, 2022
@rust-highfive
Copy link
Collaborator

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 17, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@compiler-errors
Copy link
Member

compiler-errors commented Aug 17, 2022 •
edited
Loading

r? diagnosics

ok high-five is not cooperating. feel free to reassign to wg-diagnostics once this is not in draft mode i guess.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@finalchild finalchild marked this pull request as ready for review August 18, 2022 18:32
@rustbot
Copy link
Collaborator

rustbot commented Aug 18, 2022

rustc_macros::diagnostics was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank

@finalchild finalchild changed the title [WIP] Migrate rustc_ast_passes diagnostics to SessionDiagnostic and translatable messages Migrate rustc_ast_passes diagnostics to SessionDiagnostic and translatable messages (first PR) Aug 18, 2022
@JeanCASPAR JeanCASPAR mentioned this pull request Aug 18, 2022
Merged
@finalchild
Copy link
Contributor Author

finalchild commented Aug 18, 2022 •
edited
Loading

While migrating the diagnostics, I had to do some work that might be needed to other contributors too.

  • Modified BufferedEarlyLint.msg to use DiagnosticMessage instead of String
  • Added #[fatal(..)] attribute support to diagnostic macros
  • Fixed a bug in build_format to unescape opening braces properly

The work is far from complete, but it might be better to merge these changes now and open a separate PR for remaining strings.

r? rust-lang/diagnostics

@rust-highfive rust-highfive assigned estebank and unassigned lcnr Aug 18, 2022
@finalchild finalchild changed the title Migrate rustc_ast_passes diagnostics to SessionDiagnostic and translatable messages (first PR) Migrate rustc_ast_passes diagnostics to SessionDiagnostic and translatable messages (first part) Aug 18, 2022
@CleanCut CleanCut mentioned this pull request Aug 18, 2022
Merged
4 tasks
@davidtwco
Copy link
Member

r? @davidtwco

@rust-highfive rust-highfive assigned davidtwco and unassigned estebank Aug 19, 2022
davidtwco
davidtwco approved these changes Aug 19, 2022
View reviewed changes
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

This is fantastic! #[fatal(..)] support is great and will unblock a bunch of others (including me in rustc_incremental!).

compiler/rustc_error_messages/locales/en-US/ast_passes.ftl
functions in traits cannot be declared `async`
.label = `async` because of this
.note = `async` trait functions are not currently supported
.note2 = consider using the `async-trait` crate: https://crates.io/crates/async-trait
Copy link
Member

Choose a reason for hiding this comment

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

nit: it would be nice if this was slightly more descriptive

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 19, 2022

📌 Commit dd4be85b31556e10fd6c509333b1da7acd18a1b6 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 Aug 19, 2022
@davidtwco davidtwco mentioned this pull request Aug 19, 2022
Open
84 tasks
@5225225 5225225 mentioned this pull request Aug 19, 2022
Merged
@davidtwco davidtwco mentioned this pull request Aug 19, 2022
Merged
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@finalchild
Copy link
Contributor Author

r? diagnosics

@finalchild
Copy link
Contributor Author

r? diagnostics

@rust-highfive rust-highfive assigned TaKO8Ki and unassigned davidtwco Aug 21, 2022
@compiler-errors
Copy link
Member

@finalchild please be patient when waiting for reviews. it has been less than a day since you requested a follow-up review from davidtwco, and he may not be working on Rust this weekend.

@TaKO8Ki
Copy link
Member

TaKO8Ki commented Aug 21, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Aug 21, 2022

📌 Commit 09d495c has been approved by TaKO8Ki

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 21, 2022
@finalchild
Copy link
Contributor Author

@compiler-errors Sorry, I was just misunderstanding how the command works. 🙏

@Dylan-DPC Dylan-DPC mentioned this pull request Aug 22, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 22, 2022
@bors
Auto merge of rust-lang#100868 - Dylan-DPC:rollup-a1hfi1r, r=Dylan-DPC
ee8c31e 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#93162 (Std module docs improvements)
 - rust-lang#99386 (Add tests that check `Vec::retain` predicate execution order.)
 - rust-lang#99915 (Recover keywords in trait bounds)
 - rust-lang#100694 (Migrate rustc_ast_passes diagnostics to `SessionDiagnostic` and translatable messages (first part))
 - rust-lang#100757 (Catch overflow early)

Failed merges:

 - rust-lang#99917 (Move Error trait into core)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 57e521e into rust-lang:master Aug 22, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 22, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 23, 2022
@Dylan-DPC
Rollup merge of rust-lang#100768 - Facel3ss1:plugin-impl-translation,…
39c8814 
… r=davidtwco

Migrate `rustc_plugin_impl` to `SessionDiagnostic`

Migration of the `rustc_plugin_impl` crate.
~Draft PR because it is blocked on rust-lang#100694 for `#[fatal(...)]` support~ (this has been merged, and I've changed over to `#[diag(...)]` now too), but I would also like to know if what I did with `LoadPluginError` is okay, because all it does is display the error message from `libloading` ([See conversation on zulip](https://rust-lang.zulipchat.com/#narrow/stream/147480-t-compiler.2Fwg-diagnostics/topic/.23100717.20diagnostic.20translation/near/294327843)). This crate is apparently for a deprecated feature which is used by servo, so I don't know how much this matters anyway.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 23, 2022
@Dylan-DPC
Rollup merge of rust-lang#100768 - Facel3ss1:plugin-impl-translation,…
35f2d12 
… r=davidtwco

Migrate `rustc_plugin_impl` to `SessionDiagnostic`

Migration of the `rustc_plugin_impl` crate.
~Draft PR because it is blocked on rust-lang#100694 for `#[fatal(...)]` support~ (this has been merged, and I've changed over to `#[diag(...)]` now too), but I would also like to know if what I did with `LoadPluginError` is okay, because all it does is display the error message from `libloading` ([See conversation on zulip](https://rust-lang.zulipchat.com/#narrow/stream/147480-t-compiler.2Fwg-diagnostics/topic/.23100717.20diagnostic.20translation/near/294327843)). This crate is apparently for a deprecated feature which is used by servo, so I don't know how much this matters anyway.
@beowolx beowolx mentioned this pull request Aug 23, 2022
Merged
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 24, 2022
@JohnTitor
Rollup merge of rust-lang#100780 - wonchulee:translation_save_analysi…
d0b2295 
…s, r=davidtwco

save_analysis: Migrate diagnostic

* Migrate the `rustc_save_analysis` crate's diagnostic to translatable diagnostic structs.

Depends on rust-lang#100694 and rust-lang#100754 for #[fatal(..)] support, then rust-lang@aa68eb4, rust-lang@f5219a3, rust-lang@7da52f6 can be removed. (I copied commits from rust-lang#100754)
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 24, 2022
@JohnTitor
Rollup merge of rust-lang#100780 - wonchulee:translation_save_analysi…
2e4760c 
…s, r=davidtwco

save_analysis: Migrate diagnostic

* Migrate the `rustc_save_analysis` crate's diagnostic to translatable diagnostic structs.

Depends on rust-lang#100694 and rust-lang#100754 for #[fatal(..)] support, then rust-lang@aa68eb4, rust-lang@f5219a3, rust-lang@7da52f6 can be removed. (I copied commits from rust-lang#100754)
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 26, 2022
@Dylan-DPC
Rollup merge of rust-lang#100724 - JeanCASPAR:migrate-ast_lowering-to…
ef95092 
…-session-diagnostic, r=davidtwco

Migrate ast lowering to session diagnostic

I migrated the whole rustc_ast_lowering crate to session diagnostic *except* the for the use of `span_fatal` at /compiler/rustc_ast_lowering/src/expr.rs#L1268 because `#[fatal(...)]` is not yet supported (see rust-lang#100694).
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 26, 2022
@Dylan-DPC
Rollup merge of rust-lang#100724 - JeanCASPAR:migrate-ast_lowering-to…
acad0ee 
…-session-diagnostic, r=davidtwco

Migrate ast lowering to session diagnostic

I migrated the whole rustc_ast_lowering crate to session diagnostic *except* the for the use of `span_fatal` at /compiler/rustc_ast_lowering/src/expr.rs#L1268 because `#[fatal(...)]` is not yet supported (see rust-lang#100694).
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 26, 2022
@Dylan-DPC
Rollup merge of rust-lang#100724 - JeanCASPAR:migrate-ast_lowering-to…
8a56eb6 
…-session-diagnostic, r=davidtwco

Migrate ast lowering to session diagnostic

I migrated the whole rustc_ast_lowering crate to session diagnostic *except* the for the use of `span_fatal` at /compiler/rustc_ast_lowering/src/expr.rs#L1268 because `#[fatal(...)]` is not yet supported (see rust-lang#100694).
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 26, 2022
@Dylan-DPC
Rollup merge of rust-lang#100724 - JeanCASPAR:migrate-ast_lowering-to…
a0b8f22 
…-session-diagnostic, r=davidtwco

Migrate ast lowering to session diagnostic

I migrated the whole rustc_ast_lowering crate to session diagnostic *except* the for the use of `span_fatal` at /compiler/rustc_ast_lowering/src/expr.rs#L1268 because `#[fatal(...)]` is not yet supported (see rust-lang#100694).
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Aug 26, 2022
@compiler-errors
Rollup merge of rust-lang#100724 - JeanCASPAR:migrate-ast_lowering-to…
bc1d205 
…-session-diagnostic, r=davidtwco

Migrate ast lowering to session diagnostic

I migrated the whole rustc_ast_lowering crate to session diagnostic *except* the for the use of `span_fatal` at /compiler/rustc_ast_lowering/src/expr.rs#L1268 because `#[fatal(...)]` is not yet supported (see rust-lang#100694).
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Aug 31, 2022
@GuillaumeGomez
Rollup merge of rust-lang#100730 - CleanCut:diagnostics-rustc_monomor…
ac7705f 
…phize, r=davidtwco

Migrate rustc_monomorphize to use SessionDiagnostic

### Description

- Migrates diagnostics in `rustc_monomorphize` to use `SessionDiagnostic`
- Adds an `impl IntoDiagnosticArg for PathBuf`

### TODO / Help!
- [x] I'm having trouble figuring out how to apply an optional note. 😕  Help!?
  - Resolved. It was bad docs. Fixed in https://github.com/rust-lang/rustc-dev-guide/pull/1437/files
- [x] `errors:RecursionLimit` should be `#[fatal ...]`, but that doesn't exist so it's `#[error ...]` at the moment.
  - Maybe I can switch after this is merged in? --> rust-lang#100694
  - Or maybe I need to manually implement `SessionDiagnostic` instead of deriving it?
- [x] How does one go about converting an error inside of [a call to struct_span_lint_hir](https://github.com/rust-lang/rust/blob/8064a495086c2e63c0ef77e8e82fe3b9b5dc535f/compiler/rustc_monomorphize/src/collector.rs#L917-L927)?
- [x] ~What placeholder do you use in the fluent template to refer to the value in a vector? It seems like [this code](https://github.com/rust-lang/rust/blob/0b79f758c9aa6646606662a6d623a0752286cd17/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs#L83-L114) ought to have the answer (or something near it)...but I can't figure it out.~ You can't. Punted.
RalfJung added a commit to RalfJung/rust that referenced this pull request Aug 31, 2022
@RalfJung
Rollup merge of rust-lang#100730 - CleanCut:diagnostics-rustc_monomor…
6c4bda6 
…phize, r=davidtwco

Migrate rustc_monomorphize to use SessionDiagnostic

### Description

- Migrates diagnostics in `rustc_monomorphize` to use `SessionDiagnostic`
- Adds an `impl IntoDiagnosticArg for PathBuf`

### TODO / Help!
- [x] I'm having trouble figuring out how to apply an optional note. 😕  Help!?
  - Resolved. It was bad docs. Fixed in https://github.com/rust-lang/rustc-dev-guide/pull/1437/files
- [x] `errors:RecursionLimit` should be `#[fatal ...]`, but that doesn't exist so it's `#[error ...]` at the moment.
  - Maybe I can switch after this is merged in? --> rust-lang#100694
  - Or maybe I need to manually implement `SessionDiagnostic` instead of deriving it?
- [x] How does one go about converting an error inside of [a call to struct_span_lint_hir](https://github.com/rust-lang/rust/blob/8064a495086c2e63c0ef77e8e82fe3b9b5dc535f/compiler/rustc_monomorphize/src/collector.rs#L917-L927)?
- [x] ~What placeholder do you use in the fluent template to refer to the value in a vector? It seems like [this code](https://github.com/rust-lang/rust/blob/0b79f758c9aa6646606662a6d623a0752286cd17/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs#L83-L114) ought to have the answer (or something near it)...but I can't figure it out.~ You can't. Punted.
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Sep 4, 2022
@RalfJung
Rollup merge of #100730 - CleanCut:diagnostics-rustc_monomorphize, r=…
d60e930 
…davidtwco

Migrate rustc_monomorphize to use SessionDiagnostic

### Description

- Migrates diagnostics in `rustc_monomorphize` to use `SessionDiagnostic`
- Adds an `impl IntoDiagnosticArg for PathBuf`

### TODO / Help!
- [x] I'm having trouble figuring out how to apply an optional note. 😕  Help!?
  - Resolved. It was bad docs. Fixed in https://github.com/rust-lang/rustc-dev-guide/pull/1437/files
- [x] `errors:RecursionLimit` should be `#[fatal ...]`, but that doesn't exist so it's `#[error ...]` at the moment.
  - Maybe I can switch after this is merged in? --> rust-lang/rust#100694
  - Or maybe I need to manually implement `SessionDiagnostic` instead of deriving it?
- [x] How does one go about converting an error inside of [a call to struct_span_lint_hir](https://github.com/rust-lang/rust/blob/8064a495086c2e63c0ef77e8e82fe3b9b5dc535f/compiler/rustc_monomorphize/src/collector.rs#L917-L927)?
- [x] ~What placeholder do you use in the fluent template to refer to the value in a vector? It seems like [this code](https://github.com/rust-lang/rust/blob/0b79f758c9aa6646606662a6d623a0752286cd17/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs#L83-L114) ought to have the answer (or something near it)...but I can't figure it out.~ You can't. Punted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SkiFire13 SkiFire13 SkiFire13 left review comments

@davidtwco davidtwco Awaiting requested review from davidtwco

Assignees

@TaKO8Ki TaKO8Ki

Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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
Milestone
1.65.0
Development

Successfully merging this pull request may close these issues.

None yet