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, replace target_item with ast_target, which is an enum that contains all possible types of target Item (obtained by grepping all functions that call lower_attrs()).

This change is needed as methods defined under traits & impls are represented as ast::AssocItem. Lastly, move check_naked to the callback returned by NakedParser::deferred_finalize_check().

Part of #153101. r?@JonathanBrouwer

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

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>

…functions calling lower_attrs() Signed-off-by: Richard Tjokroutomo <richard.tjokro2@gmail.com>

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

rustbot · 2026-10-06T09:07:25Z

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.

RichardTjokroutomo · 2026-10-06T09:12:48Z

I just read the LLM guidelines and found out disclosure is needed only if a part of the PR (code, PR desc, etc) is generated by LLM.

Since I only use it to learn the codebase, I'll remove the disclosure.

JonathanBrouwer · 2026-10-07T09:48:06Z

+    WherePredicate(&'a WherePredicate),
+
+    /// Used when it is not possible to get detailed information about the target.
+    None,


Yeah that makes sense, we can have a 'AstTarget::None` for this case then to keep things simple

JonathanBrouwer · 2026-10-07T09:55:18Z

I just read the LLM guidelines and found out disclosure is needed only if a part of the PR (code, PR desc, etc) is generated by LLM.

Yeah as long as reviewers are not seeing any AI generated output you don't need to disclose.
Disclosure of that is still allowed tho ofc :)

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

JonathanBrouwer · 2026-10-07T13:11:47Z

+            &l.attrs,
+            l.span,
+            Target::Statement,
+            rustc_attr_ir::target::AstTarget::None,


Why can't this be AstTarget::Statement?

View changes since the review

Sorry... should've checked the caller function

JonathanBrouwer · 2026-10-07T13:11:52Z

+            &attrs,
+            span,
+            Target::Expression,
+            rustc_attr_ir::target::AstTarget::None,


Why can't this be AstTarget::Expression?

View changes since the review

I didn't check caller functions, sorry....

JonathanBrouwer · 2026-10-07T13:12:56Z

+                    &param.attrs,
+                    param.span,
+                    Target::Param,
+                    AstTarget::None,


Why can't this be AstTarget::Pram?

View changes since the review

Sorry... I was being careless..

JonathanBrouwer · 2026-10-07T13:13:42Z

+                        &f.attrs,
+                        f.span,
+                        Target::ExprField,
+                        AstTarget::Expression(expr),


I don't think this is correct

View changes since the review

JonathanBrouwer · 2026-10-07T13:14:43Z

