`#![register_{attribute,lint}_tool]`
Co-authored by @BD103.
Thank you to everyone who gave feedback on early versions of this RFC.
going to go ahead and ping all the authors of extern tools that i know, in no particular order:
- @xd009642 (tarpaulin)
- @kkysen @thedataking (c2rust)
- @Daalbrecht @bd103 (bevy_lint)
- @obi1kenobi (cargo semver-checks)
- @smoelius (dylint)
- @xfrednet (marker)
- @nbdd0121 (klint)
- not sure who maintains kani, maybe @celinval ?
- not sure who maintains prusti, maybe @fpoli ?
- @eddyb (rust-gpu)
- @chorman0773 (lccc)
- @Nadrieril (charon)
- not sure who maintains crubit, maybe @ssbr ?
I would happily use this in cargo-semver-checks. Users keep asking for things like "how can I disable lint X on item Y"; right now the only options we can offer them are:
- disable the lint everywhere
- mark the module
#[doc(hidden)], thereby making it non-public API
Both of those are very blunt instruments, and thoroughly unsatisfying as an answer to our users. This RFC addresses those problems directly. I'm excited to see it.
I would happily use this in
cargo-semver-checks. Users keep asking for things like "how can I disable lint X on item Y"; right now the only options we can offer them are:* disable the lint everywhere * mark the module `#[doc(hidden)]`, thereby making it non-public APIBoth of those are very blunt instruments, and thoroughly unsatisfying as an answer to our users. This RFC addresses those problems directly. I'm excited to see it.
How would you be able to leverage this? irrc rustdoc gives you immediate attributes but not enough other attributes to resolve lint levels.
How would you be able to leverage this? irrc rustdoc gives you immediate attributes but not enough other attributes to resolve lint levels.
this is not a rustdoc problem. this is a language level problem. semver-checks would have the same problem if it were using syn to parse; today, there is simply no place to put the attributes where rustc doesn't hard error.
oh, i misunderstood, you were asking how semver-checks would use this if it were implemented. i would expect semver-checks to reimplement the precedence rules for lints, or to document that it doesn't support module-level lint controls. it doesn't have CLI flags, which means [lints] in Cargo.toml won't work, that's unfortunate. maybe rustdoc-json could expose those flags.
I (static analysis software developer) would totally use this for any annotation that is not a contract. Thanks for your work!
not sure who maintains crubit, maybe @ssbr ?
(Yup). FWIW, the main downside with this proposal is that it requires some extra work on the user's POV.
Here's the current Crubit workflow:
- You depend on a crubit_annotate crate, which has a bunch of attribute-macros which will check that you have well-formed inputs, and expand to a structured doc-comment.
- Crubit itself parses the doc-comment.
(We used to use tool attributes, before @cramertj created this new setup.)
Now, with the proposal, AIUI, to have a similar workflow using tool attributes, the user must both depend on the attribute macro crate, as well as independently register the tool attribute which the proc macro will generate. This is unfortunate -- the user needs to do two things, and needs to know the name of the tool attribute even if they never spell it.
I like the proc macro wrapper, because it can force compile-time errors even if you happen to not be running the tool at the moment. The metadata Crubit consumes is not necessarily simple and so does need some validation. My preference as far as user experiences go are that a proc macro can generate inert attributes without them being registered by the crate that uses the proc macro, or else that there's a globally-available tool attribute, similar to clang::annotate, so that we could generate something like rust::tool_attribute(crubit::attribute_name, extra, args, go, here). That way, the only thing the user needs to do is depend on the attribute macro crate, and not worry about what it does past that.
I'm hoping this helps. FWIW, we have a working process, and I think this proposal is certainly compatible with many paths forward that lead to what I want, and definitely doc-comments are a bit... egh. Thanks for all your work here, and thanks for soliciting more feedback! I really appreciate it.
Very nice! 🙌
I'm not understanding 100% the reasoning behind denying the ambiguous inert tool attribute vs. a genuine attribute, although maybe my confusion stems from rustfmt::skip being rather special / purely syntactical (although I could envision there being other purely-syntactical markers by some "cheap" tools or whatnot, right?).
Consider the following:
mod rustfmt {
pub use ::tokio::main as skip;
}
#[rustfmt::skip]
async fn main()
{}
- this, currently, does compile perfectly fine;
- and
rustfmtdoes see therustfmt::skipand does not reformat thatfn main.
A tool (inert) attribute is just that, an inert / nothing-doing syntactical marker that the compiler is to let slide. But a genuine attribute, is just as much a syntactical marker as an inert attribute; so I do wonder about non-compiler-name-res-using tools such as rustfmt not making a difference between the two.
But I imagine that if a tool were to hook itself onto compiler machinery so as to actually involve name res and semantics, then a difference between a genuine attribute and an inert one could be made.
In which case, denying the ambiguity makes sense.
But all this may hint at some mention of the distinction between semantics-capable tools (e.g., able to resolve cfgs), vs. purely-syntactical ones being warranted? 🙂
Would #![custom::foo] be supported? Currently custom_inner_attributes feature is required even if register_tool is used to register the custom tool.
Feels like it's worth mentioning in the RFC about this, regardless if it's going to be supported.
Just asking. Is it possible to implement any custom "attribute -> extra-codegen" using this feature?
Such register_tool(llvm) for example to add llvm-attrs to codegen/IR. I really need some probably-always-unstable llvm attributes like noprofile, skipprofile and no_sanitize(“address”)/disable_sanitizer_instrumentation.
Just asking. Is it possible to implement any custom "attribute -> extra-codegen" using this feature? Such
register_tool(llvm)for example to add llvm-attrs to codegen/IR. I really need some probably-always-unstable llvm attributes likenoprofile,skipprofileandno_sanitize(“address”)/disable_sanitizer_instrumentation.
From my understanding, no. Registered tool attributes do not affect compilation using rustc, they're only there to be parsed by 3rd-party tools. If you want to introduce an attribute that affects how the code is compiled, you'll need to introduce that into the Rust compiler itself.
@rfcbot fcp merge lang
This RFC would allow code to use tool-defined attributes and lints without relying on nightly features. These are used during development with custom tools that are specially configured to run on that codebase. The RFC proposes the simplest thing that can work for this purpose, and indeed one that exists unstably in the compiler today.
Future possibilities
I have suggested some future possibilities that make sense to me, noting some drawbacks that came up in the Zulip discussion. These would have to overcome implementation complexity involving name resolution in attributes, or find a way to avoid doing that.
While I find these possibilities appealing, I don't think we should let the perfect be the enemy of the good. First, because we might not need it. Second, because there aren't significant ecosystem effects of someone adopting #![register_tool] in their own codebase. If we release a more fancy version of this feature in the future, users will be free to migrate to it independent of their dependencies or dependents. In the less-common case where a tool operates on attribute metadata from multiple crates in a crate graph, it should still be possible to define a migration story that supports both schemes simultaneously.
Team member @tmandry has proposed to merge this. The next step is review by the rest of the tagged team members:
- [ ] @joshtriplett
- [ ] @nikomatsakis
- [ ] @scottmcm
- [x] @tmandry
- [ ] @traviscross
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!
cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. See this document for info about what commands tagged team members can give me.
Given that this is part of the RFL project goal, I will nominate it for brief discussion in a lang triage meeting for visibility and to answer questions. (I don't expect to review the actual RFC in the meeting, but given that the feature so is simple some people may be comfortable checking their boxes regardless.)
lang-ops note: I don't think this RFC is especially urgent, just want to make sure we get to it eventually.
@rustbot label I-lang-nominated