Skip to content

Commit

Permalink
Rollup merge of rust-lang#124875 - compiler-errors:more-diagnostics-i…
Browse files Browse the repository at this point in the history
…ces, r=estebank

Fix more ICEs in `diagnostic::on_unimplemented`

There were 8 other calls to `expect_local` left in `on_unimplemented.rs` -- all of which (afaict) could be turned into ICEs.

I would really like to see validation of `on_unimplemented` separated from parsing, so we only emit errors here:
https://github.com/rust-lang/rust/blob/a60f077c38fe66d05449919842d3d73e3299bbab/compiler/rustc_hir_analysis/src/check/check.rs#L836-L839
...And gracefully fail instead when emitting trait predicate failures, not *ever* even trying to emit an error or a lint. But that's left for a separate PR.

r? `@estebank`
  • Loading branch information
matthiaskrgr committed May 9, 2024
2 parents 4510ee3 + 7dbdbaa commit c49436d
Show file tree
Hide file tree
Showing 6 changed files with 259 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -350,12 +350,14 @@ impl IgnoredDiagnosticOption {
option_name: &'static str,
) {
if let (Some(new_item), Some(old_item)) = (new, old) {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id.expect_local()),
new_item,
IgnoredDiagnosticOption { span: new_item, prev_span: old_item, option_name },
);
if let Some(item_def_id) = item_def_id.as_local() {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id),
new_item,
IgnoredDiagnosticOption { span: new_item, prev_span: old_item, option_name },
);
}
}
}
}
Expand Down Expand Up @@ -639,30 +641,38 @@ impl<'tcx> OnUnimplementedDirective {
AttrArgs::Eq(span, AttrArgsEq::Hir(expr)) => span.to(expr.span),
};

tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id.expect_local()),
report_span,
MalformedOnUnimplementedAttrLint::new(report_span),
);
if let Some(item_def_id) = item_def_id.as_local() {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id),
report_span,
MalformedOnUnimplementedAttrLint::new(report_span),
);
}
Ok(None)
}
} else if is_diagnostic_namespace_variant {
match &attr.kind {
AttrKind::Normal(p) if !matches!(p.item.args, AttrArgs::Empty) => {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id.expect_local()),
attr.span,
MalformedOnUnimplementedAttrLint::new(attr.span),
);
if let Some(item_def_id) = item_def_id.as_local() {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id),
attr.span,
MalformedOnUnimplementedAttrLint::new(attr.span),
);
}
}
_ => {
if let Some(item_def_id) = item_def_id.as_local() {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id),
attr.span,
MissingOptionsForOnUnimplementedAttr,
)
}
}
_ => tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id.expect_local()),
attr.span,
MissingOptionsForOnUnimplementedAttr,
),
};

