#![register_{attribute,lint}_tool]

[jyn514]

Rendered

Co-authored by @BD103.

Thank you to everyone who gave feedback on early versions of this RFC.

[jyn514]

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 ?

[obi1kenobi]

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.

[petrochenkov]

I think that's what I'd just do.
(Perhaps together with the "ambiguity is always an error" change if we can pull it off, to maximize future possibilities.)

rustfmt misinterpreting mod rustfmt { pub use my_attr_macro as skip; } for #[rustfmt::skip] isn't really an issue in practice, and for tools working post-expansion this is not an issue even in theory.

extern crate rustfmt as my_rustfmt is a good enough way to avoid breaking a tool when you need a crate with the same name.

[jyn514]

to be clear, you are suggesting that the following be a hard error?

#![register_attribute_tool(pyo3)]
extern crate pyo3;
#[pyo3::pymodule]  // error: ambiguous between tool and crate
struct Foo {}

I think that would not be ideal, but I guess I could live with it as long as #[::pyo3::pymodule] is still allowed. I feel somewhat strongly that #[::pyo3::pymodule] should not give an error, the extern crate as workaround is not very well known. even if people use it as my_pyo3 = { package = "pyo3" } in Cargo.toml it's still a bit indirect.

cc @mejrs

[petrochenkov]

to be clear, you are suggesting that the following be a hard error?

Yes, just because if it's an error initially, then in the future we'll be able to backward compatibly do any of the resolution changes from this RFC (precedence, subnamespaces) if really necessary.

#[::pyo3::pymodule] should certainly work, there's no ambiguity there, ::foo only looks into extern prelude.

[jyn514]

I thought of another complication. Say we want to add more built-in tools in the future, or to reserve a tool namespace today. If adding a built-in tool causes breakage because it makes crate names ambiguous, suddenly it's much harder to extend the namespace.

I guess we could have different rules for built-in tools than for registered tools, but that kinda sucks.

[mejrs]

I think that would not be ideal, but I guess I could live with it as long as #[::pyo3::pymodule] is still allowed.

I've thought about this and and having #[::pyo3::pymodule] work in this case but not #[pyo3::pymodule] seems like a bad idea to me. I think it'd be better if they both errored or if both worked. If we allow the former then it's gonna be one of those gotchas that everyone runs into. "Oh yeah you have to prefix the macro path with :: because otherwise it's ambiguous between the tool and the macro". I think I'd like to have them named the same but it might be better if authors are forced to choose different names.

[jyn514]

I am very opposed to that idea. We should not be adding needless restrictions. If you don't want the tool/macro distinction to be confusing, you as the author can choose a different name for your tool. We should not prevent people from disambiguating their code (for one thing, it makes adding a new built-in tool over an edition much harder).

[mejrs]

you as the author can choose a different name for your tool

I do not disagree in principle, but I'm just a bit paranoid of the situation where someone chooses the crate name as their tool name without thinking about it very much and it ends up being awkward later on. Imagine a situation like:

• author creates a library (without proc macros)
• author creates a tool attribute with the same name, this works fine because of no proc macros
• author later decides to add proc macros
• (without prefix ::) these macros collide with the tool name
• the author would have to decide to either rename the tool or tell the users to prefix the macros. The latter sounds like a noob trap.

[jyn514]

I don't think we can prevent this situation. The last thing still happens even if we prevent disambiguating the tool, we just force you to rename, we are strictly limiting your options.

Consider a different case: someone writes a crate, someone else writes a tool, they don't know about each other. you combine them and they conflict. I don't think we should force you in this case to use extern crate overlap as my_overlap, that seems strictly worse than allowing you to disambiguate at the call site.

[tmandry]

I think the RFC was updated to "always error" so @petrochenkov's original concern is resolved for now. Is that right?

[jyn514]

That's right, yes. Ambiguity always errors.

[epage]

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.

How would you be able to leverage this? irrc rustdoc gives you immediate attributes but not enough other attributes to resolve lint levels.

[jyn514]

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.

[jyn514]

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.

[celinval]

It's awesome to see this moving forward. Thanks!

For Kani, it's better to include @zhassan-aws / @carolynzech.

[celinval]

Where will this list be maintained?

Also, today you can't add any attribute that starts with rustc. Will that still be the case?

[jyn514]

