Fix target checking for some attributes on macro calls by JonathanBrouwer · Pull Request #156569 · rust-lang/rust

JonathanBrouwer · 2026-05-14T10:43:36Z

rustbot · 2026-05-14T10:43:40Z

Some changes occurred in compiler/rustc_attr_parsing

rustbot · 2026-05-14T10:43:43Z

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 73 candidates
Random selection from 17 candidates

JonathanBrouwer · 2026-05-14T10:45:11Z

r? @mejrs

JonathanBrouwer · 2026-05-14T10:45:59Z

+/// This is a list of default targets to which a attribute can be applied
+/// This is used for attributes that are not parted to the new target checking system yet can use this list as a placeholder.
+/// This excludes `Target::MacroCall`, as attributes on macro calls are otherwise not checked for parsed attributes.
+pub(crate) const DEFAULT_TARGETS: &'static [Policy] = {


This being a separate list is quite ugly, I'm planning on fixing this in the future by having some way to pass a pattern to the target checking, for example by having AllowedTargets::AllowFn(|target| match target { ... })

View changes since the review

Note that ALL_TARGETS is still used by the diagnostic attrs & rustc_dummy

fbstj · 2026-05-14T11:33:57Z

 }

+/// This is a list of default targets to which a attribute can be applied
+/// This is used for attributes that are not parted to the new target checking system yet can use this list as a placeholder.


Suggested change

/// This is used for attributes that are not parted to the new target checking system yet can use this list as a placeholder.

/// This is used for attributes that are not ported to the new target checking system yet and can use this list as a placeholder.

two nits:

parted => ported

it might be two sentences?

View changes since the review

These attributes don't have target checks during parsing because they need access to hir, not because we haven't gotten around to moving them around.

Unless that's something you're actively planning to change, we shouldn't say "yet".

mejrs

I know I suggested it but now I'm not sure CHECKED_LATER is a good idea. I think we should just move as many checks as we can to attribute parsing instead.

Take for example may_dangle: currently it checks in check_attr that it's on a generic param, and then that it's on a lifetime/type generic in a Drop impl.

Instead we should just move that first check into attr parsing and leave the drop impl check in check_attr.

Similar for the other attributes. Thoughts?

View changes since this review

mejrs · 2026-05-14T16:51:54Z

+/// This is used for attributes that are not parted to the new target checking system yet can use this list as a placeholder.
+/// This excludes `Target::MacroCall`, as attributes on macro calls are otherwise not checked for parsed attributes.
+pub(crate) const CHECKED_LATER: &'static [Policy] = {
+    use Policy::Allow;


Suggested change

use Policy::Allow;

Redundant import.

mejrs · 2026-05-14T16:53:01Z


+/// This is a list of default targets to which a attribute can be applied
+/// This is used for attributes that are not parted to the new target checking system yet can use this list as a placeholder.
+/// This excludes `Target::MacroCall`, as attributes on macro calls are otherwise not checked for parsed attributes.


Suggested change

/// This excludes `Target::MacroCall`, as attributes on macro calls are otherwise not checked for parsed attributes.

/// This excludes `Target::MacroCall`, as attributes on macro calls are otherwise not checked for parsed attributes because `check_attr.rs` runs after macro expansion.

or something similar

mejrs · 2026-05-14T17:01:25Z

+//~| WARN previously accepted
+test!();
+
+fn main() {}


Please merge this file into https://github.com/rust-lang/rust/blob/main/tests/ui/attributes/attr-on-mac-call.rs instead.

mejrs · 2026-05-14T17:04:54Z

+#![feature(sanitize)]
+#![feature(register_tool)]
+#![feature(export_stable)]
+#![feature(lang_items)]
+#![feature(dropck_eyepatch)]


I propose we just error for all these unstable attributes on macro call positions, no need to bother with a lint or fcw.

mejrs · 2026-05-14T17:05:49Z

    added_fake_targets.push(target_group_name);
 }

+/// This is a list of default targets to which a attribute can be applied


Suggested change

/// This is a list of default targets to which a attribute can be applied

/// This is a list of default targets to which a attribute can be applied

///

Let's not have this big paragraph included in the module level docs :)

mejrs · 2026-05-14T17:12:33Z

 }

+/// This is a list of default targets to which a attribute can be applied
+/// This is used for attributes that are not parted to the new target checking system yet can use this list as a placeholder.


These attributes don't have target checks during parsing because they need access to hir, not because we haven't gotten around to moving them around.

Unless that's something you're actively planning to change, we shouldn't say "yet".

mejrs · 2026-05-14T17:14:23Z

+   | ^^^^^^^^^^^^^^^^^^^
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = help: `#[link]` can be applied to associated consts, associated types, const parameters, const parameters, constants, crates, data types, enum variants, extern crates, foreign modules, foreign statics, function params, functions, global asms, impl blocks, lifetime parameters, lifetime parameters, macro defs, match arms, modules, pattern fields, statics, struct fields, struct fields, trait aliases, traits, type aliases, type parameters, type parameters, use statements, and where predicates


We shouldn't be saying this huge list of "valid" targets.

rustbot · 2026-05-14T17:22:20Z

Reminder, once the PR becomes ready for a review, use @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 May 14, 2026

rustbot assigned jieyouxu May 14, 2026

JonathanBrouwer force-pushed the fix-macro-call-target branch from c960552 to 5f14882 Compare May 14, 2026 10:44

rustbot assigned mejrs and unassigned jieyouxu May 14, 2026

JonathanBrouwer commented May 14, 2026

View reviewed changes

JonathanBrouwer force-pushed the fix-macro-call-target branch from 5f14882 to d98e484 Compare May 14, 2026 10:48

JonathanBrouwer added 2 commits May 14, 2026 12:52

Introduce CHECKED_LATER for all targets except MacroCall

7c72d54

Add regression test

d69f6ba

JonathanBrouwer force-pushed the fix-macro-call-target branch from d98e484 to d69f6ba Compare May 14, 2026 10:52

fbstj reviewed May 14, 2026

View reviewed changes

mejrs requested changes May 14, 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 May 14, 2026

	/// This is used for attributes that are not parted to the new target checking system yet can use this list as a placeholder.
	/// This is used for attributes that are not ported to the new target checking system yet and can use this list as a placeholder.

	/// This excludes `Target::MacroCall`, as attributes on macro calls are otherwise not checked for parsed attributes.
	/// This excludes `Target::MacroCall`, as attributes on macro calls are otherwise not checked for parsed attributes because `check_attr.rs` runs after macro expansion.

	/// This is a list of default targets to which a attribute can be applied
	/// This is a list of default targets to which a attribute can be applied
	///

Uh oh!

Conversation

JonathanBrouwer commented May 14, 2026

Uh oh!

rustbot commented May 14, 2026

Uh oh!

rustbot commented May 14, 2026

Uh oh!

JonathanBrouwer commented May 14, 2026

Uh oh!

JonathanBrouwer May 14, 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!

fbstj May 14, 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!

mejrs left a comment • 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!

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!

mejrs May 14, 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 May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JonathanBrouwer May 14, 2026 •

edited by rustbot

Loading

fbstj May 14, 2026 •

edited by rustbot

Loading

mejrs left a comment •

edited by rustbot

Loading

mejrs May 14, 2026 •

edited

Loading