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

Fix some pretty printing tests #37202

Merged
merged 1 commit into from
Oct 19, 2016
Merged

Fix some pretty printing tests #37202

merged 1 commit into from
Oct 19, 2016

Conversation

petrochenkov
Copy link
Contributor

Many pretty-printing tests are un-ignored.
Some issues in classification of comments (trailing/isolated) and blank line counting are fixed.
Some comments are printed more carefully.
Some minor refactoring in pprust.rs
no-pretty-expanded annotations are removed because this is the default now.
pretty-expanded annotations are removed from compile-fail tests, they are not tested with pretty-printer.

Closes #23623 in favor of more specific #37201 and #37199
r? @nrc

@petrochenkov
Copy link
Contributor Author

@nrc
Some more general questions:
Are there any plans regarding the rustc pretty-printer, considering that rustfmt is actively developed?
Printing of some small fragments of AST/HIR is still useful for diagnostics, but what about all the formatting stuff, comments?
Also, HIR pretty-printer could probably use some diet. It can't even print parens reliably, but at the same time it tries to print other surface stuff like comments. Do you think they are needed or they can be removed?

@nrc
Copy link
Member

nrc commented Oct 17, 2016

@petrochenkov

Are there any plans regarding the rustc pretty-printer, considering that rustfmt is actively developed?

It's been on my mind, but I don't have a plan. ATM, I think we leave it in place and maintain it to the minimal state necessary. If it becomes a burden, we might look to sharing code between Rustfmt and the pretty printer. I don't know exactly how this would work. This would probably be easier with Rustbuild and using an external crate.

Printing of some small fragments of AST/HIR is still useful for diagnostics, but what about all the formatting stuff, comments?

Agree. I think it is sometimes useful to have pretty printing of whole programs for debugging, and that requires keeping the formatting stuff. We could probably lose comments, but it doesn't seem like a burden.

Also, HIR pretty-printer could probably use some diet. It can't even print parens reliably, but at the same time it tries to print other surface stuff like comments. Do you think they are needed or they can be removed?

I'm not sure off the top of my head whether we use HIR pretty printer for error messages, etc. But it seems likely we do for types, etc. Again, it is sometimes useful for debugging to dump HIR as code. I'm honestly not sure whether it is worth spending effort on it.

@nrc
Copy link
Member

nrc commented Oct 17, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Oct 17, 2016

📌 Commit 2cb8cdc has been approved by nrc

@bors
Copy link
Contributor

bors commented Oct 18, 2016

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

@petrochenkov
Copy link
Contributor Author

@bors r=nrc

@bors
Copy link
Contributor

bors commented Oct 18, 2016

📌 Commit 4a91a80 has been approved by nrc

eddyb added a commit to eddyb/rust that referenced this pull request Oct 19, 2016
Fix some pretty printing tests

Many pretty-printing tests are un-ignored.
Some issues in classification of comments (trailing/isolated) and blank line counting are fixed.
Some comments are printed more carefully.
Some minor refactoring in pprust.rs
`no-pretty-expanded` annotations are removed because this is the default now.
`pretty-expanded` annotations are removed from compile-fail tests, they are not tested with pretty-printer.

Closes rust-lang#23623 in favor of more specific rust-lang#37201 and rust-lang#37199
r? @nrc
eddyb added a commit to eddyb/rust that referenced this pull request Oct 19, 2016
Fix some pretty printing tests

Many pretty-printing tests are un-ignored.
Some issues in classification of comments (trailing/isolated) and blank line counting are fixed.
Some comments are printed more carefully.
Some minor refactoring in pprust.rs
`no-pretty-expanded` annotations are removed because this is the default now.
`pretty-expanded` annotations are removed from compile-fail tests, they are not tested with pretty-printer.

Closes rust-lang#23623 in favor of more specific rust-lang#37201 and rust-lang#37199
r? @nrc
bors added a commit that referenced this pull request Oct 19, 2016
@bors bors merged commit 4a91a80 into rust-lang:master Oct 19, 2016
@petrochenkov petrochenkov deleted the pretty branch March 16, 2017 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pretty printer is injecting extra newlines/spaces in funky ways in some run-fail tests
3 participants