Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Some token stream cleanups #98278

Merged
merged 5 commits into from
Jun 21, 2022
Merged

Some token stream cleanups #98278

merged 5 commits into from
Jun 21, 2022

Conversation

nnethercote
Copy link
Contributor

Best reviewed one commit at a time.

r? @petrochenkov

@nnethercote
Remove Cursor::append.
ccd956a 
It's a weird function: it lets you modify the token stream in the middle
of iteration. There is only one call site, and it is only used for the
rare `ProceduralMasquerade` legacy case.
@nnethercote
Remove Cursor::index.
178b746 
It's unused.
@nnethercote
Remove TokenStream::from_streams.
f6b5788 
By inlining it into the only non-test call site. The one test call site
is changed to use `TokenStreamBuilder`.
@nnethercote
Merge TokenStreamBuilder::push into TokenStreamBuilder::build.
2a5487a 
Both functions do some modifying of streams using `make_mut`:
- `push` sometimes glues the first token of the next stream to the last
  token of the first stream.
- `build` appends tokens to the first stream.

By doing all of this in the one place, things are simpler. The first
stream can be modified in both ways (if necessary) in the one place, and
any next stream with the first token removed doesn't need to be stored.
@nnethercote
Add blank lines between methods in proc_macro_server.rs.
69f45b7 
Because that's the standard way of doing it.
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 20, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 20, 2022
@nnethercote
Copy link
Contributor Author

Not expecting much change here:

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 20, 2022
@bors
Copy link
Contributor

bors commented Jun 20, 2022

⌛ Trying commit 69f45b7 with merge a4080d0740dfac42d6c48c3a7a0e49498c03c7c9...

@bors
Copy link
Contributor

bors commented Jun 20, 2022

☀️ Try build successful - checks-actions
Build commit: a4080d0740dfac42d6c48c3a7a0e49498c03c7c9 (a4080d0740dfac42d6c48c3a7a0e49498c03c7c9)

@rust-timer
Copy link
Collaborator

Queued a4080d0740dfac42d6c48c3a7a0e49498c03c7c9 with parent 9a0b774, future comparison URL.

@petrochenkov petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 20, 2022
@petrochenkov
Copy link
Contributor

r=me when the perf run is complete.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a4080d0740dfac42d6c48c3a7a0e49498c03c7c9): comparison url.

Instruction count

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

Max RSS (memory usage)

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: 🎉 relevant improvement found
mean1 max count2
Regressions 😿
(primary)		 3.2% 3.2% 1
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 -1.3% -1.3% 1
All 😿🎉 (primary) 3.2% 3.2% 1

Cycles

Results
  • Primary benchmarks: 😿 relevant regressions found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)		 2.2% 2.7% 2
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 N/A N/A 0
All 😿🎉 (primary) 2.2% 2.7% 2

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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-review -S-waiting-on-perf -perf-regression

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2

  2. number of relevant changes ↩ ↩2

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 20, 2022
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 20, 2022

📌 Commit 69f45b7 has been approved by petrochenkov

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Jun 20, 2022
@matthiaskrgr
Copy link
Member

@bors rollup=never

@nnethercote
Copy link
Contributor Author

@matthiaskrgr: with the neutral perf CI run done, I think this one would be fine to be in a rollup. It's fairly small, and not doing anything very complicated.

@matthiaskrgr
Copy link
Member

@bors rollup=maybe

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 21, 2022
@matthiaskrgr
Rollup merge of rust-lang#98278 - nnethercote:some-token-stream-clean…
0de1e04 
…ups, r=petrochenkov

Some token stream cleanups

Best reviewed one commit at a time.

r? `@petrochenkov`
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 21, 2022
Closed
@JohnTitor JohnTitor mentioned this pull request Jun 21, 2022
Closed
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 21, 2022
@JohnTitor
Rollup merge of rust-lang#98278 - nnethercote:some-token-stream-clean…
0827359 
…ups, r=petrochenkov

Some token stream cleanups

Best reviewed one commit at a time.

r? ``@petrochenkov``
@JohnTitor JohnTitor mentioned this pull request Jun 21, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 21, 2022
@bors
Auto merge of rust-lang#98335 - JohnTitor:rollup-j2zudxv, r=JohnTitor
72fd41a 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#94033 (Improve docs for `is_running` to explain use case)
 - rust-lang#97269 (adjust transmute const stabilization version)
 - rust-lang#97805 (Add proper tracing spans to rustc_trait_selection::traits::error_reporting)
 - rust-lang#98022 (Fix erroneous span for borrowck error)
 - rust-lang#98124 (Improve loading of crates.js and sidebar-items.js)
 - rust-lang#98278 (Some token stream cleanups)
 - rust-lang#98306 (`try_fold_unevaluated` for infallible folders)
 - rust-lang#98313 (Remove lies in comments.)
 - rust-lang#98323 (:arrow_up: rust-analyzer)
 - rust-lang#98329 (Avoid an ICE and instead let the compiler report a useful error)
 - rust-lang#98330 (update ioslice docs to use shared slices)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b1d2e5c into rust-lang:master Jun 21, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 21, 2022
@nnethercote nnethercote deleted the some-token-stream-cleanups branch June 21, 2022 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@petrochenkov petrochenkov

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
Milestone
1.63.0
Development

Successfully merging this pull request may close these issues.
7 participants
@nnethercote @rust-timer @bors @petrochenkov @matthiaskrgr @rust-highfive @rustbot