Remove `NtExpr` and `NtLiteral` by nnethercote · Pull Request #138478 · rust-lang/rust

nnethercote · 2025-03-14T03:50:24Z

The next part of #124141.

rustbot · 2025-03-24T06:38:28Z

Could not assign reviewer from: petrochenkov.
User(s) petrochenkov are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

nnethercote · 2025-03-24T06:38:36Z

@bors try @rust-timer queue

bors · 2025-03-24T06:39:46Z

⌛ Trying commit 7f6b06c with merge 2f5db75...

…try> Remove `NtExpr` and `NtLiteral` The next part of rust-lang#124141. r? `@petrochenkov`

nnethercote · 2025-03-24T08:09:19Z

@rustbot ready

bors · 2025-03-24T08:49:21Z

☀️ Try build successful - checks-actions
Build commit: 2f5db75 (2f5db751163ab3fb82ba7631bd9a6e6d96c870fa)

rust-timer · 2025-03-24T10:12:40Z

Finished benchmarking commit (2f5db75): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@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.5%	[0.1%, 2.1%]	52
Regressions ❌ (secondary)	4.5%	[0.2%, 23.0%]	35
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.4%, -0.2%]	9
All ❌✅ (primary)	0.5%	[0.1%, 2.1%]	52

Max RSS (memory usage)

Results (primary 1.6%, secondary -4.3%)

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)	1.6%	[1.6%, 1.6%]	1
Regressions ❌ (secondary)	2.4%	[2.4%, 2.4%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.3%	[-12.0%, -1.5%]	13
All ❌✅ (primary)	1.6%	[1.6%, 1.6%]	1

Cycles

Results (primary 1.7%, secondary 8.5%)

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)	1.7%	[1.6%, 1.9%]	4
Regressions ❌ (secondary)	8.5%	[1.0%, 23.5%]	18
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.7%	[1.6%, 1.9%]	4

Binary size

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

Bootstrap: missing data
Artifact size: 365.48 MiB -> 365.51 MiB (0.01%)

nnethercote · 2025-03-25T05:32:50Z

The perf regressions have been seen previously in #124141, with the NtExpr/NtLiteral changes being the most impactful. Hard to avoid, unfortunately, and deep-vector is a pathological case for this PR. The last two commits in #124141 will gain back some performance.

@rustbot label: +perf-regression-triaged

nnethercote · 2025-03-26T05:50:18Z

Thanks for the review. I have added two new commits addressing the comments.

@rustbot ready

bors · 2025-04-01T11:30:19Z

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

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, but I don't think it's worth holding things up for a single slightly suboptimal error message.

In favour of the similar method on `Parser`, which works on things other than identifiers and lifetimes.

This makes the expression re-parsing more like how it's originally done in `parse_nonterminal`.

nnethercote · 2025-04-01T19:43:07Z

I rebased.

@bors r=petrochenkov

bors · 2025-04-01T19:43:09Z

📌 Commit 592d113 has been approved by petrochenkov

It is now in the queue for this repository.

bors · 2025-04-02T00:36:08Z

⌛ Testing commit 592d113 with merge 70dab5a...

bors · 2025-04-02T03:47:13Z

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

github-actions · 2025-04-02T03:48:54Z

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 9b7d5ac (parent) -> 70dab5a (this PR)

Test differences

Show 6 test diffs

Stage 1

[ui] tests/ui/attributes/dont-dup-expr-attrs.rs: [missing] -> pass (J0)
[ui] tests/ui/attributes/inner-attr-metavar.rs: [missing] -> pass (J0)

Stage 2

[ui] tests/ui/attributes/dont-dup-expr-attrs.rs: [missing] -> pass (J1)
[ui] tests/ui/attributes/inner-attr-metavar.rs: [missing] -> pass (J1)

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

Job group index

J0: x86_64-gnu-llvm-18-3, x86_64-gnu-llvm-19-3
J1: aarch64-apple, aarch64-gnu, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, test-various, x86_64-apple-2, x86_64-gnu, x86_64-gnu-llvm-18-1, x86_64-gnu-llvm-18-2, x86_64-gnu-llvm-19-1, x86_64-gnu-llvm-19-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1

Job duration changes

x86_64-apple-1: 7599.0s -> 9710.7s (27.8%)
x86_64-msvc-2: 6618.7s -> 7135.1s (7.8%)
dist-x86_64-freebsd: 5023.9s -> 5349.4s (6.5%)
dist-i686-mingw: 8154.0s -> 8671.7s (6.3%)
dist-i686-linux: 6037.2s -> 6409.6s (6.2%)
dist-aarch64-linux: 7408.3s -> 7687.7s (3.8%)
dist-various-2: 3327.4s -> 3426.4s (3.0%)
i686-gnu-nopt-1: 8462.2s -> 8697.0s (2.8%)
dist-x86_64-netbsd: 4966.0s -> 5100.4s (2.7%)
dist-x86_64-linux-alt: 7231.2s -> 7420.3s (2.6%)

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-02T05:36:19Z

Finished benchmarking commit (70dab5a): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

If the regression was expected or you think it can be justified,
please write a comment with sufficient written justification, and add
@rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
If you think that you know of a way to resolve the regression, try to create
a new PR with a fix for the regression.
If you do not understand the regression or you think that it is just noise,
you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.5%	[0.1%, 2.6%]	80
Regressions ❌ (secondary)	6.8%	[0.2%, 38.2%]	41
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.3%, -0.2%]	9
All ❌✅ (primary)	0.5%	[0.1%, 2.6%]	80

