-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
builtin_macros: Migrate diagnostic #101935
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
The job Click to see the possible cause of the failure (guessed by this bot)
|
☔ The latest upstream changes (presumably #100996) made this pull request unmergeable. Please resolve the merge conflicts. |
@rustbot author |
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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the delay in reviewing this, this is a great start - I assume from the CI failures that there are some diagnostics still to migrate.
#[primary_span] | ||
#[label] | ||
pub(crate) span: Span, | ||
// FIXME: This should be a `tool_only_span_suggestion` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we've got support for this now after #103575.
WrappingNumberInArray { span: Span, snippet: String }, | ||
} | ||
|
||
impl AddSubdiagnostic for Snippet { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that you can use the derive for this.
@wonchulee any updates on this? |
Hello, This is still a work in progress, and as I mentioned in #100717, I don't have enough time to finish it these days. If someone would like to take it on, that would be great. I'll try to keep going as long as I can until then. |
Hi @wonchulee, I'm going to close this since you're not sure when you'll have time to work on it. Thank you for the effort you've put in already, and feel free to pick up the work later if you have more time :) (although do be aware that #110092 migrated a lot of the diagnostics in the meantime, so it might make sense to pick a crate other than rustc_builtin_macros). |
…, r=davidtwco Migrate some rustc_builtin_macros to SessionDiagnostic <!-- If this PR is related to an unstable feature or an otherwise tracked effort, please link to the relevant tracking issue here. If you don't know of a related tracking issue or there are none, feel free to ignore this. This PR will get automatically assigned to a reviewer. In case you would like a specific user to review your work, you can assign it to them by using r? <reviewer name> --> Part of rust-lang#100717. pick up abandoned pr: rust-lang#101935 `@rustbot` label +A-translation
Rollup merge of rust-lang#126405 - He1pa:translate_builtin_macro_diag, r=davidtwco Migrate some rustc_builtin_macros to SessionDiagnostic <!-- If this PR is related to an unstable feature or an otherwise tracked effort, please link to the relevant tracking issue here. If you don't know of a related tracking issue or there are none, feel free to ignore this. This PR will get automatically assigned to a reviewer. In case you would like a specific user to review your work, you can assign it to them by using r? <reviewer name> --> Part of rust-lang#100717. pick up abandoned pr: rust-lang#101935 `@rustbot` label +A-translation
rustc_biltin_macros
crate's diagnostic to translatable diagnostic structs.