Skip to content

Pre lint port cleanups #162813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rust-bors merged 2 commits into from
Sep 16, 2026
Merged

Pre lint port cleanups #162813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f9d169
add pre expansion parsing test of lint attrs
mejrs Sep 15, 2026
6d505c6
check_unused_attribute: handle crate level only attrs via config, and…
mejrs Sep 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions compiler/rustc_lint_defs/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4076,8 +4076,9 @@ declare_lint! {
/// ### Example
///
/// ```rust,ignore (needs CLI args, platform-specific)
/// #[warn(linker_messages)]
/// extern "C" {
/// #![warn(linker_messages)]
///
/// unsafe extern "C" {
/// fn foo();
/// }
/// fn main () { unsafe { foo(); } }
Expand Down Expand Up @@ -4112,7 +4113,8 @@ declare_lint! {
// Linker messages don't live up to the high standard people expect of rustc's errors.
// Prevent `-D warnings` from applying to it.
// It's still possible to pass `-D linker-messages` specifically.
ignore_deny_warnings
ignore_deny_warnings,
crate_level_only
}

declare_lint! {
Expand All @@ -4121,7 +4123,8 @@ declare_lint! {
/// ### Example
///
/// ```rust,ignore (needs CLI args, platform-specific)
/// #[warn(linker_info)]
/// #![warn(linker_info)]
///
/// fn main () {}
/// ```
///
Expand All @@ -4146,7 +4149,8 @@ declare_lint! {
/// <https://github.com/rust-lang/rust/issues/136096>.
pub LINKER_INFO,
Allow,
"linker warnings known to be informational-only and not indicative of a problem"
"linker warnings known to be informational-only and not indicative of a problem",
crate_level_only
}

declare_lint! {
Expand Down
42 changes: 6 additions & 36 deletions compiler/rustc_passes/src/check_attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::cell::Cell;
use std::slice;

use rustc_abi::ExternAbi;
use rustc_ast::{AttrStyle, MetaItemKind, ast};
use rustc_ast::MetaItemKind;
use rustc_attr_parsing::AttributeParser;
use rustc_data_structures::thin_vec::ThinVec;
use rustc_errors::{DiagCtxtHandle, IntoDiagArg, MultiSpan, msg};
Expand Down Expand Up @@ -138,9 +138,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
match attr {
Attribute::Parsed(attr_kind) => {
self.check_one_parsed_attribute(hir_id, span, target, item, attr_kind);
self.check_unused_attribute(hir_id, attr, None);
self.check_unused_attribute(hir_id, attr);
}
Attribute::Unparsed(attr_item) => {
Attribute::Unparsed(_) => {
match attr.path().as_slice() {
// ok
[sym::allow | sym::expect | sym::warn | sym::deny | sym::forbid, ..] => {}
Expand All @@ -167,7 +167,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
[] => unreachable!(),
}

self.check_unused_attribute(hir_id, attr, Some(attr_item.style));
self.check_unused_attribute(hir_id, attr);
}
}
}
Expand Down Expand Up @@ -275,7 +275,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
AttributeKind::LinkName { .. } => (),
AttributeKind::LinkOrdinal { .. } => (),
AttributeKind::LinkSection { .. } => (),
AttributeKind::LoopMatch(..) => {}
AttributeKind::LoopMatch(..) => (),
AttributeKind::MacroEscape => (),
AttributeKind::MacroUse { .. } => (),
AttributeKind::Marker => (),
Expand Down Expand Up @@ -1316,7 +1316,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

fn check_unused_attribute(&self, hir_id: HirId, attr: &Attribute, style: Option<AttrStyle>) {
fn check_unused_attribute(&self, hir_id: HirId, attr: &Attribute) {
// Warn on useless empty attributes.
// FIXME(jdonszelmann): this lint should be moved to attribute parsing, see `AcceptContext::warn_empty_attribute`
let note =
Expand Down Expand Up @@ -1351,34 +1351,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
})
{
if hir_id != CRATE_HIR_ID {
match style {
Some(ast::AttrStyle::Outer) => {
let attr_span = attr.span();
let bang_position = self
.tcx
.sess
.source_map()
.span_until_char(attr_span, '[')
.shrink_to_hi();

self.tcx.emit_node_span_lint(
UNUSED_ATTRIBUTES,
hir_id,
attr_span,
diagnostics::OuterCrateLevelAttr {
suggestion: diagnostics::OuterCrateLevelAttrSuggestion {
bang_position,
},
},
)
}
Some(ast::AttrStyle::Inner) | None => self.tcx.emit_node_span_lint(
UNUSED_ATTRIBUTES,
hir_id,
attr.span(),
diagnostics::InnerCrateLevelAttr,
),
};
return;
} else {
let never_needs_link = self
Expand All @@ -1401,8 +1373,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
&& !self.tcx.crate_types().contains(&CrateType::Executable)
{
diagnostics::UnusedNote::NoEffectDeadCodePubInBinary
} else if attr.has_name(sym::default_method_body_is_const) {
diagnostics::UnusedNote::DefaultMethodBodyConst
} else {
return;
};
Expand Down
20 changes: 0 additions & 20 deletions compiler/rustc_passes/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,24 +28,6 @@ pub(crate) struct MixedExportNameAndNoMangle {
pub export_name_attr: &'static str,
}

#[derive(Diagnostic)]
#[diag("crate-level attribute should be an inner attribute")]
pub(crate) struct OuterCrateLevelAttr {
#[subdiagnostic]
pub suggestion: OuterCrateLevelAttrSuggestion,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion("add a `!`", style = "verbose")]
pub(crate) struct OuterCrateLevelAttrSuggestion {
#[suggestion_part(code = "!")]
pub bang_position: Span,
}

#[derive(Diagnostic)]
#[diag("crate-level attribute should be in the root module")]
pub(crate) struct InnerCrateLevelAttr;

#[derive(Diagnostic)]
#[diag("`#[doc(alias = \"...\")]` isn't allowed on {$location}")]
pub(crate) struct DocAliasBadLocation<'a> {
Expand Down Expand Up @@ -237,8 +219,6 @@ pub(crate) enum UnusedNote {
EmptyList { name: Symbol },
#[note("attribute `{$name}` without any lints has no effect")]
NoLints { name: Symbol },
#[note("`default_method_body_is_const` has been replaced with `const` on traits")]
DefaultMethodBodyConst,
#[note(
"the `linker_messages` and `linker_info` lints can only be controlled at the root of a crate that needs to be linked"
)]
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_span/src/symbol.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,6 @@ symbols! {
default_field_values,
default_fn,
default_lib_allocator,
default_method_body_is_const,
// --------------------------
// Lang items which are used only for experiments with auto traits with default bounds.
// These lang items are not actually defined in core/std. Experiment is a part of
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/lint/linker-warning.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
#![crate_type = "lib"]
#![warn(unused_attributes)]
#![allow(linker_messages)]
//~^ WARNING unused attribute
//~^ WARN unused attribute

#[allow(linker_messages)]
//~^ WARNING should be an inner attribute
//~^ WARN allow(linker_messages) is ignored unless specified at crate level
fn foo() {}
10 changes: 3 additions & 7 deletions tests/ui/lint/linker-warning.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
warning: crate-level attribute should be an inner attribute
--> $DIR/linker-warning.rs:7:1
warning: allow(linker_messages) is ignored unless specified at crate level
--> $DIR/linker-warning.rs:7:9
|
LL | #[allow(linker_messages)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/linker-warning.rs:3:9
|
LL | #![warn(unused_attributes)]
| ^^^^^^^^^^^^^^^^^
help: add a `!`
|
LL | #![allow(linker_messages)]
| +

warning: unused attribute
--> $DIR/linker-warning.rs:4:1
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/lint/pre-expansion-parsing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![crate_type = "lib"]

#[expect] // OK
#[expect[wut]] // OK
#[expect(expect)] // OK
#[expect(expect(expect))] //~ ERROR malformed lint attribute input
//~| ERROR malformed lint attribute input

@petrochenkov petrochenkov Sep 15, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also should be ok.

Before the const _ item is fully expanded (the item itself, not its nested nodes), all the #[expect]s (and other potential inert attributes) are just tokens without any semantic meaning, and the active attributes can change those tokens in any way or remove them like cfg(false) does, so any semantic checks like "this built-in attribute has unexpected input" should not be performed for them.

View changes since the review

@petrochenkov petrochenkov Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this sense

#[expect(expect(expect))]
expand_to_nothing!();

is also equivalent to #[expect(expect(expect))] in cfg(false) code, and also shouldn't report any "malformed" attribute errors, and #160904 is not doing the right thing.
Although it would still be nice to conservatively turn it into an error, and then try turning #[attrs] in #[attrs] mac_call!() into a part of the macro input (#63221 (comment)).

@mejrs mejrs Sep 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also should be ok.

I agree, it's annoyingly inconsistent. I'm just adding the test here to document the current behavior and I don't want to change that behavior before or during #162811 because it'll make porting lint attributes much harder to write and review.

Before the const _ item is fully expanded (the item itself, not its nested nodes), all the #[expect]s (and other potential inert attributes) are just tokens without any semantic meaning ...

They have meaning for pre-expansion lints, so it's not as simple as "they're just tokens":

//@ edition: 2015

#[warn(keyword_idents_2024)]
#[cfg(false)]
fn foo() {
    let gen = 5; //~ WARN `gen` is a keyword in the 2024 edition
}

@petrochenkov petrochenkov Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since when the example with #[warn(keyword_idents_2024)] works, and how?
Previously lint attributes were only attached to node ids, and node ids are not created for cfg(false) code.
I don't think it's generally a good idea to support this, if it goes against the core expansion model.

@petrochenkov petrochenkov Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to change that behavior before or during #162811 because it'll make porting lint attributes much harder to write and review.

With this I agree.

@mejrs mejrs Sep 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This equivalent example works in rust 1.30:

//@ edition: 2015

#[warn(keyword_idents)]
#[cfg(any())]
fn foo() {
    let async = 5; // `async` is a keyword in the 2018 edition
}

@petrochenkov petrochenkov Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, weird, I need to investigate.

#[deny(({!}))] // OK
#[expect[helix::<ub>]] // OK
#[cfg(false)]
const _: () = ();
17 changes: 17 additions & 0 deletions tests/ui/lint/pre-expansion-parsing.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error[E0452]: malformed lint attribute input
--> $DIR/pre-expansion-parsing.rs:6:10
|
LL | #[expect(expect(expect))]
| ^^^^^^^^^^^^^^ bad attribute argument

error[E0452]: malformed lint attribute input
--> $DIR/pre-expansion-parsing.rs:6:10
|
LL | #[expect(expect(expect))]
| ^^^^^^^^^^^^^^ bad attribute argument
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0452`.
Loading