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

expand/resolve: Turn #[derive] into a regular macro attribute #79078

Merged
merged 3 commits into from
Feb 7, 2021

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Nov 15, 2020

This PR turns #[derive] into a regular attribute macro declared in libcore and defined in rustc_builtin_macros, like it was previously done with other "active" attributes in #62086, #62735 and other PRs.
This PR is also a continuation of #65252, #69870 and other PRs linked from them, which layed the ground for converting #[derive] specifically.

#[derive] still asks rustc_resolve to resolve paths inside derive(...), and rustc_expand gets those resolution results through some backdoor (which I'll try to address later), but otherwise #[derive] is treated as any other macro attributes, which simplifies the resolution-expansion infra pretty significantly.

The change has several observable effects on language and library.
Some of the language changes are feature-gated by feature(macro_attributes_in_derive_output).

Library

  • derive is now available through standard library as {core,std}::prelude::v1::derive.

Language

  • derive now goes through name resolution, so it can now be renamed - use derive as my_derive; #[my_derive(Debug)] struct S;.
  • derive now goes through name resolution, so this resolution can fail in corner cases. Crater found one such regression, where import use foo as derive goes into a cycle with #[derive(Something)].
  • [feature-gated] #[derive] is now expanded as any other attributes in left-to-right order. This allows to remove the restriction on other macro attributes following #[derive] (Attribute macros must be placed before derive attributes. reference#566). The following macro attributes become a part of the derive's input (this is not a change, non-macro attributes following #[derive] were treated in the same way previously).
  • #[derive] is now expanded as any other attributes in left-to-right order. This means two derive attributes #[derive(Foo)] #[derive(Bar)] are now expanded separately rather than together. It doesn't generally make difference, except for esoteric cases. For example #[derive(Foo)] can now produce an import bringing Bar into scope, but previously both Foo and Bar were required to be resolved before expanding any of them.
  • [feature-gated] #[derive()] (with empty list in parentheses) actually becomes useful. For historical reasons #[derive] fully configures its input, eagerly evaluating cfg everywhere in its target, for example on fields.
    Expansion infra doesn't do that for other attributes, but now when macro attributes attributes are allowed to be written after #[derive], it means that derive can fully configure items for them.
    #[derive()]
    #[my_attr]
    struct S {
    	#[cfg(FALSE)] // this field in removed by `#[derive()]` and not observed by `#[my_attr]`
    	field: u8
    }
  • #[derive] on some non-item targets is now prohibited. This was accidentally allowed as noop in the past, but was warned about since early 2018 (Warn on pointless #[derive] in more places #50092), despite that crater found a few such cases in unmaintained crates.
  • Derive helper attributes used before their introduction are now reported with a deprecation lint. This change is long overdue (since macro modularization, Ambiguity between custom derive attributes and proc macro attributes #52226 (comment)), but it was hard to do without fixing expansion order for derives. The deprecation is tracked by Tracking issue for deprecation lint legacy_derive_helpers #79202.
    #[trait_helper] // warning: derive helper attribute is used before it is introduced
    #[derive(Trait)]
    struct S {}

Crater analysis: #79078 (comment)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 15, 2020
@petrochenkov
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Nov 15, 2020

⌛ Trying commit 914c2fe531ee84a0a40e42148a82b8ceb61b2fa9 with merge a5d5c8b18da6d26c8237c831cfda59089bccf845...

@bors

This comment has been minimized.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 15, 2020
@rust-log-analyzer

This comment has been minimized.

@jyn514 jyn514 added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 15, 2020
@petrochenkov
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Nov 15, 2020

⌛ Trying commit 71cf3396c9d44524cfcbbea1a375cdb2fd63871b with merge 7f64738202675455a69bc6bb3e27b8186ef98cb6...

@bors
Copy link
Contributor

bors commented Nov 15, 2020

☀️ Try build successful - checks-actions
Build commit: 7f64738202675455a69bc6bb3e27b8186ef98cb6 (7f64738202675455a69bc6bb3e27b8186ef98cb6)

@petrochenkov
Copy link
Contributor Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-79078 created and queued.
🤖 Automatically detected try build 7f64738202675455a69bc6bb3e27b8186ef98cb6
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 15, 2020
@craterbot
Copy link
Collaborator

🚧 Experiment pr-79078 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 19, 2020
expand/resolve: Pre-requisites to "Turn `#[derive]` into a regular macro attribute"

Miscellaneous refactorings and error reporting changes extracted from rust-lang#79078.

Unlike rust-lang#79078 this PR doesn't make any observable changes to the language or library.
r? `@Aaron1011`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 19, 2020
expand/resolve: Pre-requisites to "Turn `#[derive]` into a regular macro attribute"

Miscellaneous refactorings and error reporting changes extracted from rust-lang#79078.

Unlike rust-lang#79078 this PR doesn't make any observable changes to the language or library.
r? ``@Aaron1011``
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 19, 2020
expand/resolve: Pre-requisites to "Turn `#[derive]` into a regular macro attribute"

Miscellaneous refactorings and error reporting changes extracted from rust-lang#79078.

Unlike rust-lang#79078 this PR doesn't make any observable changes to the language or library.
r? ```@Aaron1011```
@pksunkara
Copy link
Contributor

I am a little late to the party, but is there any reason why some of those are feature gated?

@petrochenkov
Copy link
Contributor Author

@pksunkara
See #79078 (comment) and comments from lang team members above.
I think the primary goal was to avoid empty #[derive()] being abused as #[cfg_eval].

The good news is that I plan to make a PR for proper #[cfg_eval] in the next couple of weeks, after it's merged feature(macro_attributes_in_derive_output) will be able to start stabilization.
It doesn't really add anything new - #[derive(Foo)] ITEM fully expanding #[cfg]s for Foo is a well-defined and relied upon behavior, and #[derive(/*anything*/)] #[foo] ITEM (or #[cfg_eval] #[foo] ITEM) doing the same thing for foo is no different from that.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 8, 2021
Implement built-in attribute macro `#[cfg_eval]` + some refactoring

This PR implements a built-in attribute macro `#[cfg_eval]` as it was suggested in rust-lang#79078 to avoid `#[derive()]` without arguments being abused as a way to configure input for other attributes.

The macro is used for eagerly expanding all `#[cfg]` and `#[cfg_attr]` attributes in its input ("fully configuring" the input).
The effect is identical to effect of `#[derive(Foo, Bar)]` which also fully configures its input before passing it to macros `Foo` and `Bar`, but unlike `#[derive]` `#[cfg_eval]` can be applied to any syntax nodes supporting macro attributes, not only certain items.

`cfg_eval` was the first name suggested in rust-lang#79078, but other alternatives are also possible, e.g. `cfg_expand`.

```rust
#[cfg_eval]
#[my_attr] // Receives `struct S {}` as input, the field is configured away by `#[cfg_eval]`
struct S {
    #[cfg(FALSE)]
    field: u8,
}
```

Tracking issue: rust-lang#82679
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 8, 2021
Implement built-in attribute macro `#[cfg_eval]` + some refactoring

This PR implements a built-in attribute macro `#[cfg_eval]` as it was suggested in rust-lang#79078 to avoid `#[derive()]` without arguments being abused as a way to configure input for other attributes.

The macro is used for eagerly expanding all `#[cfg]` and `#[cfg_attr]` attributes in its input ("fully configuring" the input).
The effect is identical to effect of `#[derive(Foo, Bar)]` which also fully configures its input before passing it to macros `Foo` and `Bar`, but unlike `#[derive]` `#[cfg_eval]` can be applied to any syntax nodes supporting macro attributes, not only certain items.

`cfg_eval` was the first name suggested in rust-lang#79078, but other alternatives are also possible, e.g. `cfg_expand`.

```rust
#[cfg_eval]
#[my_attr] // Receives `struct S {}` as input, the field is configured away by `#[cfg_eval]`
struct S {
    #[cfg(FALSE)]
    field: u8,
}
```

Tracking issue: rust-lang#82679
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 8, 2021
Implement built-in attribute macro `#[cfg_eval]` + some refactoring

This PR implements a built-in attribute macro `#[cfg_eval]` as it was suggested in rust-lang#79078 to avoid `#[derive()]` without arguments being abused as a way to configure input for other attributes.

The macro is used for eagerly expanding all `#[cfg]` and `#[cfg_attr]` attributes in its input ("fully configuring" the input).
The effect is identical to effect of `#[derive(Foo, Bar)]` which also fully configures its input before passing it to macros `Foo` and `Bar`, but unlike `#[derive]` `#[cfg_eval]` can be applied to any syntax nodes supporting macro attributes, not only certain items.

`cfg_eval` was the first name suggested in rust-lang#79078, but other alternatives are also possible, e.g. `cfg_expand`.

```rust
#[cfg_eval]
#[my_attr] // Receives `struct S {}` as input, the field is configured away by `#[cfg_eval]`
struct S {
    #[cfg(FALSE)]
    field: u8,
}
```

Tracking issue: rust-lang#82679
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 8, 2021
Implement built-in attribute macro `#[cfg_eval]` + some refactoring

This PR implements a built-in attribute macro `#[cfg_eval]` as it was suggested in rust-lang#79078 to avoid `#[derive()]` without arguments being abused as a way to configure input for other attributes.

The macro is used for eagerly expanding all `#[cfg]` and `#[cfg_attr]` attributes in its input ("fully configuring" the input).
The effect is identical to effect of `#[derive(Foo, Bar)]` which also fully configures its input before passing it to macros `Foo` and `Bar`, but unlike `#[derive]` `#[cfg_eval]` can be applied to any syntax nodes supporting macro attributes, not only certain items.

`cfg_eval` was the first name suggested in rust-lang#79078, but other alternatives are also possible, e.g. `cfg_expand`.

```rust
#[cfg_eval]
#[my_attr] // Receives `struct S {}` as input, the field is configured away by `#[cfg_eval]`
struct S {
    #[cfg(FALSE)]
    field: u8,
}
```

Tracking issue: rust-lang#82679
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 8, 2021
Implement built-in attribute macro `#[cfg_eval]` + some refactoring

This PR implements a built-in attribute macro `#[cfg_eval]` as it was suggested in rust-lang#79078 to avoid `#[derive()]` without arguments being abused as a way to configure input for other attributes.

The macro is used for eagerly expanding all `#[cfg]` and `#[cfg_attr]` attributes in its input ("fully configuring" the input).
The effect is identical to effect of `#[derive(Foo, Bar)]` which also fully configures its input before passing it to macros `Foo` and `Bar`, but unlike `#[derive]` `#[cfg_eval]` can be applied to any syntax nodes supporting macro attributes, not only certain items.

`cfg_eval` was the first name suggested in rust-lang#79078, but other alternatives are also possible, e.g. `cfg_expand`.

```rust
#[cfg_eval]
#[my_attr] // Receives `struct S {}` as input, the field is configured away by `#[cfg_eval]`
struct S {
    #[cfg(FALSE)]
    field: u8,
}
```

Tracking issue: rust-lang#82679
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 15, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 16, 2021
…s, r=petrochenkov

Mention rust-lang#79078 on compatibility notes of 1.52

Closes rust-lang#85854
r? `@petrochenkov`
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 16, 2021
…s, r=petrochenkov

Mention rust-lang#79078 on compatibility notes of 1.52

Closes rust-lang#85854
r? ``@petrochenkov``
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 16, 2021
Rollup of 10 pull requests

Successful merges:

 - rust-lang#85870 (Allow whitespace in dump_mir filter)
 - rust-lang#86104 (Fix span calculation in format strings)
 - rust-lang#86140 (Mention the `Borrow` guarantee on the `Hash` implementations for Arrays and `Vec`)
 - rust-lang#86141 (Link reference in `dyn` keyword documentation)
 - rust-lang#86260 (Open trait implementations' toggles by default.)
 - rust-lang#86339 (Mention rust-lang#79078 on compatibility notes of 1.52)
 - rust-lang#86341 (Stop returning a value from `report_assert_as_lint`)
 - rust-lang#86353 (Remove `projection_ty_from_predicates`)
 - rust-lang#86361 (Add missing backslashes to prevent unwanted newlines in rustdoc HTML)
 - rust-lang#86372 (Typo correction: s/is/its)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.