Skip to content

Stabilize #[cfg_eval] and feature(macro_attributes_in_derive_output)#83824

Closed
petrochenkov wants to merge 1 commit intorust-lang:masterfrom
petrochenkov:stabcfgeval
Closed

Stabilize #[cfg_eval] and feature(macro_attributes_in_derive_output)#83824
petrochenkov wants to merge 1 commit intorust-lang:masterfrom
petrochenkov:stabcfgeval

Conversation

@petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Apr 3, 2021

TODO: Stabilization report.

Closes #82679
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 Apr 3, 2021
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-lang Relevant to the language team and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 3, 2021
@jyn514 jyn514 added the needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. label Apr 3, 2021
@bors
Copy link
Collaborator

bors commented Apr 5, 2021

☔ The latest upstream changes (presumably #83530) made this pull request unmergeable. Please resolve the merge conflicts.

pksunkara
pksunkara approved these changes Apr 5, 2021
View reviewed changes
Copy link
Contributor

@pksunkara pksunkara left a comment

Choose a reason for hiding this comment

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

Looks good. 👍

@petrochenkov
Copy link
Contributor Author

petrochenkov commented Apr 11, 2021 •
edited
Loading

I'm going to delay this stabilization a bit and try two more things first:

@pksunkara
Copy link
Contributor

pksunkara commented Apr 11, 2021

  • Double down on cfg and cfg_attr being expanded out of order, and eagerly expand them for all attributes implicitly, not only for derive and cfg_eval. Need to check backward compatibility and performance.

I am not completely sure about the code and internals, would love if you could help explain a bit of why this is a blocker for stabilising this feature.

  • Produce the original unconfigured item from #[derive(Trait1, Trait2)] while still feeding its configured version to Trait1 and Trait2. Need to check for performance.

Why do we need the original unconfigured item?

@pksunkara
Copy link
Contributor

pksunkara commented Apr 11, 2021

Ah, for others who are thinking the same, here's related links: #83331, #83354

@petrochenkov
Copy link
Contributor Author

petrochenkov commented Apr 11, 2021

@pksunkara

would love if you could help explain a bit of why this is a blocker for stabilising this feature.

If input is eagerly fully configured for all attributes, then #[cfg_eval] becomes a no-op and no longer has reasons to exist.

Why do we need the original unconfigured item?

So you can place attributes requiring the original unconfigured input after #[derive].

@Aaron1011
Copy link
Contributor

Aaron1011 commented Apr 11, 2021

Double down on cfg and cfg_attr being expanded out of order, and eagerly expand them for all attributes implicitly, not only for derive and cfg_eval. Need to check backward compatibility and performance.

One potential issue with doing this - a macro might want to do addtional processing of #[cfg] attributes. For example, someone could write an attribute #[auto_doc_cfg] which creates #[doc(cfg)] attributes for every #[cfg] attribute in the input (and copies them to any generated items).

@petrochenkov
Copy link
Contributor Author

petrochenkov commented Apr 11, 2021

Yeah, not expanding cfgs eagerly gives proc macro authors a choice, so we'll probably need some kind of opt-out.
I'm considering the variant with eagerly expanding cfgs for everything because in 90+% cases doing that is the preferable choice.

This was referenced Apr 23, 2021
Closed
Closed
@davidhewitt
Copy link
Contributor

davidhewitt commented Apr 23, 2021

As a pyO3 maintainer the evidence downstream (in pyO3 at least) is that having cfg-stripping be opt-out would be slightly more convenient for our users, who are often first-time Rustaceans coming from Python.

That said, once we've got a mechanism in the language (whether opt-in or opt-out) then it's quite plausible with pyO3's current design to conditionally compile python bindings for Rust crates just by slapping #[cfg_attr] on the right parts of the original Rust source.

We would write documentation for this regardless of whether it would need to explain #[cfg_eval] or not, so it's not a big problem either way.

My thanks to you folks for implementing this! 🚀

This was referenced May 5, 2021
Closed
Merged
@petrochenkov
Copy link
Contributor Author

petrochenkov commented May 6, 2021 •
edited
Loading

Blocked on #85073.

@petrochenkov petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 6, 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-blocked Status: Blocked on something else such as an RFC or other implementation work. labels May 30, 2021
@petrochenkov petrochenkov mentioned this pull request Jun 6, 2021
Closed
@petrochenkov
Copy link
Contributor Author

petrochenkov commented Jun 6, 2021

Blocked on #86057.

@petrochenkov petrochenkov removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 6, 2021
@petrochenkov petrochenkov added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jun 6, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 6, 2021
@bors
Auto merge of rust-lang#84995 - petrochenkov:tcollect, r=Aaron1011
86b0baf 
parser: Ensure that all nonterminals have tokens after parsing

`parse_nonterminal` should always result in something with tokens.

This requirement wasn't satisfied in two cases:
- `stmt` nonterminal with expression statements (e.g. `0`, or `{}`, or `path + 1`) because `fn parse_stmt_without_recovery` forgot to propagate `force_collect` in some cases.
- `expr` nonterminal with expressions with built-in attributes (e.g. `#[allow(warnings)] 0`) due to an incorrect optimization in `fn parse_expr_force_collect`, it assumed that all expressions starting with `#` have their tokens collected during parsing, but that's not true if all the attributes on that expression are built-in and inert.

(Discovered when trying to implement eager `cfg` expansion for all attributes rust-lang#83824 (comment).)

r? `@Aaron1011`
@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-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jun 6, 2021
@crlf0710 crlf0710 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 25, 2021
@crlf0710 crlf0710 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 17, 2021
@petrochenkov
Copy link
Contributor Author

petrochenkov commented Jul 17, 2021

Blocked on #87220.

@petrochenkov petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. 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
@petrochenkov
Copy link
Contributor Author

petrochenkov commented Jul 17, 2021

Blocked on #87220.

Actually no, looks like #[cfg_eval] can be stabilized independently from #87220, I've made a PR - #87221.
So this PR can be closed.

@petrochenkov petrochenkov deleted the stabcfgeval branch February 22, 2025 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@pksunkara pksunkara pksunkara approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Aaron1011 Aaron1011

Labels

needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-lang Relevant to the language team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tracking Issue for built-in attribute macro #[cfg_eval] Tracking issue for macro attributes in #[derive] output

8 participants

@petrochenkov @bors @pksunkara @Aaron1011 @davidhewitt @crlf0710 @rust-highfive @jyn514