Skip to content

RFC: --crate-attr - #3791

Merged
joshtriplett merged 20 commits into
rust-lang:masterfrom
jyn514:crate-attr
Jul 30, 2025
Merged

RFC: --crate-attr#3791
joshtriplett merged 20 commits into
rust-lang:masterfrom
jyn514:crate-attr

Conversation

@jyn514

@jyn514 jyn514 commented Mar 17, 2025

Copy link
Copy Markdown
Member

Rendered

cc rust-lang/rust#138287

Differences from the current nightly implementation (note that this section is non-normative):

  • -A -W -D -F become aliases for crate-attr
  • Attributes are prepended, not appended.
  • Everything about how this interacts with doctests (see RFC: --crate-attr #3791 (comment))
  • The attribute shares the same Span as the crate root. In particular file! behaves properly now and the edition is no longer always 2015.

Comment thread text/0000-crate-attr.md Outdated
Comment thread text/3791-crate-attr.md
Comment thread text/3791-crate-attr.md Outdated
@ehuss ehuss added the T-compiler Relevant to the compiler team, which will review and decide on the RFC. label Mar 17, 2025
jyn514 added 2 commits March 17, 2025 10:46
- Mention how this applies to doctests
- Mention build-std
- Remove outdated mention of custom test frameworks
Comment thread text/3791-crate-attr.md
Comment thread text/3791-crate-attr.md
Comment on lines +42 to +43
The `--crate-attr` flag allows you to inject attributes into the crate root.
For example, `--crate-attr=crate_name="test"` acts as if `#![crate_name="test"]` were present before the first source line of the crate root.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question (not necessarily for you but the Cursed Code Specialists): do we have any wonky (stable) crate-level attributes like #![crate_name = EXPR] (whose validation was only recently fixed as a breaking change in rust-lang/rust#127581) that an injection like --crate-attr=crate_name=include_str!("crate_name.txt") can lead to... interesting behaviors?

Or perhaps, can crate-root attributes take a "value" that's a macro, possibly line!()?

@jyn514 jyn514 Mar 17, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

these should be questions for the individual attributes, imo. crate-attr does not try to special-case these in any way, it just forwards the macro invocation which gets rejected later in compilation. i do not think we should try to limit the syntax to things that "look normal".

$ rustc +r2-stage1 src/main.rs -Zcrate-attr=crate_name='line!()'
error: malformed `crate_name` attribute input
 --> <crate attribute>:1:1
  |
1 | #![crate_name=line!()]
  | ^^^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#![crate_name = "name"]`

(here r2-stage1 is a checkout of rust-lang/rust#138336)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah that seems completely fair (if anything, may be QoI issue on specific crate-level attrs)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(I'll temporarily leave this thread open in case people do know of any Funny Things, but please resolve this comment chain before FCP as it is not intended to be blocking).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oh, i misunderstood. you were asking what include! and line! should expand to in this context. my understanding is that we do not currently have any crate-level attributes which accept values, right? i'm tempted to say we just delay defining this until we need to.

@fmease fmease Mar 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with Jyn that we shouldn't restrict the grammar of those attributes.

To answer the original question however, there are several. E.g., #![doc = …], #![deprecated = …], #![type_length_limit = …]. In these three cases you can use concat!("", line!()) as the RHS which expands to (lineno) "1" when passed via -Zcrate-attr (I'm using concat to essentially get rid of the numeric suffix).

(crate_name, crate_type and recursion_limit are fully locked-down nowadays in the sense that they no longer accept macro calls)

(since a lot of malformed and misplaced attrs are still accepted by rustc and only trigger the warn-by-default lint unused_attributes, you can currently also do things like #![must_use = compile_error!("…")] which isn't that interesting arguably)

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.

rustdoc -Zcrate-attr='doc=concat!("test", line!())' foo.rs gave a doc/foo/index.html containing "test1".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok. of these, line!() really does not seem to have a meaning to me, maybe the implementation comes up with "1" but i don't think that's useful. so i'm tempted to ban that particular macro in this context.

include! and friends all seem fine to me, they should be relative to whatever #![doc = include_str!()] in the source is relative to.

@jieyouxu jieyouxu Mar 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FWIW I was mostly thinking adversarially on what weird combinations this can end up with (e.g. what's prone to break, between a stable + stable feature/functionality interaction). I suppose this may also include the other "meta" ones like column!(). Not sure how to phrase this, but "observable implementation details" -- e.g. that line number 1 being observable through line!() is potentially interesting if it can become depended upon by the user.

Alternatively... explicitly carve out a stability caution/exception where it's explicitly remarked that depending on line!()/column!() etc. is not part of the stability guarantees? Idk how to word this though. It's not a big concern but it can be annoying if it ever comes up.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i still think that all these concerns apply just as much to existing crate-level attributes in source code. if there is a concern here it should be specific to attributes passed via a CLI flag.

Comment thread text/3791-crate-attr.md
Comment thread text/3791-crate-attr.md Outdated
Comment thread text/3791-crate-attr.md Outdated

- We could require `--crate-attr=#![foo]` instead. This is more verbose and requires extensive shell quoting, for not much benefit.
- We could disallow comments in the attribute. This perhaps makes the design less surprising, but complicates the implementation for little benefit.
- We could add a syntax for passing multiple attributes in a single CLI flag. We would have to find a syntax that avoids ambiguity *and* that does not mis-parse the data inside string literals (i.e. picking a fixed string, such as `|`, would not work because it has to take quote nesting into account). This greatly complicates the implementation for little benefit.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remark: +1, I think it's perfectly fine (and arguably good) to not support multi-attr in same flag. If anything, multiple --crate-attr flags is way easier to read too, IMO.

Comment thread text/3791-crate-attr.md Outdated
Comment thread text/3791-crate-attr.md
@oli-obk oli-obk added the I-compiler-nominated Indicates that an issue has been nominated for prioritizing at the next compiler team meeting. label Mar 30, 2025
Comment thread text/3791-crate-attr.md
Comment thread text/3791-crate-attr.md Outdated
Comment thread text/3791-crate-attr.md Outdated
@Mark-Simulacrum

Copy link
Copy Markdown
Member

@rfcbot fcp merge

I've wanted this functionality before myself, and it's a requested feature from Rust for Linux, so demand exists. I think the RFC does a good job motivating the feature and noting edge cases that need resolution, though personally nothing that necessarily seems like it blocks stabilization. So, proposing we merge it.

@rfcbot

rfcbot commented Mar 30, 2025

Copy link
Copy Markdown

Team member @Mark-Simulacrum has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Mar 30, 2025
Comment thread text/3791-crate-attr.md

Additionally, some existing CLI options could have been useful as attributes. This leads to the second group: Maintainers of the Rust language. Often we need to decide between attributes and flags; either we duplicate features between the two (lints, `crate-name`, `crate-type`), or we make it harder to configure the options for stakeholder group 1.

The third group is the authors of external tools. The [original motivation][impl] for this feature was for Crater, which wanted to enable a rustfix feature in *all* crates it built without having to modify the source code. Other motivations include the currently-unstable [`register-tool`], which with this RFC could be an attribute passed by the external tool (or configured in the workspace), and [build-std], which wants to inject `no_std` into all crates being compiled.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

and [build-std], which wants to inject no_std into all crates being compiled

Just as a note, RfL already does this exact thing and they would like to see -Zcrate-attr stabilized as well:

https://github.com/Rust-for-Linux/linux/blob/e6ea10d5dbe082c54add289b44f08c9fcfe658af/scripts/Makefile.build#L238-L239

@apiraino

Copy link
Copy Markdown
Contributor

visited in meeting on Zulip

@rustbot label -I-compiler-nominated

@rustbot rustbot removed the I-compiler-nominated Indicates that an issue has been nominated for prioritizing at the next compiler team meeting. label Apr 10, 2025
Comment thread text/3791-crate-attr.md Outdated
Comment thread text/3791-crate-attr.md Outdated
Comment thread text/3791-crate-attr.md Outdated
Comment thread text/3791-crate-attr.md
fn foo() {}
```

Additionally, all existing `-A -W -D -F` flags become aliases for `--crate-attr` (`allow`, `warn`, `deny`, and `forbid`, respectively). In particular, this implies that the following CLI flag combinations are exactly equivalent:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't actually affect anything, right? The flags behave exactly the same no matter whether they are aliases or not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

no, this is user-facing. see #3791 (comment) for an example of how it can affect behavior otherwise.

@Noratrieb Noratrieb May 29, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, please mention this in the RFC more explicitly in that case.

Comment thread text/3791-crate-attr.md
@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Jun 17, 2025
@rfcbot

rfcbot commented Jun 17, 2025

Copy link
Copy Markdown

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

Comment thread text/3791-crate-attr.md
Co-authored-by: Josh Triplett <josh@joshtriplett.org>
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. to-announce and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Jun 27, 2025
@rfcbot

rfcbot commented Jun 27, 2025

Copy link
Copy Markdown

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.

This will be merged soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-compiler Relevant to the compiler team, which will review and decide on the RFC. to-announce

Projects

None yet

Development

Successfully merging this pull request may close these issues.