You can see a full list at https://doc.rust-lang.org/nightly/reference/attributes.html#tool-attributes. That list is currently not as complete as the list in this RFC; I will make sure to update it.

Also, today you can't add any attribute that starts with rustc. Will that still be the case?

I assume you mean namespaced tool attributes like rustc::xxx, not "bare" attributes like rustc_const_stable. I expect those to still be banned. The story there is kind of a mess because there are rustc:: lints and they are feature-gated, but differently than the rest of the compiler's feature gates ... but I don't think we need to resolve that in this RFC. I will mention that the rustc:: tool namespace is currently and will continue to be reserved.

[celinval]

Yes, I mean namespaced tool attributes that starts with rustc, for example, rustc_foo::bar. Today you can register a tool rustc_foo, but the compiler will reject the attribute #[rustc_foo::bar] (playground link):

#![feature(register_tool)]
#![register_tool(rustc_foo)]

#[rustc_foo::bar]
fn main() {
    println!("🦀")
}

[jyn514]

oh, i see. this is actually not coming from the register_tool code, this is the same code that rejects #[rustc_baz].

i think it is fairly reasonable to keep this reserved, but i can make sure it's rejected earlier in register_tool so that people aren't confused.

[petrochenkov]

but i can make sure it's rejected earlier in register_tool so that people aren't confused.

I don't think it makes sense to prevent this, there are many other ways to make an unusable attribute that are not rejected, like mod rustc_something { pub use my_attribute; } or use my_attribute as rustc_something;, but we do not prohibit modules or imports containing rustc preventively.

[celinval]

As long as we are consistent. Either it works without any hacks or it fails early on.

[scrabsha]

I (static analysis software developer) would totally use this for any annotation that is not a contract. Thanks for your work!

[carolynzech]

Thank you!

[celinval]

May I suggest we use #![register_tool_{attribute|lint} instead? I was also wondering if we could have a #[register_tool] which would add the tool to both namespaces, the attribute and lint.

[BD103]

I worry that register_tool_{attribute|lint} will imply we are registering individual attributes or individual lints, when that is not the case. If someone sees the attribute #![register_tool_lint(...)], I worry they'll do this:

#![register_tool_lint(bevy::panicking_methods)]
#![register_tool_lint(bevy::missing_reflect)]
#![register_tool_lint(bevy::duplicate_bevy_dependencies)]
// ...

I think the name register_lint_tool much better signals that it's for registering a tool, not a lint or attribute.

[celinval]

Gotcha... That makes sense. Thanks

[kennytm]

unless we allow nested tool name like com::example::tool¹, I don't think #![register_tool_lint(bevy::panicking_methods)] will compile because the tool name can only be an $:ident not a $:path, so it is actually not an issue.

Footnotes

1. #![warn(com::example::tool::lint)] is interpreted as having a tool name com. ↩

[jyn514]

sure, but why not avoid the confusion in the first place?

[jyn514]

I have added #![register_tool] as an alias for adding both attributes.

[kennytm]

sure, but why not avoid the confusion in the first place?

IMO #![register_tool(clippy)] is also confusing, it sounds like it will make rustc run the "tool" clippy by "registering" it to the compilation process. The standard reference refers to this as "tool name" rather than just "tool". I think #![register_tool_name(clippy)] would be clear, but #![register_attribute_tool_name(clippy)] is quite verbose.

[jyn514]

That's not what "register" means. I think there is some amount of confusion possible, but I don't think it's likely enough that we need to make all the names longer. If people are adding these attributes, it's because the tool's docs told them to, and the docs should also say how to run the tool. If people are reading these attributes in existing code, then, well, I don't think it's that much more confusing than any other attribute in the language? #[inline] especially does not mean what most people think it means.

[ssbr]

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.

[nbdd0121]

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.

[boozook]

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.

[BD103]

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.

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.

[tmandry]

@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.

[rust-rfcbot]

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

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @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.

[tmandry]

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

[jyn514]

Would #![custom::foo] be supported? Currently custom_inner_attributes feature is required even if register_tool is used to register the custom tool.

@nbdd0121 hm, this is a good question. the main blocker for custom_inner_attributes is about name resolution. I think that still applies here? In order to know whether a tool name conflicts with a name in the type namespace, we need to know which scope we are resolving in.