Format heterogeneous try blocks#152293
Conversation
|
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt |
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? @ytmimi |
| // FIXME: heterogeneous try blocks, which include a type so are harder to format | ||
| ast::ExprKind::TryBlock(_, Some(_)) => Err(RewriteError::Unknown), | ||
| ast::ExprKind::TryBlock(ref block, Some(ref ty)) => { | ||
| let keyword = "try bikeshed "; |
There was a problem hiding this comment.
Is there going to be an issue with try /* comment */ bikeshed?
There was a problem hiding this comment.
Not more than with try_blocks apparently. It simply won't format. See the test case in catch.rs:
let x = try /* Invisible comment */ { foo()? };If you do:
let x = try /* Invisible comment */ { foo()?
};It won't format. I've added tests similar to the one of catch.rs.
There was a problem hiding this comment.
Let's add #![feature(try_blocks_heterogeneous)] to these tests so that it's easier to tell what experimental feature they're for.
There was a problem hiding this comment.
I've also fixed try_block.rs.
There was a problem hiding this comment.
Thanks I appreciate that 🙏🏼
| .and_then(|shape| shape.sub_width(2)) | ||
| .max_width_error(shape.width, expr.span)?; | ||
| let ty_str = ty.rewrite_result(context, ty_shape)?; | ||
| let prefix = format!("{keyword}{ty_str} "); |
There was a problem hiding this comment.
Similarly, is there going to be an issue with try bikeshed /* comment */ {ty_str}?
There was a problem hiding this comment.
Do we need to handle cases where the type can't fit on the current line and needs to wraps to the next line? Something like:
try bikeshed
{ty_str} {
}
There was a problem hiding this comment.
Good question. I gave my opinion in the PR description. I don't think this is needed given "bikeshed" is temporary so whatever we choose might not be transposable to the final syntax. But I'm happy to do it if we believe it's useful.
There was a problem hiding this comment.
This is still experimental so I think it's fine for now. T-style may want to write up some rules for this before stabilization though.
There was a problem hiding this comment.
Sorry, I thought I have linked it but apparently didn't. I also wrote a PR from that based on Zulip discussion: #152311.
|
@bors r+ rollup |
…mimi Format heterogeneous try blocks The tracking issue for `try_blocks_heterogeneous` is rust-lang#149488. This follows the formatting of homogeneous try blocks (feature `try_blocks`) by considering `try bikeshed <type>` to be the equivalent of `try` (in particular a single "token"). An alternative would be to permit breaking between `bikeshed` and `<type>`, but given that those 2 elements are an explicitly temporary part of the syntax, it doesn't seem worth it. There also doesn't seem to be any existing precedent breaking between a keyword and a type. It also doesn't seem to be useful in practice, given that the type itself doesn't break (which is how it works for the return type of closures) and has more chances to dominate the length in case a break is necessary. Happy to adapt anything in case this formatting is not optimal. The test is also copied from homogeneous try blocks with 2 additional test cases to demonstrate the behavior with long types. See [#t-lang > try blocks @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/try.20blocks/near/572387493) for context.
Rollup of 7 pull requests Successful merges: - #151152 (Add FCW for derive helper attributes that will conflict with built-in attributes) - #151954 (Add help message suggesting explicit reference cast for From/TryFrom) - #152148 (Move `impl Interner for TyCtxt` to its own submodule) - #152226 (Modernize diagnostic for indeterminate trait object lifetime bounds) - #150688 (typeck: Make it clearer that `check_pat_lit` only handles literal patterns) - #152293 (Format heterogeneous try blocks) - #152396 (Uplift `Predicate::allow_normalization` to `rustc_type_ir`)
…mimi Format heterogeneous try blocks The tracking issue for `try_blocks_heterogeneous` is rust-lang#149488. This follows the formatting of homogeneous try blocks (feature `try_blocks`) by considering `try bikeshed <type>` to be the equivalent of `try` (in particular a single "token"). An alternative would be to permit breaking between `bikeshed` and `<type>`, but given that those 2 elements are an explicitly temporary part of the syntax, it doesn't seem worth it. There also doesn't seem to be any existing precedent breaking between a keyword and a type. It also doesn't seem to be useful in practice, given that the type itself doesn't break (which is how it works for the return type of closures) and has more chances to dominate the length in case a break is necessary. Happy to adapt anything in case this formatting is not optimal. The test is also copied from homogeneous try blocks with 2 additional test cases to demonstrate the behavior with long types. See [#t-lang > try blocks @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/try.20blocks/near/572387493) for context.
…mimi Format heterogeneous try blocks The tracking issue for `try_blocks_heterogeneous` is rust-lang#149488. This follows the formatting of homogeneous try blocks (feature `try_blocks`) by considering `try bikeshed <type>` to be the equivalent of `try` (in particular a single "token"). An alternative would be to permit breaking between `bikeshed` and `<type>`, but given that those 2 elements are an explicitly temporary part of the syntax, it doesn't seem worth it. There also doesn't seem to be any existing precedent breaking between a keyword and a type. It also doesn't seem to be useful in practice, given that the type itself doesn't break (which is how it works for the return type of closures) and has more chances to dominate the length in case a break is necessary. Happy to adapt anything in case this formatting is not optimal. The test is also copied from homogeneous try blocks with 2 additional test cases to demonstrate the behavior with long types. See [#t-lang > try blocks @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/try.20blocks/near/572387493) for context.
…mimi Format heterogeneous try blocks The tracking issue for `try_blocks_heterogeneous` is rust-lang#149488. This follows the formatting of homogeneous try blocks (feature `try_blocks`) by considering `try bikeshed <type>` to be the equivalent of `try` (in particular a single "token"). An alternative would be to permit breaking between `bikeshed` and `<type>`, but given that those 2 elements are an explicitly temporary part of the syntax, it doesn't seem worth it. There also doesn't seem to be any existing precedent breaking between a keyword and a type. It also doesn't seem to be useful in practice, given that the type itself doesn't break (which is how it works for the return type of closures) and has more chances to dominate the length in case a break is necessary. Happy to adapt anything in case this formatting is not optimal. The test is also copied from homogeneous try blocks with 2 additional test cases to demonstrate the behavior with long types. See [#t-lang > try blocks @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/try.20blocks/near/572387493) for context.
…uwer Rollup of 11 pull requests Successful merges: - #152364 (Port a lot of attributes to the new parser) - #151954 (Add help message suggesting explicit reference cast for From/TryFrom) - #152148 (Move `impl Interner for TyCtxt` to its own submodule) - #152226 (Modernize diagnostic for indeterminate trait object lifetime bounds) - #152351 (Remove `SubdiagMessage` in favour of the identical `DiagMessage`) - #152417 (Move the needs-drop check for `arena_cache` queries out of macro code) - #150688 (typeck: Make it clearer that `check_pat_lit` only handles literal patterns) - #152293 (Format heterogeneous try blocks) - #152355 (Update documentation of rustc_macros) - #152396 (Uplift `Predicate::allow_normalization` to `rustc_type_ir`) - #152425 (Port #![test_runner] to the attribute parser)
…uwer Rollup of 10 pull requests Successful merges: - #152364 (Port a lot of attributes to the new parser) - #151954 (Add help message suggesting explicit reference cast for From/TryFrom) - #152148 (Move `impl Interner for TyCtxt` to its own submodule) - #152226 (Modernize diagnostic for indeterminate trait object lifetime bounds) - #152351 (Remove `SubdiagMessage` in favour of the identical `DiagMessage`) - #152417 (Move the needs-drop check for `arena_cache` queries out of macro code) - #150688 (typeck: Make it clearer that `check_pat_lit` only handles literal patterns) - #152293 (Format heterogeneous try blocks) - #152355 (Update documentation of rustc_macros) - #152396 (Uplift `Predicate::allow_normalization` to `rustc_type_ir`)
Rollup merge of #152293 - ia0:try_blocks_heterogeneous, r=ytmimi Format heterogeneous try blocks The tracking issue for `try_blocks_heterogeneous` is #149488. This follows the formatting of homogeneous try blocks (feature `try_blocks`) by considering `try bikeshed <type>` to be the equivalent of `try` (in particular a single "token"). An alternative would be to permit breaking between `bikeshed` and `<type>`, but given that those 2 elements are an explicitly temporary part of the syntax, it doesn't seem worth it. There also doesn't seem to be any existing precedent breaking between a keyword and a type. It also doesn't seem to be useful in practice, given that the type itself doesn't break (which is how it works for the return type of closures) and has more chances to dominate the length in case a break is necessary. Happy to adapt anything in case this formatting is not optimal. The test is also copied from homogeneous try blocks with 2 additional test cases to demonstrate the behavior with long types. See [#t-lang > try blocks @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/try.20blocks/near/572387493) for context.
The tracking issue for
try_blocks_heterogeneousis #149488.This follows the formatting of homogeneous try blocks (feature
try_blocks) by consideringtry bikeshed <type>to be the equivalent oftry(in particular a single "token").An alternative would be to permit breaking between
bikeshedand<type>, but given that those 2 elements are an explicitly temporary part of the syntax, it doesn't seem worth it. There also doesn't seem to be any existing precedent breaking between a keyword and a type. It also doesn't seem to be useful in practice, given that the type itself doesn't break (which is how it works for the return type of closures) and has more chances to dominate the length in case a break is necessary.Happy to adapt anything in case this formatting is not optimal.
The test is also copied from homogeneous try blocks with 2 additional test cases to demonstrate the behavior with long types.
See #t-lang > try blocks @ 💬 for context.