Rollup merge of #117205 - weiznich:multiple_notes_for_on_unimplemented, r=compiler-errors

fmease · web-flow · commit 5eb76fac7cd9 · 2023-10-30T10:48:18.000+01:00
Allows `#[diagnostic::on_unimplemented]` attributes to have multiple

notes

This commit extends the `#[diagnostic::on_unimplemented]` (and `#[rustc_on_unimplemented]`) attributes to allow multiple `note` options. This enables emitting multiple notes for custom error messages. For now I've opted to not change any of the existing usages of `#[rustc_on_unimplemented]` and just updated the relevant compile tests.

r? `@compiler-errors`

I'm happy to adjust any of the existing changed location to emit the old error message if that's desired.
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs
@@ -319,7 +319,7 @@ pub struct OnUnimplementedDirective {
     pub subcommands: Vec<OnUnimplementedDirective>,
     pub message: Option<OnUnimplementedFormatString>,
     pub label: Option<OnUnimplementedFormatString>,
-    pub note: Option<OnUnimplementedFormatString>,
+    pub notes: Vec<OnUnimplementedFormatString>,
     pub parent_label: Option<OnUnimplementedFormatString>,
     pub append_const_msg: Option<AppendConstMessage>,
 }
@@ -329,7 +329,7 @@ pub struct OnUnimplementedDirective {
 pub struct OnUnimplementedNote {
     pub message: Option<String>,
     pub label: Option<String>,
-    pub note: Option<String>,
+    pub notes: Vec<String>,
     pub parent_label: Option<String>,
     // If none, should fall back to a generic message
     pub append_const_msg: Option<AppendConstMessage>,
@@ -399,7 +399,7 @@ impl<'tcx> OnUnimplementedDirective {
 
         let mut message = None;
         let mut label = None;
-        let mut note = None;
+        let mut notes = Vec::new();
         let mut parent_label = None;
         let mut subcommands = vec![];
         let mut append_const_msg = None;
@@ -415,10 +415,12 @@ impl<'tcx> OnUnimplementedDirective {
                     label = parse_value(label_)?;
                     continue;
                 }
-            } else if item.has_name(sym::note) && note.is_none() {
+            } else if item.has_name(sym::note) {
                 if let Some(note_) = item.value_str() {
-                    note = parse_value(note_)?;
-                    continue;
+                    if let Some(note) = parse_value(note_)? {
+                        notes.push(note);
+                        continue;
+                    }
                 }
             } else if item.has_name(sym::parent_label)
                 && parent_label.is_none()
@@ -432,7 +434,7 @@ impl<'tcx> OnUnimplementedDirective {
                 && is_root
                 && message.is_none()
                 && label.is_none()
-                && note.is_none()
+                && notes.is_empty()
                 && !is_diagnostic_namespace_variant
             // FIXME(diagnostic_namespace): disallow filters for now
             {
@@ -487,7 +489,7 @@ impl<'tcx> OnUnimplementedDirective {
                 subcommands,
                 message,
                 label,
-                note,
+                notes,
                 parent_label,
                 append_const_msg,
             }))
@@ -505,12 +507,14 @@ impl<'tcx> OnUnimplementedDirective {
                     if let Some(aggr) = aggr {
                         let mut subcommands = aggr.subcommands;
                         subcommands.extend(directive.subcommands);
+                        let mut notes = aggr.notes;
+                        notes.extend(directive.notes);
                         Ok(Some(Self {
                             condition: aggr.condition.or(directive.condition),
                             subcommands,
                             message: aggr.message.or(directive.message),
                             label: aggr.label.or(directive.label),
-                            note: aggr.note.or(directive.note),
+                            notes,
                             parent_label: aggr.parent_label.or(directive.parent_label),
                             append_const_msg: aggr.append_const_msg.or(directive.append_const_msg),
                         }))
@@ -543,7 +547,7 @@ impl<'tcx> OnUnimplementedDirective {
                         value,
                         attr.span,
                     )?),
-                    note: None,
+                    notes: Vec::new(),
                     parent_label: None,
                     append_const_msg: None,
                 }))
@@ -600,7 +604,7 @@ impl<'tcx> OnUnimplementedDirective {
     ) -> OnUnimplementedNote {
         let mut message = None;
         let mut label = None;
-        let mut note = None;
+        let mut notes = Vec::new();
         let mut parent_label = None;
         let mut append_const_msg = None;
         info!("evaluate({:?}, trait_ref={:?}, options={:?})", self, trait_ref, options);
@@ -637,9 +641,7 @@ impl<'tcx> OnUnimplementedDirective {
                 label = Some(label_.clone());
             }
 
