Move #[naked] attribute check to attribute parsing stage by RichardTjokroutomo · Pull Request #162530 · rust-lang/rust

RichardTjokroutomo · 2026-09-09T12:24:57Z

Following #161482's idea to add target_item field to FinalizeCheckContext, add target_associated_item as an additional field. Lastly, move check_naked to the callback returned by NakedParser::deferred_finalize_check().

Part of #153101. r?@JonathanBrouwer

LLM disclosure: I wrote the code by hand & took inspiration from #161482. However, I did use LLM to learn how attribute checking is done in Rustc.

rustbot · 2026-09-09T12:25:00Z

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

rustbot · 2026-09-09T12:25:04Z

Thanks for the pull request, and welcome! The Rust Project has assigned @nnethercote (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks.

Please see the contribution instructions and our LLM policy for more information.

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 76 candidates
Random selection from 22 candidates

JonathanBrouwer · 2026-09-11T09:10:51Z

+                    };
+
+                    let ItemKind::Fn(fn_item) = &item.kind else {
+                        return;


This should never fail right? So please make this panic

View changes since the review

After implementing panic, through the failing CI I just found out that methods defined under traits & impls are represented as associated item instead of normal item.... lucky...

JonathanBrouwer · 2026-09-11T09:11:12Z

+                    MethodKind::Trait { body: true } | MethodKind::TraitImpl | MethodKind::Inherent,
+                ) => {
+                    let Some(item) = cx.target_item else {
+                        return;


same with this

View changes since the review

JonathanBrouwer · 2026-09-11T09:15:02Z

+    /// - support unwinding with `-Cpanic=unwind`, unlike `extern "C"`
+    /// - often diverge from the C ABI
+    /// - are subject to change between compiler versions
+    pub fn is_rustic_abi(self) -> bool {


Can we avoid duplicating this logic?
Perhaps by parsing the abi into a ExternAbi instead?

View changes since the review

I don't think we can. I originally tried to convert it to ExternAbi, but I couldn't compile as it creates circular dependency (haven't checked deeper as to why, though).

Then I saw similar check for CanonAbi, and I thought since the logic is already duplicated elsewhere, perhaps I can also add similar method to Extern.

JonathanBrouwer · 2026-09-11T09:15:25Z

        target_span: Span,
        target_hir_id: HirId,
        target: Target,
+        target_item: Option<&ast::Item>,


Please rebase on #161482 after it is merged

View changes since the review

rustbot · 2026-09-11T09:15:31Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Signed-off-by: Richard Tjokroutomo <richard.tjokro2@gmail.com>

rustbot · 2026-09-11T13:49:58Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Signed-off-by: Richard Tjokroutomo <richard.tjokro2@gmail.com>

RichardTjokroutomo · 2026-09-12T01:23:57Z

@rustbot ready

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 Sep 9, 2026

rustbot assigned nnethercote Sep 9, 2026

RichardTjokroutomo force-pushed the check-attr-naked branch from 57ae8f4 to c65aa6f Compare September 9, 2026 12:26

rustbot assigned JonathanBrouwer and unassigned nnethercote Sep 9, 2026