add rustfmt support for cfg_select

[folkertdev]

tracking issue: #115585

Done here because rust-lang/rustfmt has not been synced for a while and uses a nightly version where cfg_select! is not yet a builtin macro.

r? @ytmimi

[rustbot]

Failed to set assignee to ytmimi: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

[rustbot]

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

[folkertdev]

@ytmimi seems like I can't officially make you a reviewer, so we'll have to figure out how to merge this: I can r+, or we can find someone else to just greenlight it. The changes only touch rustfmt so I think your approval should suffice.

[ytmimi]

Maybe you can't officially assign me, but I should definitely have the ability to r+ this once I've had a chance to take a look.

[calebcartwright]

I think we might want to hold off on merging this for a tad longer. The style team has been discussing possible formatting prescriptions for the macro in our weekly meetings and while we're getting closer there's still a few things we're trying to iron out - rust-lang/style-team#201

My preference would be to give that another week or two to hopefully avoid double code churn

[joshtriplett]

I think the proposed formatting looks reasonable, as far as the examples go.

@folkertdev Could you please add some examples that include very long left-hand sides in cfg_select!, that might necessitate wrapping? (It's fine if they don't currently wrap, @traviscross and I just wanted to see some examples and how they turned out with the proposed formatting.)

Please include both examples where the line is overlong, and examples where the user manually wrapped the left-hand side.

[folkertdev]

I added some formatting examples. They do in fact already format nicely.

I did just notice that comments on the branches, like below, disappear upon formatting

cfg_select! {
    // This comment will disappear when formatting. 
    any(true) => {}
}

For lazy_static! apparently formatting is just skipped if there is a comment inside anywhere? For macro_rules! I'm not really sure what happens. It just seems to work but I can't find logic that handles comments explicitly.

Does rustfmt have some way of handling this?

[traviscross]

For context, @joshtriplett and I talked about this in the style team meeting, and @folkertdev, if you're interested in working with us on it, we were interested in using this PR to explore possible solutions to the questions we had raised about how to format this, such as what Josh mentioned about long left-hand sides.

If we can work out some good rules in this PR, and use the test cases in it to verify that we're happy with the formatting of all the hard cases that we can think of, then we can write down the rules in the style guide around the behavior that's proposed and that we agree on here.

Generally, our preference, if possible, is some wrapping rule for the LHS that keeps things below the maximum column width and that we can later apply, over a style edition, to normal cfg attributes.

[folkertdev]

well, I'd like to see cfg_select! stabilized, so whatever helps that move forward, let's do that.

I added some of the long unbreakable line examples, just so we track their behavior. There we also see that comments inside of the block are dropped which is a bit strange because there we just give a tokenstream to the formatter, and I would have assumed that it handled comments.

For the simple cases I think the current behavior is what we want right? though I assume the => { isn't really taken into account when deciding whether to break or not, which might make the line exceed the line length.

[traviscross]

For the simple cases I think the current behavior is what we want right?

The trouble is that rustfmt's stability policy is that once it formats a construct, the formatting must remain byte-identical within that style edition. So whatever our behavior for the non-simple cases ends up being what we have to commit to and document in the style guide.

So it's better if we can come up with reasonable rules for the non-common cases as well, so that we can commit to reasonable behavior for those cases too. We're open to and interested in seeing proposed behavior there as part of this PR.

[folkertdev]

Right, I just meant to say that what we need is refinement, but the broad strokes work as desired.

[folkertdev]

This does not seem to have the desired effect, am I using this right?

[folkertdev]

currently this breaks when the cfg part exceeds 80 characters. I've tried to take the => { into account, but that doesn't seem to have any effect. Also when the rhs really is empty, we'd need to consider => {} for the length.

The above still has issues with comments too, which I'm not sure how to deal with.

So @ytmimi @calebcartwright a rustfmt review is probably also a good idea at this point?

[ytmimi]

I took a very brief look at this the other day and I had some suggestions for changes. I'll do my best to wrap up my review soon.