Skip to content

Commit

Permalink
Print all labels, even if they have no span. Fall back to main item's…
Browse files Browse the repository at this point in the history
… span.
  • Loading branch information
oli-obk committed Nov 11, 2022
1 parent 21ce587 commit df2adc4
Show file tree
Hide file tree
Showing 11 changed files with 110 additions and 51 deletions.
5 changes: 4 additions & 1 deletion compiler/rustc_error_messages/locales/en-US/passes.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ passes_no_coverage_not_coverable =
passes_should_be_applied_to_fn =
attribute should be applied to a function definition
.label = not a function definition
.label = {$on_crate ->
[true] cannot be applied to crates
*[false] not a function definition
}
passes_naked_tracked_caller =
cannot use `#[track_caller]` with `#[naked]`
Expand Down
47 changes: 35 additions & 12 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2210,22 +2210,45 @@ impl FileWithAnnotatedLines {

if let Some(ref sm) = emitter.source_map() {
for span_label in msp.span_labels() {
let fixup_lo_hi = |span: Span| {
let lo = sm.lookup_char_pos(span.lo());
let mut hi = sm.lookup_char_pos(span.hi());

// Watch out for "empty spans". If we get a span like 6..6, we
// want to just display a `^` at 6, so convert that to
// 6..7. This is degenerate input, but it's best to degrade
// gracefully -- and the parser likes to supply a span like
// that for EOF, in particular.

if lo.col_display == hi.col_display && lo.line == hi.line {
hi.col_display += 1;
}
(lo, hi)
};

if span_label.span.is_dummy() {
if let Some(span) = msp.primary_span() {
// if we don't know where to render the annotation, emit it as a note
// on the primary span.

let (lo, hi) = fixup_lo_hi(span);

let ann = Annotation {
start_col: lo.col_display,
end_col: hi.col_display,
is_primary: span_label.is_primary,
label: span_label
.label
.as_ref()
.map(|m| emitter.translate_message(m, args).to_string()),
annotation_type: AnnotationType::Singleline,
};
add_annotation_to_file(&mut output, lo.file, lo.line, ann);
}
continue;
}

let lo = sm.lookup_char_pos(span_label.span.lo());
let mut hi = sm.lookup_char_pos(span_label.span.hi());

// Watch out for "empty spans". If we get a span like 6..6, we
// want to just display a `^` at 6, so convert that to
// 6..7. This is degenerate input, but it's best to degrade
// gracefully -- and the parser likes to supply a span like
// that for EOF, in particular.

if lo.col_display == hi.col_display && lo.line == hi.line {
hi.col_display += 1;
}
let (lo, hi) = fixup_lo_hi(span_label.span);

if lo.line != hi.line {
let ml = MultilineAnnotation {
Expand Down
55 changes: 42 additions & 13 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,13 @@ impl CheckAttrVisitor<'_> {
}
sym::naked => self.check_naked(hir_id, attr, span, target),
sym::rustc_legacy_const_generics => {
self.check_rustc_legacy_const_generics(&attr, span, target, item)
self.check_rustc_legacy_const_generics(hir_id, &attr, span, target, item)
}
sym::rustc_lint_query_instability => {
self.check_rustc_lint_query_instability(&attr, span, target)
self.check_rustc_lint_query_instability(hir_id, &attr, span, target)
}
sym::rustc_lint_diagnostics => {
self.check_rustc_lint_diagnostics(&attr, span, target)
self.check_rustc_lint_diagnostics(hir_id, &attr, span, target)
}
sym::rustc_lint_opt_ty => self.check_rustc_lint_opt_ty(&attr, span, target),
sym::rustc_lint_opt_deny_field_access => {
Expand All @@ -135,7 +135,9 @@ impl CheckAttrVisitor<'_> {
| sym::rustc_dirty
| sym::rustc_if_this_changed
| sym::rustc_then_this_would_need => self.check_rustc_dirty_clean(&attr),
sym::cmse_nonsecure_entry => self.check_cmse_nonsecure_entry(attr, span, target),
sym::cmse_nonsecure_entry => {
self.check_cmse_nonsecure_entry(hir_id, attr, span, target)
}
sym::collapse_debuginfo => self.check_collapse_debuginfo(attr, span, target),
sym::const_trait => self.check_const_trait(attr, span, target),
sym::must_not_suspend => self.check_must_not_suspend(&attr, span, target),
Expand Down Expand Up @@ -386,21 +388,29 @@ impl CheckAttrVisitor<'_> {
self.tcx.sess.emit_err(errors::AttrShouldBeAppliedToFn {
attr_span: attr.span,
defn_span: span,
on_crate: hir_id == CRATE_HIR_ID,
});
false
}
}
}

/// Checks if `#[cmse_nonsecure_entry]` is applied to a function definition.
fn check_cmse_nonsecure_entry(&self, attr: &Attribute, span: Span, target: Target) -> bool {
fn check_cmse_nonsecure_entry(
&self,
hir_id: HirId,
attr: &Attribute,
span: Span,
target: Target,
) -> bool {
match target {
Target::Fn
| Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true,
_ => {
self.tcx.sess.emit_err(errors::AttrShouldBeAppliedToFn {
attr_span: attr.span,
defn_span: span,
on_crate: hir_id == CRATE_HIR_ID,
});
false
}
Expand Down Expand Up @@ -465,9 +475,11 @@ impl CheckAttrVisitor<'_> {
true
}
_ => {
self.tcx
.sess
.emit_err(errors::TrackedCallerWrongLocation { attr_span, defn_span: span });
self.tcx.sess.emit_err(errors::TrackedCallerWrongLocation {
attr_span,
defn_span: span,
on_crate: hir_id == CRATE_HIR_ID,
});
false
}
}
Expand Down Expand Up @@ -576,6 +588,7 @@ impl CheckAttrVisitor<'_> {
self.tcx.sess.emit_err(errors::AttrShouldBeAppliedToFn {
attr_span: attr.span,
defn_span: span,
on_crate: hir_id == CRATE_HIR_ID,
});
false
}
Expand Down Expand Up @@ -1240,7 +1253,7 @@ impl CheckAttrVisitor<'_> {
UNUSED_ATTRIBUTES,
hir_id,
attr.span,
errors::Cold { span },
errors::Cold { span, on_crate: hir_id == CRATE_HIR_ID },
);
}
}
Expand Down Expand Up @@ -1376,6 +1389,7 @@ impl CheckAttrVisitor<'_> {
/// Checks if `#[rustc_legacy_const_generics]` is applied to a function and has a valid argument.
fn check_rustc_legacy_const_generics(
&self,
hir_id: HirId,
attr: &Attribute,
span: Span,
target: Target,
Expand All @@ -1386,6 +1400,7 @@ impl CheckAttrVisitor<'_> {
self.tcx.sess.emit_err(errors::AttrShouldBeAppliedToFn {
attr_span: attr.span,
defn_span: span,
on_crate: hir_id == CRATE_HIR_ID,
});
return false;
}
Expand Down Expand Up @@ -1450,12 +1465,19 @@ impl CheckAttrVisitor<'_> {

