Expand NtExpr tokens only in key-value attributes
#77271
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @shepmaster (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try |
|
⌛ Trying commit 7c1c7f1d17a63493486da2ed55d820cb113bf786 with merge 075992eda207e1647b699c8c8ada67b32ce7b115... |
petrochenkov
added
S-waiting-on-crater
This was referenced Sep 27, 2020
|
☀️ Try build successful - checks-actions, checks-azure |
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
RalfJung
added a commit
to RalfJung/rust
that referenced
this pull request
Sep 28, 2020
expand: Stop normalizing `NtIdent`s before passing them to built-in macros Built-in macros should be able to deal with `NtIdents` in the input by themselves like any other parser code. You can't imagine how bad mutable AST visitors are, *especially* if they are modifying tokens. This is one step towards removing token visiting from the visitor infrastructure (rust-lang#77271 also works in this direction.)
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 29, 2020
expand: Stop normalizing `NtIdent`s before passing them to built-in macros Built-in macros should be able to deal with `NtIdents` in the input by themselves like any other parser code. You can't imagine how bad mutable AST visitors are, *especially* if they are modifying tokens. This is one step towards removing token visiting from the visitor infrastructure (rust-lang#77271 also works in this direction.)
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
craterbot
added
S-waiting-on-review
petrochenkov
added
S-waiting-on-author
|
No non-spurious regressions from crater - this is an excellent news, I expected worse given the one case found in libcore. |
camelid
added
S-waiting-on-author
petrochenkov
force-pushed
the
notokenexp
branch
from
7c1c7f1
to
5e1eb6c
Compare
petrochenkov
changed the title
NtExpr tokens only in key-value attributesNtExpr tokens only in key-value attributes
bors
added
S-waiting-on-bors
Aaron1011
added
A-attributes
|
☀️ Test successful - checks-actions |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 8, 2020
rustc_ast: Visit tokens stored in AST nodes in mutable visitor After rust-lang#77271 token visiting is enabled only for one visitor in `rustc_expand\src\mbe\transcribe.rs` which applies hygiene marks to tokens produced by declarative macros (`macro_rules` or `macro`), so this change doesn't affect anything else. When a macro has some interpolated token from an outer macro in its output ```rust macro inner() { $interpolated } ``` we can use the usual interpretation of interpolated tokens in token-based model - a None-delimited group - to write this macro in an equivalent form ```rust macro inner() { ⟪ a b c d ⟫ } ``` When we are expanding the macro `inner` we need to apply hygiene marks to all tokens produced by it, including the tokens inside the group. Before this PR we did this by visiting the AST piece inside the interpolated token and applying marks to all spans in it. I'm not sure this is 100% correct (ideally we should apply the marks to tokens and then re-parse the AST from tokens), but it's a very good approximation at least. We didn't however apply the marks to actual tokens stored in the nonterminal, so if we used the nonterminal as a token rather than as an AST piece (e.g. passed it to a proc macro), then we got hygiene bugs. This PR applies the marks to tokens in addition to the AST pieces thus fixing the issue. r? `@Aaron1011`
|
@Aaron1011 What's the user facing change in this PR? It's hard to tell from the discussion and diff. |
|
I think this might not have any impact beyond fixing some ICEs. I may have confused this with the follow-up PR that allows arbitrary expressions in key-value attributes pre-expansion. |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 10, 2020
Accept arbitrary expressions in key-value attributes at parse time Continuation of rust-lang#77271. We now support arbitrary expressions in values of key-value attributes at parse time. ``` #[my_attr = EXPR] ``` Previously only unsuffixed literals and interpolated expressions (`$expr`) were accepted. There are two immediate motivational cases for this: - External doc strings (`#[doc = include_str!("my_doc.md")]`, eliminating the need in rust-lang#44732) and expanding macros in this position in general. Currently such macro expansions are supported in this position in interpolated `$expr`s (the `#[doc = $doc]` idiom). - Paths (`#[namespace = foo::bar] extern "C++" { ... }`) like proposed in rust-lang#76734. If the attribute in question survives expansion, then the value is still restricted to unsuffixed literals by a semantic check. This restriction doesn't prevent the use cases listed above, so this PR keeps it in place for now. Closes rust-lang#52607. Previous attempt - rust-lang#67121. Some more detailed write up on internals - https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455. Tracking issue - rust-lang#78835.
jyn514
added a commit
to jyn514/rust
that referenced
this pull request
May 18, 2021
# Stabilization report
## Summary
This stabilizes using macro expansion in key-value attributes, like so:
```rust
#[doc = include_str!("my_doc.md")]
struct S;
#[path = concat!(env!("OUT_DIR"), "/generated.rs")]
mod m;
```
See the changes to the reference for details on what macros are allowed;
see Petrochenkov's excellent blog post [on internals](https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455)
for alternatives that were considered and rejected ("why accept no more
and no less?")
This has been available on nightly since 1.50 with no major issues.
## Notes
### Accepted syntax
The parser accepts arbitrary Rust expressions in this position, but any expression other than a macro invocation will ultimately lead to an error because it is not expected by the built-in expression forms (e.g., `#[doc]`). Note that decorators and the like may be able to observe other expression forms.
### Expansion ordering
Expansion of macro expressions in "inert" attributes occurs after decorators have executed, analogously to macro expressions appearing in the function body or other parts of decorator input.
There is currently no way for decorators to accept macros in key-value position if macro expansion must be performed before the decorator executes (if the macro can simply be copied into the output for later expansion, that can work).
## Test cases
- https://github.com/rust-lang/rust/blob/master/src/test/ui/attributes/key-value-expansion-on-mac.rs
- https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/external-doc.rs
The feature has also been dogfooded extensively in the compiler and
standard library:
- rust-lang#83329
- rust-lang#83230
- rust-lang#82641
- rust-lang#80534
## Implementation history
- Initial proposal: rust-lang#55414 (comment)
- Experiment to see how much code it would break: rust-lang#67121
- Preliminary work to restrict expansion that would conflict with this
feature: rust-lang#77271
- Initial implementation: rust-lang#78837
- Fix for an ICE: rust-lang#80563
## Unresolved Questions
~~rust-lang#83366 (comment) listed some concerns, but they have been resolved as of this final report.~~
## Additional Information
There are two workarounds that have a similar effect for `#[doc]`
attributes on nightly. One is to emulate this behavior by using a limited version of this feature that was stabilized for historical reasons:
```rust
macro_rules! forward_inner_docs {
($e:expr => $i:item) => {
#[doc = $e]
$i
};
}
forward_inner_docs!(include_str!("lib.rs") => struct S {});
```
This also works for other attributes (like `#[path = concat!(...)]`).
The other is to use `doc(include)`:
```rust
#![feature(external_doc)]
#[doc(include = "lib.rs")]
struct S {}
```
The first works, but is non-trivial for people to discover, and
difficult to read and maintain. The second is a strange special-case for
a particular use of the macro. This generalizes it to work for any use
case, not just including files.
I plan to remove `doc(include)` when this is stabilized. The
`forward_inner_docs` workaround will still compile without warnings, but
I expect it to be used less once it's no longer necessary.
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
May 18, 2021
…=petrochenkov Stabilize extended_key_value_attributes Closes rust-lang#44732. Closes rust-lang#78835. Closes rust-lang#82768 (by making it irrelevant). # Stabilization report ## Summary This stabilizes using macro expansion in key-value attributes, like so: ```rust #[doc = include_str!("my_doc.md")] struct S; #[path = concat!(env!("OUT_DIR"), "/generated.rs")] mod m; ``` See Petrochenkov's excellent blog post [on internals](https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455) for alternatives that were considered and rejected ("why accept no more and no less?") This has been available on nightly since 1.50 with no major issues. ## Notes ### Accepted syntax The parser accepts arbitrary Rust expressions in this position, but any expression other than a macro invocation will ultimately lead to an error because it is not expected by the built-in expression forms (e.g., `#[doc]`). Note that decorators and the like may be able to observe other expression forms. ### Expansion ordering Expansion of macro expressions in "inert" attributes occurs after decorators have executed, analogously to macro expressions appearing in the function body or other parts of decorator input. There is currently no way for decorators to accept macros in key-value position if macro expansion must be performed before the decorator executes (if the macro can simply be copied into the output for later expansion, that can work). ## Test cases - https://github.com/rust-lang/rust/blob/master/src/test/ui/attributes/key-value-expansion-on-mac.rs - https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/external-doc.rs The feature has also been dogfooded extensively in the compiler and standard library: - rust-lang#83329 - rust-lang#83230 - rust-lang#82641 - rust-lang#80534 ## Implementation history - Initial proposal: rust-lang#55414 (comment) - Experiment to see how much code it would break: rust-lang#67121 - Preliminary work to restrict expansion that would conflict with this feature: rust-lang#77271 - Initial implementation: rust-lang#78837 - Fix for an ICE: rust-lang#80563 ## Unresolved Questions ~~rust-lang#83366 (comment) listed some concerns, but they have been resolved as of this final report.~~ ## Additional Information There are two workarounds that have a similar effect for `#[doc]` attributes on nightly. One is to emulate this behavior by using a limited version of this feature that was stabilized for historical reasons: ```rust macro_rules! forward_inner_docs { ($e:expr => $i:item) => { #[doc = $e] $i }; } forward_inner_docs!(include_str!("lib.rs") => struct S {}); ``` This also works for other attributes (like `#[path = concat!(...)]`). The other is to use `doc(include)`: ```rust #![feature(external_doc)] #[doc(include = "lib.rs")] struct S {} ``` The first works, but is non-trivial for people to discover, and difficult to read and maintain. The second is a strange special-case for a particular use of the macro. This generalizes it to work for any use case, not just including files. I plan to remove `doc(include)` when this is stabilized (rust-lang#82539). The `forward_inner_docs` workaround will still compile without warnings, but I expect it to be used less once it's no longer necessary.
jackh726
added a commit
to jackh726/rust
that referenced
this pull request
May 19, 2021
…=petrochenkov Stabilize extended_key_value_attributes Closes rust-lang#44732. Closes rust-lang#78835. Closes rust-lang#82768 (by making it irrelevant). # Stabilization report ## Summary This stabilizes using macro expansion in key-value attributes, like so: ```rust #[doc = include_str!("my_doc.md")] struct S; #[path = concat!(env!("OUT_DIR"), "/generated.rs")] mod m; ``` See Petrochenkov's excellent blog post [on internals](https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455) for alternatives that were considered and rejected ("why accept no more and no less?") This has been available on nightly since 1.50 with no major issues. ## Notes ### Accepted syntax The parser accepts arbitrary Rust expressions in this position, but any expression other than a macro invocation will ultimately lead to an error because it is not expected by the built-in expression forms (e.g., `#[doc]`). Note that decorators and the like may be able to observe other expression forms. ### Expansion ordering Expansion of macro expressions in "inert" attributes occurs after decorators have executed, analogously to macro expressions appearing in the function body or other parts of decorator input. There is currently no way for decorators to accept macros in key-value position if macro expansion must be performed before the decorator executes (if the macro can simply be copied into the output for later expansion, that can work). ## Test cases - https://github.com/rust-lang/rust/blob/master/src/test/ui/attributes/key-value-expansion-on-mac.rs - https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/external-doc.rs The feature has also been dogfooded extensively in the compiler and standard library: - rust-lang#83329 - rust-lang#83230 - rust-lang#82641 - rust-lang#80534 ## Implementation history - Initial proposal: rust-lang#55414 (comment) - Experiment to see how much code it would break: rust-lang#67121 - Preliminary work to restrict expansion that would conflict with this feature: rust-lang#77271 - Initial implementation: rust-lang#78837 - Fix for an ICE: rust-lang#80563 ## Unresolved Questions ~~rust-lang#83366 (comment) listed some concerns, but they have been resolved as of this final report.~~ ## Additional Information There are two workarounds that have a similar effect for `#[doc]` attributes on nightly. One is to emulate this behavior by using a limited version of this feature that was stabilized for historical reasons: ```rust macro_rules! forward_inner_docs { ($e:expr => $i:item) => { #[doc = $e] $i }; } forward_inner_docs!(include_str!("lib.rs") => struct S {}); ``` This also works for other attributes (like `#[path = concat!(...)]`). The other is to use `doc(include)`: ```rust #![feature(external_doc)] #[doc(include = "lib.rs")] struct S {} ``` The first works, but is non-trivial for people to discover, and difficult to read and maintain. The second is a strange special-case for a particular use of the macro. This generalizes it to work for any use case, not just including files. I plan to remove `doc(include)` when this is stabilized (rust-lang#82539). The `forward_inner_docs` workaround will still compile without warnings, but I expect it to be used less once it's no longer necessary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement the experiment described in #55414 (comment)
This PR also removes some customization points and token visiting functionality from AST visitors.
Read-only visitor no longer visits tokens, mutable visitor visits tokens only when specifically enabled, mutable token visiting is restricted to its single intended use case.
I haven't changed the representation of
MacArgs::Eqyet, but it potentially can use aTokenTreeor aTokeninstead ofTokenStream.It's hard to get rid of
Nonterminal::NtExprthere (and e.g. replace it withast::Expr) due to the dual nature of key-value attributes (the value is both an expression and a token stream, depending on context), andNonterminalhas all the machinery for maintaining both representations in sync.Fixes #55414
Fixes #43860