Port dead_code lints to be translatable.
#103397
Conversation
|
cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki |
9d9821a to
b754a2b
Compare
| @@ -170,6 +171,36 @@ impl IntoDiagnosticArg for Level { | |||
| } | |||
| } | |||
|
|
|||
| #[derive(Clone)] | |||
| pub struct DiagnosticSymbolList(Vec<Symbol>); | |||
There was a problem hiding this comment.
nit: ideally this is generic over T: Display, and also, ideally there's an And version and an Or version
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
A proper list formatter would basically just need to live on the session/emitter alongside the fluent bundle stuff.
b754a2b to
03cf48c
Compare
|
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, |
03cf48c to
113e8df
Compare
| @@ -170,6 +171,36 @@ impl IntoDiagnosticArg for Level { | |||
| } | |||
| } | |||
|
|
|||
| #[derive(Clone)] | |||
| pub struct DiagnosticSymbolList(Vec<Symbol>); | |||
There was a problem hiding this comment.
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.
|
@bors r+ |
…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
This adds an additional comma to lists with three or more items, to be consistent with list formatters like
icu4x.r? @davidtwco