fix: prevent rustfmt from removing necessary blocks in `cfg_select!` arms by ytmimi · Pull Request #7183 · rust-lang/rustfmt

ytmimi · 2026-10-14T18:48:53Z

I did not use an LLM to create a change in this PR.
I used an LLM to create a change in this PR, and I have explained below how it was used.

jieyouxu

Thanks. Unfortunately as we know from

the "how many braces" problem is quite quirky. The heuristics here is not sufficiently precise (i.e. currently we would try to format more than we actually can).

Disclaimer: I reviewed this in a first pass, and then co-reviewed it again with an LLM, which came up with some adversarial cases I didn't initially consider (that caught me off guard as well). I double-checked the cases/reasoning. All communications are hand-written.

View changes since this review

ytmimi · 2026-10-15T03:51:01Z

the "how many braces" problem is quite quirky. The heuristics here is not sufficiently precise (i.e. currently we would try to format more than we actually can).

Could you elaborate with a concrete example? I'm not sure that I follow.

jieyouxu · 2026-10-15T04:43:40Z

Could you elaborate with a concrete example? I'm not sure that I follow.

Sorry, to clarify, I specifically mean

fix: prevent rustfmt from removing necessary blocks in cfg_select! arms #7183 (comment), and
and that the adding brace apparently can change drop order situation.

