-
Notifications
You must be signed in to change notification settings - Fork 490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update attribute documentation. #528
Conversation
This explicitly does not talk about attribute name scoping and resolution. That is a much bigger topic that I might tackle separately. I'm also looking at documenting the syntax for every built-in attribute, better incorporating MetaItem. I think this should be done separately since it will likely be a large change. Finally, I've noted a number of built-in attributes that are not documented that probably should be. I'll try to follow up on those later. Can someone clarify the distinction of inert/active attributes? I can't find anything in rustc that tracks that. In what way is that difference ever observable by the user? |
src/attributes.md
Outdated
the bang after the hash, apply to the thing that follows the attribute. | ||
* Built-in attributes | ||
* [Macro attributes][attribute macro] | ||
* [Derive mode helper attributes] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#412 (comment)
I still think "mode" should not be used here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, "mode" has always seemed strange to me.
src/attributes.md
Outdated
* Built-in attributes | ||
* Macro attributes | ||
* Derive mode helper attributes | ||
A "meta item" is the syntax used for the _Attr_ rule by built-in attributes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"built-in attributes" -> "most of built-in attributes", this is not a requirement but rather a "tradition" and implementation detail.
cfg_attr
for example, doesn't fit into the meta item syntax and may contain attributes with "unrestricted" input inside it.
Also, meta
macro fragment not accepting attributes with the unrestricted input is generally a bug / something under-implemented, but I guess the reference needs to describe the actual compiler behavior rather than the intent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good point. If it changes, I'll try to make sure this gets updated.
My plan is to expand this section in the future to include some of the common templates you added (like MetaNameValueStr) so that other places in the documentation can just say something like "crate_name
uses the MetaNameValueStr syntax to …". When I do that, I might tweak the wording here to make the relationship to built-in attributes clearer.
|
||
// Controls the "cyclomatic complexity" threshold for the cLippy tool. | ||
#[clippy::cyclomatic_complexity = "100"] | ||
pub fn f() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably makes sense to say that only two "tool modules" are currently available - rustfmt
and clippy
, and both are hard-coded into the compiler, and there's no way to introduce a custom tool module right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that, but I was reluctant to enshrine "clippy" and "rustfmt" as part of the language. I don't want to be too pedantic, but maybe the added note is ok? To me it's a question of the balance of the reference targeting the language vs the rustc implementation.
An active attribute (a macro attribute, |
Thanks for the speedy review! From a language perspective, it seems like the only scenario where the presence/absence of an attribute matters is for attribute and derive macros, right? I've been doing some tests, and it doesn't seem to match my expectations. Attribute macros see EDIT: I was wrong here, the order of attributes matters. However, I do see that previously processed attribute macros are removed when you have multiple of them. I want to be clear if Tool Attributes are always inert. I can't find the code where this removal happens, and I can't find a way to observe it (other than the multiple attribute macro situation). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm; I'll let @petrochenkov sign off on this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a cursory look at this, it all looks good to me, with the one suggestion of adding an "only".
Re: "derive mode", I needed a name for each thing you pass to a derive. People are just calling them derive macros, so changing mode->macro would be fine.
I don't understand what this means.
Generally, yes, but in details it's a mess that needs fixing (individual pieces were implemented in different time by different people without understanding the whole picture and having a common "vision").
This is an implementation defect, it causes cfg predicates being evaluated multiple times.
The compiler always treats tool attributes as inert. |
Meta: looks like I'm now assigned, but I cannot merge PRs in this repo. |
@petrochenkov Oh; uhmm, just leave a note when you think this is in a good state and someone will merge it. :) |
Thanks everyone for taking a look! @petrochenkov thanks, I think that makes it clearer to me. I'll follow up with separate PR's for some of the other things noted here. |
Co-Authored-By: ehuss <[email protected]>
Update books Update nomicon, reference, book, edition-guide, embedded-book rust-by-example cannot be updated because it is currently broken (rust-lang/rust-by-example#1161) ## nomicon 8 commits in b7eb4a087207af2405c0669fa577f8545b894c66..f1ff93b66844493a7b03101c7df66ac958c62418 2018-11-30 08:52:24 -0500 to 2019-02-26 13:37:28 -0500 - Fix typo in other-reprs.md (rust-lang/nomicon#112) - Remove duplicate 'the' in aliasing.md (rust-lang/nomicon#116) - Fix typo in subtyping.md (rust-lang/nomicon#117) - Trivial updates to the coercions chapter (rust-lang/nomicon#118) - Fix double "the" in aliasing.md (rust-lang/nomicon#119) - Fixes outdated bindgen link (rust-lang/nomicon#110) - Fix link to type layout reference (rust-lang/nomicon#121) - Fix capitalization of Rust in races.md (rust-lang/nomicon#107) ## reference 18 commits in 1c775a1..41493ff 2019-01-13 21:12:05 +0100 to 2019-03-05 12:32:22 +0100 - Fix broken link. (rust-lang/reference#527) - Update attribute documentation. (rust-lang/reference#528) - Remove target_has_atomic. (rust-lang/reference#529) - Remove "mode" from derive macro terminology. (rust-lang/reference#530) - Clarifications (rust-lang/reference#493) - Fix an incorrect note (rust-lang/reference#522) - Document extern_crate_self (rust-lang/reference#517) - Fix spelling (rust-lang/reference#520) - Update conditional-compilation.md (rust-lang/reference#503) - Document if_while_or_patterns (rust-lang/reference#516) - Fix some broken links. (rust-lang/reference#519) - Clarify what access struct updates do (rust-lang/reference#518) - Document irrefutable_let_patterns (rust-lang/reference#515) - Document Rc/Arc method receivers. (rust-lang/reference#494) - unions have no active field (rust-lang/reference#478) - attributes.md Outer -> Inner (rust-lang/reference#510) - Let bindings are now available in const contexts (rust-lang/reference#512) - Add missed literal at MacroFragSpec (rust-lang/reference#513) ## book 2 commits in fab9419503f0e34c1a06f9350a6705d0ce741dc6..9cffbeabec3bcec42d09432bfe7705125c848889 2019-02-25 07:53:23 -0500 to 2019-03-02 08:22:41 -0500 - More edits (rust-lang/book#1838) - Remove the 2018 edition nostarch directory ## edition-guide 1 commits in 5f3cc2a5618700efcde3bc00799744f21fa9ad2e..aa0022c875907886cae8f3ef8e9ebf6e2a5e728d 2019-02-27 20:11:50 -0800 to 2019-02-27 22:10:39 -0800 - Fix one last link in readme. (rust-lang/edition-guide#153) ## embedded-book 12 commits in bd2778f304989ee52be8201504d6ec621dd60ca9..9e656ead82bfe869493dec82653a52e27fa6a05c 2019-02-10 12:37:14 +0000 to 2019-03-03 16:03:26 +0000 - Merge rust-embedded/book#174 - Merge rust-embedded/book#172 - Merge rust-embedded/book#169 - Merge rust-embedded/book#171 - Merge rust-embedded/book#168 - Merge rust-embedded/book#153 - Merge rust-embedded/book#142 rust-embedded/book#151 - Merge rust-embedded/book#133 rust-embedded/book#135 - Merge rust-embedded/book#150 - Merge rust-embedded/book#145 - Merge rust-embedded/book#129 - Merge rust-embedded/book#128
unrestricted_attribute_tokens
, closes Documentunrestricted_attribute_tokens
#525tool_attributes
, closes Document RFC 2103tool_attributes
#526cfg_attr_multi
, closes Document RFC 2539 (#[cfg_attr(p, attr1, attr2)]
) #500linked_from
, it was removed in Implement RFC 1717 rust#37973macro_reexport
, it was removed in Remove unstablemacro_reexport
rust#49982linkage
,link_args