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.

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

Improve token stream pretty printing. #97340

Conversation

nnethercote
Copy link
Contributor

@nnethercote nnethercote commented May 24, 2022

Specifically:

  • No space before or after ::, e.g. a::b instead of a :: b.
  • No space between ! and (, e.g. foo!() instead of foo! ().
  • No space between ! and [, e.g. #![...] instead of #! [...].
  • No space before :, e.g. a: u32 instead of a : u32.
  • No space before ;, e.g. struct A; instead of struct A ;.

r? @petrochenkov

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 24, 2022
@rust-highfive
Copy link
Collaborator

r? @cjgillot

(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 24, 2022
@nnethercote
Copy link
Contributor Author

The a::b change is a necessary part of #96724, without it some doctests fail. While fixing that I saw some other easy improvements. I figure it's worth pulling these out on their own because #96724 is going to take a while.

@nnethercote
Copy link
Contributor Author

r? @petrochenkov

Specifically:
- No space before or after `::`, e.g. `a::b` instead of `a :: b`.
- No space between `!` and `(`, e.g. `foo!()` instead of `foo! ()`.
- No space between `!` and `[`, e.g. `#![...]` instead of `#! [...]`.
- No space before `:`, e.g. `a: u32` instead of `a : u32`.
- No space before `;`, e.g. `struct A;` instead of `struct A ;`.
@nnethercote nnethercote force-pushed the improve-token-stream-pretty-printing branch from 1129138 to 20d4424 Compare May 24, 2022 02:54
@@ -636,7 +636,7 @@ fn test_item() {
() => {};
}
),
"macro_rules! stringify { () => {} ; }", // FIXME
"macro_rules! stringify { () => {}; }", // FIXME
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the FIXME for?

@petrochenkov
Copy link
Contributor

Originally spaces were always printed because proc macros worked through pretty-printing and reparsing, and spaces prevented tokens from being glued together and forming other tokens, like : : -> ::.
Note that in proc macros spaces are significant for all punctuation tokens, not only those that can form multi-character operators in Rust syntax, e.g. !+ and ! + can be treated differently, similarly to < < and << in Rust syntax.

Then we started skipping some spaces to avoid breaking code (#63897).

Eventually people just started skipping them to beautify the pretty-printed code, which is incorrect in theory.
But it may be less important now when proc macros very rarely have to go through pretty-printing and reparsing.

@petrochenkov
Copy link
Contributor

petrochenkov commented May 24, 2022

Ideally we would preserve jointness information for all tokens and then use it during pretty-printing (#76285 went into that direction, but was reverted later due to some issues and never re-landed).

@petrochenkov
Copy link
Contributor

To keep documentation links working with #96724 it should be enough to elide spaces in cases :: IDENT and IDENT ::, so I'd personally prefer to limit this PR to that.

Although ! ( and ! [ should be fine too because they can never be joined.

@petrochenkov 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 May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

6 participants