Skip to content

Implement declarative (macro_rules!) attribute macros (RFC 3697) #144579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Aug 9, 2025

Conversation

joshtriplett
Copy link
Member

@joshtriplett joshtriplett commented Jul 28, 2025

This implements RFC 3697, "Declarative (macro_rules!) attribute macros".

I would suggest reading this commit-by-commit. This first introduces the
feature gate, then adds parsing for attribute rules (doing nothing with them),
then adds the ability to look up and apply macro_rules! attributes by path,
then adds support for local attributes, then adds a test, and finally makes
various improvements to errors.

@rustbot
Copy link
Collaborator

rustbot commented Jul 28, 2025

r? @petrochenkov

rustbot has assigned @petrochenkov.
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

@rustbot 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 Jul 28, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 28, 2025

The Miri subtree was changed

cc @rust-lang/miri

@rust-log-analyzer

This comment was marked as outdated.

@jdonszelmann jdonszelmann self-assigned this Jul 28, 2025
@theemathas
Copy link
Contributor

Here's an edge case that probably needs a test.

Code and error
#![feature(macro_attr)]

#[foo]
macro_rules! foo {
    attr() { $($body:tt)* } => {
        $($body)*
    }
}
use foo;
warning: unused macro definition: `foo`
 --> src/lib.rs:4:14
  |
