[WIP] Token-based outer attributes handling#76130
[WIP] Token-based outer attributes handling#76130Aaron1011 wants to merge 6 commits intorust-lang:masterfrom
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
In PR rust-lang#76130, I add a fourth field, which makes using a tuple variant somewhat unwieldy.
This comment has been minimized.
This comment has been minimized.
…nkov Factor out StmtKind::MacCall fields into `MacCallStmt` struct In PR rust-lang#76130, I add a fourth field, which makes using a tuple variant somewhat unwieldy.
602b066 to
083792b
Compare
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 083792bcf67d1e50afc9ad50500ab011d34b0bee with merge 30ddab741f8c02392c4383445436138f5a6ec53d... |
|
☀️ Try build successful - checks-actions, checks-azure |
|
Queued 30ddab741f8c02392c4383445436138f5a6ec53d with parent 80cacd7, future comparison URL. |
|
Finished benchmarking try commit (30ddab741f8c02392c4383445436138f5a6ec53d): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
083792b to
23fd9fb
Compare
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 23fd9fbd9d07096ca254592013a42b4cae12ba29 with merge b3535fba76a3fc7202284b7699afff8f8831e461... |
|
☀️ Try build successful - checks-actions, checks-azure |
|
Queued b3535fba76a3fc7202284b7699afff8f8831e461 with parent 42d896a, future comparison URL. |
|
Finished benchmarking try commit (b3535fba76a3fc7202284b7699afff8f8831e461): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
@Aaron1011 |
|
We need to trim this PR a bit first. The next changes are improvements on their own and can be landed in separate PRs:
Additionally:
|
| pub enum PreexpTokenTree { | ||
| Token(Token), | ||
| Delimited(DelimSpan, DelimToken, PreexpTokenStream), | ||
| OuterAttributes(AttributesData), |
There was a problem hiding this comment.
Inlining AttributeData will improve readability here, IMO.
We could, but that seems weirdly inconsistent - we would then have a span that doesn't include some of the tokens we capture. However, I could split this out into a separate PR to minimize the diff. |
|
I'll work on splitting things up. I opened this PR mainly to do a Crater run to check that everything fit together. |
Split out from rust-lang#76130 This tests our handling of combining derives, derive helper attributes, attribute macros, and `cfg`/`cfg_attr`
|
With this PR we perform transformations like cfg-expansion on both tokens and AST simultaneously, and then make sure that they are still in sync with the pretty-print-reparse hack. In the end state, I think, we should treat the AST part as a cache (used for performance only) and always reparse it from tokens after any transformations (which will only be performed on tokens). The reparse hack will then be eliminated. This is kinda opposite to what we do now by treating the tokens as a "cache" and regenerating them from AST when necessary (via pretty-printing), but unlike the "AST -> tokens", the "tokens -> AST" conversion is always lossless. Having only tokens without any AST cache should be functionally correct, but it's pretty reasonable to predict that it will be a performance regression because we'll have to parse same tokens at least twice and maybe more. |
|
Blocked on #77250 |
|
Triage: A reminder that #77250 has been merged, so it seems maybe this can continue... |
|
This is currently blocked on further work on statement attributes |
|
Closing in favor of #80689 |
Makes progress towards #43081
We now capture tokens for attributes, and remove them from the tokenstream when applying
#[cfg]attributes (in addition to modifying the AST). As a result, derive-proc-macros now receive the exact input (instead of the pretty-printed/retokenized input), even when fields/variants get#[cfg]-stripped.Several changes are made in order to accomplish this:
PreexpTokenStreamandPreexpTokenTreeare added. These are identical toTokenStreamandPreexpTokenstream,with the exception of anOuterAttributesvariant inPreexpTokenTree. This is used to represent captured attributes, allowing the target tokens to be removed by#[cfg]-stripping(and when invoking attribute macros).
Attribute. This allows us to removeprepend_attributes, which required pretty-printing/retokenizing attributes in certain cases.collect_tokenswas rewritten. The implementation is now much simpler - instead of keeping track of nestedTokenTree::Delimitedat various deps, we collect all tokens into a flat buffer (e.g. we pushTokenKind::OpenDelimandTokenKind::CloseDelim. After capturing, we losslessly re-package these tokens back into the normalTokenTree::Delimitedstructure.PreexpTokenStreamon AST s structs instead of a plainTokenStream. This contains both the attributes and the target itself.parse_outer_attributesnow passes the parsed attributes to a closure, instead of simply returning them. The closure is responsible for parsing the attribute target, and allows us to automatically construct the properPreexpTokenStream.HasAttrsnow has avisit_tokensmethod. This is used during#[cfg]-stripping to allow us to remove attribute targets from thePreexpTokenStream.This PR is quite large (though ~1000 lines are tests). I opened it mainly to show how all of the individual pieces fit together, since some of them (e.g. the
parse_outer_attributeschange) don't make sense if we're not going to land this PR. However, many pieces are mergeable on their own - I plan to split them into their own PRs.TODO:
#[cfg]/#[cfg_attr]are duplicated. This is because we run the same code for both the AST-based attributes and the token-based attributes. We could possibly skip validating the token-based attributes, but I was worried about accidentally skipping gating if the pretty-print/reparse check ever fails. These are deduplicated by the diagnostic infrastructure outside of compiletest, but it would be nice to fix this.PreexpTokenStreamalongside outer attributes, which allows us to handle#![cfg(FALSE)]and remove its parent. This should be good enough to handle all stable uses of inner attributes that are observable by proc-macros (currently, just cfg-stripping before#[derive]macros are invoked). Custom inner attributes are not handled.Parser::parse_stmt_without_recoverydoes not eat a trailing semicolon, which means we will not capture it in the tokenstream. I need to either refactor statement parsing to eat the semicolon inside theparse_outer_attributes_with_tokens, or manually adjust the captured tokens.cfg,cfg_attr,derive, or any non-builtin attribute). There are many scenarios where we can skip token collection for some/all ast structs (noderiveon an an item, nocfginside an item, no custom attributes on an item, etc.)