Port `dead_code` lints to be translatable. #103397

crlf0710 · 2022-10-22T10:51:35Z

This adds an additional comma to lists with three or more items, to be consistent with list formatters like icu4x.

rustbot · 2022-10-22T10:51:38Z

rustc_error_messages was changed

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

Manishearth · 2022-10-22T19:06:59Z

compiler/rustc_errors/src/diagnostic_impls.rs

@@ -170,6 +171,36 @@ impl IntoDiagnosticArg for Level {
    }
 }

+#[derive(Clone)]
+pub struct DiagnosticSymbolList(Vec<Symbol>);


nit: ideally this is generic over T: Display, and also, ideally there's an And version and an Or version

honestly I think the right path forward is to give the custom derive first-class support for vecs and special case them, because it can't use IntoDiagnosticArg anyway once we're using a real list formatter since there's no way to pass in the list formatter.

But this is still a positive step.

(I'm wondering what @davidtwco thinks)

re: Generic over T: Display or T: IntoDiagnosticArg, yes that's better in the long run. However the IntoDiagnosticArg impl for Symbol Currently doesn't emit the `delimiter that many diagnostic message expected. That would change a lot of code, so i'd prefer leave that to a follow work. Created #103422

For accessing the list formatter i don't think it's hard, we can just store tcx inside the DiagnosticSymbolList, so it will have access to any necessary information.

I've thought before that we'd need some optional way of having a TyCtxt field in diagnostic structs that we could annotate with #[tcx] so that the derive knows about it. We could thread that through to a IntoDiagnosticArg impl. I also want something like that to be able to support DefId fields that we can annotate with #[primary_span(def_span)] to avoid having to call def_span in the struct creation, or with #[def_path] and things like that.

I think if we did this then we could avoid making the derive special-case some types too much more than it does now, which I think would be good.

I don't have a good sense of what integrating a proper list formatter looks like in terms of support in the rest of the infrastructure.

I think what this is doing now is an improvement though.

A proper list formatter would basically just need to live on the session/emitter alongside the fluent bundle stuff.

compiler/rustc_errors/src/diagnostic_impls.rs

Noratrieb · 2022-10-23T11:46:03Z

Hi, I've seen you changed some diagnostic structs in your PR. After #103345, the way we refer to fluent messages changed. They are now in a flat namespace with the same identifier as in the fluent file. For example, parser::cool_thing is now parser_cool_thing and parser::suggestion just suggestion.
You should rebase to the latest master and change your fluent message references as described above. Thanks!

davidtwco

Apologies for the delay in responding, some minor comments then r=me

compiler/rustc_passes/src/errors.rs

davidtwco · 2022-11-02T13:45:03Z

compiler/rustc_errors/src/diagnostic_impls.rs

@@ -170,6 +171,36 @@ impl IntoDiagnosticArg for Level {
    }
 }

+#[derive(Clone)]
+pub struct DiagnosticSymbolList(Vec<Symbol>);


I've thought before that we'd need some optional way of having a TyCtxt field in diagnostic structs that we could annotate with #[tcx] so that the derive knows about it. We could thread that through to a IntoDiagnosticArg impl. I also want something like that to be able to support DefId fields that we can annotate with #[primary_span(def_span)] to avoid having to call def_span in the struct creation, or with #[def_path] and things like that.

I think if we did this then we could avoid making the derive special-case some types too much more than it does now, which I think would be good.

I don't have a good sense of what integrating a proper list formatter looks like in terms of support in the rest of the infrastructure.

I think what this is doing now is an improvement though.

davidtwco · 2022-11-04T14:33:21Z

@bors r+

bors · 2022-11-04T14:33:23Z

📌 Commit a777c46 has been approved by davidtwco

It is now in the queue for this repository.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#103367 (Remove std's transitive dependency on cfg-if 0.1) - rust-lang#103397 (Port `dead_code` lints to be translatable.) - rust-lang#103681 (libtest: run all tests in their own thread, if supported by the host) - rust-lang#103792 (Migrate `codegen_ssa` to diagnostics structs - [Part 2]) - rust-lang#103897 (asm: Work around LLVM bug on AArch64) - rust-lang#103937 (minor changes to make method lookup diagnostic code easier to read) - rust-lang#103958 (Test tidy should not count untracked paths towards entries limit) - rust-lang#103964 (Give a specific lint for unsafety not being inherited) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned davidtwco Oct 22, 2022

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 Oct 22, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 22, 2022

crlf0710 mentioned this pull request Oct 22, 2022

Diagnostic Translation #100717

Open

84 tasks

crlf0710 force-pushed the port_dead_code_lint branch 2 times, most recently from 9d9821a to b754a2b Compare October 22, 2022 11:12

Manishearth reviewed Oct 22, 2022

View reviewed changes

crlf0710 force-pushed the port_dead_code_lint branch from b754a2b to 03cf48c Compare October 23, 2022 02:40

crlf0710 mentioned this pull request Oct 23, 2022

Move the emitting of inline code delimiters for Symbol inside the IntoDiagnosticArg impl #103422

Open

Port dead_code lints to be translatable.

113e8df

crlf0710 force-pushed the port_dead_code_lint branch from 03cf48c to 113e8df Compare October 24, 2022 09:02

davidtwco reviewed Nov 2, 2022

View reviewed changes

crlf0710 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 Nov 3, 2022

SLASHLogin mentioned this pull request Nov 3, 2022

Migrate rustc_codegen_llvm to SessionDiagnostics #101005

Merged

Use derive(Subdiagnostic) for ChangeFieldsToBeOfUnitType.

a777c46

crlf0710 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 Nov 3, 2022

davidtwco approved these changes Nov 4, 2022

View reviewed changes

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 4, 2022

matthiaskrgr mentioned this pull request Nov 4, 2022

Rollup of 8 pull requests #103978

Merged

bors merged commit 612bb78 into rust-lang:master Nov 4, 2022

rustbot added this to the 1.67.0 milestone Nov 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port `dead_code` lints to be translatable. #103397

Port `dead_code` lints to be translatable. #103397

crlf0710 commented Oct 22, 2022 •

edited

Loading

rustbot commented Oct 22, 2022

Manishearth Oct 22, 2022

Manishearth Oct 22, 2022

crlf0710 Oct 23, 2022

crlf0710 Oct 23, 2022

davidtwco Nov 2, 2022 •

edited

Loading

Manishearth Nov 3, 2022

Noratrieb commented Oct 23, 2022

davidtwco left a comment

davidtwco Nov 2, 2022 •

edited

Loading

davidtwco commented Nov 4, 2022

bors commented Nov 4, 2022

Port dead_code lints to be translatable. #103397

Port dead_code lints to be translatable. #103397

Conversation

crlf0710 commented Oct 22, 2022 • edited Loading

rustbot commented Oct 22, 2022

Manishearth Oct 22, 2022

Choose a reason for hiding this comment

Manishearth Oct 22, 2022

Choose a reason for hiding this comment

crlf0710 Oct 23, 2022

Choose a reason for hiding this comment

crlf0710 Oct 23, 2022

Choose a reason for hiding this comment

davidtwco Nov 2, 2022 • edited Loading

Choose a reason for hiding this comment

Manishearth Nov 3, 2022

Choose a reason for hiding this comment

Noratrieb commented Oct 23, 2022

davidtwco left a comment

Choose a reason for hiding this comment

davidtwco Nov 2, 2022 • edited Loading

Choose a reason for hiding this comment

davidtwco commented Nov 4, 2022

bors commented Nov 4, 2022

Port `dead_code` lints to be translatable. #103397

Port `dead_code` lints to be translatable. #103397

crlf0710 commented Oct 22, 2022 •

edited

Loading

davidtwco Nov 2, 2022 •

edited

Loading

davidtwco Nov 2, 2022 •

edited

Loading