Skip to content
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

Document inert vs active attributes #1110

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Aaron1011
Copy link
Member

This PR adds a subsection to the 'Syntax and AST' section describing
inert vs active attributes.

For consistency, I've also updated the '#[test] implementation' page to
stop referring to `#[test]' as a 'built in' attribute, since that has a
specific meaning defined on this page.

This PR adds a subsection to the 'Syntax and AST' section describing
inert vs active attributes.

For consistency, I've also updated the '#[test] implementation' page to
stop referring to `#[test]' as a 'built in' attribute, since that has a
specific meaning defined on this page.
@camelid camelid added the waiting-on-review This PR is waiting for a reviewer to verify its content label Apr 16, 2021
Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

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

This looks really good, thanks for writing it up! I left a few typographical nits. I can't comment much on the content because I'm learning about attributes by reading this PR ;)

Comment on lines +19 to +21
As a result, any behavior comes as a result of the compiler explicitly checking for their presence
(for example, lint-related code explicitly checks for `#[allow]`, `#[warn]`, `#[deny]`, and
`#[forbid]`)
Copy link
Member

Choose a reason for hiding this comment

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

Might be nice to add a bit more to make it clear what the distinction is:

Suggested change
As a result, any behavior comes as a result of the compiler explicitly checking for their presence
(for example, lint-related code explicitly checks for `#[allow]`, `#[warn]`, `#[deny]`, and
`#[forbid]`)
As a result, any behavior comes as a result of the compiler explicitly checking for their presence.
For example, lint-related code explicitly checks for `#[allow]`, `#[warn]`, `#[deny]`, and
`#[forbid]`, rather than the behavior coming from the expansion of the attributes themselves.

Comment on lines +34 to +35
2. Ast-based attributes, defined in the standard library. These attributes have special 'stub'
macros defined in places like [`library/core/src/macros/mod.rs`][core_macros]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
2. Ast-based attributes, defined in the standard library. These attributes have special 'stub'
macros defined in places like [`library/core/src/macros/mod.rs`][core_macros]
2. AST-based attributes, defined in the standard library. These attributes have special 'stub'
macros defined in places like [`library/core/src/macros/mod.rs`][core_macros].

These definitions exist to allow the macros to participate in typical path-based resolution - they
can be imported, re-exported, and renamed just like any other item definition. However, the body of
the definition is empty. Instead, the macro is annotated with the `#[rustc_builtin_macro]`
attribute, which tells the compiler to run a corresponding function in `rustc_builtin_macros`.
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a link to the crate's API docs? That way it is easy for people to see more about it, and it will make our CI fail if the name ever changes.

@@ -0,0 +1,49 @@
# Attributes

Attributes come in two types - 'builtin'/'inert' attributes, and 'non-builtin'/'active' attributes:
Copy link
Member

Choose a reason for hiding this comment

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

I feel like the single quotes here and in the headings make the text a bit harder to read; are you okay with removing them?


They are 'inert', meaning they are left as-is by the macro expansion code.

This one seems fine to keep, but I feel that the others make it harder to read.

Comment on lines +7 to +10
These attributes are defined in the compiler itself, in
[`compiler/rustc_feature/src/builtin_attrs.rs`][builtin_attrs]

Example include `#[allow]` and `#[macro_use]`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
These attributes are defined in the compiler itself, in
[`compiler/rustc_feature/src/builtin_attrs.rs`][builtin_attrs]
Example include `#[allow]` and `#[macro_use]`
These attributes are defined in the compiler itself, in
[`compiler/rustc_feature/src/builtin_attrs.rs`][builtin_attrs].
Examples include `#[allow]` and `#[macro_use]`.

Comment on lines +25 to +26
These attributes are defined by a crate - either the standard library, or a proc-macro crate.
**Important**: Many non-builtin attributes, such as `#[derive]`, are still considered part of the
Copy link
Member

Choose a reason for hiding this comment

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

I think this note might be easier to read as a new paragraph:

Suggested change
These attributes are defined by a crate - either the standard library, or a proc-macro crate.
**Important**: Many non-builtin attributes, such as `#[derive]`, are still considered part of the
These attributes are defined by a crate - either the standard library, or a proc-macro crate.
**Important**: Many non-builtin attributes, such as `#[derive]`, are still considered part of the

@petrochenkov
Copy link
Contributor

cfg and cfg_attr are probably worth mentioning as third category because they are both built-in (in the sense that they don't go through name resolution) and active (expanded directly by the compiler).

@camelid camelid added waiting-on-author This PR is waiting for additional action by the OP and removed waiting-on-review This PR is waiting for a reviewer to verify its content labels Apr 19, 2021
@rylev
Copy link
Member

rylev commented Jul 4, 2021

@Aaron1011 will you be able to work on this to get it over the finish line? I'd love to see it merged.

@@ -0,0 +1,49 @@
# Attributes

Attributes come in two types - 'builtin'/'inert' attributes, and 'non-builtin'/'active' attributes:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Attributes come in two types - 'builtin'/'inert' attributes, and 'non-builtin'/'active' attributes:
Attributes come in two types: *inert* (or *built-in*) and *active* (*non-builtin*).

Copy link
Member

Choose a reason for hiding this comment

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

Could switch these around as well if we prefer. (*built-in* (or *inert*))

I like italics for the first time a new term is introduced.


Attributes come in two types - 'builtin'/'inert' attributes, and 'non-builtin'/'active' attributes:

## 'Builtin'/'inert' attributes
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## 'Builtin'/'inert' attributes
## Builtin/inert attributes

@@ -2,7 +2,7 @@

<!-- toc -->

Today, rust programmers rely on a built in attribute called `#[test]`. All
Today, rust programmers rely on an attribute called `#[test]`. All
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Today, rust programmers rely on an attribute called `#[test]`. All
Today, Rust programmers rely on an attribute called `#[test]`. All

@JohnTitor
Copy link
Member

@Aaron1011 I'm not sure if this is still up-to-date, could you check? It'd also be great if you could address review comments.

@spastorino
Copy link
Member

I'm not sure what's the current status of this PR, but I guess it may be better to merge it and continue working on it on master.
Opinions?.

@spastorino
Copy link
Member

I'm not sure what's the current status of this PR, but I guess it may be better to merge it and continue working on it on master. Opinions?.

Any comment about this ☝️ ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-author This PR is waiting for additional action by the OP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants