Error on non-literal expressions in doc attributes on macro calls

[mejrs]

View all comments

Recently I discovered that, since Rust 1.94, doc attributes on macro invocations can have arbitrary expressions in them:

// accidentally stabilized in 1.94
#[doc = concat!("", "")]
#[doc = {
    let a = 1;
    let b = 1;
    let sum = a + b;
    assert_eq!(sum, 2);
}]
println!();

As part of the attribute parsing rework this was accidentally allowed. Note that doc attributes (or any doc comment) on macro invocations do nothing, because documentation for macro invocations is not rendered - this emits a lint saying macros must produce doc comments as part of their expansion.

With this PR, it now emits a FCW, like #57571. As this is so niche it's probable this could go straight to an error but there's quite a crater queue so I'd rather do this now and try turning it into an error later.

error: invalid expression in `doc` attribute on macro invocation
  --> $DIR/attr-on-mac-call.rs:115:13
   |
LL |     #[doc = concat!("", "")]
   |     --------^^^^^^^^^^^^^^^^^^^^^^^- help: remove the attribute
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571>
   = note: `#[deny(ill_formed_attribute_input)]` (part of `#[deny(future_incompatible)]`) on by default

error: invalid expression in `doc` attribute on macro invocation
  --> $DIR/attr-on-mac-call.rs:118:13
   |
LL |        #[doc = {
   |  ______-       ^
   | | _____________|
LL | ||         let a = 1;
LL | ||         let b = 1;
LL | ||         let sum = a + b;
LL | ||         assert_eq!(sum, 2);
LL | ||     }]
   | ||_____^- help: remove the attribute
   |  |_____|
   |
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571>

error: aborting due to 7 previous errors; 30 warnings emitted

Note that #[doc = mac!()] is included in this. While this is allowed everywhere else as normally attribute parsing only sees it after its expansion, it is not expanded here, but we do need to check attributes here since we can't check them later as they're lost by then.

#[doc = mac!()] // this would be expanded second, but any remaining attributes
                // on macro invocations are dropped before they are expanded.
println!(); // this is expanded first

I don't think it is worth trying to make particular case work - this would involve checking that the expression would expand to a string literal:

• These expansions have a defined order (see also @petrochenkov's comment at #t-compiler > attribute parsing rework @ 💬)
• so it would be quite a hack to check whether the expression would expand to a string literal
• changing macro expansion order might be possible but is a big can of worms and undesirable.
• it would serve no use case, as the attribute is dropped regardless

This change would also make it consistent with all other key-value attributes. For an example, the following are allowed

#[deprecated = "foo"] // just `unused_attributes` warning
println!();
    
#[deprecated = concat!()]
struct Foo;

but this is not:

#[deprecated = concat!()]
println!();

error: attribute value must be a literal
 --> src/main.rs:2:20
  |
2 | #[deprecated = concat!()]

r? @JonathanBrouwer

[rustbot]

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[mejrs]

@JonathanBrouwer this is ready for review now

[JonathanBrouwer]

r=me after lang FCP
Crater results look reasonable to me, only 1 crate broken, which is already fixed by @mejrs (thanks!) in Martan03/termint#13

View changes since this review

[scottmcm]

Always happy to go straight to error for things that are clearly bugs that weren't supposed to work, so glad crater said we could :)

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rustbot]

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.

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[JonathanBrouwer]

@bors r+ rollup

[rust-bors]

📋 This PR cannot be approved because it currently has the following label: S-waiting-on-t-lang.

[JonathanBrouwer]

@bors r+ rollup

[rust-bors]

📌 Commit 6539813 has been approved by JonathanBrouwer

It is now in the queue for this repository.