Remove `Nonterminal` and `TokenKind::Interpolated` #124141

nnethercote · 2024-04-18T23:28:21Z

A third attempt at this; the first attempt was #96724 and the second was #114647.

rustbot · 2024-04-18T23:28:25Z

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

ijackson · 2024-05-10T15:28:11Z

❤️ @nnethercote for working on this. Thank you! I'm not sure if there's a way for me to help, as someone who doesn't really know much about the compiler innards, but please LMK if you think of something.

nnethercote · 2024-05-10T20:45:22Z

@ijackson: thanks! I'm curious why you are interested in this change, given that it's a compiler internals rearrangement?

nnethercote · 2024-05-10T20:47:31Z

@ijackson: Oh, I see, you are interested in #67062 being fixed. Unfortunately my current thoughts are that this PR alone won't be enough to fix that issue, though it's a necessary stepping stone.

ijackson · 2024-05-10T22:40:41Z

@ijackson: Oh, I see, you are interested in #67062 being fixed. Unfortunately my current thoughts are that this PR alone won't be enough to fix that issue, though it's a necessary stepping stone.

Right. It seems ... quite nontrivial. So, thanks.

oriongonza · 2024-05-12T21:49:53Z

After this is done TokenKind will become Copy right?

nnethercote · 2024-05-12T22:35:11Z

After this is done TokenKind will become Copy right?

Yes.