-            if let Some(ref note_) = command.note {
-                note = Some(note_.clone());
-            }
+            notes.extend(command.notes.clone());
 
             if let Some(ref parent_label_) = command.parent_label {
                 parent_label = Some(parent_label_.clone());
@@ -651,7 +653,7 @@ impl<'tcx> OnUnimplementedDirective {
         OnUnimplementedNote {
             label: label.map(|l| l.format(tcx, trait_ref, &options_map)),
             message: message.map(|m| m.format(tcx, trait_ref, &options_map)),
-            note: note.map(|n| n.format(tcx, trait_ref, &options_map)),
+            notes: notes.into_iter().map(|n| n.format(tcx, trait_ref, &options_map)).collect(),
             parent_label: parent_label.map(|e_s| e_s.format(tcx, trait_ref, &options_map)),
             append_const_msg,
         }
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
@@ -445,29 +445,29 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                         let OnUnimplementedNote {
                             message,
                             label,
-                            note,
+                            notes,
                             parent_label,
                             append_const_msg,
                         } = self.on_unimplemented_note(trait_ref, &obligation);
                         let have_alt_message = message.is_some() || label.is_some();
                         let is_try_conversion = self.is_try_conversion(span, trait_ref.def_id());
                         let is_unsize =
                             Some(trait_ref.def_id()) == self.tcx.lang_items().unsize_trait();
-                        let (message, note, append_const_msg) = if is_try_conversion {
+                        let (message, notes, append_const_msg) = if is_try_conversion {
                             (
                                 Some(format!(
                                     "`?` couldn't convert the error to `{}`",
                                     trait_ref.skip_binder().self_ty(),
                                 )),
-                                Some(
+                                vec![
                                     "the question mark operation (`?`) implicitly performs a \
                                      conversion on the error value using the `From` trait"
                                         .to_owned(),
-                                ),
+                                ],
                                 Some(AppendConstMessage::Default),
                             )
                         } else {
-                            (message, note, append_const_msg)
+                            (message, notes, append_const_msg)
                         };
 
                         let err_msg = self.get_standard_error_message(
@@ -588,9 +588,9 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                         if let Some((msg, span)) = type_def {
                             err.span_label(span, msg);
                         }
-                        if let Some(s) = note {
+                        for note in notes {
                             // If it has a custom `#[rustc_on_unimplemented]` note, let's display it
-                            err.note(s);
+                            err.note(note);
                         }
                         if let Some(s) = parent_label {
                             let body = obligation.cause.body_id;
diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs
@@ -621,7 +621,20 @@ impl<T: ?Sized> Copy for &T {}
         note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicBool` instead",
     ),
     on(
-        _Self = "core::cell::Cell<T>",
+        all(
+            _Self = "core::cell::Cell<T>",
+            not(_Self = "core::cell::Cell<u8>"),
+            not(_Self = "core::cell::Cell<u16>"),
+            not(_Self = "core::cell::Cell<u32>"),
+            not(_Self = "core::cell::Cell<u64>"),
+            not(_Self = "core::cell::Cell<usize>"),
+            not(_Self = "core::cell::Cell<i8>"),
+            not(_Self = "core::cell::Cell<i16>"),
+            not(_Self = "core::cell::Cell<i32>"),
+            not(_Self = "core::cell::Cell<i64>"),
+            not(_Self = "core::cell::Cell<isize>"),
+            not(_Self = "core::cell::Cell<bool>")
+        ),
         note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock`",
     ),
     on(
diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr
@@ -26,6 +26,7 @@ LL |     takes_foo(());
    |
    = help: the trait `Foo` is not implemented for `()`
    = note: custom note
+   = note: fallback note
 help: this trait has no implementations, consider adding one
   --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:13:1
    |
diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/multiple_notes.rs b/tests/ui/diagnostic_namespace/on_unimplemented/multiple_notes.rs
@@ -0,0 +1,18 @@
+#![feature(diagnostic_namespace)]
+
+#[diagnostic::on_unimplemented(message = "Foo", label = "Bar", note = "Baz", note = "Boom")]
+trait Foo {}
+
+#[diagnostic::on_unimplemented(message = "Bar", label = "Foo", note = "Baz")]
+#[diagnostic::on_unimplemented(note = "Baz2")]
+trait Bar {}
+
+fn takes_foo(_: impl Foo) {}
+fn takes_bar(_: impl Bar) {}
+
+fn main() {
+    takes_foo(());
+    //~^ERROR Foo
+    takes_bar(());
+    //~^ERROR Bar
+}
diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/multiple_notes.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/multiple_notes.stderr
@@ -0,0 +1,47 @@
+error[E0277]: Foo
+  --> $DIR/multiple_notes.rs:14:15
+   |
+LL |     takes_foo(());
+   |     --------- ^^ Bar
+   |     |
+   |     required by a bound introduced by this call
+   |
+   = help: the trait `Foo` is not implemented for `()`
+   = note: Baz
+   = note: Boom
+help: this trait has no implementations, consider adding one
+  --> $DIR/multiple_notes.rs:4:1
+   |
+LL | trait Foo {}
+   | ^^^^^^^^^
+note: required by a bound in `takes_foo`
+  --> $DIR/multiple_notes.rs:10:22
+   |
+LL | fn takes_foo(_: impl Foo) {}
+   |                      ^^^ required by this bound in `takes_foo`
+
+error[E0277]: Bar
+  --> $DIR/multiple_notes.rs:16:15
+   |
+LL |     takes_bar(());
+   |     --------- ^^ Foo
+   |     |
+   |     required by a bound introduced by this call
+   |
+   = help: the trait `Bar` is not implemented for `()`
+   = note: Baz
+   = note: Baz2
+help: this trait has no implementations, consider adding one
+  --> $DIR/multiple_notes.rs:8:1
+   |
+LL | trait Bar {}
+   | ^^^^^^^^^
+note: required by a bound in `takes_bar`
+  --> $DIR/multiple_notes.rs:11:22
+   |
+LL | fn takes_bar(_: impl Bar) {}
+   |                      ^^^ required by this bound in `takes_bar`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0277`.
diff --git a/tests/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr b/tests/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr
@@ -58,6 +58,7 @@ 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: `#[target_feature]` functions do not implement the `Fn` traits
 note: required by a bound in `call`
@@ -75,6 +76,7 @@ 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: `#[target_feature]` functions do not implement the `Fn` traits
 note: required by a bound in `call_mut`
@@ -92,6 +94,7 @@ 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: `#[target_feature]` functions do not implement the `Fn` traits
 note: required by a bound in `call_once`
diff --git a/tests/ui/suggestions/path-display.stderr b/tests/ui/suggestions/path-display.stderr
@@ -5,6 +5,7 @@ LL |     println!("{}", path);
    |                    ^^^^ `Path` cannot be formatted with the default formatter; call `.display()` on it
    |
    = 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: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
 
@@ -15,6 +16,7 @@ LL |     println!("{}", path);
    |                    ^^^^ `PathBuf` cannot be formatted with the default formatter; call `.display()` on it
    |
    = 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: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
 
diff --git a/tests/ui/traits/new-solver/fn-trait.stderr b/tests/ui/traits/new-solver/fn-trait.stderr
@@ -7,6 +7,7 @@ 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: required by a bound in `require_fn`
   --> $DIR/fn-trait.rs:3:23
@@ -97,6 +98,7 @@ 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: required by a bound in `require_fn`
   --> $DIR/fn-trait.rs:3:23

Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,7 @@ LL \| takes_foo(());`
`26`	`26`	`\|`
`27`	`27`	= help: the trait `Foo` is not implemented for `()`
`28`	`28`	`= note: custom note`
	`29`	`+ = note: fallback note`
`29`	`30`	`help: this trait has no implementations, consider adding one`
`30`	`31`	`--> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:13:1`
`31`	`32`	`\|`
Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@ LL \| println!("{}", path);`
`5`	`5`	\| ^^^^ `Path` cannot be formatted with the default formatter; call `.display()` on it
`6`	`6`	`\|`
`7`	`7`	= help: the trait `std::fmt::Display` is not implemented for `Path`
	`8`	+ = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
`8`	`9`	= note: call `.display()` or `.to_string_lossy()` to safely print paths, as they may contain non-Unicode data
`9`	`10`	= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
`10`	`11`
`@@ -15,6 +16,7 @@ LL \| println!("{}", path);`
`15`	`16`	\| ^^^^ `PathBuf` cannot be formatted with the default formatter; call `.display()` on it
`16`	`17`	`\|`
`17`	`18`	= help: the trait `std::fmt::Display` is not implemented for `PathBuf`
	`19`	+ = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
`18`	`20`	= note: call `.display()` or `.to_string_lossy()` to safely print paths, as they may contain non-Unicode data
`19`	`21`	= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
`20`	`22`