Skip to content

Improve if/else pretty printing #140280

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

Merged
merged 4 commits into from
Apr 27, 2025

Conversation

nnethercote
Copy link
Contributor

@nnethercote nnethercote commented Apr 25, 2025

AST/HIR pretty printing of if/else is currently pretty bad. This PR improves it a lot.

r? @Nadrieril

The AST pretty printing is a bit wonky. The HIR pretty printing is
extremely wonky.
Indents for `cbox` and `ibox` are 0 or `INDENT_UNIT` (4) except for a
couple of places which are `INDENT_UNIT - 1` for no clear reason.

This commit changes the three space indents to four spaces.
By removing some of the over-indenting. AST pretty printing now looks
correct. HIR pretty printing is better, but still over-indents some.
@rustbot

This comment was marked as resolved.

@rustbot
Copy link
Collaborator

rustbot commented Apr 25, 2025

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Apr 25, 2025
@rustbot

This comment was marked as resolved.

@rust-log-analyzer

This comment has been minimized.

In the AST the "then" block is represented as a `Block`. In HIR the
"then" block is represented as an `Expr` that happens to always be.
`ExprKind::Block`. By deconstructing the `ExprKind::Block` to extract
the block within, things print properly.

For `issue-82392.rs`, note that we no longer print a type after the
"then" block. This is good, it now matches how we don't print a type for
the "else" block. (Well, we do print a type after the "else" block, but
it's for the whole if/else.)

Also tighten up some of the pattern matching -- these block expressions
within if/else will never have labels.
@nnethercote nnethercote force-pushed the improve-if-else-printing branch from e484839 to 7ac2d1f Compare April 25, 2025 20:38
@Urgau
Copy link
Member

Urgau commented Apr 27, 2025

Not yet perfect, but definitively an improvement. Thanks!

r? Urgau @bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 27, 2025

📌 Commit 7ac2d1f has been approved by Urgau

It is now in the queue for this repository.

@rustbot rustbot assigned Urgau and unassigned Nadrieril Apr 27, 2025
@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 Apr 27, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 27, 2025
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#140246 (Fix never pattern printing)
 - rust-lang#140280 (Improve if/else pretty printing)
 - rust-lang#140348 (Update lint-docs to default to Rust 2024)
 - rust-lang#140358 (Use `search_for_cycle_permutation` to look for `variances_of`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 405c8af into rust-lang:master Apr 27, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 27, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 27, 2025
Rollup merge of rust-lang#140280 - nnethercote:improve-if-else-printing, r=Urgau

Improve if/else pretty printing

AST/HIR pretty printing of if/else is currently pretty bad. This PR improves it a lot.

r? `@Nadrieril`
@nnethercote nnethercote deleted the improve-if-else-printing branch April 27, 2025 20:53
nnethercote added a commit to nnethercote/rust that referenced this pull request Apr 28, 2025
The pretty-printers open and close "boxes" of text a lot. The open and
close operations must be matched. The matching is currently all implicit
and very easy to get wrong. (rust-lang#140280 and rust-lang#140246 are two recent
pretty-printing fixes that both involved unclosed boxes.)

This commit introduced `BoxMarker`, a marker type that represents an
open box. It makes box opening/closing explicit, which makes it much
easier to understand and harder to get wrong.

The commit also removes many comments are on `end` calls saying things
like "end outer head-block", "Close the outer-box". These demonstrate
how confusing the implicit approach was, but aren't necessary any more.
nnethercote added a commit to nnethercote/rust that referenced this pull request Apr 28, 2025
The pretty-printers open and close "boxes" of text a lot. The open and
close operations must be matched. The matching is currently all implicit
and very easy to get wrong. (rust-lang#140280 and rust-lang#140246 are two recent
pretty-printing fixes that both involved unclosed boxes.)

This commit introduces `BoxMarker`, a marker type that represents an
open box. It makes box opening/closing explicit, which makes it much
easier to understand and harder to get wrong.

The commit also removes many comments are on `end` calls saying things
like "end outer head-block", "Close the outer-box". These demonstrate
how confusing the implicit approach was, but aren't necessary any more.
nnethercote added a commit to nnethercote/rust that referenced this pull request Apr 28, 2025
The pretty-printers open and close "boxes" of text a lot. The open and
close operations must be matched. The matching is currently all implicit
and very easy to get wrong. (rust-lang#140280 and rust-lang#140246 are two recent
pretty-printing fixes that both involved unclosed boxes.)

This commit introduces `BoxMarker`, a marker type that represents an
open box. It makes box opening/closing explicit, which makes it much
easier to understand and harder to get wrong.

The commit also removes many comments are on `end` calls saying things
like "end outer head-block", "Close the outer-box". These demonstrate
how confusing the implicit approach was, but aren't necessary any more.
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.

6 participants