Remove `AttributeLintKind` by GuillaumeGomez · Pull Request #155760 · rust-lang/rust

GuillaumeGomez · 2026-04-25T02:20:52Z

View all comments

Part of #153099.

The AttributeLintKind type is finally gone! \o/

Diff is this big because I moved a file and a lot of Diagnostic types. :')

r? @JonathanBrouwer

…_parsing` lints

rustbot · 2026-04-25T02:20:56Z

Some changes occurred to diagnostic attributes.

cc @mejrs

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

petrochenkov · 2026-04-25T06:26:13Z

(Assigning myself for the suspicious infrastructural changes in the first commit.)

JonathanBrouwer · 2026-04-25T07:11:28Z

+        let Self { callback, info } = self;
+        // FIXME: remove this transmute once lifetime coercion blocker is solved.
+        let info: rustc_session::SessionAndCrateName<'static> =
+            unsafe { std::mem::transmute(info) };


Is does not sound correct to me that the lifetime is static here, shouldn't this be the lifetime of the session?
I'm also missing a safety comment (and kinda sad that we need unsafe for this in the first place)

JonathanBrouwer · 2026-04-25T07:13:43Z

                cx.emit_dyn_lint(
                    rustc_session::lint::builtin::INVALID_DOC_ATTRIBUTES,
-                    move |dcx, level| {
+                    move |dcx, level, _| {


Also kinda sad that all the calls need to ignore the third argument now except the few cursed ones that need it.
Some alternatives are:

Create a separate function next to emit_dyn_lint that takes a closure with the special argument

My idea in the next comment, with making session the first argument

View changes since the review

I think having a different function is likely better. For the session idea, answered in the other comment.

JonathanBrouwer · 2026-04-25T07:15:12Z

    pub span: MultiSpan,
    pub callback: Box<
-        dyn for<'a> Fn(DiagCtxtHandle<'a>, Level) -> Diag<'a, ()> + DynSend + DynSync + 'static,
+        dyn for<'a> Fn(DiagCtxtHandle<'a>, Level, &dyn std::any::Any) -> Diag<'a, ()>


If I understand correctly, the big problem is the crate name, we could just pass Session here without a dyn Any. How much would the diagnostics regress if we just don't make crate name available anymore? I think that shouldn't be too bad, and would prevent us from adding all this ugly code

View changes since the review

In that case maybe we could even replace the DiagCtxtHandle<'a> with &'a Session since I think you can get the DiagCtxtHandle<'a> from the session

It's not the current crate name but other crates name. So Session wouldn't solve our issue here.

What I'm saying is I believe there's only one diagnostic that needs the crate name, would it be that bad to just remove the crate name from the help message where it's used

In tests/ui/check-cfg/report-in-external-macros.rs#cargo it would do:

33 | 34 = note: no expected values for `feature` - = note: using a cfg inside a macro will use the cfgs from the destination crate and not the ones from the defining crate - = help: try referring to `cfg_macro::my_lib_macro_feature` crate for guidance on how handle this unexpected cfg - = help: the macro `cfg_macro::my_lib_macro_feature` may come from an old version of the `cfg_macro` crate, try updating your dependency with `cargo update -p cfg_macro` 38 = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration 39 = note: this warning originates in the macro `cfg_macro::my_lib_macro_feature` (in Nightly builds, run with -Z macro-backtrace for more info)

I personally don't care but maybe @Urgau has another opinion?

I don't think we need to fully remove the help message, only change it to not have the crate name in it

We could do away with just the macro name in most places in the diagnostic, except for the part where we tell people to update there dependency: cargo update -p cfg_macro.

We obviously should not suggest cargo update, that's too big of a hammer.

If it's too big of a hassle to keep a TyCtxt for the diagnostic, we can probably remove the suggestion to update the crate. That suggestion was added when we stabilized --check-cfg, at a time when basically no crate was --check-cfg compatible yet; the stabilization happened over 1 year ago, so the suggestion is probably no longer relevant.

As a note for myself, it means we can remove:

- = help: the macro `cfg_macro::my_lib_macro_feature` may come from an old version of the `cfg_macro` crate, try updating your dependency with `cargo update -p cfg_macro`

JonathanBrouwer · 2026-04-25T07:20:20Z

+        };
+        let sess = SessionAndCrateName { sess: self.sess, crate_name };
+        // FIXME: remove this transmute call once lifetime coercion issue is fixed.
+        let sess: SessionAndCrateName<'static> = unsafe { std::mem::transmute(sess) };


Same thing here about `static being questionable and the safety comment and about unsafe being sad

