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

Remove some unncessary spaces from pretty-printed tokenstream output #84920

Merged
merged 1 commit into from
May 15, 2021
Merged

Remove some unncessary spaces from pretty-printed tokenstream output #84920

merged 1 commit into from
May 15, 2021

Conversation

Aaron1011
Copy link
Member

@Aaron1011 Aaron1011 commented May 4, 2021 •
edited
Loading

In addition to making the output look nicer for all crates, this also
aligns the pretty-printing output with what the rental crate expects.
This will allow us to eventually disable a backwards-compat hack in a
follow-up PR.

See #84428 for some background information about why we want to make this change. Note that this change would be desirable (but not particularly necessary) even if rental didn't exist, so we're not adding any crate-specific hacks into the compiler.

@rust-highfive
Copy link
Collaborator

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 4, 2021
@Aaron1011
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented May 4, 2021

⌛ Trying commit 28ea465de8d66b5beffb6ada79005cfa8699d441 with merge 6f8a6450478f69b8510c8976ab6c2a70b3b0e47c...

@Aaron1011 Aaron1011 mentioned this pull request May 4, 2021
Closed
2 tasks
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented May 4, 2021

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

@Aaron1011
Copy link
Member Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-84920 created and queued.
🤖 Automatically detected try build 6f8a6450478f69b8510c8976ab6c2a70b3b0e47c
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 4, 2021
@craterbot
Copy link
Collaborator

🚧 Experiment pr-84920 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-84920 is completed!
📊 13 regressed and 6 fixed (160545 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels May 13, 2021
@Aaron1011
Copy link
Member Author

All of the regressions look spurious

@Aaron1011
Copy link
Member Author

r? @petrochenkov

@Aaron1011
Remove some unncessary spaces from pretty-printed tokenstream output
357c013 
In addition to making the output look nicer for all crates, this also
aligns the pretty-printing output with what the `rental` crate expects.
This will allow us to eventually disable a backwards-compat hack in a
follow-up PR.
@petrochenkov
Copy link
Contributor

The plan was to preserve spacing ("jointness") information for all tokens and print spaces accordingly to it, but the first step (#76285) was unfortunately reverted due to regressions and it didn't go anywhere since then.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 15, 2021

📌 Commit 357c013 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 May 15, 2021
@Aaron1011
Copy link
Member Author

@petrochenkov The current proc-macro API only exposes jointness for punctuation, meaning that any extra information we store will be lost after a deep recollection. So, I think we'd still want this PR even if we start trying to preserve more jointness information.

@Aaron1011
Copy link
Member Author

Just in case this cases some non-spurious regressions:
@bors rollup=never

@bors
Copy link
Contributor

bors commented May 15, 2021

⌛ Testing commit 357c013 with merge 8cf990c...

@bors
Copy link
Contributor

bors commented May 15, 2021

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 15, 2021
@bors bors merged commit 8cf990c into rust-lang:master May 15, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 15, 2021
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
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.
8 participants
@Aaron1011 @rust-highfive @bors @rust-log-analyzer @craterbot @petrochenkov @jackh726 @rustbot