Format the world (considering let-chains and let-else) #95262
Conversation
rustbot
added
the
T-compiler
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
compiler-errors
force-pushed
the
format-let-chains
branch
from
df592b6
to
0578555
Compare
|
I don't think we should do this until bootstrap rustfmt is updated to something including rust-lang/rustfmt#5203. |
I don't think so, a diff like this is far from being large enough. |
petrochenkov
added
S-waiting-on-author
|
I personally think unnecessary (let chains formatting should default to the current expression formatting) but people are debating rust-lang/style-team#169 so upstream might take a while. |
|
☔ The latest upstream changes (presumably #91030) made this pull request unmergeable. Please resolve the merge conflicts. |
|
I agree with @petrochenkov - let's wait until we know what the formatting will be. I might even have suggested holding off on usage of these features - particularly some of the "rewrite all cases" for now, but that's not something we've done, so sort of a moot point. |
|
I agree as well. I appreciate the motivation, but would recommend closing this. We're still a good ways away from even having the formatting rules defined and that just is the first step in the process. Even if the rules happen to go this particular direction, you'd still be left with a moving target here relying on periodic manual passes. For more context, we on the rustfmt team don't decide amongst ourselves how any given AST node gets formatted. There's a similar RFC based process to define the formatting rules for any and all AST constructs, and rustfmt then implements the codified rules. Until the rules for a given node actually exist, rustfmt will leave whatever the author originally wrote. That's especially important because once rustfmt starts rewriting something that becomes permanent; rustfmt can't later decide to format it differently. Unfortunately, the formatting aspects have often been forgotten or secondary concerns during language/syntax RFCs and discussions, or at least rarely make their way into the formatting process we have to follow. There's efforts underway to improve some of these processes going forward, but not really anything that can be done about the past. As such, folks will need to sit tight on let chains and let else statements. If you'd like to help accelerate those efforts, the best way would be to review and weigh in on the formatting rule proposals (rust-lang/style-team#169 & rust-lang/style-team#165) and encourage others to do the same.
This should be categorically false. If you are seeing this behavior please open an issue in the rustfmt repo with a complete and minimal repro |
…Lapkin,Nilstrieb Format all the let-chains in compiler crates Since rust-lang/rustfmt#5910 has landed, soon we will have support for formatting let-chains (as soon as rustfmt syncs and beta gets bumped). This PR applies the changes [from master rustfmt to rust-lang/rust eagerly](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/out.20formatting.20of.20prs/near/374997516), so that the next beta bump does not have to deal with a 200+ file diff and can remain concerned with other things like `cfg(bootstrap)` -- rust-lang#113637 was a pain to land, for example, because of let-else. I will also add this commit to the ignore list after it has landed. The commands that were run -- I'm not great at bash-foo, but this applies rustfmt to every compiler crate, and then reverts the two crates that should probably be formatted out-of-tree. ``` ~/rustfmt $ ls -1d ~/rust/compiler/* | xargs -I@ cargo run --bin rustfmt -- `@/src/lib.rs` --config-path ~/rust --edition=2021 # format all of the compiler crates ~/rust $ git checkout HEAD -- compiler/rustc_codegen_{gcc,cranelift} # revert changes to cg-gcc and cg-clif ``` cc `@rust-lang/rustfmt` r? `@WaffleLapkin` or `@Nilstrieb` who said they may be able to review this purely mechanical PR :> cc `@Mark-Simulacrum` and `@petrochenkov,` who had some thoughts on the order of operations with big formatting changes in rust-lang#95262 (comment). I think the situation has changed since then, given that let-chains support exists on master rustfmt now, and I'm fairly confident that this formatting PR should land even if *bootstrap* rustfmt doesn't yet format let-chains in order to lessen the burden of the next beta bump.
…lstrieb Format all the let-chains in compiler crates Since rust-lang/rustfmt#5910 has landed, soon we will have support for formatting let-chains (as soon as rustfmt syncs and beta gets bumped). This PR applies the changes [from master rustfmt to rust-lang/rust eagerly](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/out.20formatting.20of.20prs/near/374997516), so that the next beta bump does not have to deal with a 200+ file diff and can remain concerned with other things like `cfg(bootstrap)` -- #113637 was a pain to land, for example, because of let-else. I will also add this commit to the ignore list after it has landed. The commands that were run -- I'm not great at bash-foo, but this applies rustfmt to every compiler crate, and then reverts the two crates that should probably be formatted out-of-tree. ``` ~/rustfmt $ ls -1d ~/rust/compiler/* | xargs -I@ cargo run --bin rustfmt -- `@/src/lib.rs` --config-path ~/rust --edition=2021 # format all of the compiler crates ~/rust $ git checkout HEAD -- compiler/rustc_codegen_{gcc,cranelift} # revert changes to cg-gcc and cg-clif ``` cc `@rust-lang/rustfmt` r? `@WaffleLapkin` or `@Nilstrieb` who said they may be able to review this purely mechanical PR :> cc `@Mark-Simulacrum` and `@petrochenkov,` who had some thoughts on the order of operations with big formatting changes in rust-lang/rust#95262 (comment). I think the situation has changed since then, given that let-chains support exists on master rustfmt now, and I'm fairly confident that this formatting PR should land even if *bootstrap* rustfmt doesn't yet format let-chains in order to lessen the burden of the next beta bump.
rustfmt(and thus./x.py fmt) does not currently format let-chains and let-else at all. As far as I can tell, it will totally refuse to format a block (or even a file) that contains one of these newletusages.This PR aims to format all of
compiler/under rustfmt with rustfmt#5203 applied, because I fear that rustc source is building up a lot of "formatting debt", since there are many folks introducing let-chains and let-else into the codebase with rustfmt doing nothing to help them.Feel free to close this PR if this change is unwanted. It's purely aesthetic.
cc: @Mark-Simulacrum who did another big format-the-world in #67540 -- do I need to go thru some formal process for a big formatting diff?