Instead of using AST pretty printing. This is a step towards removing `token::Interpolated`, which will eventually (in rust-lang#124141) be replaced with a token stream within invisible delimiters. This changes (improves) the output of the `stringify!` macro in some cases. This is allowed. As the `stringify!` docs say: "Note that the expanded results of the input tokens may change in the future. You should be careful if you rely on the output." Test changes: - tests/ui/macros/stringify.rs: this used to test both token stream pretty printing and AST pretty printing via different ways of invoking of `stringify!` (i.e. `$expr` vs `$tt`). But those two different invocations now give the same result, which is a nice consistency improvement. This removes the need for the `c2!` macro. - tests/ui/macros/trace_faulty_macros.rs: there is some sub-optimal spacing in the printing of `A { a : a, b : 0, c : _, .. }`, which will be fixed in the next commit. The spacing of `1+1` improves -- it now matches the formatting in the source code. - tests/ui/proc-macro/*: minor improvements where small differences between `INPUT (DISPLAY)` output and `DEEP-RE-COLLECTED (DISPLAY)` output disappear.

…, r=<try> Print `token::Interpolated` with token stream pretty printing. This is a step towards removing `token::Interpolated` (rust-lang#124141). It unavoidably changes the output of the `stringify!` macro, generally for the better. r? `@petrochenkov`

nnethercote · 2024-05-17T06:47:27Z

#125174 carves off a piece of this PR so it can be merged separately.

bors · 2024-05-18T00:46:59Z

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

Add tests for `-Zunpretty=expanded` ported from stringify's tests This PR adds a new set of tests for the AST pretty-printer. Previously, pretty-printer edge cases were tested by way of `stringify!` in [tests/ui/macros/stringify.rs](https://github.com/rust-lang/rust/blob/1.78.0/tests/ui/macros/stringify.rs), such as the tests added by rust-lang@419b269 and rust-lang@527e2ea. Those tests will no longer provide effective coverage of the AST pretty-printer after rust-lang#124141. `Nonterminal` and `TokenKind::Interpolated` are being removed, and a consequence is that `stringify!` will perform token stream pretty printing, instead of AST pretty printing, in all of the `stringify!` cases including $:expr and all other interpolations. This PR adds 2 new ui tests with `compile-flags: -Zunpretty=expanded`: - **tests/ui/unpretty/expanded-exhaustive.rs** — this test aims for exhaustive coverage of all the variants of `ExprKind`, `ItemKind`, `PatKind`, `StmtKind`, `TyKind`, and `VisibilityKind`. Some parts could use being fleshed out further, but the current state is roughly on par with what exists in the old stringify-based tests. - **tests/ui/unpretty/expanded-interpolation.rs** — this test covers tricky macro metavariable edge cases that require the AST pretty printer to synthesize parentheses in order for the printed code to be valid Rust syntax. r? `@nnethercote`

Rollup merge of rust-lang#125236 - dtolnay:expandtest, r=nnethercote Add tests for `-Zunpretty=expanded` ported from stringify's tests This PR adds a new set of tests for the AST pretty-printer. Previously, pretty-printer edge cases were tested by way of `stringify!` in [tests/ui/macros/stringify.rs](https://github.com/rust-lang/rust/blob/1.78.0/tests/ui/macros/stringify.rs), such as the tests added by rust-lang@419b269 and rust-lang@527e2ea. Those tests will no longer provide effective coverage of the AST pretty-printer after rust-lang#124141. `Nonterminal` and `TokenKind::Interpolated` are being removed, and a consequence is that `stringify!` will perform token stream pretty printing, instead of AST pretty printing, in all of the `stringify!` cases including $:expr and all other interpolations. This PR adds 2 new ui tests with `compile-flags: -Zunpretty=expanded`: - **tests/ui/unpretty/expanded-exhaustive.rs** — this test aims for exhaustive coverage of all the variants of `ExprKind`, `ItemKind`, `PatKind`, `StmtKind`, `TyKind`, and `VisibilityKind`. Some parts could use being fleshed out further, but the current state is roughly on par with what exists in the old stringify-based tests. - **tests/ui/unpretty/expanded-interpolation.rs** — this test covers tricky macro metavariable edge cases that require the AST pretty printer to synthesize parentheses in order for the printed code to be valid Rust syntax. r? `@nnethercote`

Instead of using AST pretty printing. This is a step towards removing `token::Interpolated`, which will eventually (in rust-lang#124141) be replaced with a token stream within invisible delimiters. This changes (improves) the output of the `stringify!` macro in some cases. This is allowed. As the `stringify!` docs say: "Note that the expanded results of the input tokens may change in the future. You should be careful if you rely on the output." Test changes: - tests/ui/macros/stringify.rs: this used to test both token stream pretty printing and AST pretty printing via different ways of invoking of `stringify!` (i.e. `$expr` vs `$tt`). But those two different invocations now give the same result, which is a nice consistency improvement. This removes the need for all the `c2*` macros. The AST pretty printer now has more thorough testing thanks to rust-lang#125236. - tests/ui/proc-macro/*: minor improvements where small differences between `INPUT (DISPLAY)` output and `DEEP-RE-COLLECTED (DISPLAY)` output disappear.

petrochenkov · 2024-05-23T20:01:24Z

It's great to see that enum InvisibleOrigin allows to migrate the parser to delimited groups relatively simply, with just the maybe_whole to maybe_reparse_metavar_seq replacement.

Of course it prevents a lot of interesting stuff like reparsing expr as pat and similar, like it would work in a purely token-based model, but all that can be carefully introduced later, when it's possible to do backward compatibly.

petrochenkov · 2024-05-23T20:02:23Z

How hard would it be to get this to a perf run?
(With or without the NtExpr/NtLiteral stuff.)

petrochenkov · 2024-05-23T20:03:04Z

Blocked on #125174.
@rustbot blocked

…enkov Remove `NtVis` and `NtTy` The next part of rust-lang#124141. The first actual remove of `Nonterminal` variants. `NtVis` is a simple case that doesn't get much use, but `NtTy` is more complex. r? `@petrochenkov`

…r=<try> Remove `NtPat`, `NtItem`, and `NtStmt` Another part of rust-lang#124141. r? `@petrochenkov`

…r=<try> Remove `NtPat` Another part of rust-lang#124141. r? `@petrochenkov`

…r=<try> Remove `NtPat`, `NtMeta`, and `NtPath` Another part of rust-lang#124141. r? `@petrochenkov`

bors · 2025-02-25T02:36:43Z

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

…r=<try> Remove `NtPat`, `NtMeta`, and `NtPath` Another part of rust-lang#124141. r? `@petrochenkov`

…r=petrochenkov Remove `NtPat`, `NtMeta`, and `NtPath` Another part of rust-lang#124141. r? `@petrochenkov`

This involves replacing `nt_pretty_printing_compatibility_hack` with `stream_pretty_printing_compatibility_hack`. The handling of statements in `transcribe` is slightly different to other nonterminal kinds, due to the lack of `from_ast` implementation for empty statements. Notable test changes: - `tests/ui/proc-macro/expand-to-derive.rs`: the diff looks large but the only difference is the insertion of a single invisible-delimited group around a metavar.

This time when converting them to proc-macro `Group` form.

Notes about tests: - tests/ui/rfcs/rfc-2294-if-let-guard/feature-gate.rs: some messages are now duplicated due to repeated parsing. - tests/ui/rfcs/rfc-2497-if-let-chains/disallowed-positions.rs: ditto. - `tests/ui/proc-macro/macro-rules-derive-cfg.rs`: the diff looks large but the only difference is the insertion of a single invisible-delimited group around a metavar. - `tests/ui/attributes/nonterminal-expansion.rs`: a slight span degradation, somehow related to the recent massive attr parsing rewrite (rust-lang#135726). I couldn't work out exactly what is going wrong, I don't think it's worth holding things up for a single slightly suboptimal error message.

`NtBlock` is the last remaining variant of `Nonterminal`, so once it is gone then `Nonterminal` can be removed as well.

They are no longer needed. This does slightly worsen the error message for a single test, but that test contains code that is so badly broken that I'm not worried about it.

rustbot · 2025-03-05T03:52:19Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

nnethercote · 2025-03-05T03:55:03Z

@petrochenkov: this is ready for final review. There is one commit marked with "XXX" that is taken from #137758; it should be merged soon. I will update this PR once that has happened.

petrochenkov · 2025-03-05T12:47:00Z

@nnethercote
Could you continue splitting this like previously?
I'd rather review the Nt* removals individually because some of them are nontrivial (and it will land faster this way due to my review scheduling).

nnethercote · 2025-03-05T20:34:35Z

@petrochenkov: I would also prefer to split them, but the "Remove NtItem and NtStmt" commit introduces a rustdoc error that doesn't go away until "Remove NtBlock, Nonterminal, and TokenKind::Interpolated". Some more details are in this comment. I already reordered the Nt* removal somewhat to remove as many of them as possible before this sequence of rustdoc-affecting commits.

This is temporarily needed for `x doc compiler` to work. They can be removed once the `Nonterminal` is removed (rust-lang#124141).

nnethercote · 2025-03-06T02:07:22Z

Ok, I filed #138083 for NtItem/NtStmt. It required increasing the recursion_limit to 256 for 29 crates. That can be undone once Nonterminal is removed.

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 18, 2024

nnethercote marked this pull request as draft April 18, 2024 23:28

nnethercote mentioned this pull request Apr 18, 2024

Start removing Nonterminal #114647

Closed

nnethercote force-pushed the rm-Nonterminal-and-TokenKind-Interpolated branch 2 times, most recently from 42a623a to c133e16 Compare April 23, 2024 03:51

petrochenkov self-assigned this Apr 28, 2024

nnethercote force-pushed the rm-Nonterminal-and-TokenKind-Interpolated branch from c133e16 to 7aef5db Compare May 16, 2024 10:50

This comment has been minimized.

Sign in to view

nnethercote mentioned this pull request May 17, 2024

Print token::Interpolated with token stream pretty printing. #125174

Merged

dtolnay mentioned this pull request May 18, 2024

Add tests for -Zunpretty=expanded ported from stringify's tests #125236

Merged

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 23, 2024

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 19, 2025

nnethercote force-pushed the rm-Nonterminal-and-TokenKind-Interpolated branch from 2479f6c to 4fc4749 Compare February 24, 2025 07:44

nnethercote mentioned this pull request Feb 24, 2025

Remove NtPat, NtMeta, and NtPath #137517

Merged

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 24, 2025

Auto merge of rust-lang#137517 - nnethercote:rm-NtPat-NtItem-NtStmt, …

e6d1e42

…r=<try> Remove `NtPat`, `NtItem`, and `NtStmt` Another part of rust-lang#124141. r? `@petrochenkov`

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 24, 2025

Auto merge of rust-lang#137517 - nnethercote:rm-NtPat-NtItem-NtStmt, …

2c86f0c

…r=<try> Remove `NtPat` Another part of rust-lang#124141. r? `@petrochenkov`

nnethercote force-pushed the rm-Nonterminal-and-TokenKind-Interpolated branch from 4fc4749 to 2eef60e Compare February 25, 2025 00:52

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 25, 2025

Auto merge of rust-lang#137517 - nnethercote:rm-NtPat-NtItem-NtStmt, …

52886b3

…r=<try> Remove `NtPat`, `NtMeta`, and `NtPath` Another part of rust-lang#124141. r? `@petrochenkov`

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 25, 2025

Auto merge of rust-lang#137517 - nnethercote:rm-NtPat-NtItem-NtStmt, …

fc289d3

…r=<try> Remove `NtPat`, `NtMeta`, and `NtPath` Another part of rust-lang#124141. r? `@petrochenkov`

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2025

Auto merge of rust-lang#137517 - nnethercote:rm-NtPat-NtItem-NtStmt, …

0740d9f

…r=petrochenkov Remove `NtPat`, `NtMeta`, and `NtPath` Another part of rust-lang#124141. r? `@petrochenkov`

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2025

Auto merge of rust-lang#137517 - nnethercote:rm-NtPat-NtItem-NtStmt, …

aa3c2d7

…r=petrochenkov Remove `NtPat`, `NtMeta`, and `NtPath` Another part of rust-lang#124141. r? `@petrochenkov`

nnethercote added 8 commits March 5, 2025 10:23

XXX: Jana's fix

eb0843f

More denesting of invisibly-delimited groups.

f1bed64

This time when converting them to proc-macro `Group` form.

Fix a problem with metavars and inner attributes.

28452b6

Remove NtBlock, Nonterminal, and TokenKind::Interpolated.

310805d

`NtBlock` is the last remaining variant of `Nonterminal`, so once it is gone then `Nonterminal` can be removed as well.

Impl Copy for Token and TokenKind.

d8dd1b4

Remove TokenStream::flattened and InvisibleOrigin::FlattenToken.

8b55797

They are no longer needed. This does slightly worsen the error message for a single test, but that test contains code that is so badly broken that I'm not worried about it.

nnethercote force-pushed the rm-Nonterminal-and-TokenKind-Interpolated branch from 2eef60e to 8b55797 Compare March 5, 2025 03:52

rustbot added the A-attributes Area: Attributes (`#[…]`, `#![…]`) label Mar 5, 2025

nnethercote added a commit to nnethercote/rust that referenced this pull request Mar 6, 2025

Increase recursion_limit in numerous crates.

a604115

This is temporarily needed for `x doc compiler` to work. They can be removed once the `Nonterminal` is removed (rust-lang#124141).

nnethercote mentioned this pull request Mar 6, 2025

Remove NtItem and NtStmt #138083

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `Nonterminal` and `TokenKind::Interpolated` #124141

Remove `Nonterminal` and `TokenKind::Interpolated` #124141

nnethercote commented Apr 18, 2024 •

edited

Loading

rustbot commented Apr 18, 2024

ijackson commented May 10, 2024

nnethercote commented May 10, 2024

nnethercote commented May 10, 2024

ijackson commented May 10, 2024

oriongonza commented May 12, 2024

nnethercote commented May 12, 2024

This comment has been minimized.

nnethercote commented May 17, 2024 •

edited

Loading

bors commented May 18, 2024

petrochenkov commented May 23, 2024 •

edited

Loading

petrochenkov commented May 23, 2024

petrochenkov commented May 23, 2024

bors commented Feb 25, 2025

rustbot commented Mar 5, 2025

nnethercote commented Mar 5, 2025

petrochenkov commented Mar 5, 2025

nnethercote commented Mar 5, 2025

nnethercote commented Mar 6, 2025

Remove Nonterminal and TokenKind::Interpolated #124141

Are you sure you want to change the base?

Remove Nonterminal and TokenKind::Interpolated #124141

Conversation

nnethercote commented Apr 18, 2024 • edited Loading

rustbot commented Apr 18, 2024

ijackson commented May 10, 2024

nnethercote commented May 10, 2024

nnethercote commented May 10, 2024

ijackson commented May 10, 2024

oriongonza commented May 12, 2024

nnethercote commented May 12, 2024

This comment has been minimized.

nnethercote commented May 17, 2024 • edited Loading

bors commented May 18, 2024

petrochenkov commented May 23, 2024 • edited Loading

petrochenkov commented May 23, 2024

petrochenkov commented May 23, 2024

bors commented Feb 25, 2025

rustbot commented Mar 5, 2025

nnethercote commented Mar 5, 2025

petrochenkov commented Mar 5, 2025

nnethercote commented Mar 5, 2025

nnethercote commented Mar 6, 2025

Remove `Nonterminal` and `TokenKind::Interpolated` #124141

Remove `Nonterminal` and `TokenKind::Interpolated` #124141

nnethercote commented Apr 18, 2024 •

edited

Loading

nnethercote commented May 17, 2024 •

edited

Loading

petrochenkov commented May 23, 2024 •

edited

Loading