Don't flatten a block containing a single macro call #4496
Conversation
We no longer flatten a block that looks like this:
```rust
match val {
pat => { macro_call!() }
}
```
Currently, rust ignores trailing semicolons in macro expansion in
expression position (see rust-lang/rust#33953)
If this is changed, flattening a block with a macro call may break the
user's code - the trailing semicolon will no longer parse if the macro
call occurs immediately on the right-hand side of the match arm
(e.g. `pat => macro_call!()`)
|
I started tracing through the issues to rust-lang/rust#78449 and the trailing semi on the return and may have gotten myself confused. I assume there's no expected relation between that and flattening of the blocks? |
|
@calebcartwright: That's correct. PR rust-lang/rust#78449 is due to a different macro-related issue in rustfmt - I'll be opening a separate PR for that. |
Thanks for the clarification. Happy to discuss on that PR but my initial reaction is that's the correct formatting as the return expression should have a trailing semi https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/statements.md#expressions-in-statement-position
I understand the motivation for this change, though the fact that the semis are currently ignored along with the rustfmt stability guarantee that bans formatting changes is going to make this tough to release. Are there planned changes upstream around how those semis will be handled? Maybe one way around this, and to avoid having to wait until this becomes an explicit breaking bug, would be to update the style guide with an additional clause around when the bodies can be flattened @topecongiro - do you have any thoughts on how to best proceed with this? Seems like the right thing to do, just more a question of how/when for me |
|
From that document:
The macro expansion determines what get used as the value for a block. However, there's no explicit mention of macro expressions, which is kind of unfortunate.
We have a sort of cyclic dependency here. At some point, we will probably want to start warning about code using a trailing-semi macro expansion in expression position. However, I think it will be extremely frustating to users if rustfmt is actively undoing their attempts to fix the warning. |
Agreed, and I think that's where updating the Style Guide will give us the CYA we need to be able to justify making breaking formatting changes on the rustfmt side. The Style Guide is rustfmt's proverbial north star, so if we augment the Style Guide with these clarifications then that makes this more of a bug fix/style guide re-alignment change that won't impact our stability guarantee. |
|
We've updated the style guide with some guidance on when block-wrapping the body is require with mac call expressions. Given all of the discussion and context, this change makes sense as we don't want rustfmt to forcibly flatten the arm bodies in these cases and instead defer to the programmer. We will still need to backport this for the next 1.x release (1.4.24 since 1.4.23 is already out the door) so we can revisit if you have any concerns @topecongiro but going to go ahead merge 👍 |
|
backported in #4514 |
We no longer flatten a block that looks like this:
Currently, rust ignores trailing semicolons in macro expansion in
expression position (see rust-lang/rust#33953)
If this is changed, flattening a block with a macro call may break the
user's code - the trailing semicolon will no longer parse if the macro
call occurs immediately on the right-hand side of the match arm
(e.g.
pat => macro_call!())