(I know that this PR is only about not removing when we can't, not adding.)

jieyouxu · 2026-10-16T05:19:54Z

Can we add snapshot test cases for

cfg_select! { _ => { // l1 { x } // l2 } }

and

cfg_select! { _ => /* comment */ {{ x }} }

I don't think this is blocking though (snapshot moreso to capture if that gets fixed in a follow-up).

I think we might be able to check the spans between

=> { { inner_stmts } } arrow_span outer_lbrace_span inner_lbrace_span inner_stmts[0].span ... inner_rbrace_span outer_rbrace_span

i.e. if there are comments in any of the following intervals

arrow_span.between(outer_lbrace_span)

outer_lbrace_span.between(inner_lbrace_span)

inner_rbrace_span.between(outer_rbrace_span)

outer_rbrace_span to end of body (?)

we don't take the "happy" {{}} path. EDIT: maybe only between outer bracee and inner braces?

View changes since the review

Tracked unexpectedly removed comments as follow-up in #7191.

jieyouxu · 2026-10-16T05:22:24Z

+                    let trailing_comma = if arm.trailing_comma.is_some() {
+                        ","
+                    } else {
+                        ""
+                    };


Should/does this need to account for match_block_trailing_comma? Since we add this unconditionally for this "happy" case but conditionally wrt. match_block_trailing_comma for the None branch via rewrite_match_body?

rustfmt/src/matches.rs

Lines 476 to 480 in 1738c93

let comma = if context.config.match_block_trailing_comma() {

","

} else {

""

};

View changes since the review

^ Only if you want to address this in this PR. I don't want to block on this, fine as a follow-up as well.

Tracked as follow-up #7191.

jieyouxu · 2026-10-16T05:56:51Z

(Sorry, I know this is a lot of back-and-forth, didn't notice some things in earlier review passes)

ytmimi · 2026-10-16T14:03:24Z

(Sorry, I know this is a lot of back-and-forth, didn't notice some things in earlier review passes)

@jieyouxu I genuinely appreciate it. You're helping me catch a lot of things I missed too. I felt a real sense of urgency to get this fixed after learning that this was impacting miri, and I definitely jumped in without considering a lot of these edge cases.

jieyouxu · 2026-10-16T14:06:08Z

Yeah no worries, it's very subtle and I would've made exactly the same mistakes... There's just, edge cases for edge cases.

…arms Fixes rustfmt 7045

rustbot · 2026-10-16T17:33:44Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

ytmimi · 2026-10-16T17:34:22Z

Okay, added more edge case tests based on the discussion here and a few other things I wanted to check.

…nner Also adds some extra test cases to make sure that rustfmt isn't adding / removing braces that would change the code semantics.

jieyouxu

Final round of reviews I promise

View changes since this review

jieyouxu · 2026-10-17T02:24:12Z

+fn cfg_select_comments_that_might_get_lost() {
+    cfg_select! {
+        _ => {{ x }}
+    }
+    cfg_select! {
+        _ => {{ x }}
+    }
+    cfg_select! {
+        _ => {{ x }}
+    }
+
+    cfg_select! {


Yeah, this is unfortunate but reasonable to address this in a follow-up.

Tracked as #7191.

jieyouxu · 2026-10-17T02:26:58Z

+                    let trailing_comma = if arm.trailing_comma.is_some() {
+                        ","
+                    } else {
+                        ""
+                    };


^ Only if you want to address this in this PR. I don't want to block on this, fine as a follow-up as well.

jieyouxu · 2026-10-17T02:32:00Z

+        _ => {
+            (///
+            {})
+        }


Interesting, we actually add a brace here.

Tracked as #7191.

jieyouxu · 2026-10-17T02:36:13Z

+fn fails() {
+    let x = cfg_select! {
+        // rustfmt will add braces here
+        _ =>
+        {
+            #[cfg(true)]
+            1
+        }
+    };
+}


This one seems quite surprising, but still I'm fine as a follow-up since it only affects unstable language w/ stmt_expr_attributes. The brace adding here is a bit funny, because

#![feature(stmt_expr_attributes)] fn main() { let x = cfg_select! { // rustfmt will add braces here _ => #[cfg(true)] 1 }; }

is invalid syntax, but

#![feature(stmt_expr_attributes)] fn main() { let x = cfg_select! { // rustfmt will add braces here _ => { #[cfg(true)] 1 } }; }

now compiles 😁 So this is a invalid syntax => compile pass. Program repair gaming.

Considering this is fairly niche it's probably best as a follow-up.

Tracked as #7191.

jieyouxu · 2026-10-17T02:45:50Z

+                    let body = expr
+                        .rewrite_result(context, nested_shape)
+                        .map(|rw| format!("{{{rw}}}"))?;


I think we forgot to reserve the RHS } and trailing , here. For https://github.com/rust-lang/rustfmt/pull/7183/changes#r4291168609 and this, I think the nested_shape can be something to the effect of

let nested_shape = nested_shape. .offset_left(last_line_width(&predicate_str) + " => {".len(), arm.span()) .sub_width("}".len() + trailing_comma.len(), arm.span())?;

Re. the predicate_str.len(), that doesn't yet use last_line_width so something like

let a = cfg_select! { all(target_os = "linux", target_arch = "x86_64", target_env = "gnu", feature = "hello", feature = "world") => {{ x }}, };

will give up.

Some cases we may want to add:

Source

fn with_trailing_comma_exceeds_max_width() { cfg_select! { // max_width = 100 ------------------------------------------------------------------------| _ => {{ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx }}, } } fn with_trailing_comma_exceeds_max_width() { cfg_select! { // max_width = 100 ------------------------------------------------------------------------| _ => {{ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx }}, } } fn without_trailing_comma_fits_max_width() { cfg_select! { // max_width = 100 ------------------------------------------------------------------------| _ => {{ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx }} } } fn without_trailing_comma_exceeds_max_width() { cfg_select! { // max_width = 100 ------------------------------------------------------------------------| _ => {{ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx }} } }

Which should probably format into, target

fn with_trailing_comma_fits_max_width() { cfg_select! { // max_width = 100 ------------------------------------------------------------------------| _ => {{ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx }}, } } fn with_trailing_comma_exceeds_max_width() { cfg_select! { // max_width = 100 ------------------------------------------------------------------------| _ => {{ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx }}, } } fn without_trailing_comma_fits_max_width() { cfg_select! { // max_width = 100 ------------------------------------------------------------------------| _ => {{ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx }} } } fn without_trailing_comma_exceeds_max_width() { cfg_select! { // max_width = 100 ------------------------------------------------------------------------| _ => {{ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx }} } }

This is a bit pedantic so I'm also okay to defer this as follow-up.

ytmimi · 2026-10-17T03:23:41Z

@jieyouxu I'll address the wrapping behavior / last_line_width change tomorrow and add some more test cases around that. I appreciate the additional suggestions for those test cases 🙏🏼

rustbot added the S-waiting-on-review Status: awaiting review from the assignee but also interested parties. label Oct 14, 2026

ytmimi assigned jieyouxu Oct 14, 2026

ytmimi mentioned this pull request Oct 14, 2026

cfg_select! in expr position broken after formatting #7045

Open

ytmimi force-pushed the issue_7045 branch from 343c113 to 2954e37 Compare October 14, 2026 20:59

jieyouxu reviewed Oct 15, 2026

View reviewed changes

Comment thread tests/source/issue_7045.rs Outdated

Comment thread src/parse/macros/cfg_select.rs

Comment thread src/parse/macros/cfg_select.rs

Comment thread src/macros.rs

Comment thread tests/source/issue_7045.rs

Comment thread tests/source/issue_7045.rs

Comment thread src/macros.rs Outdated

jieyouxu reviewed Oct 15, 2026

View reviewed changes

Comment thread tests/source/issue_7045.rs

ytmimi force-pushed the issue_7045 branch from 2954e37 to 4662c47 Compare October 15, 2026 16:58

ytmimi commented Oct 15, 2026

View reviewed changes

Comment thread src/macros.rs

ytmimi added S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: awaiting review from the assignee but also interested parties. labels Oct 16, 2026

ytmimi force-pushed the issue_7045 branch from 4662c47 to dffce89 Compare October 16, 2026 02:16

This comment has been minimized.

Sign in to view

jieyouxu reviewed Oct 16, 2026

View reviewed changes

Comment thread src/parse/macros/cfg_select.rs Outdated

Comment thread src/parse/macros/cfg_select.rs

Comment thread src/parse/macros/cfg_select.rs

jieyouxu reviewed Oct 16, 2026

View reviewed changes

Comment thread tests/source/cfg_select_edge_cases.rs Outdated

fix: prevent rustfmt from removing necessary blocks in cfg_select! …

3ffc7e3

…arms Fixes rustfmt 7045

ytmimi force-pushed the issue_7045 branch from dffce89 to 4fd2d6f Compare October 16, 2026 17:33

fix: Add some extra heuristic checks for CfgSelectArm::double_block_i…

49c8c2f

…nner Also adds some extra test cases to make sure that rustfmt isn't adding / removing braces that would change the code semantics.

ytmimi force-pushed the issue_7045 branch from 4fd2d6f to 49c8c2f Compare October 16, 2026 18:53

jieyouxu added S-waiting-on-review Status: awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. labels Oct 17, 2026

jieyouxu reviewed Oct 17, 2026

View reviewed changes

jieyouxu mentioned this pull request Oct 17, 2026

cfg_select! follow-ups to #7183 #7191

Open

	let comma = if context.config.match_block_trailing_comma() {
	","
	} else {
	""
	};

Conversation

ytmimi commented Oct 14, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jieyouxu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ytmimi commented Oct 15, 2026

Uh oh!

jieyouxu commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jieyouxu Oct 16, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu Oct 16, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jieyouxu commented Oct 16, 2026

Uh oh!

ytmimi commented Oct 16, 2026

Uh oh!

jieyouxu commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 16, 2026

Uh oh!

ytmimi commented Oct 16, 2026

Uh oh!

jieyouxu left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ytmimi commented Oct 14, 2026 •

edited by rustbot

Loading

jieyouxu left a comment •

edited

Loading

jieyouxu commented Oct 15, 2026 •

edited

Loading

jieyouxu Oct 16, 2026 •

edited by rustbot

Loading

jieyouxu Oct 16, 2026 •

edited by rustbot

Loading

jieyouxu commented Oct 16, 2026 •

edited

Loading

jieyouxu left a comment •

edited by rustbot

Loading

jieyouxu Oct 17, 2026 •

edited

Loading