Ok(None)
Expand Down Expand Up @@ -791,12 +801,14 @@ impl<'tcx> OnUnimplementedFormatString {
|| format_spec.precision_span.is_some()
|| format_spec.fill_span.is_some())
{
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id.expect_local()),
self.span,
InvalidFormatSpecifier,
);
if let Some(item_def_id) = item_def_id.as_local() {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id),
self.span,
InvalidFormatSpecifier,
);
}
}
match a.position {
Position::ArgumentNamed(s) => {
Expand All @@ -812,15 +824,17 @@ impl<'tcx> OnUnimplementedFormatString {
s if generics.params.iter().any(|param| param.name == s) => (),
s => {
if self.is_diagnostic_namespace_variant {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id.expect_local()),
self.span,
UnknownFormatParameterForOnUnimplementedAttr {
argument_name: s,
trait_name,
},
);
if let Some(item_def_id) = item_def_id.as_local() {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id),
self.span,
UnknownFormatParameterForOnUnimplementedAttr {
argument_name: s,
trait_name,
},
);
}
} else {
result = Err(struct_span_code_err!(
tcx.dcx(),
Expand All @@ -842,12 +856,14 @@ impl<'tcx> OnUnimplementedFormatString {
// `{:1}` and `{}` are not to be used
Position::ArgumentIs(..) | Position::ArgumentImplicitlyIs(_) => {
if self.is_diagnostic_namespace_variant {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id.expect_local()),
self.span,
DisallowedPositionalArgument,
);
if let Some(item_def_id) = item_def_id.as_local() {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id),
self.span,
DisallowedPositionalArgument,
);
}
} else {
let reported = struct_span_code_err!(
tcx.dcx(),
Expand All @@ -870,12 +886,14 @@ impl<'tcx> OnUnimplementedFormatString {
// so that users are aware that something is not correct
for e in parser.errors {
if self.is_diagnostic_namespace_variant {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id.expect_local()),
self.span,
WrappedParserError { description: e.description, label: e.label },
);
if let Some(item_def_id) = item_def_id.as_local() {
tcx.emit_node_span_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id),
self.span,
WrappedParserError { description: e.description, label: e.label },
);
}
} else {
let reported =
struct_span_code_err!(tcx.dcx(), self.span, E0231, "{}", e.description,).emit();
Expand Down
26 changes: 25 additions & 1 deletion tests/ui/diagnostic_namespace/auxiliary/bad_on_unimplemented.rs
Original file line number Diff line number Diff line change
@@ -1,2 +1,26 @@
#[diagnostic::on_unimplemented(aa = "broken")]
pub trait Test {}
pub trait MissingAttr {}

#[diagnostic::on_unimplemented(label = "a", label = "b")]
pub trait DuplicateAttr {}

#[diagnostic::on_unimplemented = "broken"]
pub trait NotMetaList {}

#[diagnostic::on_unimplemented]
pub trait Empty {}

#[diagnostic::on_unimplemented {}]
pub trait WrongDelim {}

#[diagnostic::on_unimplemented(label = "{A:.3}")]
pub trait BadFormatter<A> {}

#[diagnostic::on_unimplemented(label = "test {}")]
pub trait NoImplicitArgs {}

#[diagnostic::on_unimplemented(label = "{missing}")]
pub trait MissingArg {}

#[diagnostic::on_unimplemented(label = "{_}")]
pub trait BadArg {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//@ edition:2021
//@ aux-build:bad_on_unimplemented.rs

// Do not ICE when encountering a malformed `#[diagnostic::on_unimplemented]` annotation in a
// dependency when incorrectly used (#124651).

extern crate bad_on_unimplemented;

use bad_on_unimplemented::*;

fn missing_attr<T: MissingAttr>(_: T) {}
fn duplicate_attr<T: DuplicateAttr>(_: T) {}
fn not_meta_list<T: NotMetaList>(_: T) {}
fn empty<T: Empty>(_: T) {}
fn wrong_delim<T: WrongDelim>(_: T) {}
fn bad_formatter<T: BadFormatter<()>>(_: T) {}
fn no_implicit_args<T: NoImplicitArgs>(_: T) {}
fn missing_arg<T: MissingArg>(_: T) {}
fn bad_arg<T: BadArg>(_: T) {}

fn main() {
missing_attr(()); //~ ERROR E0277
duplicate_attr(()); //~ ERROR E0277
not_meta_list(()); //~ ERROR E0277
empty(()); //~ ERROR E0277
wrong_delim(()); //~ ERROR E0277
bad_formatter(()); //~ ERROR E0277
no_implicit_args(()); //~ ERROR E0277
missing_arg(()); //~ ERROR E0277
bad_arg(()); //~ ERROR E0277
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
error[E0277]: the trait bound `(): bad_on_unimplemented::MissingAttr` is not satisfied
--> $DIR/malformed_foreign_on_unimplemented.rs:22:18
|
LL | missing_attr(());
| ------------ ^^ the trait `bad_on_unimplemented::MissingAttr` is not implemented for `()`
| |
| required by a bound introduced by this call
|
note: required by a bound in `missing_attr`
--> $DIR/malformed_foreign_on_unimplemented.rs:11:20
|
LL | fn missing_attr<T: MissingAttr>(_: T) {}
| ^^^^^^^^^^^ required by this bound in `missing_attr`

error[E0277]: the trait bound `(): bad_on_unimplemented::DuplicateAttr` is not satisfied
--> $DIR/malformed_foreign_on_unimplemented.rs:23:20
|
LL | duplicate_attr(());
| -------------- ^^ a
| |
| required by a bound introduced by this call
|
= help: the trait `bad_on_unimplemented::DuplicateAttr` is not implemented for `()`
note: required by a bound in `duplicate_attr`
--> $DIR/malformed_foreign_on_unimplemented.rs:12:22
|
LL | fn duplicate_attr<T: DuplicateAttr>(_: T) {}
| ^^^^^^^^^^^^^ required by this bound in `duplicate_attr`

error[E0277]: the trait bound `(): bad_on_unimplemented::NotMetaList` is not satisfied
--> $DIR/malformed_foreign_on_unimplemented.rs:24:19
|
LL | not_meta_list(());
| ------------- ^^ the trait `bad_on_unimplemented::NotMetaList` is not implemented for `()`
| |
| required by a bound introduced by this call
|
note: required by a bound in `not_meta_list`
--> $DIR/malformed_foreign_on_unimplemented.rs:13:21
|
LL | fn not_meta_list<T: NotMetaList>(_: T) {}
| ^^^^^^^^^^^ required by this bound in `not_meta_list`

error[E0277]: the trait bound `(): bad_on_unimplemented::Empty` is not satisfied
--> $DIR/malformed_foreign_on_unimplemented.rs:25:11
|
LL | empty(());
| ----- ^^ the trait `bad_on_unimplemented::Empty` is not implemented for `()`
| |
| required by a bound introduced by this call
|
note: required by a bound in `empty`
--> $DIR/malformed_foreign_on_unimplemented.rs:14:13
|
LL | fn empty<T: Empty>(_: T) {}
| ^^^^^ required by this bound in `empty`

error[E0277]: the trait bound `(): bad_on_unimplemented::WrongDelim` is not satisfied
--> $DIR/malformed_foreign_on_unimplemented.rs:26:17
|
LL | wrong_delim(());
| ----------- ^^ the trait `bad_on_unimplemented::WrongDelim` is not implemented for `()`
| |
| required by a bound introduced by this call
|
note: required by a bound in `wrong_delim`
--> $DIR/malformed_foreign_on_unimplemented.rs:15:19
|
LL | fn wrong_delim<T: WrongDelim>(_: T) {}
| ^^^^^^^^^^ required by this bound in `wrong_delim`

error[E0277]: the trait bound `(): bad_on_unimplemented::BadFormatter<()>` is not satisfied
--> $DIR/malformed_foreign_on_unimplemented.rs:27:19
|
LL | bad_formatter(());
| ------------- ^^ ()
| |
| required by a bound introduced by this call
|
= help: the trait `bad_on_unimplemented::BadFormatter<()>` is not implemented for `()`
note: required by a bound in `bad_formatter`
--> $DIR/malformed_foreign_on_unimplemented.rs:16:21
|
LL | fn bad_formatter<T: BadFormatter<()>>(_: T) {}
| ^^^^^^^^^^^^^^^^ required by this bound in `bad_formatter`

error[E0277]: the trait bound `(): bad_on_unimplemented::NoImplicitArgs` is not satisfied
--> $DIR/malformed_foreign_on_unimplemented.rs:28:22
|
LL | no_implicit_args(());
| ---------------- ^^ test {}
| |
| required by a bound introduced by this call
|
= help: the trait `bad_on_unimplemented::NoImplicitArgs` is not implemented for `()`
note: required by a bound in `no_implicit_args`
--> $DIR/malformed_foreign_on_unimplemented.rs:17:24
|
LL | fn no_implicit_args<T: NoImplicitArgs>(_: T) {}
| ^^^^^^^^^^^^^^ required by this bound in `no_implicit_args`

error[E0277]: the trait bound `(): bad_on_unimplemented::MissingArg` is not satisfied
--> $DIR/malformed_foreign_on_unimplemented.rs:29:17
|
LL | missing_arg(());
| ----------- ^^ {missing}
| |
| required by a bound introduced by this call
|
= help: the trait `bad_on_unimplemented::MissingArg` is not implemented for `()`
note: required by a bound in `missing_arg`
--> $DIR/malformed_foreign_on_unimplemented.rs:18:19
|
LL | fn missing_arg<T: MissingArg>(_: T) {}
| ^^^^^^^^^^ required by this bound in `missing_arg`

error[E0277]: the trait bound `(): bad_on_unimplemented::BadArg` is not satisfied
--> $DIR/malformed_foreign_on_unimplemented.rs:30:13
|
LL | bad_arg(());
| ------- ^^ {_}
| |
| required by a bound introduced by this call
|
= help: the trait `bad_on_unimplemented::BadArg` is not implemented for `()`
note: required by a bound in `bad_arg`
--> $DIR/malformed_foreign_on_unimplemented.rs:19:15
|
LL | fn bad_arg<T: BadArg>(_: T) {}
| ^^^^^^ required by this bound in `bad_arg`

error: aborting due to 9 previous errors

For more information about this error, try `rustc --explain E0277`.
17 changes: 0 additions & 17 deletions tests/ui/diagnostic_namespace/on_unimplemented_ice.rs

This file was deleted.

17 changes: 0 additions & 17 deletions tests/ui/diagnostic_namespace/on_unimplemented_ice.stderr

This file was deleted.

0 comments on commit c49436d

Please sign in to comment.