clarify match block requirement for single mac call

[calebcartwright]

cc @Aaron1011 re: rust-lang/rustfmt#4496 - think this minor clarification should cover the needed change but please let me know if you think anything additional is needed for the arm body flattening

@joshtriplett if you have time to review the text that'd be most appreciated as well!

[Aaron1011]

This isn't quite right. Both => macro_call!() and => { macro_call!() } are fine - the issue is automatically transforming one into the other.

For example, the code:

macro_rules! expr {
	() =>  { true };
}

macro_rules! stmt {
	() => { true; };
}

fn main() {
	let _val: bool = match true {
		true => expr!(),
		false => false,
	};

	match true {
		true => { stmt!() },
		false => {}
	}
}

will compile if rust-lang/rust#33953 is fixed. None of the match arms should be flattened/un-flattend, since that can change the meaning.

[calebcartwright]

@Aaron1011

Note that the Style Guide provides guidance/prescriptions and best practices for formatting in general, which rustc doesn't necessarily care about one way or the other. The compiler is fine with spaces vs. tabs and doesn't care whether a control flow expression in a match arm body is block-wrapped or not, but the Style Guide still provides explicit guidance on both fronts.

I understand that => macro_call!(), will be fine in some cases even after rust-lang/rust#33953 is fixed, but that doesn't necessarily preclude us from being able to provide guidance here (much in the same way that the compiler is fine with => if x { 1 } else { 2 }, but the Style Guide still says such a body, unrealistic as it may be, needs to be block-wrapped).

So I think the question for the Style Guide is whether it's worth prescribing block wrapping a single macro call expression as a general strategy to avoid running into parsing errors in cases where there's a trailing semi in the expansion.

If we don't want to go with something that strong, and still allow authors to write things like => println!("foo"), without rustfmt block wrapping it, that's fine too. Sure we could incorporate a general recommendation to be cognizant about block wrapping the body in cases where the expansion includes a semi which should provide sufficient cover for rustfmt to not flatten such cases.

None of the match arms should be flattened/un-flattend, since that can change the meaning.

Just to clarify, are you saying that block wrapping an arm body that has a single mac call expression could potentially fail to be semantics preserving (e.g. changing => macro_call!() to => { macro_call!() })? I can't quite see how that'd happen, though if so that'd be a separate issue we'd want to dig into as there are a few cases where rustfmt will do this today such as scenarios running up against max width with guards on the arm

[Aaron1011]

Just to clarify, are you saying that block wrapping an arm body that has a single mac call expression could potentially fail to be semantics preserving

I was thinking that it could change the code from failing to compile (if the macro expands to a let statment, for example), to compiling. However, if the code already compiles, then introducing a block won't change the behavior.

[Aaron1011]

@calebcartwright: I don't really have any opinion about what the style guide should say. As long as rust-lang/rustfmt#4496 is consistent with the style guide, I'll defer to your preferred wording.

[calebcartwright]

This actually uses more forceful wording and would require additional work on the rustfmt side to explicitly reformat all such cases with a block wrapping, whereas rust-lang/rustfmt#4496 is just going to preserve the programmers original arm body structure.

Given the additional discussion in rust-lang/rustfmt#4496 about introducing a rustc warning in such cases, I'm actually now leaning towards changing the text here to make it a bit softer. The softer text would then align with the change rust-lang/rustfmt#4496, and although rustfmt couldn't be used to automatically correct/update the problematic cases, it will defer to devs and allow them to block wrap the arm bodies in need, and keep the ones that don't.

Thoughts?

[Aaron1011]

That sounds good to me. In general, I think rustfmt should be very cautious about making changes around macro invocations/definitions. I'm happy with accepting both the wrapped and unwrapped invocation, and adding something to the style guide about the pitfalls of the unwrapped case

[Aaron1011]

@calebcartwright The new text looks good to me!

[calebcartwright]

@calebcartwright The new text looks good to me!

Awesome, thanks so much really appreciate your help on this one @Aaron1011! Fighting some indentation issues from copy/pasting out of GH but will go ahead and merge once I get that sorted and then we can proceed with rust-lang/rustfmt#4496