-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
More translatable diagnostics #123822
More translatable diagnostics #123822
Conversation
This looks fine to me from what I understand about it, but I am not an expert on the fluent machinery. The review should perhaps be assigned to @davidtwco or someone else who is more knowledgable there. In any case, since this is marked as draft PR, I'm going to switch it back to waiting-on-author, to get it out of my review queue while it is in draft. @rustbot author |
ca845fe
to
6d0b4f2
Compare
This comment has been minimized.
This comment has been minimized.
compiler/rustc_session/src/errors.rs
Outdated
f: F, | ||
) { | ||
if let Some(issue) = self.issue { | ||
issue.add_to_diag_with(diag, |diag, msg| f(diag, msg)); |
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'm not happy about this, ideally F
would be a reference so it can be copied into nested add_to_diag_with()
calls instead of being wrappen in another closure.
b41bc78
to
0094065
Compare
This comment has been minimized.
This comment has been minimized.
0094065
to
a1d7d38
Compare
This comment has been minimized.
This comment has been minimized.
a1d7d38
to
99f8205
Compare
This comment has been minimized.
This comment has been minimized.
99f8205
to
b1ae167
Compare
This comment has been minimized.
This comment has been minimized.
b1ae167
to
fb1b758
Compare
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #124424) made this pull request unmergeable. Please resolve the merge conflicts. |
fb1b758
to
f887d9f
Compare
☔ The latest upstream changes (presumably #125219) made this pull request unmergeable. Please resolve the merge conflicts. |
@Xiretza can you resolve the latest conflicts? then we can push this further after that :) thanks |
da8f1bd
to
a8265ed
Compare
|
☔ The latest upstream changes (presumably #129665) made this pull request unmergeable. Please resolve the merge conflicts. |
r? compiler |
a8265ed
to
f490819
Compare
☔ The latest upstream changes (presumably #130050) made this pull request unmergeable. Please resolve the merge conflicts. |
Suggesting to remove the entire thing makes no sense, especially not as MachineApplicable.
Using: - icu4x-datagen 1.5.0 - CLDR 45.0.0 - icuexport icu4x/2024-05-16/75.x The "pub use __impliterable*" lines in macros.rs need to be removed manually to avoid unused reexport errors.
f490819
to
bd98f08
Compare
@cjgillot ping |
mod fallback_supplement_co_v1; | ||
#[doc(inline)] | ||
pub use __impl_fallback_supplement_co_v1 as impl_fallback_supplement_co_v1; | ||
#[macro_use] | ||
#[path = "macros/list_and_v1.data.rs"] | ||
#[path = "macros/list_and_v1.rs.data"] | ||
mod list_and_v1; | ||
#[doc(inline)] | ||
pub use __impl_list_and_v1 as impl_list_and_v1; |
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 don't understand this sentence in the commit message:
The "pub use __impliterable*" lines in macros.rs need to be removed manually to avoid unused reexport errors.
Could an #[allow(unused_something)]
help?
if sess.psess.unstable_features.is_nightly_build() { | ||
if feature_from_cli { | ||
err.subdiagnostic(CliFeatureDiagnosticHelp { feature }); | ||
let (enable_feature, upgrade_compiler) = if sess.psess.unstable_features.is_nightly_build() { |
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.
Could you split this in two? That will be more readable. Maybe cache is_nightly_build()
as it looks at env vars.
self.dcx().create_err(errors::BadReturnTypeNotation::NeedsDots { | ||
span: data.inputs_span, | ||
}) | ||
errors::BadReturnTypeNotation::NeedsDots { span: data.inputs_span } |
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.
No subdiagnostic for this one?
@@ -987,6 +988,8 @@ pub(crate) struct MissingTraitItemUnstable { | |||
pub some_note: bool, | |||
#[note(hir_analysis_none_note)] | |||
pub none_note: bool, | |||
#[subdiagnostic] | |||
pub subdiag: FeatureGateSubdiagnostic, |
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.
Could you use a more descriptive name?
@@ -386,12 +387,16 @@ pub(crate) enum BadReturnTypeNotation { | |||
#[primary_span] | |||
#[suggestion(code = "()", applicability = "maybe-incorrect")] | |||
span: Span, | |||
#[subdiagnostic] | |||
subdiag: Option<FeatureGateSubdiagnostic>, |
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.
Could you use a more descriptive name?
}, | ||
#[diag(ast_lowering_bad_return_type_notation_output)] | ||
Output { | ||
#[primary_span] | ||
#[suggestion(code = "", applicability = "maybe-incorrect")] | ||
span: Span, | ||
#[subdiagnostic] | ||
subdiag: Option<FeatureGateSubdiagnostic>, |
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.
Could you use a more descriptive name?
DiagArgValue::Str(Cow::Owned(pprust::tt_to_string(&self))) | ||
} | ||
} | ||
|
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.
Could you put this change in the commit where you actually use it?
DiagArgValue::Str(Cow::from(self.to_string())) | ||
} | ||
} | ||
|
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.
Could you put this change in the commit where you actually use it?
#[primary_span] | ||
pub span: Span, | ||
#[subdiagnostic] | ||
pub subdiag: FeatureGateSubdiagnostic, |
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.
Could you use a more descriptive name?
#[primary_span] | ||
pub span: Span, | ||
#[subdiagnostic] | ||
pub subdiag: FeatureGateSubdiagnostic, |
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.
Could you use a more descriptive name?
☔ The latest upstream changes (presumably #130724) made this pull request unmergeable. Please resolve the merge conflicts. |
@Xiretza any updates on this? thanks |
I haven't had the required motivation to work on this, and now that the translation effort seems to have somwhat failed anyway, I doubt I will find it in the future. |
cc #100717