Skip to content

Commit b7c6551

Browse files
jhprattjhpratt
authored
Rollup merge of #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 cc82c42 + 0610172 commit b7c6551

8 files changed

Lines changed: 79 additions & 106 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 & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@ use crate::attrs::PrintAttribute;
1313
#[derive(Clone, Default, Debug, StableHash, Encodable, Decodable, PrintAttribute)]
1414
pub struct Directive {
1515
pub is_rustc_attr: bool,
16-
pub condition: Option<OnUnimplementedCondition>,
17-
pub subcommands: ThinVec<Directive>,
16+
/// This is never nested more than once, i.e. the directives in this
17+
/// thinvec have no filters of their own.
18+
pub filters: ThinVec<(Filter, Directive)>,
1819
pub message: Option<(Span, FormatString)>,
1920
pub label: Option<(Span, FormatString)>,
2021
pub notes: ThinVec<FormatString>,
@@ -28,11 +29,8 @@ impl Directive {
2829
/// We can't check this while parsing the attribute because `rustc_attr_parsing` doesn't have
2930
/// access to the item an attribute is on. Instead we later call this function in `check_attr`.
3031
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 {
32+
for (filter, subcommand) in &self.filters {
33+
filter.visit_params(visit);
3634
subcommand.visit_params(visit);
3735
}
3836

@@ -54,61 +52,33 @@ impl Directive {
5452

5553
pub fn eval(
5654
&self,
57-
condition_options: Option<&ConditionOptions>,
55+
filter_options: Option<&FilterOptions>,
5856
args: &FormatArgs,
5957
) -> CustomDiagnostic {
6058
let this = &args.this;
6159
debug!(
62-
"Directive::eval({self:?}, this={this}, options={condition_options:?}, args ={args:?})"
60+
"Directive::eval({self:?}, this={this}, options={filter_options:?}, args ={args:?})"
6361
);
6462

65-
let Some(condition_options) = condition_options else {
63+
let mut ret = CustomDiagnostic::default();
64+
65+
if let Some(filter_options) = filter_options {
66+
for (filter, directive) in &self.filters {
67+
if filter.matches_predicate(filter_options) {
68+
debug!("eval: {filter:?} succeeded");
69+
ret.update(directive, args);
70+
} else {
71+
debug!("eval: skipping {filter:?} due to {filter_options:?}");
72+
}
73+
}
74+
} else {
6675
debug_assert!(
6776
!self.is_rustc_attr,
68-
"Directive::eval called for `rustc_on_unimplemented` without `condition_options`"
77+
"Directive::eval called for `rustc_on_unimplemented` without `filter_options`"
6978
);
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-
};
7679
};
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-
}
80+
ret.update(self, args);
81+
ret
11282
}
11383
}
11484

@@ -121,6 +91,22 @@ pub struct CustomDiagnostic {
12191
pub parent_label: Option<String>,
12292
}
12393

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

253239
/// Represents the `on` filter in `#[rustc_on_unimplemented]`.
254240
#[derive(Clone, Debug, StableHash, Encodable, Decodable, PrintAttribute)]
255-
pub struct OnUnimplementedCondition {
241+
pub struct Filter {
256242
pub span: Span,
257243
pub pred: Predicate,
258244
}
259-
impl OnUnimplementedCondition {
260-
pub fn matches_predicate(self: &OnUnimplementedCondition, options: &ConditionOptions) -> bool {
245+
impl Filter {
246+
pub fn matches_predicate(&self, options: &FilterOptions) -> bool {
261247
self.pred.eval(&mut |p| match p {
262248
FlagOrNv::Flag(b) => options.has_flag(*b),
263249
FlagOrNv::NameValue(NameValue { name, value }) => {
@@ -272,7 +258,7 @@ impl OnUnimplementedCondition {
272258
}
273259
}
274260

275-
/// Predicate(s) in `#[rustc_on_unimplemented]`'s `on` filter. See [`OnUnimplementedCondition`].
261+
/// Predicate(s) in `#[rustc_on_unimplemented]`'s `on` filter. See [`Filter`].
276262
///
277263
/// It is similar to the predicate in the `cfg` attribute,
278264
/// and may contain nested predicates.
@@ -406,8 +392,7 @@ pub enum LitOrArg {
406392
Arg(Symbol),
407393
}
408394

409-
/// Used with `OnUnimplementedCondition::matches_predicate` to evaluate the
410-
/// [`OnUnimplementedCondition`].
395+
/// Used with `Filter::matches_predicate` to evaluate the [`Filter`].
411396
///
412397
/// For example, given a
413398
/// ```rust,ignore (just an example)
@@ -433,7 +418,7 @@ pub enum LitOrArg {
433418
/// it will look like this:
434419
///
435420
/// ```rust,ignore (just an example)
436-
/// ConditionOptions {
421+
/// FilterOptions {
437422
/// self_types: ["u32", "{integral}"],
438423
/// from_desugaring: Some("QuestionMark"),
439424
/// cause: None,
@@ -446,7 +431,7 @@ pub enum LitOrArg {
446431
/// }
447432
/// ```
448433
#[derive(Debug)]
449-
pub struct ConditionOptions {
434+
pub struct FilterOptions {
450435
/// All the self types that may apply.
451436
pub self_types: Vec<String>,
452437
// The kind of compiler desugaring.
@@ -460,7 +445,7 @@ pub struct ConditionOptions {
460445
pub generic_args: Vec<(Symbol, String)>,
461446
}
462447

463-
impl ConditionOptions {
448+
impl FilterOptions {
464449
pub fn has_flag(&self, name: Flag) -> bool {
465450
match name {
466451
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

‎tests/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ LL | call(foo_unsafe);
7878
| required by a bound introduced by this call
7979
|
8080
= help: the trait `Fn()` is not implemented for fn item `unsafe fn() {foo_unsafe}`
81-
= note: unsafe function cannot be called generically without an unsafe block
8281
= note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ }`
82+
= note: unsafe function cannot be called generically without an unsafe block
8383
= note: `#[target_feature]` functions do not implement the `Fn` traits
8484
= note: try casting the function to a `fn` pointer or wrapping it in a closure
8585
note: required by a bound in `call`
@@ -97,8 +97,8 @@ LL | call_mut(foo_unsafe);
9797
| required by a bound introduced by this call
9898
|
9999
= help: the trait `FnMut()` is not implemented for fn item `unsafe fn() {foo_unsafe}`
100-
= note: unsafe function cannot be called generically without an unsafe block
101100
= note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ }`
101+
= note: unsafe function cannot be called generically without an unsafe block
102102
= note: `#[target_feature]` functions do not implement the `Fn` traits
103103
= note: try casting the function to a `fn` pointer or wrapping it in a closure
104104
note: required by a bound in `call_mut`
@@ -116,8 +116,8 @@ LL | call_once(foo_unsafe);
116116
| required by a bound introduced by this call
117117
|
118118
= help: the trait `FnOnce()` is not implemented for fn item `unsafe fn() {foo_unsafe}`
119-
= note: unsafe function cannot be called generically without an unsafe block
120119
= note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ }`
120+
= note: unsafe function cannot be called generically without an unsafe block
121121
= note: `#[target_feature]` functions do not implement the `Fn` traits
122122
= note: try casting the function to a `fn` pointer or wrapping it in a closure
123123
note: required by a bound in `call_once`

0 commit comments

Comments
 (0)