Skip to content

Make TokenTreeCursor private#159601

Merged
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
nnethercote:privatize-TokenTreeCursor
Jul 21, 2026
Merged

Make TokenTreeCursor private#159601
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
nnethercote:privatize-TokenTreeCursor

Conversation

@nnethercote

Copy link
Copy Markdown
Contributor

And also the fields of TokenCursor.

It's good hygiene in general, and will allow larger changes to TokenCursor down the road (e.g. hopefully avoiding the flattening of token trees to a linear token stream).

r? @Kobzol

@rustbot

rustbot commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

The parser was modified, potentially altering the grammar of (stable) Rust
which would be a breaking change.

cc @fmease

@rustbot 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 Jul 20, 2026
@rustbot

rustbot commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Kobzol is not on the review rotation at the moment.
They may take a while to respond.

@Kobzol

Kobzol commented Jul 20, 2026

Copy link
Copy Markdown
Member

Thanks! I was also thinking that there was too much pub and too many implementation details shared :) Btw, I think that you can also turn pub struct TokenStream(pub(crate) Arc<Vec<TokenTree>>); into pub struct TokenStream(Arc<Vec<TokenTree>>);? Unless I missed some usage.

You can r=me once CI passes, but in the meantime, just in case:

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 20, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jul 20, 2026
@nnethercote

Copy link
Copy Markdown
Contributor Author

I think that you can also turn pub struct TokenStream(pub(crate) Arc<Vec>); into pub struct TokenStream(Arc<Vec>);?

You are correct. I'll add a commit doing that once the perf run is complete.

@rust-bors

rust-bors Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 819ef6f (819ef6fab44b5e95c2fa0a5d203a3a3ea139628d)
Base parent: 14ead7d (14ead7d3d509f50efe9afb84673e65885dfd4d07)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (819ef6f): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

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

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

Results (primary -2.3%, secondary -2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.3% [-2.6%, -2.0%] 2
Improvements ✅
(secondary)
-2.8% [-3.1%, -2.4%] 3
All ❌✅ (primary) -2.3% [-2.6%, -2.0%] 2

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 485.673s -> 486.009s (0.07%)
Artifact size: 391.74 MiB -> 391.72 MiB (-0.01%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jul 20, 2026
And also the fields of `TokenCursor`.

It's good hygiene in general, and will allow larger changes to
`TokenCursor` down the road (e.g. hopefully avoiding the flattening of
token trees to a linear token stream).
@nnethercote
nnethercote force-pushed the privatize-TokenTreeCursor branch from ee34325 to 73de522 Compare July 20, 2026 20:55
@nnethercote

Copy link
Copy Markdown
Contributor Author

A slight regression for tt-muncher in parse_token_tree. I've added #[inline] to the new small functions, hopefully that will fix it.

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 20, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jul 20, 2026
@rust-bors

rust-bors Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 19c78b2 (19c78b25589d79b5220bc5dff1926ee5c56e8a41)
Base parent: d527bc9 (d527bc9bfa297ca7fd7f5ae93781eeec42073170)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (19c78b2): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (secondary -5.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-5.3% [-5.3%, -5.3%] 1
All ❌✅ (primary) - - 0

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 489.235s -> 488.876s (-0.07%)
Artifact size: 391.81 MiB -> 391.80 MiB (-0.00%)

@rustbot rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Jul 20, 2026
@nnethercote

Copy link
Copy Markdown
Contributor Author

Inlining fixed the perf regression, yay.

@bors r=Kobzol

@rust-bors

rust-bors Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 73de522 has been approved by Kobzol

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 20, 2026
@nnethercote

Copy link
Copy Markdown
Contributor Author

@bors rollup (perf-neutral)

rust-bors Bot pushed a commit that referenced this pull request Jul 21, 2026
Rollup of 14 pull requests

Successful merges:

 - #159307 (Improve cross-namespace name diagnostics)
 - #159543 (Remove extra semicolons in parsing item lists)
 - #157270 (ergonomic_clones_dotuse_capture_by_ref: Capture upvar by ref for `.use` in non-move closures)
 - #158496 (Move `check_rustc_pub_transparent` into the attribute parser)
 - #158547 (Move `std::io::buffered` to `alloc::io`)
 - #158808 (Filter host libstdc++ ABI flag in rustc_llvm cross builds)
 - #159362 (Add regression test for #120328)
 - #159472 (Support creating float constants in rustc_public mir)
 - #159505 (make rustdoc::bare_urls strip trailing periods from url)
 - #159568 (Suggest close compiler options)
 - #159578 (Extract coroutine closure helper functions)
 - #159601 (Make `TokenTreeCursor` private)
 - #159613 (Set the rustc lib path for unstable-book-gen)
 - #159616 (Clarify the comment about stage1/stage2 discrepancy in input-stats test)
@rust-bors
rust-bors Bot merged commit d2f94b7 into rust-lang:main Jul 21, 2026
14 checks passed
@rustbot rustbot added this to the 1.99.0 milestone Jul 21, 2026
rust-timer added a commit that referenced this pull request Jul 21, 2026
Rollup merge of #159601 - nnethercote:privatize-TokenTreeCursor, r=Kobzol

Make `TokenTreeCursor` private

And also the fields of `TokenCursor`.

It's good hygiene in general, and will allow larger changes to `TokenCursor` down the road (e.g. hopefully avoiding the flattening of token trees to a linear token stream).

r? @Kobzol
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants