Skip to content

Commit

Permalink
Rollup merge of #127591 - compiler-errors:label-after-primary, r=lcnr
Browse files Browse the repository at this point in the history
Make sure that labels are defined after the primary span in diagnostics

Putting a `#[label]` before a `#[primary_span]` results in that label being overwritten, due to the semantics of `Diagnostic::span` and the fact that labels are stored in the `MultiSpan` of the diagnostic.

This isn't possible to fix in general, since a lot of code actually *relies* in this overwriting behavior (e.g. `rustc_on_unimplemented`). However, it's useful to enforce this for derive-diagnostics, since this is certainly never what you intend to do in a derived diagnostic, where all the fields are meaningful parts of the diagnostic being rendered.

This only matters for `#[label]`, since those are the ones stored in the `MultiSpan` of the error.

We could also make this "just work" by sorting the attrs or processing the primary span attr first, however I think it's kinda pointless to do.

There was 1 case where this mattered, but we literally didn't have a test exercising that diagnostic 🙃
  • Loading branch information
matthiaskrgr authored Jul 11, 2024
2 parents 380c787 + 12ae282 commit 73c500b
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 1 deletion.
9 changes: 9 additions & 0 deletions compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ impl DiagnosticDeriveVariantBuilder {
let field_binding = &binding_info.binding;

let inner_ty = FieldInnerTy::from_type(&field.ty);
let mut seen_label = false;

field
.attrs
Expand All @@ -280,6 +281,14 @@ impl DiagnosticDeriveVariantBuilder {
}

let name = attr.path().segments.last().unwrap().ident.to_string();

if name == "primary_span" && seen_label {
span_err(attr.span().unwrap(), format!("`#[primary_span]` must be placed before labels, since it overwrites the span of the diagnostic")).emit();
}
if name == "label" {
seen_label = true;
}

let needs_clone =
name == "primary_span" && matches!(inner_ty, FieldInnerTy::Vec(_));
let (binding, needs_destructure) = if needs_clone {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1089,8 +1089,8 @@ pub(crate) struct ToolWasAlreadyRegistered {
#[derive(Diagnostic)]
#[diag(resolve_tool_only_accepts_identifiers)]
pub(crate) struct ToolOnlyAcceptsIdentifiers {
#[label]
#[primary_span]
#[label]
pub(crate) span: Span,
pub(crate) tool: Symbol,
}
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/tool-attributes/invalid-tool.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#![feature(register_tool)]

#![register_tool(1)]
//~^ ERROR `register_tool` only accepts identifiers

fn main() {}
8 changes: 8 additions & 0 deletions tests/ui/tool-attributes/invalid-tool.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: `register_tool` only accepts identifiers
--> $DIR/invalid-tool.rs:3:18
|
LL | #![register_tool(1)]
| ^ not an identifier

error: aborting due to 1 previous error

0 comments on commit 73c500b

Please sign in to comment.