4 | macro_rules! foo {
  |              ^^^
  |
  = note: `#[warn(unused_macros)]` on by default

error: cannot determine resolution for the attribute macro `foo`
 --> src/lib.rs:3:3
  |
3 | #[foo]
  |   ^^^
  |
  = note: import resolution is stuck, try simplifying macro imports

warning: `foo` (lib) generated 1 warning
error: could not compile `foo` (lib) due to 1 previous error; 1 warning emitted

@rust-log-analyzer

This comment was marked as outdated.

@joshtriplett
Copy link
Member Author

@theemathas I can't reproduce that here. I get a different error instead:

error: cannot find attribute `foo` in this scope
  --> $DIR/macro-rules-attr-cycle.rs:4:3
   |
LL | #[foo]
   |   ^^^
LL | macro_rules! foo {
   |              --- `foo` exists, but has no `attr` rules

This isn't the right error message, but the error is getting caught.

(Suggestions welcome for how to catch and handle this case better.)

@rust-log-analyzer

This comment was marked as outdated.

@joshtriplett
Copy link
Member Author

@theemathas I looked into your test case more. I managed to find a way to reproduce the error you got; it only reproduces if there aren't other errors. So, I can reproduce it with the following test:

#![crate_type = "lib"]
#![feature(macro_attr)]

#[attr]
macro_rules! attr {
    attr() {} => {}
}

#[attr]
struct S;

This gives:

error: cannot find attribute `attr` in this scope
  --> /home/josh/src/rust/tests/ui/macros/macro-rules-attr-apply-to-self.rs:4:3
   |
LL | #[attr]
   |   ^^^^
LL | macro_rules! attr {
   |              ---- `attr` exists, but has no `attr` rules

error: cannot determine resolution for the attribute macro `attr`
  --> /home/josh/src/rust/tests/ui/macros/macro-rules-attr-apply-to-self.rs:9:3
   |
LL | #[attr]
   |   ^^^^
   |
   = note: import resolution is stuck, try simplifying macro imports

The second error seems reasonable for attempting to invoke an attribute on itself.

The first error also largely seems fine, except that it shouldn't say has no `attr` rules when the real problem is a cycle.

Note that "cannot find" is the same error that you get if you try this:

mac! {
    macro_rules! mac {
        {} => {}
    }
}

mac! {
    struct S;
}

This gives:

error: cannot find macro `mac` in this scope
 --> src/main.rs:1:1
  |
1 | mac! {
  | ^^^

error: cannot find macro `mac` in this scope
 --> src/main.rs:7:1
  |
7 | mac! {
  | ^^^

So, I think "cannot find ... in this scope" is an acceptable error, albeit one we might be able to improve. I think the main problem here is the spurious `attr` exists, but has no `attr` rules.

@rust-log-analyzer

This comment has been minimized.

@joshtriplett
Copy link
Member Author

I think this is ready for review. The error messages could always be better, and there are additional cases that need handling (notably glob imports), but I think it would help to have some review before tackling those.

@theemathas

This comment was marked as resolved.

@petrochenkov
Copy link
Contributor

(I started looking today, but will continue only on ~Monday.)

@petrochenkov petrochenkov 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 Aug 6, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 6, 2025

rustc_errors::emitter was changed

cc @Muscraft

This handles various kinds of errors, but does not allow applying the
attributes yet.

This adds the feature gate `macro_attr`.
…ttribute

Avoid saying "a declarative macro cannot be used as an attribute macro";
instead, say that the macro has no `attr` rules.
Add a FIXME for moving this error earlier.
Add infrastructure to apply an attribute macro given argument tokens and
body tokens.

Teach the resolver to consider `macro_rules` macros when looking for an
attribute via a path.

This does not yet handle local `macro_rules` attributes.
Teach the resolver to consider `macro_rules` macros when looking for a
local attribute. When looking for an attribute and considering a
`macro_rules` macro, load the macro in order to see if it has attribute
rules.

Include a FIXME about tracking multiple macro kinds for a Def instead.
Test macros via path and local macros.
…cursively

This allows a macro attribute to implement default arguments by
reapplying itself with the defaults filled in, for instance.
@joshtriplett
Copy link
Member Author

I've rebased the series to eliminate all cases of a commit later in the series fixing code introduced earlier in the series. I've confirmed that the result is identical to the final result after the previous series, and that ./x test ui --test-args macro compiles and passes tests after every commit.

The commits now follow a logical progression:

  • Fix an error message (from outside this series) that assumed all attributes were proc macros.
  • Parse attr rules, including tests for the parser and its error messages. This makes MacroRule an enum, and updates every user of it to check for the Func variant, but leaves the Attr variant marked as unused since nothing ever looks for it.
  • Introduce some new error messages that make sense now that attr rules exist.
  • Handle non-local macro attributes.
  • Handle local macro attributes.
  • Add a pile of tests.

@bors r=petrochenkov rollup

@bors
Copy link
Collaborator

bors commented Aug 8, 2025

📌 Commit f88839d has been approved by petrochenkov

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 8, 2025

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 8, 2025
bors added a commit that referenced this pull request Aug 8, 2025
Rollup of 8 pull requests

Successful merges:

 - #139451 (Add `target_env = "macabi"` and `target_env = "sim"`)
 - #144039 (Use `tcx.short_string()` in more diagnostics)
 - #144192 (atomicrmw on pointers: move integer-pointer cast hacks into backend)
 - #144545 (In rustc_pattern_analysis, put `true` witnesses before `false` witnesses)
 - #144579 (Implement declarative (`macro_rules!`) attribute macros (RFC 3697))
 - #144649 (Account for bare tuples and `Pin` methods in field searching logic)
 - #144775 (more strongly dissuade use of `skip_binder`)
 - #144987 (Enable f16 and f128 on targets that were fixed in LLVM21)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 804d1a1 into rust-lang:master Aug 9, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 9, 2025
rust-timer added a commit that referenced this pull request Aug 9, 2025
Rollup merge of #144579 - joshtriplett:mbe-attr, r=petrochenkov

Implement declarative (`macro_rules!`) attribute macros (RFC 3697)

This implements [RFC 3697](#143547), "Declarative (`macro_rules!`) attribute macros".

I would suggest reading this commit-by-commit. This first introduces the
feature gate, then adds parsing for attribute rules (doing nothing with them),
then adds the ability to look up and apply `macro_rules!` attributes by path,
then adds support for local attributes, then adds a test, and finally makes
various improvements to errors.
@tgross35
Copy link
Contributor

tgross35 commented Aug 9, 2025

Somehow this seems to have slightly improved tokentree muching performance :) #145126 (comment)

@joshtriplett
Copy link
Member Author

Somehow this seems to have slightly improved tokentree muching performance :) #145126 (comment)

Fun! That's comforting to hear, I had worried it might have a performance impact.

@joshtriplett joshtriplett deleted the mbe-attr branch August 9, 2025 02:16
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Aug 9, 2025
Rollup of 8 pull requests

Successful merges:

 - rust-lang/rust#139451 (Add `target_env = "macabi"` and `target_env = "sim"`)
 - rust-lang/rust#144039 (Use `tcx.short_string()` in more diagnostics)
 - rust-lang/rust#144192 (atomicrmw on pointers: move integer-pointer cast hacks into backend)
 - rust-lang/rust#144545 (In rustc_pattern_analysis, put `true` witnesses before `false` witnesses)
 - rust-lang/rust#144579 (Implement declarative (`macro_rules!`) attribute macros (RFC 3697))
 - rust-lang/rust#144649 (Account for bare tuples and `Pin` methods in field searching logic)
 - rust-lang/rust#144775 (more strongly dissuade use of `skip_binder`)
 - rust-lang/rust#144987 (Enable f16 and f128 on targets that were fixed in LLVM21)

r? `@ghost`
`@rustbot` modify labels: rollup
@joshtriplett
Copy link
Member Author

@theemathas Turns out your cyclic attr test case is also getting fixed by @petrochenkov's suggestion to handle multiple macro kinds up front. That led to fixing the diagnostics to better distinguish between various error cases.

@joshtriplett
Copy link
Member Author

@petrochenkov #145153 implements the change to support a macro with multiple kinds. Turned out very nicely, and led naturally to fixing and cleaning up various things. Thanks!

github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Aug 12, 2025
Rollup of 8 pull requests

Successful merges:

 - rust-lang#139451 (Add `target_env = "macabi"` and `target_env = "sim"`)
 - rust-lang#144039 (Use `tcx.short_string()` in more diagnostics)
 - rust-lang#144192 (atomicrmw on pointers: move integer-pointer cast hacks into backend)
 - rust-lang#144545 (In rustc_pattern_analysis, put `true` witnesses before `false` witnesses)
 - rust-lang#144579 (Implement declarative (`macro_rules!`) attribute macros (RFC 3697))
 - rust-lang#144649 (Account for bare tuples and `Pin` methods in field searching logic)
 - rust-lang#144775 (more strongly dissuade use of `skip_binder`)
 - rust-lang#144987 (Enable f16 and f128 on targets that were fixed in LLVM21)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants