Remove `token::{Open,Close}Delim` #139897

nnethercote · 2025-04-16T07:43:17Z

By replacing them with {Open,Close}{Param,Brace,Bracket,Invisible}.

PR #137902 made ast::TokenKind more like lexer::TokenKind by
replacing the compound BinOp{,Eq}(BinOpToken) variants with fieldless
variants Plus, Minus, Star, etc. This commit does a similar thing
with delimiters. It also makes ast::TokenKind more similar to
parser::TokenType.

This requires a few new methods:

TokenKind::is_{,open_,close_}delim() replace various kinds of
pattern matches.
Delimiter::as_{open,close}_token_kind are used to convert
Delimiter values to TokenKind.

Despite these additions, it's a net reduction in lines of code. This is
because e.g. token::OpenParen is so much shorter than
token::OpenDelim(Delimiter::Parenthesis) that many multi-line forms
reduce to single line forms. And many places where the number of lines
doesn't change are still easier to read, just because the names are
shorter, e.g.:

-   } else if self.token != token::CloseDelim(Delimiter::Brace) {
+   } else if self.token != token::CloseBrace {

r? @petrochenkov

nnethercote · 2025-04-16T11:18:34Z

@bors try @rust-timer queue

… r=<try> Remove `token::{Open,Close}Delim` r? `@ghost`

bors · 2025-04-16T11:19:47Z

⌛ Trying commit d6622d6 with merge 448bb73...

bors · 2025-04-16T13:26:21Z

☀️ Try build successful - checks-actions
Build commit: 448bb73 (448bb7314ad82fe33016e188f27ebe57376d93cd)

rust-timer · 2025-04-16T14:54:13Z

Finished benchmarking commit (448bb73): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.5%, -0.2%]	14
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.4%	[-0.5%, -0.2%]	14

Max RSS (memory usage)

Results (primary -0.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.8%	[2.8%, 2.8%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.7%	[-1.1%, -0.4%]	10
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.4%	[-1.1%, 2.8%]	11

Cycles

Results (primary -0.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.6%	[-0.7%, -0.4%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.6%	[-0.7%, -0.4%]	2

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 776.859s -> 775.51s (-0.17%)
Artifact size: 364.72 MiB -> 364.80 MiB (0.02%)

nnethercote · 2025-04-17T00:51:31Z

Slight perf wins too, that's nice.

compiler/rustc_ast/src/token.rs

compiler/rustc_expand/src/proc_macro_server.rs

compiler/rustc_parse/src/lexer/tokentrees.rs

compiler/rustc_parse/src/parser/path.rs

compiler/rustc_parse/src/parser/diagnostics.rs

compiler/rustc_parse/src/parser/expr.rs

compiler/rustc_parse/src/parser/diagnostics.rs

compiler/rustc_parse/src/parser/item.rs

compiler/rustc_parse/src/parser/stmt.rs

rustbot · 2025-04-17T15:06:51Z

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

rustbot · 2025-04-17T15:07:08Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

nnethercote · 2025-04-17T22:22:40Z

I added two new commits.

The first addresses the review comments. I grepped for matches! calls that could be converted to ==, and found a few more.
The second removes an extraneous diff file that was accidentally added in Explicitly annotate edition for unpretty=expanded and unpretty=hir tests #139904.

nnethercote · 2025-04-17T22:33:25Z

@rustbot ready

jieyouxu · 2025-04-18T06:45:49Z

Can you drop the diff removal commit (since there's #139991 already)?

nnethercote · 2025-04-18T07:00:47Z

Can you drop the diff removal commit (since there's #139991 already)?

Done.

petrochenkov · 2025-04-18T11:48:48Z

r=me after squashing commits.
@rustbot author

By replacing them with `{Open,Close}{Param,Brace,Bracket,Invisible}`. PR rust-lang#137902 made `ast::TokenKind` more like `lexer::TokenKind` by replacing the compound `BinOp{,Eq}(BinOpToken)` variants with fieldless variants `Plus`, `Minus`, `Star`, etc. This commit does a similar thing with delimiters. It also makes `ast::TokenKind` more similar to `parser::TokenType`. This requires a few new methods: - `TokenKind::is_{,open_,close_}delim()` replace various kinds of pattern matches. - `Delimiter::as_{open,close}_token_kind` are used to convert `Delimiter` values to `TokenKind`. Despite these additions, it's a net reduction in lines of code. This is because e.g. `token::OpenParen` is so much shorter than `token::OpenDelim(Delimiter::Parenthesis)` that many multi-line forms reduce to single line forms. And many places where the number of lines doesn't change are still easier to read, just because the names are shorter, e.g.: ``` - } else if self.token != token::CloseDelim(Delimiter::Brace) { + } else if self.token != token::CloseBrace { ```

nnethercote · 2025-04-20T21:37:33Z

I squashed the commits.

@bors r=petrochenkov

bors · 2025-04-20T21:37:37Z

📌 Commit bf8ce32 has been approved by petrochenkov

It is now in the queue for this repository.

bors · 2025-04-22T01:15:12Z

⌛ Testing commit bf8ce32 with merge fae7785...

bors · 2025-04-22T04:43:06Z

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing fae7785 to master...

github-actions · 2025-04-22T04:44:48Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing d6c1e45 (parent) -> fae7785 (this PR)

Test differences

Show 20 test diffs

20 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard fae7785b60ea7fe1ad293352c057a5b7be73d245 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

x86_64-apple-1: 7008.7s -> 11590.5s (65.4%)
x86_64-apple-2: 8031.9s -> 3889.3s (-51.6%)
dist-apple-various: 6570.7s -> 7874.4s (19.8%)
aarch64-apple: 4822.6s -> 3928.1s (-18.5%)
x86_64-gnu-llvm-20-1: 6328.4s -> 5195.9s (-17.9%)
dist-riscv64-linux: 5313.8s -> 5867.9s (10.4%)
dist-x86_64-apple: 10849.1s -> 11641.8s (7.3%)
x86_64-msvc-2: 7263.4s -> 6775.7s (-6.7%)
x86_64-gnu-llvm-19-3: 7010.5s -> 6622.7s (-5.5%)
dist-loongarch64-musl: 5343.8s -> 5129.0s (-4.0%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2025-04-22T06:10:42Z

Finished benchmarking commit (fae7785): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.6%, -0.2%]	14
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.4%	[-0.6%, -0.2%]	14

Max RSS (memory usage)

Results (primary 0.3%, secondary 3.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.5%, 1.0%]	10
Regressions ❌ (secondary)	3.4%	[3.4%, 3.4%]	1
Improvements ✅ (primary)	-1.8%	[-2.0%, -1.5%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[-2.0%, 1.0%]	12

Cycles

Results (primary -0.6%, secondary 1.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.4%	[2.1%, 3.0%]	6
Improvements ✅ (primary)	-0.6%	[-0.6%, -0.5%]	2
Improvements ✅ (secondary)	-1.6%	[-1.6%, -1.6%]	1
All ❌✅ (primary)	-0.6%	[-0.6%, -0.5%]	2