/// Helper function for checking that the provided attribute is only applied to a function or
/// method.
fn check_applied_to_fn_or_method(&self, attr: &Attribute, span: Span, target: Target) -> bool {
fn check_applied_to_fn_or_method(
&self,
hir_id: HirId,
attr: &Attribute,
span: Span,
target: Target,
) -> bool {
let is_function = matches!(target, Target::Fn | Target::Method(..));
if !is_function {
self.tcx.sess.emit_err(errors::AttrShouldBeAppliedToFn {
attr_span: attr.span,
defn_span: span,
on_crate: hir_id == CRATE_HIR_ID,
});
false
} else {
Expand All @@ -1467,17 +1489,24 @@ impl CheckAttrVisitor<'_> {
/// or method.
fn check_rustc_lint_query_instability(
&self,
hir_id: HirId,
attr: &Attribute,
span: Span,
target: Target,
) -> bool {
self.check_applied_to_fn_or_method(attr, span, target)
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
}

/// Checks that the `#[rustc_lint_diagnostics]` attribute is only applied to a function or
/// method.
fn check_rustc_lint_diagnostics(&self, attr: &Attribute, span: Span, target: Target) -> bool {
self.check_applied_to_fn_or_method(attr, span, target)
fn check_rustc_lint_diagnostics(
&self,
hir_id: HirId,
attr: &Attribute,
span: Span,
target: Target,
) -> bool {
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
}

/// Checks that the `#[rustc_lint_opt_ty]` attribute is only applied to a struct.
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ pub struct AttrShouldBeAppliedToFn {
pub attr_span: Span,
#[label]
pub defn_span: Span,
pub on_crate: bool,
}

#[derive(Diagnostic)]
Expand All @@ -97,6 +98,7 @@ pub struct TrackedCallerWrongLocation {
pub attr_span: Span,
#[label]
pub defn_span: Span,
pub on_crate: bool,
}

#[derive(Diagnostic)]
Expand Down Expand Up @@ -367,6 +369,7 @@ pub struct MustNotSuspend {
pub struct Cold {
#[label]
pub span: Span,
pub on_crate: bool,
}

#[derive(LintDiagnostic)]
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,12 @@ impl<'a> Resolver<'a> {
));

err.span_label(span, format!("`{}` re{} here", name, new_participle));
err.span_label(
self.session.source_map().guess_head_span(old_binding.span),
format!("previous {} of the {} `{}` here", old_noun, old_kind, name),
);
if !old_binding.span.is_dummy() && old_binding.span != span {
err.span_label(
self.session.source_map().guess_head_span(old_binding.span),
format!("previous {} of the {} `{}` here", old_noun, old_kind, name),
);
}

// See https://github.com/rust-lang/rust/issues/32354
use NameBindingKind::Import;
Expand Down
18 changes: 10 additions & 8 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -806,14 +806,16 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
err.code(rustc_errors::error_code!(E0411));
err.span_label(span, "`Self` is only available in impls, traits, and type definitions");
if let Some(item_kind) = self.diagnostic_metadata.current_item {
err.span_label(
item_kind.ident.span,
format!(
"`Self` not allowed in {} {}",
item_kind.kind.article(),
item_kind.kind.descr()
),
);
if !item_kind.ident.span.is_dummy() {
err.span_label(
item_kind.ident.span,
format!(
"`Self` not allowed in {} {}",
item_kind.kind.article(),
item_kind.kind.descr()
),
);
}
}
true
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/asm/naked-invalid-attr.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ error: attribute should be applied to a function definition
--> $DIR/naked-invalid-attr.rs:5:1
|
LL | #![naked]
| ^^^^^^^^^
| ^^^^^^^^^ cannot be applied to crates

error: aborting due to 5 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -110,19 +110,19 @@ error: attribute should be applied to an `extern crate` item
--> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:25:1
|
LL | #![no_link]
| ^^^^^^^^^^^
| ^^^^^^^^^^^ not an `extern crate` item

error: attribute should be applied to a free function, impl method or static
--> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:27:1
|
LL | #![export_name = "2200"]
| ^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^ not a free function, impl method or static

error[E0518]: attribute should be applied to function or closure
--> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:29:1
|
LL | #![inline]
| ^^^^^^^^^^
| ^^^^^^^^^^ not a function or closure

error: `macro_export` attribute cannot be used at crate level
--> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:12:1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//~ NOTE not a function
//~| NOTE not a foreign function or static
//~| NOTE not a function or static
//~| NOTE cannot be applied to crates
//~| NOTE not an `extern` block
// This test enumerates as many compiler-builtin ungated attributes as
// possible (that is, all the mutually compatible ones), and checks
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,31 +403,31 @@ warning: attribute should be applied to a function definition
--> $DIR/issue-43106-gating-of-builtin-attrs.rs:62:1
|
LL | #![cold]
| ^^^^^^^^
| ^^^^^^^^ cannot be applied to crates
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

warning: attribute should be applied to an `extern` block with non-Rust ABI
--> $DIR/issue-43106-gating-of-builtin-attrs.rs:64:1
|
LL | #![link()]
| ^^^^^^^^^^
| ^^^^^^^^^^ not an `extern` block
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

warning: attribute should be applied to a foreign function or static
--> $DIR/issue-43106-gating-of-builtin-attrs.rs:66:1
|
LL | #![link_name = "1900"]
| ^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^ not a foreign function or static
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

warning: attribute should be applied to a function or static
--> $DIR/issue-43106-gating-of-builtin-attrs.rs:69:1
|
LL | #![link_section = "1800"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^ not a function or static
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

Expand Down
5 changes: 1 addition & 4 deletions src/test/ui/issues/issue-69396-const-no-type-in-macro.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ error[E0428]: the name `A` is defined multiple times
--> $DIR/issue-69396-const-no-type-in-macro.rs:4:13
|
LL | const A = "A".$fn();
| ^^^^^^^^^^^^^^^^^^^^
| |
| `A` redefined here
| previous definition of the value `A` here
| ^^^^^^^^^^^^^^^^^^^^ `A` redefined here
...
LL | / suite! {
LL | | len;
Expand Down

0 comments on commit df2adc4

Please sign in to comment.