Skip to content

Commit f2d111e

Browse files
GuillaumeGomezGuillaumeGomez
authored
Rollup merge of rust-lang#155940 - mejrs:filter, r=jdonszelmann
refactor rustc_on_unimplemented's filtering Previously when you had a ```rust pub struct Directive { pub is_rustc_attr: bool, pub condition: Option<OnUnimplementedCondition>, pub subcommands: ThinVec<Directive>, pub message: Option<(Span, FormatString)>, ... } ``` that condition would control the emission of the message, label, notes etc. I've changed that to ```rust pub struct Directive { pub is_rustc_attr: bool, pub filters: ThinVec<(Filter, Directive)>, pub message: Option<(Span, FormatString)>, ... ``` so that the message etc is always emitted, and there's a vec of tuples with (filter, directive) where the filter controls whether that directive is even emitted, which i think is much clearer. That also makes it easier to not have to do the reverse iteration thing and this makes it so that notes are emitted in declaration order (with nonfiltered options always last). The rename is because I plan on making it available to other diagnostic attributes at some point (very wip) so `OnUnimplementedCondition` and the like would have to be renamed anyway.
2 parents c389685 + f9e052d commit f2d111e

8 files changed

Lines changed: 79 additions & 107 deletions

File tree

‎compiler/rustc_attr_parsing/src/attributes/diagnostic/mod.rs‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ use std::ops::Range;
33
use rustc_errors::E0232;
44
use rustc_hir::AttrPath;
55
use rustc_hir::attrs::diagnostic::{
6-
Directive, FilterFormatString, Flag, FormatArg, FormatString, LitOrArg, Name, NameValue,
7-
OnUnimplementedCondition, Piece, Predicate,
6+
Directive, Filter, FilterFormatString, Flag, FormatArg, FormatString, LitOrArg, Name,
7+
NameValue, Piece, Predicate,
88
};
99
use rustc_macros::Diagnostic;
1010
use rustc_parse_format::{
@@ -201,12 +201,11 @@ fn parse_directive_items<'p>(
201201
items: impl Iterator<Item = &'p MetaItemOrLitParser>,
202202
is_root: bool,
203203
) -> Option<Directive> {
204-
let condition = None;
205204
let mut message: Option<(Span, _)> = None;
206205
let mut label: Option<(Span, _)> = None;
207206
let mut notes = ThinVec::new();
208207
let mut parent_label = None;
209-
let mut subcommands = ThinVec::new();
208+
let mut filters = ThinVec::new();
210209

211210
for item in items {
212211
let span = item.span();
@@ -330,29 +329,27 @@ fn parse_directive_items<'p>(
330329
if is_root {
331330
let items = or_malformed!(item.args().as_list()?);
332331
let mut iter = items.mixed();
333-
let condition: &MetaItemOrLitParser = match iter.next() {
332+
let filter: &MetaItemOrLitParser = match iter.next() {
334333
Some(c) => c,
335334
None => {
336335
cx.emit_err(InvalidOnClause::Empty { span });
337336
continue;
338337
}
339338
};
340339

341-
let condition = parse_condition(condition);
340+
let filter = parse_filter(filter);
342341

343342
if items.len() < 2 {
344343
// Something like `#[rustc_on_unimplemented(on(.., /* nothing */))]`
345-
// There's a condition but no directive behind it, this is a mistake.
344+
// There's a filter but no directive behind it, this is a mistake.
346345
malformed!();
347346
}
348347

349-
let mut directive =
350-
or_malformed!(parse_directive_items(cx, mode, iter, false)?);
351-
352-
match condition {
353-
Ok(c) => {
354-
directive.condition = Some(c);
355-
subcommands.push(directive);
348+
match filter {
349+
Ok(filter) => {
350+
let directive =
351+
or_malformed!(parse_directive_items(cx, mode, iter, false)?);
352+
filters.push((filter, directive));
356353
}
357354
Err(e) => {
358355
cx.emit_err(e);
@@ -371,8 +368,7 @@ fn parse_directive_items<'p>(
371368

372369
Some(Directive {
373370
is_rustc_attr: matches!(mode, Mode::RustcOnUnimplemented),
374-
condition,
375-
subcommands,
371+
filters,
376372
message,
377373
label,
378374
notes,
@@ -513,12 +509,10 @@ fn slice_span(input: Span, Range { start, end }: Range<usize>, is_source_literal
513509
if is_source_literal { input.from_inner(InnerSpan { start, end }) } else { input }
514510
}
515511

516-
pub(crate) fn parse_condition(
517-
input: &MetaItemOrLitParser,
518-
) -> Result<OnUnimplementedCondition, InvalidOnClause> {
512+
pub(crate) fn parse_filter(input: &MetaItemOrLitParser) -> Result<Filter, InvalidOnClause> {
519513
let span = input.span();
520514
let pred = parse_predicate(input)?;
521-
Ok(OnUnimplementedCondition { span, pred })
515+
Ok(Filter { span, pred })
522516
}
523517

524518
fn parse_predicate(input: &MetaItemOrLitParser) -> Result<Predicate, InvalidOnClause> {
@@ -553,7 +547,7 @@ fn parse_predicate(input: &MetaItemOrLitParser) -> Result<Predicate, InvalidOnCl
553547
return Err(InvalidOnClause::UnsupportedLiteral { span: p.args_span() });
554548
};
555549
let name = parse_name(predicate.name);
556-
let value = parse_filter(value.name);
550+
let value = parse_filter_format(value.name);
557551
let kv = NameValue { name, value };
558552
Ok(Predicate::Match(kv))
559553
}
@@ -588,7 +582,7 @@ fn parse_name(name: Symbol) -> Name {
588582
}
589583
}
590584

591-
fn parse_filter(input: Symbol) -> FilterFormatString {
585+
fn parse_filter_format(input: Symbol) -> FilterFormatString {
592586
let pieces = Parser::new(input.as_str(), None, None, false, ParseMode::Diagnostic)
593587
.map(|p| match p {
594588
RpfPiece::Lit(s) => LitOrArg::Lit(Symbol::intern(s)),

‎compiler/rustc_hir/src/attrs/diagnostic.rs‎

Lines changed: 46 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
use std::fmt;
33
use std::fmt::Debug;
44

5-
pub use rustc_ast::attr::data_structures::*;
65
use rustc_macros::{Decodable, Encodable, HashStable, PrintAttribute};
76
use rustc_span::{DesugaringKind, Span, Symbol, kw};
87
use thin_vec::ThinVec;
@@ -13,8 +12,9 @@ use crate::attrs::PrintAttribute;
1312
#[derive(Clone, Default, Debug, HashStable, Encodable, Decodable, PrintAttribute)]
1413
pub struct Directive {
1514
pub is_rustc_attr: bool,
16-
pub condition: Option<OnUnimplementedCondition>,
17-
pub subcommands: ThinVec<Directive>,
15+
/// This is never nested more than once, i.e. the directives in this
16+
/// thinvec have no filters of their own.
17+
pub filters: ThinVec<(Filter, Directive)>,
1818
pub message: Option<(Span, FormatString)>,
1919
pub label: Option<(Span, FormatString)>,
2020
pub notes: ThinVec<FormatString>,
@@ -28,11 +28,8 @@ impl Directive {
2828
/// We can't check this while parsing the attribute because `rustc_attr_parsing` doesn't have
2929
/// access to the item an attribute is on. Instead we later call this function in `check_attr`.
3030
pub fn visit_params(&self, visit: &mut impl FnMut(Symbol, Span)) {
31-
if let Some(condition) = &self.condition {
32-
condition.visit_params(visit);
33-
}
34-
35-
for subcommand in &self.subcommands {
31+
for (filter, subcommand) in &self.filters {
32+
filter.visit_params(visit);
3633
subcommand.visit_params(visit);
3734
}
3835

@@ -54,61 +51,33 @@ impl Directive {
5451

5552
pub fn eval(
5653
&self,
57-
condition_options: Option<&ConditionOptions>,
54+
filter_options: Option<&FilterOptions>,
5855
args: &FormatArgs,
5956
) -> CustomDiagnostic {
6057
let this = &args.this;
6158
debug!(
62-
"Directive::eval({self:?}, this={this}, options={condition_options:?}, args ={args:?})"
59+
"Directive::eval({self:?}, this={this}, options={filter_options:?}, args ={args:?})"
6360
);
6461

65-
let Some(condition_options) = condition_options else {
62+
let mut ret = CustomDiagnostic::default();
63+
64+
if let Some(filter_options) = filter_options {
65+
for (filter, directive) in &self.filters {
66+
if filter.matches_predicate(filter_options) {
67+
debug!("eval: {filter:?} succeeded");
68+
ret.update(directive, args);
69+
} else {
70+
debug!("eval: skipping {filter:?} due to {filter_options:?}");
71+
}
72+
}
73+
} else {
6674
debug_assert!(
6775
!self.is_rustc_attr,
68-
"Directive::eval called for `rustc_on_unimplemented` without `condition_options`"
76+
"Directive::eval called for `rustc_on_unimplemented` without `filter_options`"
6977
);
70-
return CustomDiagnostic {
71-
label: self.label.as_ref().map(|l| l.1.format(args)),
72-
message: self.message.as_ref().map(|m| m.1.format(args)),
73-
notes: self.notes.iter().map(|n| n.format(args)).collect(),
74-
parent_label: None,
75-
};
7678
};
77-
let mut message = None;
78-
let mut label = None;
79-
let mut notes = Vec::new();
80-
let mut parent_label = None;
81-
82-
for command in self.subcommands.iter().chain(Some(self)).rev() {
83-
debug!(?command);
84-
if let Some(ref condition) = command.condition
85-
&& !condition.matches_predicate(condition_options)
86-
{
87-
debug!("eval: skipping {command:?} due to condition");
88-
continue;
89-
}
90-
debug!("eval: {command:?} succeeded");
91-
if let Some(ref message_) = command.message {
92-
message = Some(message_.clone());
93-
}
94-
95-
if let Some(ref label_) = command.label {
96-
label = Some(label_.clone());
97-
}
98-
99-
notes.extend(command.notes.clone());
100-
101-
if let Some(ref parent_label_) = command.parent_label {
102-
parent_label = Some(parent_label_.clone());
103-
}
104-
}
105-
106-
CustomDiagnostic {
107-
label: label.map(|l| l.1.format(args)),
108-
message: message.map(|m| m.1.format(args)),
109-
notes: notes.into_iter().map(|n| n.format(args)).collect(),
110-
parent_label: parent_label.map(|e_s| e_s.format(args)),
111-
}
79+
ret.update(self, args);
80+
ret
11281
}
11382
}
11483

@@ -121,6 +90,22 @@ pub struct CustomDiagnostic {
12190
pub parent_label: Option<String>,
12291
}
12392

93+
impl CustomDiagnostic {
94+
fn update(&mut self, di: &Directive, args: &FormatArgs) {
95+
if self.message.is_none() {
96+
self.message = di.message.as_ref().map(|m| m.1.format(args));
97+
}
98+
if self.label.is_none() {
99+
self.label = di.label.as_ref().map(|l| l.1.format(args));
100+
}
101+
if self.parent_label.is_none() {
102+
self.parent_label = di.parent_label.as_ref().map(|p| p.format(args));
103+
}
104+
105+
self.notes.extend(di.notes.iter().map(|n| n.format(args)))
106+
}
107+
}
108+
124109
/// Like [std::fmt::Arguments] this is a string that has been parsed into "pieces",
125110
/// either as string pieces or dynamic arguments.
126111
#[derive(Clone, Debug, HashStable, Encodable, Decodable, PrintAttribute)]
@@ -252,12 +237,12 @@ pub enum FormatArg {
252237

253238
/// Represents the `on` filter in `#[rustc_on_unimplemented]`.
254239
#[derive(Clone, Debug, HashStable, Encodable, Decodable, PrintAttribute)]
255-
pub struct OnUnimplementedCondition {
240+
pub struct Filter {
256241
pub span: Span,
257242
pub pred: Predicate,
258243
}
259-
impl OnUnimplementedCondition {
260-
pub fn matches_predicate(self: &OnUnimplementedCondition, options: &ConditionOptions) -> bool {
244+
impl Filter {
245+
pub fn matches_predicate(&self, options: &FilterOptions) -> bool {
261246
self.pred.eval(&mut |p| match p {
262247
FlagOrNv::Flag(b) => options.has_flag(*b),
263248
FlagOrNv::NameValue(NameValue { name, value }) => {
@@ -272,7 +257,7 @@ impl OnUnimplementedCondition {
272257
}
273258
}
274259

275-
/// Predicate(s) in `#[rustc_on_unimplemented]`'s `on` filter. See [`OnUnimplementedCondition`].
260+
/// Predicate(s) in `#[rustc_on_unimplemented]`'s `on` filter. See [`Filter`].
276261
///
277262
/// It is similar to the predicate in the `cfg` attribute,
278263
/// and may contain nested predicates.
@@ -406,8 +391,7 @@ pub enum LitOrArg {
406391
Arg(Symbol),
407392
}
408393

409-
/// Used with `OnUnimplementedCondition::matches_predicate` to evaluate the
410-
/// [`OnUnimplementedCondition`].
394+
/// Used with `Filter::matches_predicate` to evaluate the [`Filter`].
411395
///
412396
/// For example, given a
413397
/// ```rust,ignore (just an example)
@@ -433,7 +417,7 @@ pub enum LitOrArg {
433417
/// it will look like this:
434418
///
435419
/// ```rust,ignore (just an example)
436-
/// ConditionOptions {
420+
/// FilterOptions {
437421
/// self_types: ["u32", "{integral}"],
438422
/// from_desugaring: Some("QuestionMark"),
439423
/// cause: None,
@@ -446,7 +430,7 @@ pub enum LitOrArg {
446430
/// }
447431
/// ```
448432
#[derive(Debug)]
449-
pub struct ConditionOptions {
433+
pub struct FilterOptions {
450434
/// All the self types that may apply.
451435
pub self_types: Vec<String>,
452436
// The kind of compiler desugaring.
@@ -460,7 +444,7 @@ pub struct ConditionOptions {
460444
pub generic_args: Vec<(Symbol, String)>,
461445
}
462446

463-
impl ConditionOptions {
447+
impl FilterOptions {
464448
pub fn has_flag(&self, name: Flag) -> bool {
465449
match name {
466450
Flag::CrateLocal => self.crate_local,

‎compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::path::PathBuf;
22

33
use rustc_hir as hir;
4-
use rustc_hir::attrs::diagnostic::{ConditionOptions, CustomDiagnostic, FormatArgs};
4+
use rustc_hir::attrs::diagnostic::{CustomDiagnostic, FilterOptions, FormatArgs};
55
use rustc_hir::def_id::LocalDefId;
66
use rustc_hir::find_attr;
77
use rustc_middle::ty::print::PrintTraitRefExt;
@@ -40,11 +40,11 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
4040
if trait_pred.polarity() != ty::PredicatePolarity::Positive {
4141
return CustomDiagnostic::default();
4242
}
43-
let (condition_options, format_args) =
43+
let (filter_options, format_args) =
4444
self.on_unimplemented_components(trait_pred, obligation, long_ty_path);
4545
if let Some(command) = find_attr!(self.tcx, trait_pred.def_id(), OnUnimplemented {directive, ..} => directive.as_deref()).flatten() {
4646
command.eval(
47-
Some(&condition_options),
47+
Some(&filter_options),
4848
&format_args,
4949
)
5050
} else {
@@ -57,7 +57,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
5757
trait_pred: ty::PolyTraitPredicate<'tcx>,
5858
obligation: &PredicateObligation<'tcx>,
5959
long_ty_path: &mut Option<PathBuf>,
60-
) -> (ConditionOptions, FormatArgs) {
60+
) -> (FilterOptions, FormatArgs) {
6161
let (def_id, args) = (trait_pred.def_id(), trait_pred.skip_binder().trait_ref.args);
6262
let trait_pred = trait_pred.skip_binder();
6363

@@ -219,14 +219,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
219219
let this = self.tcx.def_path_str(trait_pred.trait_ref.def_id);
220220
let this_sugared = trait_pred.trait_ref.print_trait_sugared().to_string();
221221

222-
let condition_options = ConditionOptions {
223-
self_types,
224-
from_desugaring,
225-
cause,
226-
crate_local,
227-
direct,
228-
generic_args,
229-
};
222+
let filter_options =
223+
FilterOptions { self_types, from_desugaring, cause, crate_local, direct, generic_args };
230224

231225
// Unlike the generic_args earlier,
232226
// this one is *not* collected under `with_no_trimmed_paths!`
@@ -256,6 +250,6 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
256250
.collect();
257251

258252
let format_args = FormatArgs { this, this_sugared, generic_args, item_context };
259-
(condition_options, format_args)
253+
(filter_options, format_args)
260254
}
261255
}

‎tests/ui/abi/bad-custom.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,8 @@ LL | f
194194
| - return type was inferred to be `unsafe extern "custom" fn()` here
195195
|
196196
= help: the trait `Fn()` is not implemented for `unsafe extern "custom" fn()`
197-
= note: unsafe function cannot be called generically without an unsafe block
198197
= note: wrap the `unsafe extern "custom" fn()` in a closure with no arguments: `|| { /* code */ }`
198+
= note: unsafe function cannot be called generically without an unsafe block
199199

200200
error: items with the "custom" ABI can only be declared externally or defined via naked functions
201201
--> $DIR/bad-custom.rs:25:1

0 commit comments

Comments
 (0)