Skip to content

[CRATER] Deny almost all attributes on macro calls#158816

Closed
mejrs wants to merge 1 commit into
rust-lang:mainfrom
mejrs:no_macro_call_attrs
Closed

[CRATER] Deny almost all attributes on macro calls#158816
mejrs wants to merge 1 commit into
rust-lang:mainfrom
mejrs:no_macro_call_attrs

Conversation

@mejrs

@mejrs mejrs commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

It is not yet time to start erroring on these, but there are no PRs queued up in crater right now so I'm seizing the opportunity to check the fallout.

This doesn't error for doc/diagnostic/rustc_dummy etc attributes, those are still allowed.

@JonathanBrouwer does this appear correct?

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 5, 2026
@mejrs

mejrs commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@bors try

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jul 5, 2026
[CRATER] Deny almost all attributes on macro calls
@rust-bors

rust-bors Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 4d8aa70 (4d8aa70caf2d2f6ef5a31eab47961bf7d0acacb7)
Base parent: 4eb6253 (4eb62535fc12dd1a63fd43a4173e224e79313c4d)

@mejrs

mejrs commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@craterbot check

@craterbot

Copy link
Copy Markdown
Collaborator

👌 Experiment pr-158816 created and queued.
🤖 Automatically detected try build 4d8aa70
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 6, 2026
@craterbot

Copy link
Copy Markdown
Collaborator

🚧 Experiment pr-158816 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@petrochenkov

Copy link
Copy Markdown
Contributor

A relevant old issue - #63221.

@craterbot

Copy link
Copy Markdown
Collaborator

🎉 Experiment pr-158816 is completed!
📊 15145 regressed and 5 fixed (1012225 total)
📊 5239 spurious results on the retry-regressed-list.txt, consider a retry1 if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

  1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-158816/retry-regressed-list.txt

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jul 9, 2026
@mejrs

mejrs commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

There aren't t that many root regressions actually, there are just some old versions of big ecosystem crates that mess this up. This isn't exhaustive:

#[repr]

used on macro that expands on enum, doesnt look like anyone is actually relying on layout https://crates.io/crates/dmap/0.2.1/code/src/lib.rs#L20
used on bitflags macro, also declared as u8 repr inside macro itself JeromeHen/max30101-rs#1

#[target_feature]

v_escape zzau13/v_escape#182

#[non_exhaustive]

crossterm https://github.com/crossterm-rs/crossterm/blob/9033a483f80a9e549744cb9851ae3815abb2fa5a/src/style/types/attribute.rs#L91

#[inline]

many old versions of crates like itertools and hashbrown etc do e.g.
https://github.com/rust-lang/hashbrown/blob/34c11891e13fa3c0d08b0540e869aace9d347c26/src/raw/mod.rs#L1250-L1257
where

#[cfg(feature = "nightly")]
macro_rules! default_fn {
	(#[$($a:tt)*] $($tt:tt)*) => {
        #[$($a)*] default $($tt)*
    }
}
#[cfg(not(feature = "nightly"))]
macro_rules! default_fn {
	($($tt:tt)*) => {
        $($tt)*
    }
}

https://docs.rs/af_packet/latest/src/af_packet/tpacket3.rs.html#110
https://github.com/qwertz19281/rust_utils/blob/833f581b5e63117fb87a7dad9f41a6532113fd88/src/static_stor/mod.rs#L30

#[deprecated]

raw-cpuid https://github.com/gz/rust-cpuid/blob/6554bb914214934b15cccb9d8be4d0f71ac69232/src/lib.rs#L2510
gtk https://github.com/gtk-rs/gtk/blob/d6233591b3994cee74efe90ccf3e2dd76693c941/src/auto/constants.rs#L159-L162
trust-dns-resolver https://crates.io/crates/trust-dns-resolver/0.18.0-alpha.2/code/src/async_resolver/mod.rs#L326

#[automatically_derived]

Applied to inventory's submit!:
cucumber https://github.com/cucumber-rs/cucumber/blob/d95872541ee2a43d71089c64bd3fa8848e24c922/codegen/src/attribute.rs#L126
pyo3-stub-gen https://github.com/Jij-Inc/pyo3-stub-gen/blob/087ecfbe87758549efe94eb569cc847b28600a0d/pyo3-stub-gen-derive/src/gen_stub.rs#L190

https://github.com/vtio-rs/vtio/blob/49e0aee188bb05599346782e07b069406a65ef2b/crates/vtansi_derive/src/macros/ansi_control.rs#L166

#[cold]

Applied to panic! in generator https://github.com/Xudong-Huang/generator-rs/blob/9ede1477f615787e8e214c40a685363efd52da92/src/yield_.rs#L50

#[must_use]

Applied over bitflags! in naga https://docs.rs/naga/0.5.0/src/naga/valid/mod.rs.html#50

#[macro_use]

Used on cfg_if invocation https://crates.io/crates/xpsupport/0.2.2/code/src/lib.rs#L60

#[no_std]

Mistakenly used as outer attr and applied to first item (happens to be macro invoc) in crate root in trunkver https://github.com/urothis/trunkver/blob/main/src/lib.rs

#[ignore]

https://github.com/ajzaff/substrate-mfm-engine/blob/63222eb29832f04118f8cdc72a11fbb05f2643f5/src/code.rs#L15

#[should_panic]

So can one divide by zero or not? https://github.com/JesterOrNot/MathExpression/blob/master/tests/test.rs#L24-L33
https://github.com/conwayste/netwayste/blob/986b6eb3f09744651524a628ec74dbb47d7063f1/src/server.rs#L1471


impl AllowedTargets<'_> {
pub(crate) fn is_allowed(&self, target: Target) -> AllowedResult {
if target == Target::MacroCall {

@JonathanBrouwer JonathanBrouwer Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be correct, but I'd prefer to implement this by explicitly listing Error(Target::MacroCall) in all attributes that are AllowedTargets::AllowListWarnRest if we're actually going to merge this. That makes it more explicit what the allowed targets are for each attribute

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, we are not merging this. I just want to know how many crates are doing this.

@JonathanBrouwer

Copy link
Copy Markdown
Contributor

So the big crates that make this mistake are (old versions of?) cgmath, crossterm, gtk, hashbrown, itertools, ndarray, raw-cpuid, rkyv, trust-dns-resolver, v_htmlescape.

That's a lot of breakage, probably too much to merge this, but also that so many big crates are making this mistake is worrying

@mejrs mejrs closed this Jul 10, 2026
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) 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.

5 participants