Max RSS (memory usage)

Results (primary 1.0%, secondary 0.1%)

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)	1.0%	[1.0%, 1.0%]	2
Regressions ❌ (secondary)	7.7%	[2.4%, 18.0%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.6%	[-8.0%, -2.7%]	8
All ❌✅ (primary)	1.0%	[1.0%, 1.0%]	2

Cycles

Results (primary 1.6%, secondary 14.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)	1.6%	[1.4%, 2.0%]	6
Regressions ❌ (secondary)	15.9%	[9.9%, 42.2%]	16
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.6%	[-3.6%, -3.6%]	1
All ❌✅ (primary)	1.6%	[1.4%, 2.0%]	6

Binary size

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

Bootstrap: 774.901s -> 774.398s (-0.06%)
Artifact size: 365.95 MiB -> 365.94 MiB (-0.00%)

nnethercote · 2025-04-02T06:48:36Z

Regressions were expected, alas. deep-vector is a worst-case scenario for this change.

@rustbot label: +perf-regression-triaged

nnethercote · 2025-05-01T03:50:50Z

Note: when it comes to release notes, #140241 is relevant for this PR, which is effectively a sub-PR of #124141.

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 Mar 14, 2025

This comment has been minimized.
Sign in to view

petrochenkov self-assigned this Mar 14, 2025

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 14, 2025

nnethercote force-pushed the rm-NtExpr-NtLiteral branch from 1b55759 to 7f6b06cCompare March 24, 2025 03:38

nnethercote marked this pull request as ready for review March 24, 2025 06:38

This comment has been minimized.
Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 24, 2025

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2025
Auto merge ofrust-lang#138478- nnethercote:rm-NtExpr-NtLiteral, r=<…
2f5db75
…try> Remove `NtExpr` and `NtLiteral` The next part of rust-lang#124141. r? `@petrochenkov`

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 24, 2025

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Mar 24, 2025

rustbot added the perf-regression-triaged The performance regression has been triaged. label Mar 25, 2025

petrochenkov reviewed Mar 25, 2025
View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 25, 2025

nnethercote force-pushed the rm-NtExpr-NtLiteral branch from 7f6b06c to 37af248Compare March 26, 2025 05:48

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 26, 2025

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 31, 2025

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 1, 2025

nnethercote added 4 commits April 2, 2025 06:20

Remove Token::uninterpolated_span.
d59b17c
In favour of the similar method on `Parser`, which works on things other than identifiers and lifetimes.

Fix a problem with metavars and inner attributes.
81afdbc

Fix problem causing rusqlite compilation to OOM.
592d113
This makes the expression re-parsing more like how it's originally done in `parse_nonterminal`.

nnethercote force-pushed the rm-NtExpr-NtLiteral branch from 59f6ba4 to 592d113Compare April 1, 2025 19:42

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 1, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 2, 2025

bors merged commit 70dab5a into rust-lang:masterApr 2, 2025

rustbot added this to the 1.88.0 milestone Apr 2, 2025

bors mentioned this pull request Apr 2, 2025
Stabilize let chains in the 2024 edition #132833
Merged
9 tasks

nnethercote deleted the rm-NtExpr-NtLiteral branch April 2, 2025 05:00

nnethercote mentioned this pull request Apr 2, 2025
Remove Nonterminal and TokenKind::Interpolated#124141
Merged

This was referenced Apr 2, 2025
ICE: no close delim when reparsing Expr#139248
Closed
ICE: didn't reparse an expr#139495
Closed

matthiaskrgr mentioned this pull request Apr 21, 2025
ICE: didn't reparse a literal#140098
Closed

nnethercote mentioned this pull request Apr 29, 2025
Remove NtItem and NtStmt#138083
Merged

cuviper mentioned this pull request May 2, 2025
Tracking issue for release notes of #124141: Remove Nonterminal and TokenKind::Interpolated (part 1) #140241
Closed

Uh oh!

Uh oh!

Conversation

nnethercote commented Mar 14, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

rustbot commented Mar 24, 2025

Uh oh!

nnethercote commented Mar 24, 2025

Uh oh!

This comment has been minimized.

bors commented Mar 24, 2025

Uh oh!

nnethercote commented Mar 24, 2025

Uh oh!

bors commented Mar 24, 2025

Uh oh!

rust-timer commented Mar 24, 2025

Overall result: ❌✅ regressions and improvements - please read the text below

Uh oh!

nnethercote commented Mar 25, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nnethercote commented Mar 26, 2025

Uh oh!

bors commented Apr 1, 2025

Uh oh!

nnethercote commented Apr 1, 2025

Uh oh!

bors commented Apr 1, 2025

Uh oh!

bors commented Apr 2, 2025

Uh oh!

bors commented Apr 2, 2025

Uh oh!

github-actionsBot commented Apr 2, 2025

Test differences

Stage 1

Stage 2

Job duration changes

Uh oh!

rust-timer commented Apr 2, 2025

Overall result: ❌✅ regressions and improvements - please read the text below

Uh oh!

nnethercote commented Apr 2, 2025

Uh oh!

nnethercote commented May 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nnethercote commented Mar 14, 2025•
edited
Loading