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

Make #[derive(A, B, ...)] cfg-eval its input only for A, B, ... and stabilize feature(macro_attributes_in_derive_output) #87220

Merged
merged 3 commits into from
Sep 25, 2021

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Jul 17, 2021

Stabilization report: #87220 (comment)

Closes #81119
r? @Aaron1011

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 17, 2021
@petrochenkov petrochenkov 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 Jul 17, 2021
@jonas-schievink jonas-schievink added relnotes Marks issues that should be documented in the release notes of the next release. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jul 17, 2021
@petrochenkov
Copy link
Contributor Author

Stabilization Report

Historically #[derive] attribute eagerly expands all cfg and cfg_attr attributes in its item ("fully configures" it) before passing that item to derive macros.

For example, in the next example

#[derive(Trait)]
struct S {
    #[cfg(FALSE)]
    field: u8,
}

Trait will see tokens struct S {} rather than struct S { #[cfg(FALSE)] field: u8, }.

PR #79078 ("Turn #[derive] into a regular macro attribute") supported macro attributes written after #[derive]:

#[derive(Trait)]
#[my_macro]
struct S {
    #[cfg(FALSE)]
    field: u8,
}

Attributes on the same item are expanded in left to right order, so my_macro is expanded immediately after derive.
The question is: which tokens my_macro receives as an input?

  1. It can receive the original, not configured, item. In other words #[derive] feeds a configured version of the item to Trait, but otherwise emit the item in unmodified form so all the cfgs and cfg_attrs in it are expanded later in natural order.
  2. It can receive the same fully configured item as Trait.
  3. It can receive the item transformed in some entirely different way (not considering this variant here, no motivation to do that).

Code depending on the answer is currently gated by feature(macro_attributes_in_derive_output).

We propose stabilizing the variant 1. and feeding the fully configured item only to traits inside #[derive] and keeping the original item it for later expansion.
This seems to be the most principled solution - #[derive] only affects itself and not attributes written after it.
As a result attributes that want to do custom cfg processing (like those found by crater in #85073 (comment)) can be written after #[derive] too.
The variant 1. also prevents abusing empty #[derive()] as a cfg-expanding attribute (this problem is going to be addressed by #[cfg_eval] (#82679) which I'm planning to stabilize next).
The main concern here is performance because some things may need to be configured twice, but as a perf experiment in #86057 shows, it's not an issue in practice.

The more radical alternative is to fully configure items before expanding any attributes, not only derive.
That alternative was implemented as an experiment in #85073.
Crater showed that there are indeed attributes that want to do some custom cfg processing (#85073 (comment)) which are broken by this change, so we need some kind opt-out from eager configuration, but it's not clear how that opt-out should look like.
We suggest considering switching to this alternative for the next edition (~Rust 2024), but leave it alone for now.

Examples

#[derive(Trait)] // (1) `Trait` sees `struct S {}` as input
#[my_macro] // (2) `my_macro` sees `struct S { #[cfg(FALSE)] field: u8, }` as input
struct S {
    #[cfg(FALSE)]
    field: u8,
}

Test cases

  • src/test/ui/proc-macro/attribute-after-derive.rs - (2) in the example above
  • src\test\ui\proc-macro\expand-to-derive.rs or any other test with derive and cfg - (1) in the example above

Documentation

rust-lang/reference#1074

@petrochenkov petrochenkov added I-nominated S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 17, 2021
@bors

This comment has been minimized.

@joshtriplett
Copy link
Member

We discussed this in today's @rust-lang/lang meeting. We agreed with this proposal, and we think it makes sense to change this for current Rust. We do feel that we should move to eager-evaluation by default for a future edition, with an opt-out for macros/attributes that really want to deal with unevaluated cfg. But for now, this change seems like the right approach.

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Sep 7, 2021

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

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!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Sep 7, 2021
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Sep 14, 2021
@rfcbot
Copy link

rfcbot commented Sep 14, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Sep 24, 2021
@rfcbot
Copy link

rfcbot commented Sep 24, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@Aaron1011
Copy link
Member

r=me with the merge conflicts addressed

@petrochenkov
Copy link
Contributor Author

@bors r=Aaron1011

@bors
Copy link
Contributor

bors commented Sep 24, 2021

📌 Commit 85f0290 has been approved by Aaron1011

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Sep 24, 2021
@bors
Copy link
Contributor

bors commented Sep 25, 2021

⌛ Testing commit 85f0290 with merge 60fe8b3...

@bors
Copy link
Contributor

bors commented Sep 25, 2021

☀️ Test successful - checks-actions
Approved by: Aaron1011
Pushing 60fe8b3 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 25, 2021
@bors bors merged commit 60fe8b3 into rust-lang:master Sep 25, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 25, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (60fe8b3): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@jplatte jplatte mentioned this pull request Sep 28, 2021
65 tasks
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Sep 30, 2021
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Dec 6, 2021
Pkgsrc changes:
 * Adapt a couple of patches

Upstream changes:

Version 1.57.0 (2021-12-02)
==========================

Language
--------

- [Macro attributes may follow `#[derive]` and will see the original
  (pre-`cfg`) input.][87220]
- [Accept curly-brace macros in expressions, like `m!{ .. }.method()`
  and `m!{ .. }?`.][88690]
- [Allow panicking in constant evaluation.][89508]

Compiler
--------

- [Create more accurate debuginfo for vtables.][89597]
- [Add `armv6k-nintendo-3ds` at Tier 3\*.][88529]
- [Add `armv7-unknown-linux-uclibceabihf` at Tier 3\*.][88952]
- [Add `m68k-unknown-linux-gnu` at Tier 3\*.][88321]
- [Add SOLID targets at Tier 3\*:][86191] `aarch64-kmc-solid_asp3`,
  `armv7a-kmc-solid_asp3-eabi`, `armv7a-kmc-solid_asp3-eabihf`

\* Refer to Rust's [platform support page][platform-support-doc] for more
   information on Rust's tiered platform support.

Libraries
---------

- [Avoid allocations and copying in `Vec::leak`][89337]
- [Add `#[repr(i8)]` to `Ordering`][89507]
- [Optimize `File::read_to_end` and `read_to_string`][89582]
- [Update to Unicode 14.0][89614]
- [Many more functions are marked `#[must_use]`][89692], producing a warning
  when ignoring their return value. This helps catch mistakes such as expecting
  a function to mutate a value in place rather than return a new value.

Stabilised APIs
---------------

- [`[T; N]::as_mut_slice`][`array::as_mut_slice`]
- [`[T; N]::as_slice`][`array::as_slice`]
- [`collections::TryReserveError`]
- [`HashMap::try_reserve`]
- [`HashSet::try_reserve`]
- [`String::try_reserve`]
- [`String::try_reserve_exact`]
- [`Vec::try_reserve`]
- [`Vec::try_reserve_exact`]
- [`VecDeque::try_reserve`]
- [`VecDeque::try_reserve_exact`]
- [`Iterator::map_while`]
- [`iter::MapWhile`]
- [`proc_macro::is_available`]
- [`Command::get_program`]
- [`Command::get_args`]
- [`Command::get_envs`]
- [`Command::get_current_dir`]
- [`CommandArgs`]
- [`CommandEnvs`]

These APIs are now usable in const contexts:

- [`hint::unreachable_unchecked`]

Cargo
-----

- [Stabilize custom profiles][cargo/9943]

Compatibility notes
-------------------

Internal changes
----------------
These changes provide no direct user facing benefits, but represent significant
improvements to the internals and overall performance of rustc
and related tools.

- [Added an experimental backend for codegen with `libgccjit`.][87260]

[86191]: rust-lang/rust#86191
[87220]: rust-lang/rust#87220
[87260]: rust-lang/rust#87260
[88243]: rust-lang/rust#88243
[88321]: rust-lang/rust#88321
[88529]: rust-lang/rust#88529
[88690]: rust-lang/rust#88690
[88952]: rust-lang/rust#88952
[89337]: rust-lang/rust#89337
[89507]: rust-lang/rust#89507
[89508]: rust-lang/rust#89508
[89582]: rust-lang/rust#89582
[89597]: rust-lang/rust#89597
[89614]: rust-lang/rust#89614
[89692]: rust-lang/rust#89692
[cargo/9943]: rust-lang/cargo#9943
[`array::as_mut_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_mut_slice
[`array::as_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_slice
[`collections::TryReserveError`]: https://doc.rust-lang.org/std/collections/struct.TryReserveError.html
[`HashMap::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_map/struct.HashMap.html#method.try_reserve
[`HashSet::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_set/struct.HashSet.html#method.try_reserve
[`String::try_reserve`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve
[`String::try_reserve_exact`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve_exact
[`Vec::try_reserve`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve
[`Vec::try_reserve_exact`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve_exact
[`VecDeque::try_reserve`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve
[`VecDeque::try_reserve_exact`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve_exact
[`Iterator::map_while`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.map_while
[`iter::MapWhile`]: https://doc.rust-lang.org/std/iter/struct.MapWhile.html
[`proc_macro::is_available`]: https://doc.rust-lang.org/proc_macro/fn.is_available.html
[`Command::get_program`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_program
[`Command::get_args`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_args
[`Command::get_envs`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_envs
[`Command::get_current_dir`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_current_dir
[`CommandArgs`]: https://doc.rust-lang.org/std/process/struct.CommandArgs.html
[`CommandEnvs`]: https://doc.rust-lang.org/std/process/struct.CommandEnvs.html
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 22, 2022
Pkgsrc changes:
 * Adjust line numbers in a number of patches
 * remove the --disable-dist-src option, so that we produce
   the rust-src rust component, which we upload to LOCALSRC
   to allow the rust-src package to build, which is needed
   for rust-analyzer.
 * Cargo checksum for vendor/cc no longer needs patching;
   checksum for vendor/libc updated

Upstream changes:

Version 1.57.0 (2021-12-02)
==========================

Language
--------

- [Macro attributes may follow `#[derive]` and will see the original
  (pre-`cfg`) input.][87220]
- [Accept curly-brace macros in expressions, like `m!{ .. }.method()`
  and `m!{ .. }?`.][88690]
- [Allow panicking in constant evaluation.][89508]

Compiler
--------

- [Create more accurate debuginfo for vtables.][89597]
- [Add `armv6k-nintendo-3ds` at Tier 3\*.][88529]
- [Add `armv7-unknown-linux-uclibceabihf` at Tier 3\*.][88952]
- [Add `m68k-unknown-linux-gnu` at Tier 3\*.][88321]
- [Add SOLID targets at Tier 3\*:][86191] `aarch64-kmc-solid_asp3`,
  `armv7a-kmc-solid_asp3-eabi`, `armv7a-kmc-solid_asp3-eabihf`

\* Refer to Rust's [platform support page][platform-support-doc] for more
   information on Rust's tiered platform support.

Libraries
---------

- [Avoid allocations and copying in `Vec::leak`][89337]
- [Add `#[repr(i8)]` to `Ordering`][89507]
- [Optimize `File::read_to_end` and `read_to_string`][89582]
- [Update to Unicode 14.0][89614]
- [Many more functions are marked `#[must_use]`][89692], producing a warning
  when ignoring their return value. This helps catch mistakes such as expecting
  a function to mutate a value in place rather than return a new value.

Stabilised APIs
---------------

- [`[T; N]::as_mut_slice`][`array::as_mut_slice`]
- [`[T; N]::as_slice`][`array::as_slice`]
- [`collections::TryReserveError`]
- [`HashMap::try_reserve`]
- [`HashSet::try_reserve`]
- [`String::try_reserve`]
- [`String::try_reserve_exact`]
- [`Vec::try_reserve`]
- [`Vec::try_reserve_exact`]
- [`VecDeque::try_reserve`]
- [`VecDeque::try_reserve_exact`]
- [`Iterator::map_while`]
- [`iter::MapWhile`]
- [`proc_macro::is_available`]
- [`Command::get_program`]
- [`Command::get_args`]
- [`Command::get_envs`]
- [`Command::get_current_dir`]
- [`CommandArgs`]
- [`CommandEnvs`]

These APIs are now usable in const contexts:

- [`hint::unreachable_unchecked`]

Cargo
-----

- [Stabilize custom profiles][cargo/9943]

Compatibility notes
-------------------

Internal changes
----------------
These changes provide no direct user facing benefits, but represent significant
improvements to the internals and overall performance of rustc
and related tools.

- [Added an experimental backend for codegen with `libgccjit`.][87260]

[86191]: rust-lang/rust#86191
[87220]: rust-lang/rust#87220
[87260]: rust-lang/rust#87260
[88243]: rust-lang/rust#88243
[88321]: rust-lang/rust#88321
[88529]: rust-lang/rust#88529
[88690]: rust-lang/rust#88690
[88952]: rust-lang/rust#88952
[89337]: rust-lang/rust#89337
[89507]: rust-lang/rust#89507
[89508]: rust-lang/rust#89508
[89582]: rust-lang/rust#89582
[89597]: rust-lang/rust#89597
[89614]: rust-lang/rust#89614
[89692]: rust-lang/rust#89692
[cargo/9943]: rust-lang/cargo#9943
[`array::as_mut_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_mut_slice
[`array::as_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_slice
[`collections::TryReserveError`]: https://doc.rust-lang.org/std/collections/struct.TryReserveError.html
[`HashMap::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_map/struct.HashMap.html#method.try_reserve
[`HashSet::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_set/struct.HashSet.html#method.try_reserve
[`String::try_reserve`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve
[`String::try_reserve_exact`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve_exact
[`Vec::try_reserve`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve
[`Vec::try_reserve_exact`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve_exact
[`VecDeque::try_reserve`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve
[`VecDeque::try_reserve_exact`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve_exact
[`Iterator::map_while`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.map_while
[`iter::MapWhile`]: https://doc.rust-lang.org/std/iter/struct.MapWhile.html
[`proc_macro::is_available`]: https://doc.rust-lang.org/proc_macro/fn.is_available.html
[`Command::get_program`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_program
[`Command::get_args`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_args
[`Command::get_envs`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_envs
[`Command::get_current_dir`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_current_dir
[`CommandArgs`]: https://doc.rust-lang.org/std/process/struct.CommandArgs.html
[`CommandEnvs`]: https://doc.rust-lang.org/std/process/struct.CommandEnvs.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. relnotes Marks issues that should be documented in the release notes of the next release. 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.

Tracking issue for macro attributes in #[derive] output
10 participants