prettier error for parsed attrs on `Target::Param` #150903

Bryntet · 2026-01-09T22:55:30Z

Couple things to note:

This change includes making all so-far parsed attrs emit an error if they are targeting Param (by adding Error(Target::Param))

^ The above is not a breaking change, because there was previously a step that emitted an error pre-attr parsing, for all attrs that were not in the list [allow, cfg_trace, cfg_attr_trace, deny, expect, forbid, warn]

The check mentioned above has been disabled for all parsed attrs, meaning we in practice have replaced the error lint above, with the nicer lint from the attribute parser

I also removed Allow(Target::Param) from the ALL_TARGETS const, since all attrs with the exception of allow, cfg_trace, cfg_attr_trace, deny, expect, forbid, warn had this emitting error previously anyways

The latter two commits I would want someone to verify closely that I have not made some mistake/broken something with! I think they're good, but I still feel like I don't know exactly enough about the compiler to say that they are 100% OK

This also fixes this part of #147417

r? @JonathanBrouwer

rustbot · 2026-01-09T22:55:34Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Bryntet · 2026-01-09T23:15:00Z

@jdonszelmann think it would be good if you did a quick look at this as well when you have time, just so you are up to date with the updates to const ALL_TARGETS as well as AllowedTargets::AllowListWarnRest

Bryntet · 2026-01-09T23:24:25Z

compiler/rustc_ast_lowering/src/item.rs

+                    attrs = self.lower_attrs_with_extra(
+                        hir_id,
+                        param.attrs(),
+                        param.span,
+                        Target::Param,
+                        attrs,
+                    );


unsure if this should actually be appending the param attrs here or not

I think this should just be a lower_attrs call, the extra attributes from attrs are the attributes of the foreign fn, and the params should not inherit those.

compiler/rustc_attr_parsing/src/target_checking.rs

JonathanBrouwer · 2026-01-10T12:26:45Z

compiler/rustc_ast_lowering/src/item.rs

+                    attrs = self.lower_attrs_with_extra(
+                        hir_id,
+                        param.attrs(),
+                        param.span,
+                        Target::Param,
+                        attrs,
+                    );


I think this should just be a lower_attrs call, the extra attributes from attrs are the attributes of the foreign fn, and the params should not inherit those.

compiler/rustc_attr_parsing/src/attributes/util.rs

compiler/rustc_attr_parsing/src/target_checking.rs

rustbot · 2026-01-10T12:42:27Z

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

JonathanBrouwer · 2026-01-10T12:44:38Z

compiler/rustc_attr_parsing/src/target_checking.rs

+                // All attributes were previously erroring on Target::Param except a few that are allow-listed.
+                // Therefore, to avoid accidentally regressing from emitting error to emitting warn when parsing new attrs
+                // we have this check that always emits an error if Target::Param unless it's explicitly been allowed
+                if target == Target::Param || list.contains(&Policy::Error(target)) {


I don't like how special Target::Param is here.

There are other targets, such as Target::WherePredicate, that similarly were previously checked separately, and now have to be added to all attributes with AllowListWarnRest. We should have some way to ensure that these weird targets are not forgotten, but this doesn't feel right.

If we do this, we should at least rename AllowListWarnRest so the name is something like AllowListWarnMost, but even then this feels iffy to me.

@jdonszelmann what is your opinion?

hm, we could:

force people to list exhaustively (infeasible)

write a rustc lint (could be neat)

have some special sets that allow most (like you suggest, seems reasonable)

I've never written a rustc lint before, but I've seen them around. Could be worth considering.

@Bryntet
What Jana meant is defining a full-on lint pass in rustc_lint/src/internal.rs, that checks for misuse of these allowed target lists. This is a possible solution but I feel like it's too overkill for this relatively simple problem.

I don't like this approach to "linting" because it has runtime overhead.
I'd personally prefer to go for the "special sets" approach

this improvement is only relevant for parsed attrs that use `AllowedTargets::AllowListWarnRest`

rustbot · 2026-01-12T21:40:47Z

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.

Bryntet · 2026-01-12T21:55:46Z

i tried the approach of linting inside of rustc instead

unsure if this is the proper way of making a rustc lint though?

just did this to pass tests, commit will look different before merge

rust-bors · 2026-01-13T14:37:51Z

☔ The latest upstream changes (presumably #151051) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot added the A-attributes Area: Attributes (`#[…]`, `#![…]`) label Jan 9, 2026

rustbot assigned JonathanBrouwer Jan 9, 2026

rustbot added 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 Jan 9, 2026

Bryntet force-pushed the edv/prettier_error_for_attr_on_fn_param branch from 8597baf to 4d39647 Compare January 9, 2026 23:13

jdonszelmann self-requested a review January 9, 2026 23:18

Bryntet commented Jan 9, 2026

View reviewed changes

compiler/rustc_attr_parsing/src/target_checking.rs Outdated Show resolved Hide resolved

Bryntet force-pushed the edv/prettier_error_for_attr_on_fn_param branch from 4d39647 to 34a3158 Compare January 9, 2026 23:28

JonathanBrouwer requested changes Jan 10, 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 Jan 10, 2026

JonathanBrouwer reviewed Jan 10, 2026

View reviewed changes

Bryntet added 3 commits January 12, 2026 20:39

improve the error attrs give when put on fn params

5f92b0b

this improvement is only relevant for parsed attrs that use `AllowedTargets::AllowListWarnRest`

lower foreign fn param attrs to attr parser

9555ea7

lower FnPtr fn param attrs to attr parser

7334161

Bryntet force-pushed the edv/prettier_error_for_attr_on_fn_param branch from 34a3158 to 7cbb620 Compare January 12, 2026 21:40

add lint preventing attr target regression

7d4a0f5

Bryntet force-pushed the edv/prettier_error_for_attr_on_fn_param branch from 7cbb620 to 7d4a0f5 Compare January 12, 2026 21:41

This comment has been minimized.

Sign in to view

fix tests

0b3e7b9

just did this to pass tests, commit will look different before merge

Uh oh!

prettier error for parsed attrs on Target::Param #150903

Are you sure you want to change the base?

prettier error for parsed attrs on Target::Param #150903

Uh oh!

Conversation

Bryntet commented Jan 9, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jan 9, 2026

Uh oh!

Bryntet commented Jan 9, 2026

Uh oh!

Bryntet Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

jdonszelmann Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JonathanBrouwer Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rustbot commented Jan 10, 2026

Uh oh!

JonathanBrouwer Jan 10, 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!

jdonszelmann Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Jan 13, 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!

rustbot commented Jan 12, 2026

Uh oh!

Bryntet commented Jan 12, 2026

Uh oh!

This comment has been minimized.

rust-bors bot commented Jan 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

prettier error for parsed attrs on `Target::Param` #150903

prettier error for parsed attrs on `Target::Param` #150903

Bryntet commented Jan 9, 2026 •

edited by rustbot

Loading

JonathanBrouwer Jan 10, 2026 •

edited

Loading

JonathanBrouwer Jan 13, 2026 •

edited

Loading