rustc_on_unimplemented: emit notes in declared order

Author: mejrs

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

@@ -210,12 +210,11 @@ fn parse_directive_items<'p>(
     items: impl Iterator<Item = &'p MetaItemOrLitParser>,
     is_root: bool,
 ) -> Option<Directive> {
-    let condition = None;
     let mut message: Option<(Span, _)> = None;
     let mut label: Option<(Span, _)> = None;
     let mut notes = ThinVec::new();
     let mut parent_label = None;
-    let mut subcommands = ThinVec::new();
+    let mut filters = ThinVec::new();
 
     for item in items {
         let span = item.span();
@@ -357,21 +356,19 @@ fn parse_directive_items<'p>(
                         }
                     };
 
-                    let condition = parse_condition(condition);
+                    let filter = parse_condition(condition);
 
                     if items.len() < 2 {
                         // Something like `#[rustc_on_unimplemented(on(.., /* nothing */))]`
                         // There's a condition but no directive behind it, this is a mistake.
                         malformed!();
                     }
 
-                    let mut directive =
-                        or_malformed!(parse_directive_items(cx, mode, iter, false)?);
-
-                    match condition {
-                        Ok(c) => {
-                            directive.condition = Some(c);
-                            subcommands.push(directive);
+                    match filter {
+                        Ok(filter) => {
+                            let directive =
+                                or_malformed!(parse_directive_items(cx, mode, iter, false)?);
+                            filters.push((filter, directive));
                         }
                         Err(e) => {
                             cx.emit_err(e);
@@ -390,8 +387,7 @@ fn parse_directive_items<'p>(
 
     Some(Directive {
         is_rustc_attr: matches!(mode, Mode::RustcOnUnimplemented),
-        condition,
-        subcommands,
+        filters,
         message,
         label,
         notes,

compiler/rustc_hir/src/attrs/diagnostic.rs

@@ -2,7 +2,6 @@
 use std::fmt;
 use std::fmt::Debug;
 
-pub use rustc_ast::attr::data_structures::*;
 use rustc_macros::{Decodable, Encodable, HashStable, PrintAttribute};
 use rustc_span::{DesugaringKind, Span, Symbol, kw};
 use thin_vec::ThinVec;
@@ -13,8 +12,9 @@ use crate::attrs::PrintAttribute;
 #[derive(Clone, Default, Debug, HashStable, Encodable, Decodable, PrintAttribute)]
 pub struct Directive {
     pub is_rustc_attr: bool,
-    pub condition: Option<OnUnimplementedCondition>,
-    pub subcommands: ThinVec<Directive>,
+    /// This is never nested more than once, i.e. the directives in this
+    /// thinvec have no filters of their own.
+    pub filters: ThinVec<(OnUnimplementedCondition, Directive)>,
     pub message: Option<(Span, FormatString)>,
     pub label: Option<(Span, FormatString)>,
     pub notes: ThinVec<FormatString>,
@@ -28,11 +28,8 @@ impl Directive {
     /// We can't check this while parsing the attribute because `rustc_attr_parsing` doesn't have
     /// access to the item an attribute is on. Instead we later call this function in `check_attr`.
     pub fn visit_params(&self, visit: &mut impl FnMut(Symbol, Span)) {
-        if let Some(condition) = &self.condition {
-            condition.visit_params(visit);
-        }
-
-        for subcommand in &self.subcommands {
+        for (filter, subcommand) in &self.filters {
+            filter.visit_params(visit);
             subcommand.visit_params(visit);
         }
 
@@ -62,53 +59,25 @@ impl Directive {
             "Directive::eval({self:?}, this={this}, options={condition_options:?}, args ={args:?})"
         );
 
-        let Some(condition_options) = condition_options else {
+        let mut ret = CustomDiagnostic::default();
+
+        if let Some(condition_options) = condition_options {
+            for (filter, directive) in &self.filters {
+                if filter.matches_predicate(condition_options) {
+                    debug!("eval: {filter:?} succeeded");
+                    ret.update(directive, args);
+                } else {
+                    debug!("eval: skipping {filter:?} due to condition");
+                }
+            }
+        } else {
             debug_assert!(
                 !self.is_rustc_attr,
                 "Directive::eval called for `rustc_on_unimplemented` without `condition_options`"
             );
-            return CustomDiagnostic {
-                label: self.label.as_ref().map(|l| l.1.format(args)),
-                message: self.message.as_ref().map(|m| m.1.format(args)),
-                notes: self.notes.iter().map(|n| n.format(args)).collect(),
-                parent_label: None,
-            };
         };
-        let mut message = None;
-        let mut label = None;
-        let mut notes = Vec::new();
-        let mut parent_label = None;
-
-        for command in self.subcommands.iter().chain(Some(self)).rev() {
-            debug!(?command);
-            if let Some(ref condition) = command.condition
-                && !condition.matches_predicate(condition_options)
-            {
-                debug!("eval: skipping {command:?} due to condition");
-                continue;
-            }
-            debug!("eval: {command:?} succeeded");
-            if let Some(ref message_) = command.message {
-                message = Some(message_.clone());
-            }
-
-            if let Some(ref label_) = command.label {
-                label = Some(label_.clone());
-            }
-
-            notes.extend(command.notes.clone());
-
-            if let Some(ref parent_label_) = command.parent_label {
-                parent_label = Some(parent_label_.clone());
-            }
-        }
-
-        CustomDiagnostic {
-            label: label.map(|l| l.1.format(args)),
-            message: message.map(|m| m.1.format(args)),
-            notes: notes.into_iter().map(|n| n.format(args)).collect(),
-            parent_label: parent_label.map(|e_s| e_s.format(args)),
-        }
+        ret.update(self, args);
+        ret
     }
 }
 
@@ -121,6 +90,22 @@ pub struct CustomDiagnostic {
     pub parent_label: Option<String>,
 }
 
+impl CustomDiagnostic {
+    fn update(&mut self, di: &Directive, args: &FormatArgs) {
+        if self.message.is_none() {
+            self.message = di.message.as_ref().map(|m| m.1.format(args));
+        }
+        if self.label.is_none() {
+            self.label = di.label.as_ref().map(|l| l.1.format(args));
+        }
+        if self.parent_label.is_none() {
+            self.parent_label = di.parent_label.as_ref().map(|p| p.format(args));
+        }
+
+        self.notes.extend(di.notes.iter().map(|n| n.format(args)))
+    }
+}
+
 /// Like [std::fmt::Arguments] this is a string that has been parsed into "pieces",
 /// either as string pieces or dynamic arguments.
 #[derive(Clone, Debug, HashStable, Encodable, Decodable, PrintAttribute)]

tests/ui/abi/bad-custom.stderr

@@ -194,8 +194,8 @@ LL |     f
    |     - return type was inferred to be `unsafe extern "custom" fn()` here
    |
    = help: the trait `Fn()` is not implemented for `unsafe extern "custom" fn()`
-   = note: unsafe function cannot be called generically without an unsafe block
    = note: wrap the `unsafe extern "custom" fn()` in a closure with no arguments: `|| { /* code */ }`
+   = note: unsafe function cannot be called generically without an unsafe block
 
 error: items with the "custom" ABI can only be declared externally or defined via naked functions
   --> $DIR/bad-custom.rs:25:1

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

@@ -78,8 +78,8 @@ LL |     call(foo_unsafe);
    |     required by a bound introduced by this call
    |
    = help: the trait `Fn()` is not implemented for fn item `unsafe fn() {foo_unsafe}`
-   = note: unsafe function cannot be called generically without an unsafe block
    = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ }`
+   = note: unsafe function cannot be called generically without an unsafe block
    = note: `#[target_feature]` functions do not implement the `Fn` traits
    = note: try casting the function to a `fn` pointer or wrapping it in a closure
 note: required by a bound in `call`
@@ -97,8 +97,8 @@ LL |     call_mut(foo_unsafe);
    |     required by a bound introduced by this call
    |
    = help: the trait `FnMut()` is not implemented for fn item `unsafe fn() {foo_unsafe}`
-   = note: unsafe function cannot be called generically without an unsafe block
    = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ }`
+   = note: unsafe function cannot be called generically without an unsafe block
    = note: `#[target_feature]` functions do not implement the `Fn` traits
    = note: try casting the function to a `fn` pointer or wrapping it in a closure
 note: required by a bound in `call_mut`
@@ -116,8 +116,8 @@ LL |     call_once(foo_unsafe);
    |     required by a bound introduced by this call
    |
    = help: the trait `FnOnce()` is not implemented for fn item `unsafe fn() {foo_unsafe}`
-   = note: unsafe function cannot be called generically without an unsafe block
    = note: wrap the `unsafe fn() {foo_unsafe}` in a closure with no arguments: `|| { /* code */ }`
+   = note: unsafe function cannot be called generically without an unsafe block
    = note: `#[target_feature]` functions do not implement the `Fn` traits
    = note: try casting the function to a `fn` pointer or wrapping it in a closure
 note: required by a bound in `call_once`

tests/ui/suggestions/path-display.stderr

@@ -7,8 +7,8 @@ LL |     println!("{}", path);
    |               required by this formatting parameter
    |
    = help: the trait `std::fmt::Display` is not implemented for `Path`
-   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
    = note: call `.display()` or `.to_string_lossy()` to safely print paths, as they may contain non-Unicode data
+   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
    = note: required for `&Path` to implement `std::fmt::Display`
 
 error[E0277]: `PathBuf` doesn't implement `std::fmt::Display`
@@ -20,8 +20,8 @@ LL |     println!("{}", path);
    |               required by this formatting parameter
    |
    = help: the trait `std::fmt::Display` is not implemented for `PathBuf`
-   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
    = note: call `.display()` or `.to_string_lossy()` to safely print paths, as they may contain non-Unicode data
+   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
 
 error: aborting due to 2 previous errors
 

tests/ui/traits/next-solver/fn-trait.stderr

@@ -7,8 +7,8 @@ LL |     require_fn(f as unsafe fn() -> i32);
    |     required by a bound introduced by this call
    |
    = help: the trait `Fn()` is not implemented for `unsafe fn() -> i32`
-   = note: unsafe function cannot be called generically without an unsafe block
    = note: wrap the `unsafe fn() -> i32` in a closure with no arguments: `|| { /* code */ }`
+   = note: unsafe function cannot be called generically without an unsafe block
 note: required by a bound in `require_fn`
   --> $DIR/fn-trait.rs:3:23
    |
@@ -56,8 +56,8 @@ LL |     require_fn(h);
    |     required by a bound introduced by this call
    |
    = help: the trait `Fn()` is not implemented for fn item `unsafe fn() -> i32 {h}`
-   = note: unsafe function cannot be called generically without an unsafe block
    = note: wrap the `unsafe fn() -> i32 {h}` in a closure with no arguments: `|| { /* code */ }`
+   = note: unsafe function cannot be called generically without an unsafe block
 note: required by a bound in `require_fn`
   --> $DIR/fn-trait.rs:3:23
    |

tests/ui/unpretty/diagnostic-attr.stdout

@@ -7,7 +7,7 @@ use ::std::prelude::rust_2015::*;
 //@ edition: 2015
 
 #[attr = OnUnimplemented {directive: Directive {is_rustc_attr: false,
-subcommands: [],
+filters: [],
 message: FormatString {input: "My Message for `ImportantTrait<{A}>` implemented for `{Self}`",
 pieces: [Lit("My Message for `ImportantTrait<"),
 Arg(GenericParam {generic_param: "A"}), Lit(">` implemented for `"),
@@ -22,7 +22,7 @@ impl <T> ImportantTrait<T> for T where T: Clone { }
 
 struct X;
 
-#[attr = OnConst {directive: Directive {is_rustc_attr: false, subcommands: [],
+#[attr = OnConst {directive: Directive {is_rustc_attr: false, filters: [],
 message: FormatString {input: "My Message for `ImportantTrait<u8>` implemented for `{Self}`",
 pieces: [Lit("My Message for `ImportantTrait<u8>` implemented for `"),
 Arg(SelfUpper), Lit("`")]}, label: FormatString {input: "My Label",