+#[derive(Clone, Copy, Debug)]
+pub enum AstTarget<'a> {
+    // Target types that may correspond to different kinds of items.
+    Delegation { target: DelegationAstTarget<'a>, mac: bool },


Also here I'm leaning towards that we don't need the fields of this, and it adds a lot of complexity

View changes since the review

JonathanBrouwer · 2026-10-07T13:17:27Z

+    pub fn from_ast_item(kind: &'a ast::ItemKind) -> Self {
+        match kind {
+            ast::ItemKind::ExternCrate(..) => AstTarget::ExternCrate(kind),
+            ast::ItemKind::Use(..) => AstTarget::Use(kind),


I'd prefer if we map each ItemKind to its field where reasonable.
Like have AstTarget::Use take a UseTree rather than a ast::ItemKind.

View changes since the review

JonathanBrouwer · 2026-10-07T13:17:59Z

+    Union(&'a ItemKind),
+    Trait(&'a ItemKind),
+    TraitAlias(&'a ItemKind),
+    Impl { item: &'a ItemKind, of_trait: bool },


Do we need this of_trait field? That should already be stored in the Impl right?

View changes since the review

yeah. It's irrelevant now after the most recent commit

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

RichardTjokroutomo · 2026-10-07T15:28:36Z

@JonathanBrouwer I think everything is correct now. Let's just wait for CI results...

Previously there were a lot of changes when making AstTarget map 1-1 with Target, so TBH, at some point I stopped giving full attention when passing AstTarget as argument of various functions. I'm really sorry for this, but thankfully you catched them before merging

RichardTjokroutomo · 2026-10-08T00:56:16Z

@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

This comment has been minimized.

Sign in to view

RichardTjokroutomo force-pushed the check-attr-naked branch from c65aa6f to 4c2e629 Compare September 9, 2026 13:43

This comment has been minimized.

Sign in to view

RichardTjokroutomo force-pushed the check-attr-naked branch from 4c2e629 to ee7a87a Compare September 9, 2026 13:48

RichardTjokroutomo changed the title ~~move check_naked() from check_attr.rs to codegen_attrs.rs~~ Move #[naked] attribute check to attribute parsing stage Sep 10, 2026

RichardTjokroutomo force-pushed the check-attr-naked branch from ee7a87a to 9d144c2 Compare September 10, 2026 06:56

This comment has been minimized.

Sign in to view

JonathanBrouwer reviewed Sep 11, 2026

View reviewed changes

Comment thread compiler/rustc_attr_parsing/src/attributes/codegen_attrs.rs

JonathanBrouwer reviewed Sep 11, 2026

View reviewed changes

Comment thread compiler/rustc_attr_parsing/src/attributes/codegen_attrs.rs

JonathanBrouwer reviewed Sep 11, 2026

View reviewed changes

Comment thread compiler/rustc_ast/src/ast.rs Outdated

JonathanBrouwer reviewed Sep 11, 2026

View reviewed changes

Comment thread compiler/rustc_ast_lowering/src/lib.rs Outdated

JonathanBrouwer requested changes Sep 11, 2026

View reviewed changes

rustbot 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 Sep 11, 2026

RichardTjokroutomo force-pushed the check-attr-naked branch from 9d144c2 to 7e93b9a Compare September 11, 2026 12:00

This comment has been minimized.

Sign in to view

RichardTjokroutomo force-pushed the check-attr-naked branch from 7e93b9a to cdc980c Compare September 11, 2026 13:49

This comment has been minimized.

Sign in to view

RichardTjokroutomo force-pushed the check-attr-naked branch from cdc980c to 4fbadae Compare September 11, 2026 13:57

RichardTjokroutomo force-pushed the check-attr-naked branch from dd825f3 to 2d331a2 Compare October 5, 2026 15:39

This comment has been minimized.

Sign in to view

RichardTjokroutomo added 8 commits October 6, 2026 13:58

move check_naked() from check_attr.rs to codegen_attrs.rs

cb7bb90

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

also consider associate item

ba530aa

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

wrap target_item & target_assoc_item into an enum

6e2a7e4

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

addressing review comments

114aa52

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

exhaust all possible types of AST Target (obtained from grepping all …

cbd16d2

…functions calling lower_attrs() Signed-off-by: Richard Tjokroutomo <richard.tjokro2@gmail.com>

addressing comments & fixing code after 0929 rebase

2f29a0b

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

make AstTarget map 1-1 with Target

9bf8c82

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

remove options

7889ef6

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

RichardTjokroutomo force-pushed the check-attr-naked branch from 2d331a2 to 566fa74 Compare October 6, 2026 09:07

JonathanBrouwer requested changes Oct 7, 2026

View reviewed changes

rustbot 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 Oct 7, 2026

rebase latest changes & addressing latest comments

35b1fab

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

RichardTjokroutomo force-pushed the check-attr-naked branch from 566fa74 to f12bf6b Compare October 7, 2026 12:49

JonathanBrouwer requested changes Oct 7, 2026

View reviewed changes

RichardTjokroutomo force-pushed the check-attr-naked branch from f12bf6b to 4280fe0 Compare October 7, 2026 13:34

some tidy ups

b567ca0

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

RichardTjokroutomo force-pushed the check-attr-naked branch from 4280fe0 to b567ca0 Compare October 7, 2026 13:55

RichardTjokroutomo added 2 commits October 7, 2026 22:47

remove Item, AsocItem, & ForeignItem

bba59d2

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

pass AstTarget::Expression too

f2d3305

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

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

RichardTjokroutomo requested a review from JonathanBrouwer October 8, 2026 00:56

Conversation

RichardTjokroutomo commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 9, 2026

Uh oh!

rustbot commented Sep 9, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rustbot commented Sep 11, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Oct 6, 2026

Uh oh!

RichardTjokroutomo commented Oct 6, 2026

Uh oh!

Uh oh!

JonathanBrouwer Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JonathanBrouwer commented Oct 7, 2026

Uh oh!

JonathanBrouwer Oct 7, 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!

RichardTjokroutomo Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Oct 7, 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!

RichardTjokroutomo Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Oct 7, 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!

RichardTjokroutomo Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Oct 7, 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!

JonathanBrouwer Oct 7, 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!

JonathanBrouwer Oct 7, 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!

JonathanBrouwer Oct 7, 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!

RichardTjokroutomo Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

RichardTjokroutomo commented Oct 7, 2026

Uh oh!

RichardTjokroutomo commented Oct 8, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

RichardTjokroutomo commented Sep 9, 2026 •

edited

Loading

JonathanBrouwer Oct 7, 2026 •

edited by rustbot

Loading

JonathanBrouwer Oct 7, 2026 •

edited by rustbot

Loading

JonathanBrouwer Oct 7, 2026 •

edited by rustbot

Loading

JonathanBrouwer Oct 7, 2026 •

edited by rustbot

Loading

JonathanBrouwer Oct 7, 2026 •

edited by rustbot

Loading

JonathanBrouwer Oct 7, 2026 •

edited by rustbot

Loading

JonathanBrouwer Oct 7, 2026 •

edited by rustbot

Loading