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 160b179
Show file tree
Hide file tree
Showing 9 changed files with 111 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
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
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 160b179

Please sign in to comment.