Attach tokens to all AST types used in Nonterminal
#75800
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit f1624da705b3b5c569ab49f0560c22e652b9f3ad with merge 249ff4f3249e07e4b951856c7d9938186efce584... |
|
☀️ Try build successful - checks-actions, checks-azure |
|
Queued 249ff4f3249e07e4b951856c7d9938186efce584 with parent de521cb, future comparison URL. |
|
Finished benchmarking try commit (249ff4f3249e07e4b951856c7d9938186efce584): 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 |
|
Even if this PR doesn't land in its current form, we'll want to make this change eventually. Let's see what the breakage looks like. @craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
petrochenkov
added
the
S-waiting-on-review
|
Code LGTM, perf is acceptable, so only waiting for crater. |
petrochenkov
removed
the
S-waiting-on-review
|
☔ The latest upstream changes (presumably #73084) made this pull request unmergeable. Please resolve the merge conflicts. |
Aaron1011
force-pushed
the
feature/full-nt-tokens
branch
from
b681f77
to
6da31a8
Compare
|
@bors try |
|
⌛ Trying commit 6da31a8b9bc332944b69d94c9c28ca5b61ad3dc4 with merge fb5211a6951b98ccbb5503375fa78704f59200fe... |
|
☀️ Try build successful - checks-actions, checks-azure |
|
@craterbot abort |
|
🗑️ Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-review
A `Visibility` does not have outer attributes, so we only capture tokens when parsing a `macro_rules!` matcher
We currently only attach tokens when parsing a `:stmt` matcher for a `macro_rules!` macro. Proc-macro attributes on statements are still unstable, and need additional work.
This comment has been minimized.
This comment has been minimized.
This commit contains miscellaneous changes that don't fit into any of the other commits in this PR
Aaron1011
force-pushed
the
feature/full-nt-tokens
branch
from
cdbaf88
to
fec0479
Compare
Aaron1011
added
S-waiting-on-review
|
@bors r+ |
|
📌 Commit fec0479 has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit fec0479 with merge 3673b18616a0a8abaf68ff34da09037d9c42ccfe... |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
This looks somewhat concerning, but seems unrelated to this PR. @bors retry |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions, checks-azure |
The `doc_test!` macro defines two local variables (`entries` and `doc`),
which are then accessed from the provided `block`.
However, this only compiled due to a bug in rustc: substituted
metavariables cannot refer to names defined within the macro body. For
example, the following ode does not compile:
```rust
macro_rules! foo {
($block:expr) => {
let mut bar = false;
$block
}
}
fn main() {
foo!({bar = true});
}
```
In this case, the `doc_test!` macro was incorrectly allowed to compile
due to the presence of the `#[tokio::test]` macro on the enclosing
function. When the underlying compiler bug is fixed in
rust-lang/rust#75800,
this macro will stop compiling.
…etrochenkov Attach tokens to all AST types used in `Nonterminal` We perform token capturing when we have outer attributes (for nonterminals that support attributes - e.g. `Stmt`), or when we parse a `Nonterminal` for a `macro_rules!` argument. The full list of `Nonterminals` affected by this PR is: * `NtBlock` * `NtStmt` * `NtTy` * `NtMeta` * `NtPath` * `NtVis` * `NtLiteral` Of these nonterminals, only `NtStmt` and `NtLiteral` (which is actually just an `Expr`), support outer attributes - the rest only ever have token capturing perform when they match a `macro_rules!` argument. This makes progress towards solving rust-lang#43081 - we now collect tokens for everything that might need them. However, we still need to handle `#[cfg]`, inner attributes, and misc pretty-printing issues (e.g. rust-lang#75734) I've separated the changes into (mostly) independent commits, which could be split into individual PRs for each `Nonterminal` variant. The purpose of having them all in one PR is to do a single Crater run for all of them. Most of the changes in this PR are trivial (adding `tokens: None` everywhere we construct the various AST structs). The significant changes are: * `ast::Visibility` is changed from `type Visibility = Spanned<VisibilityKind>` to a `struct Visibility { kind, span, tokens }`. * `maybe_collect_tokens` is made generic, and used for both `ast::Expr` and `ast::Stmt`. * Some of the statement-parsing functions are refactored so that we can capture the trailing semicolon. * `Nonterminal` and `Expr` both grew by 8 bytes, as some of the structs which are stored inline (rather than behind a `P`) now have an `Option<TokenStream>` field. Hopefully the performance impact of doing this is negligible.
We perform token capturing when we have outer attributes (for nonterminals that support attributes - e.g.
Stmt), or when we parse aNonterminalfor amacro_rules!argument. The full list ofNonterminalsaffected by this PR is:NtBlockNtStmtNtTyNtMetaNtPathNtVisNtLiteralOf these nonterminals, only
NtStmtandNtLiteral(which is actually just anExpr), support outer attributes - the rest only ever have token capturing perform when they match amacro_rules!argument.This makes progress towards solving #43081 - we now collect tokens for everything that might need them. However, we still need to handle
#[cfg], inner attributes, and misc pretty-printing issues (e.g. #75734)I've separated the changes into (mostly) independent commits, which could be split into individual PRs for each
Nonterminalvariant. The purpose of having them all in one PR is to do a single Crater run for all of them.Most of the changes in this PR are trivial (adding
tokens: Noneeverywhere we construct the various AST structs). The significant changes are:ast::Visibilityis changed fromtype Visibility = Spanned<VisibilityKind>to astruct Visibility { kind, span, tokens }.maybe_collect_tokensis made generic, and used for bothast::Exprandast::Stmt.NonterminalandExprboth grew by 8 bytes, as some of the structs which are stored inline (rather than behind aP) now have anOption<TokenStream>field. Hopefully the performance impact of doing this is negligible.