Skip to content

Commit

Permalink
Allows #[diagnostic::on_unimplemented] attributes to have multiple
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
weiznich committed Oct 27, 2023
1 parent 6d674af commit 3c7daf9
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}
Expand All @@ -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>,
Expand Down Expand Up @@ -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;
Expand All @@ -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()
Expand All @@ -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
{
Expand Down Expand Up @@ -487,7 +489,7 @@ impl<'tcx> OnUnimplementedDirective {
subcommands,
message,
label,
note,
notes,
parent_label,
append_const_msg,
}))
Expand All @@ -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),
}))
Expand Down Expand Up @@ -543,7 +547,7 @@ impl<'tcx> OnUnimplementedDirective {
value,
attr.span,
)?),
note: None,
notes: Vec::new(),
parent_label: None,
append_const_msg: None,
}))
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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());
Expand All @@ -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,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,29 +428,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(
Expand Down Expand Up @@ -569,9 +569,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;
Expand Down
15 changes: 14 additions & 1 deletion library/core/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/diagnostic_namespace/on_unimplemented/multiple_notes.rs
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -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`.
3 changes: 3 additions & 0 deletions tests/ui/rfcs/rfc-2396-target_feature-11/fn-traits.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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`
Expand All @@ -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`
Expand Down
1 change: 1 addition & 0 deletions tests/ui/stdlib-unit-tests/not-sync.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | test::<Cell<i32>>();
| ^^^^^^^^^ `Cell<i32>` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `Cell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead
note: required by a bound in `test`
--> $DIR/not-sync.rs:5:12
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/suggestions/path-display.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand Down
1 change: 1 addition & 0 deletions tests/ui/sync/mutexguard-sync.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ LL | test_sync(guard);
| required by a bound introduced by this call
|
= help: the trait `Sync` is not implemented for `Cell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead
= note: required for `MutexGuard<'_, Cell<i32>>` to implement `Sync`
note: required by a bound in `test_sync`
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/traits/new-solver/fn-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 3c7daf9

Please sign in to comment.