View changes since the review

Yeah, some deeper investigations is needed. At the beginning, I simply used tcx.sess and no transmute was needed because lifetime coercion was working fine. But for SessionAndCrateName, coercion doesn't work. I have some suspicions about the closure. Anyway, to be investigated.

Urgau · 2026-04-25T11:57:11Z

    sess: &Session,
    inst: &impl Fn(EscapeQuotes) -> String,
-) -> lints::UnexpectedCfgCargoHelp {
+) -> errors::UnexpectedCfgCargoHelp {


nit: I find it weird that the unexpected_cfgs diagnostic structs are in an errors module, while we are talking about a warn-by-default lint. I would expected only hard errors in an errors module, not warnings.

View changes since the review

There is no clear pattern of where you put Diagnostic types. By default, it's mostly in crate::errors, even if it's lints.

One of the things on my todo list is do find a better name for these modules, perhaps diagnostics.rs

I would second such MCP.

…ify code when `Session` is not needed

rustbot · 2026-04-25T14:23:58Z

triagebot.toml has been modified, there may have been changes to the review queue.

cc @davidtwco, @wesleywiser

GuillaumeGomez · 2026-04-25T14:24:31Z

Not ready yet. Just cleaning up a few things. Still need to update the check_cfg code to remove the need for TyCtxt.

GuillaumeGomez added 3 commits April 25, 2026 04:12

Make it possible to have access to Session and crates name in `attr…

36459d9

…_parsing` lints

Remove remaining AttributeLintKind variants

b80d0b8

Remove unused diagnostic types

e7b9d94

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 25, 2026

rustbot assigned JonathanBrouwer Apr 25, 2026

GuillaumeGomez mentioned this pull request Apr 25, 2026

Tracking Issue for LintDiagnostic migration #153099

Open

3 tasks

This comment has been minimized.

Sign in to view

petrochenkov self-assigned this Apr 25, 2026

JonathanBrouwer reviewed Apr 25, 2026

View reviewed changes

Update triagebot.toml file to take into account the moved file

a070732

JonathanBrouwer added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 25, 2026

Urgau reviewed Apr 25, 2026

View reviewed changes

Create a new SharedContext::emit_dyn_lint_with_sess method to simpl…

0b0c8f0

…ify code when `Session` is not needed

rustbot added the A-meta Area: Issues & PRs about the rust-lang/rust repository itself label Apr 25, 2026

Uh oh!

Conversation

GuillaumeGomez commented Apr 25, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 25, 2026

Uh oh!

This comment has been minimized.

petrochenkov commented Apr 25, 2026

Uh oh!

JonathanBrouwer Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Apr 25, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Apr 25, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Urgau Apr 25, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Apr 25, 2026

Uh oh!

GuillaumeGomez commented Apr 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

GuillaumeGomez commented Apr 25, 2026 •

edited by rustbot

Loading

JonathanBrouwer Apr 25, 2026 •

edited

Loading

JonathanBrouwer Apr 25, 2026 •

edited

Loading

JonathanBrouwer Apr 25, 2026 •

edited by rustbot

Loading

JonathanBrouwer Apr 25, 2026 •

edited

Loading

JonathanBrouwer Apr 25, 2026 •

edited by rustbot

Loading

Urgau Apr 25, 2026 •

edited by rustbot

Loading