Skip to content

Commit

Permalink
Rollup merge of #102623 - davidtwco:translation-eager, r=compiler-errors
Browse files Browse the repository at this point in the history
translation: eager translation

Part of #100717. See [Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/336883-i18n/topic/.23100717.20lists!/near/295010720) for additional context.

- **Store diagnostic arguments in a `HashMap`**: Eager translation will enable subdiagnostics to be translated multiple times with different arguments - this requires the ability to replace the value of one argument with a new value, which is better suited to a `HashMap` than the previous storage, a `Vec`.
- **Add `AddToDiagnostic::add_to_diagnostic_with`**: `AddToDiagnostic::add_to_diagnostic_with` is similar to the previous `AddToDiagnostic::add_to_diagnostic` but takes a function that can be used by the caller to modify diagnostic messages originating from the subdiagnostic (such as performing translation eagerly). `add_to_diagnostic` now just calls `add_to_diagnostic_with` with an empty closure.
- **Add `DiagnosticMessage::Eager`**: Add variant of `DiagnosticMessage` for eagerly translated messages
(messages in the target language which don't need translated by the emitter during emission). Also adds `eager_subdiagnostic` function which is intended to be invoked by the diagnostic derive for subdiagnostic fields which are marked as needing eager translation.
- **Support `#[subdiagnostic(eager)]`**: Add support for `eager` argument to the `subdiagnostic` attribute which generates a call to `eager_subdiagnostic`.
- **Finish migrating `rustc_query_system`**: Using eager translation, migrate the remaining repeated cycle stack diagnostic.
- **Split formatting initialization and use in diagnostic derives**: Diagnostic derives have previously had to take special care when ordering the generated code so that fields were not used after a move.

  This is unlikely for most fields because a field is either annotated with a subdiagnostic attribute and is thus likely a `Span` and copiable, or is a argument, in which case it is only used once by `set_arg`
anyway.

  However, format strings for code in suggestions can result in fields being used after being moved if not ordered carefully. As a result, the derive currently puts `set_arg` calls last (just before emission), such as:

      let diag = { /* create diagnostic */ };

      diag.span_suggestion_with_style(
          span,
          fluent::crate::slug,
          format!("{}", __binding_0),
          Applicability::Unknown,
          SuggestionStyle::ShowAlways
      );
      /* + other subdiagnostic additions */

      diag.set_arg("foo", __binding_0);
      /* + other `set_arg` calls */

      diag.emit();

  For eager translation, this doesn't work, as the message being translated eagerly can assume that all arguments are available - so arguments _must_ be set first.

  Format strings for suggestion code are now separated into two parts - an initialization line that performs the formatting into a variable, and a usage in the subdiagnostic addition.

  By separating these parts, the initialization can happen before arguments are set, preserving the desired order so that code compiles, while still enabling arguments to be set before subdiagnostics are added.

      let diag = { /* create diagnostic */ };

      let __code_0 = format!("{}", __binding_0);
      /* + other formatting */

      diag.set_arg("foo", __binding_0);
      /* + other `set_arg` calls */

      diag.span_suggestion_with_style(
          span,
          fluent::crate::slug,
          __code_0,
          Applicability::Unknown,
          SuggestionStyle::ShowAlways
      );
      /* + other subdiagnostic additions */

      diag.emit();

- **Remove field ordering logic in diagnostic derive:** Following the approach taken in earlier commits to separate formatting initialization from use in the subdiagnostic derive, simplify the diagnostic derive by removing the field-ordering logic that previously solved this problem.

r? ```@compiler-errors```
  • Loading branch information
Dylan-DPC committed Oct 12, 2022
2 parents 50f6d33 + fbac1f2 commit dc9f6f3
Show file tree
Hide file tree
Showing 26 changed files with 539 additions and 235 deletions.
15 changes: 12 additions & 3 deletions compiler/rustc_ast_lowering/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use rustc_errors::{fluent, AddToDiagnostic, Applicability, Diagnostic, DiagnosticArgFromDisplay};
use rustc_errors::{
fluent, AddToDiagnostic, Applicability, Diagnostic, DiagnosticArgFromDisplay,
SubdiagnosticMessage,
};
use rustc_macros::{Diagnostic, Subdiagnostic};
use rustc_span::{symbol::Ident, Span, Symbol};

Expand All @@ -19,7 +22,10 @@ pub struct UseAngleBrackets {
}

impl AddToDiagnostic for UseAngleBrackets {
fn add_to_diagnostic(self, diag: &mut Diagnostic) {
fn add_to_diagnostic_with<F>(self, diag: &mut Diagnostic, _: F)
where
F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage,
{
diag.multipart_suggestion(
fluent::ast_lowering::use_angle_brackets,
vec![(self.open_param, String::from("<")), (self.close_param, String::from(">"))],
Expand Down Expand Up @@ -69,7 +75,10 @@ pub enum AssocTyParenthesesSub {
}

impl AddToDiagnostic for AssocTyParenthesesSub {
fn add_to_diagnostic(self, diag: &mut Diagnostic) {
fn add_to_diagnostic_with<F>(self, diag: &mut Diagnostic, _: F)
where
F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage,
{
match self {
Self::Empty { parentheses_span } => diag.multipart_suggestion(
fluent::ast_lowering::remove_parentheses,
Expand Down
12 changes: 9 additions & 3 deletions compiler/rustc_ast_passes/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Errors emitted by ast_passes.

use rustc_errors::{fluent, AddToDiagnostic, Applicability, Diagnostic};
use rustc_errors::{fluent, AddToDiagnostic, Applicability, Diagnostic, SubdiagnosticMessage};
use rustc_macros::{Diagnostic, Subdiagnostic};
use rustc_span::{Span, Symbol};

Expand All @@ -17,7 +17,10 @@ pub struct ForbiddenLet {
}

impl AddToDiagnostic for ForbiddenLetReason {
fn add_to_diagnostic(self, diag: &mut Diagnostic) {
fn add_to_diagnostic_with<F>(self, diag: &mut Diagnostic, _: F)
where
F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage,
{
match self {
Self::GenericForbidden => {}
Self::NotSupportedOr(span) => {
Expand Down Expand Up @@ -228,7 +231,10 @@ pub struct ExternBlockSuggestion {
}

impl AddToDiagnostic for ExternBlockSuggestion {
fn add_to_diagnostic(self, diag: &mut Diagnostic) {
fn add_to_diagnostic_with<F>(self, diag: &mut Diagnostic, _: F)
where
F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage,
{
let start_suggestion = if let Some(abi) = self.abi {
format!("extern \"{}\" {{", abi)
} else {
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ use rustc_data_structures::profiling::TimingGuard;
use rustc_data_structures::profiling::VerboseTimingGuard;
use rustc_data_structures::sync::Lrc;
use rustc_errors::emitter::Emitter;
use rustc_errors::{translation::Translate, DiagnosticId, FatalError, Handler, Level};
use rustc_errors::{
translation::{to_fluent_args, Translate},
DiagnosticId, FatalError, Handler, Level,
};
use rustc_fs_util::link_or_copy;
use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
use rustc_incremental::{
Expand Down Expand Up @@ -1740,7 +1743,7 @@ impl Translate for SharedEmitter {

impl Emitter for SharedEmitter {
fn emit_diagnostic(&mut self, diag: &rustc_errors::Diagnostic) {
let fluent_args = self.to_fluent_args(diag.args());
let fluent_args = to_fluent_args(diag.args());
drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic {
msg: self.translate_messages(&diag.message, &fluent_args).to_string(),
code: diag.code.clone(),
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_error_messages/locales/en-US/query_system.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ query_system_cycle_usage = cycle used when {$usage}
query_system_cycle_stack_single = ...which immediately requires {$stack_bottom} again
query_system_cycle_stack_middle = ...which requires {$desc}...
query_system_cycle_stack_multiple = ...which again requires {$stack_bottom}, completing the cycle
query_system_cycle_recursive_ty_alias = type aliases cannot be recursive
Expand Down
29 changes: 28 additions & 1 deletion compiler/rustc_error_messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,18 @@ pub enum SubdiagnosticMessage {
/// Non-translatable diagnostic message.
// FIXME(davidtwco): can a `Cow<'static, str>` be used here?
Str(String),
/// Translatable message which has already been translated eagerly.
///
/// Some diagnostics have repeated subdiagnostics where the same interpolated variables would
/// be instantiated multiple times with different values. As translation normally happens
/// immediately prior to emission, after the diagnostic and subdiagnostic derive logic has run,
/// the setting of diagnostic arguments in the derived code will overwrite previous variable
/// values and only the final value will be set when translation occurs - resulting in
/// incorrect diagnostics. Eager translation results in translation for a subdiagnostic
/// happening immediately after the subdiagnostic derive's logic has been run. This variant
/// stores messages which have been translated eagerly.
// FIXME(#100717): can a `Cow<'static, str>` be used here?
Eager(String),
/// Identifier of a Fluent message. Instances of this variant are generated by the
/// `Subdiagnostic` derive.
FluentIdentifier(FluentId),
Expand Down Expand Up @@ -304,8 +316,20 @@ impl<S: Into<String>> From<S> for SubdiagnosticMessage {
#[rustc_diagnostic_item = "DiagnosticMessage"]
pub enum DiagnosticMessage {
/// Non-translatable diagnostic message.
// FIXME(davidtwco): can a `Cow<'static, str>` be used here?
// FIXME(#100717): can a `Cow<'static, str>` be used here?
Str(String),
/// Translatable message which has already been translated eagerly.
///
/// Some diagnostics have repeated subdiagnostics where the same interpolated variables would
/// be instantiated multiple times with different values. As translation normally happens
/// immediately prior to emission, after the diagnostic and subdiagnostic derive logic has run,
/// the setting of diagnostic arguments in the derived code will overwrite previous variable
/// values and only the final value will be set when translation occurs - resulting in
/// incorrect diagnostics. Eager translation results in translation for a subdiagnostic
/// happening immediately after the subdiagnostic derive's logic has been run. This variant
/// stores messages which have been translated eagerly.
// FIXME(#100717): can a `Cow<'static, str>` be used here?
Eager(String),
/// Identifier for a Fluent message (with optional attribute) corresponding to the diagnostic
/// message.
///
Expand All @@ -324,6 +348,7 @@ impl DiagnosticMessage {
pub fn with_subdiagnostic_message(&self, sub: SubdiagnosticMessage) -> Self {
let attr = match sub {
SubdiagnosticMessage::Str(s) => return DiagnosticMessage::Str(s),
SubdiagnosticMessage::Eager(s) => return DiagnosticMessage::Eager(s),
SubdiagnosticMessage::FluentIdentifier(id) => {
return DiagnosticMessage::FluentIdentifier(id, None);
}
Expand All @@ -332,6 +357,7 @@ impl DiagnosticMessage {

match self {
DiagnosticMessage::Str(s) => DiagnosticMessage::Str(s.clone()),
DiagnosticMessage::Eager(s) => DiagnosticMessage::Eager(s.clone()),
DiagnosticMessage::FluentIdentifier(id, _) => {
DiagnosticMessage::FluentIdentifier(id.clone(), Some(attr))
}
Expand Down Expand Up @@ -367,6 +393,7 @@ impl Into<SubdiagnosticMessage> for DiagnosticMessage {
fn into(self) -> SubdiagnosticMessage {
match self {
DiagnosticMessage::Str(s) => SubdiagnosticMessage::Str(s),
DiagnosticMessage::Eager(s) => SubdiagnosticMessage::Eager(s),
DiagnosticMessage::FluentIdentifier(id, None) => {
SubdiagnosticMessage::FluentIdentifier(id)
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

use crate::emitter::FileWithAnnotatedLines;
use crate::snippet::Line;
use crate::translation::Translate;
use crate::translation::{to_fluent_args, Translate};
use crate::{
CodeSuggestion, Diagnostic, DiagnosticId, DiagnosticMessage, Emitter, FluentBundle,
LazyFallbackBundle, Level, MultiSpan, Style, SubDiagnostic,
Expand Down Expand Up @@ -46,7 +46,7 @@ impl Translate for AnnotateSnippetEmitterWriter {
impl Emitter for AnnotateSnippetEmitterWriter {
/// The entry point for the diagnostics generation
fn emit_diagnostic(&mut self, diag: &Diagnostic) {
let fluent_args = self.to_fluent_args(diag.args());
let fluent_args = to_fluent_args(diag.args());

let mut children = diag.children.clone();
let (mut primary_span, suggestions) = self.primary_span_formatted(&diag, &fluent_args);
Expand Down
57 changes: 46 additions & 11 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ pub struct SuggestionsDisabled;
/// Simplified version of `FluentArg` that can implement `Encodable` and `Decodable`. Collection of
/// `DiagnosticArg` are converted to `FluentArgs` (consuming the collection) at the start of
/// diagnostic emission.
pub type DiagnosticArg<'source> = (Cow<'source, str>, DiagnosticArgValue<'source>);
pub type DiagnosticArg<'iter, 'source> =
(&'iter DiagnosticArgName<'source>, &'iter DiagnosticArgValue<'source>);

/// Name of a diagnostic argument.
pub type DiagnosticArgName<'source> = Cow<'source, str>;

/// Simplified version of `FluentValue` that can implement `Encodable` and `Decodable`. Converted
/// to a `FluentValue` by the emitter to be used in diagnostic translation.
Expand Down Expand Up @@ -199,9 +203,20 @@ impl IntoDiagnosticArg for ast::token::TokenKind {
/// `#[derive(Subdiagnostic)]` -- see [rustc_macros::Subdiagnostic].
#[cfg_attr(bootstrap, rustc_diagnostic_item = "AddSubdiagnostic")]
#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "AddToDiagnostic")]
pub trait AddToDiagnostic {
pub trait AddToDiagnostic
where
Self: Sized,
{
/// Add a subdiagnostic to an existing diagnostic.
fn add_to_diagnostic(self, diag: &mut Diagnostic);
fn add_to_diagnostic(self, diag: &mut Diagnostic) {
self.add_to_diagnostic_with(diag, |_, m| m);
}

/// Add a subdiagnostic to an existing diagnostic where `f` is invoked on every message used
/// (to optionally perform eager translation).
fn add_to_diagnostic_with<F>(self, diag: &mut Diagnostic, f: F)
where
F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage;
}

/// Trait implemented by lint types. This should not be implemented manually. Instead, use
Expand Down Expand Up @@ -229,7 +244,7 @@ pub struct Diagnostic {
pub span: MultiSpan,
pub children: Vec<SubDiagnostic>,
pub suggestions: Result<Vec<CodeSuggestion>, SuggestionsDisabled>,
args: Vec<DiagnosticArg<'static>>,
args: FxHashMap<DiagnosticArgName<'static>, DiagnosticArgValue<'static>>,

/// This is not used for highlighting or rendering any error message. Rather, it can be used
/// as a sort key to sort a buffer of diagnostics. By default, it is the primary span of
Expand Down Expand Up @@ -321,7 +336,7 @@ impl Diagnostic {
span: MultiSpan::new(),
children: vec![],
suggestions: Ok(vec![]),
args: vec![],
args: Default::default(),
sort_span: DUMMY_SP,
is_lint: false,
}
Expand Down Expand Up @@ -917,13 +932,30 @@ impl Diagnostic {
self
}

/// Add a subdiagnostic from a type that implements `Subdiagnostic` - see
/// [rustc_macros::Subdiagnostic].
/// Add a subdiagnostic from a type that implements `Subdiagnostic` (see
/// [rustc_macros::Subdiagnostic]).
pub fn subdiagnostic(&mut self, subdiagnostic: impl AddToDiagnostic) -> &mut Self {
subdiagnostic.add_to_diagnostic(self);
self
}

/// Add a subdiagnostic from a type that implements `Subdiagnostic` (see
/// [rustc_macros::Subdiagnostic]). Performs eager translation of any translatable messages
/// used in the subdiagnostic, so suitable for use with repeated messages (i.e. re-use of
/// interpolated variables).
pub fn eager_subdiagnostic(
&mut self,
handler: &crate::Handler,
subdiagnostic: impl AddToDiagnostic,
) -> &mut Self {
subdiagnostic.add_to_diagnostic_with(self, |diag, msg| {
let args = diag.args();
let msg = diag.subdiagnostic_message_to_diagnostic_message(msg);
handler.eagerly_translate(msg, args)
});
self
}

pub fn set_span<S: Into<MultiSpan>>(&mut self, sp: S) -> &mut Self {
self.span = sp.into();
if let Some(span) = self.span.primary_span() {
Expand Down Expand Up @@ -956,16 +988,19 @@ impl Diagnostic {
self
}

pub fn args(&self) -> &[DiagnosticArg<'static>] {
&self.args
// Exact iteration order of diagnostic arguments shouldn't make a difference to output because
// they're only used in interpolation.
#[allow(rustc::potential_query_instability)]
pub fn args<'a>(&'a self) -> impl Iterator<Item = DiagnosticArg<'a, 'static>> {
self.args.iter()
}

pub fn set_arg(
&mut self,
name: impl Into<Cow<'static, str>>,
arg: impl IntoDiagnosticArg,
) -> &mut Self {
self.args.push((name.into(), arg.into_diagnostic_arg()));
self.args.insert(name.into(), arg.into_diagnostic_arg());
self
}

Expand All @@ -976,7 +1011,7 @@ impl Diagnostic {
/// Helper function that takes a `SubdiagnosticMessage` and returns a `DiagnosticMessage` by
/// combining it with the primary message of the diagnostic (if translatable, otherwise it just
/// passes the user's string along).
fn subdiagnostic_message_to_diagnostic_message(
pub(crate) fn subdiagnostic_message_to_diagnostic_message(
&self,
attr: impl Into<SubdiagnosticMessage>,
) -> DiagnosticMessage {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_span::{FileLines, SourceFile, Span};

use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, Style, StyledString};
use crate::styled_buffer::StyledBuffer;
use crate::translation::Translate;
use crate::translation::{to_fluent_args, Translate};
use crate::{
CodeSuggestion, Diagnostic, DiagnosticId, DiagnosticMessage, FluentBundle, Handler,
LazyFallbackBundle, Level, MultiSpan, SubDiagnostic, SubstitutionHighlight, SuggestionStyle,
Expand Down Expand Up @@ -535,7 +535,7 @@ impl Emitter for EmitterWriter {
}

fn emit_diagnostic(&mut self, diag: &Diagnostic) {
let fluent_args = self.to_fluent_args(diag.args());
let fluent_args = to_fluent_args(diag.args());

let mut children = diag.children.clone();
let (mut primary_span, suggestions) = self.primary_span_formatted(&diag, &fluent_args);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_errors/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_span::source_map::{FilePathMapping, SourceMap};

use crate::emitter::{Emitter, HumanReadableErrorType};
use crate::registry::Registry;
use crate::translation::Translate;
use crate::translation::{to_fluent_args, Translate};
use crate::DiagnosticId;
use crate::{
CodeSuggestion, FluentBundle, LazyFallbackBundle, MultiSpan, SpanLabel, SubDiagnostic,
Expand Down Expand Up @@ -312,7 +312,7 @@ struct UnusedExterns<'a, 'b, 'c> {

impl Diagnostic {
fn from_errors_diagnostic(diag: &crate::Diagnostic, je: &JsonEmitter) -> Diagnostic {
let args = je.to_fluent_args(diag.args());
let args = to_fluent_args(diag.args());
let sugg = diag.suggestions.iter().flatten().map(|sugg| {
let translated_message = je.translate_message(&sugg.msg, &args);
Diagnostic {
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,17 @@ impl Handler {
}
}

/// Translate `message` eagerly with `args`.
pub fn eagerly_translate<'a>(
&self,
message: DiagnosticMessage,
args: impl Iterator<Item = DiagnosticArg<'a, 'static>>,
) -> SubdiagnosticMessage {
let inner = self.inner.borrow();
let args = crate::translation::to_fluent_args(args);
SubdiagnosticMessage::Eager(inner.emitter.translate_message(&message, &args).to_string())
}

// This is here to not allow mutation of flags;
// as of this writing it's only used in tests in librustc_middle.
pub fn can_emit_warnings(&self) -> bool {
Expand Down
Loading

0 comments on commit dc9f6f3

Please sign in to comment.