macros: separate suggestion fmt'ing and emission
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: ```rust 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. ```rust 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(); ``` Signed-off-by: David Wood <david.wood@huawei.com>
This commit is contained in:
parent
113e94369c
commit
7e20929e55
4 changed files with 105 additions and 23 deletions
|
@ -4,6 +4,7 @@ use crate::diagnostics::error::{
|
|||
use proc_macro::Span;
|
||||
use proc_macro2::TokenStream;
|
||||
use quote::{format_ident, quote, ToTokens};
|
||||
use std::cell::RefCell;
|
||||
use std::cmp::Ordering;
|
||||
use std::collections::{BTreeSet, HashMap};
|
||||
use std::fmt;
|
||||
|
@ -14,6 +15,19 @@ use synstructure::{BindStyle, BindingInfo, VariantInfo};
|
|||
|
||||
use super::error::invalid_nested_attr;
|
||||
|
||||
thread_local! {
|
||||
pub static CODE_IDENT_COUNT: RefCell<u32> = RefCell::new(0);
|
||||
}
|
||||
|
||||
/// Returns an ident of the form `__code_N` where `N` is incremented once with every call.
|
||||
pub(crate) fn new_code_ident() -> syn::Ident {
|
||||
CODE_IDENT_COUNT.with(|count| {
|
||||
let ident = format_ident!("__code_{}", *count.borrow());
|
||||
*count.borrow_mut() += 1;
|
||||
ident
|
||||
})
|
||||
}
|
||||
|
||||
/// Checks whether the type name of `ty` matches `name`.
|
||||
///
|
||||
/// Given some struct at `a::b::c::Foo`, this will return true for `c::Foo`, `b::c::Foo`, or
|
||||
|
@ -142,6 +156,15 @@ impl<'ty> FieldInnerTy<'ty> {
|
|||
unreachable!();
|
||||
}
|
||||
|
||||
/// Returns `true` if `FieldInnerTy::with` will result in iteration for this inner type (i.e.
|
||||
/// that cloning might be required for values moved in the loop body).
|
||||
pub(crate) fn will_iterate(&self) -> bool {
|
||||
match self {
|
||||
FieldInnerTy::Vec(..) => true,
|
||||
FieldInnerTy::Option(..) | FieldInnerTy::None => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns `Option` containing inner type if there is one.
|
||||
pub(crate) fn inner_type(&self) -> Option<&'ty Type> {
|
||||
match self {
|
||||
|
@ -434,7 +457,12 @@ pub(super) enum SubdiagnosticKind {
|
|||
Suggestion {
|
||||
suggestion_kind: SuggestionKind,
|
||||
applicability: SpannedOption<Applicability>,
|
||||
code: TokenStream,
|
||||
/// Identifier for variable used for formatted code, e.g. `___code_0`. Enables separation
|
||||
/// of formatting and diagnostic emission so that `set_arg` calls can happen in-between..
|
||||
code_field: syn::Ident,
|
||||
/// Initialization logic for `code_field`'s variable, e.g.
|
||||
/// `let __formatted_code = /* whatever */;`
|
||||
code_init: TokenStream,
|
||||
},
|
||||
/// `#[multipart_suggestion{,_short,_hidden,_verbose}]`
|
||||
MultipartSuggestion {
|
||||
|
@ -469,7 +497,8 @@ impl SubdiagnosticKind {
|
|||
SubdiagnosticKind::Suggestion {
|
||||
suggestion_kind,
|
||||
applicability: None,
|
||||
code: TokenStream::new(),
|
||||
code_field: new_code_ident(),
|
||||
code_init: TokenStream::new(),
|
||||
}
|
||||
} else if let Some(suggestion_kind) =
|
||||
name.strip_prefix("multipart_suggestion").and_then(|s| s.parse().ok())
|
||||
|
@ -548,9 +577,10 @@ impl SubdiagnosticKind {
|
|||
};
|
||||
|
||||
match (nested_name, &mut kind) {
|
||||
("code", SubdiagnosticKind::Suggestion { .. }) => {
|
||||
("code", SubdiagnosticKind::Suggestion { code_field, .. }) => {
|
||||
let formatted_str = fields.build_format(&value.value(), value.span());
|
||||
code.set_once(formatted_str, span);
|
||||
let code_init = quote! { let #code_field = #formatted_str; };
|
||||
code.set_once(code_init, span);
|
||||
}
|
||||
(
|
||||
"applicability",
|
||||
|
@ -582,13 +612,13 @@ impl SubdiagnosticKind {
|
|||
}
|
||||
|
||||
match kind {
|
||||
SubdiagnosticKind::Suggestion { code: ref mut code_field, .. } => {
|
||||
*code_field = if let Some((code, _)) = code {
|
||||
code
|
||||
SubdiagnosticKind::Suggestion { ref code_field, ref mut code_init, .. } => {
|
||||
*code_init = if let Some(init) = code.value() {
|
||||
init
|
||||
} else {
|
||||
span_err(span, "suggestion without `code = \"...\"`").emit();
|
||||
quote! { "" }
|
||||
}
|
||||
quote! { let #code_field: String = unreachable!(); }
|
||||
};
|
||||
}
|
||||
SubdiagnosticKind::Label
|
||||
| SubdiagnosticKind::Note
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue