rustfmt: Format `cfg_select!` by ytmimi · Pull Request #154202 · rust-lang/rust

ytmimi · 2026-03-22T05:04:25Z

tracking issue: #115585

Implementing cfg_select! formatting here in rust-lang/rust so that the feature can get out to nightly quicker.
The previous PR (#144323) is a bit old at this point and I felt like I could simplify the implementation so I've opted to reimplement the formatting instead of building off the previous PR.

I've tried to break the PR up into logical commits and I think this would be best reviewed one commit at a time.

Previous PR:

add rustfmt support for cfg_select #144323

See also:

Formatting for cfg_select style-team#201

`cfg_select!` parsing needs to be implemented in rustfmt right now because there's no good way to call `rustc_attr_parsing::parse_cfg_select`.

The plan is to leverage `rewrite_match_body` to help with `cfg_select!` formatting.

Since macro calls can use `{}`, `()`, or `[]` delimiters I'm making this change to handle cases where users write: ```rust cfg_select!(); cfg_select![]; ```

rustbot · 2026-03-22T05:04:29Z

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

rustbot · 2026-03-22T05:04:31Z

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: rustfmt, rustfmt-contributors
rustfmt, rustfmt-contributors expanded to 6 candidates
Random selection from fee1-dead, jieyouxu

ytmimi · 2026-03-22T05:07:25Z

src/tools/rustfmt/tests/target/cfg_select.rs

+    // Original `()` delimiters
+    cfg_select!( // opening brace comment
+
+);
+    std::cfg_select!( // opening brace comment
+
+);
+    core::cfg_select!( // opening brace comment
+
+);
+
+    // Original `[]` delimiters
+    cfg_select![ // opening brace comment
+
+];
+    std::cfg_select![ // opening brace comment
+
+];
+    core::cfg_select![ // opening brace comment
+
+];


The current implementation leaves empty cfg_select! calls written with () or [] delimiters unchanged so that comments aren't accidentally removed. That's why these examples aren't properly indented.

Suggestion: could you leave this comment in the test too? (um, slightly funny for rustfmt tests because well, comments can be formatting-meaningful themselves)

ytmimi · 2026-03-22T05:08:45Z

src/tools/rustfmt/tests/target/cfg_select.rs

+// trailing comments on the last line are a little buggy and always wrap back up
+cfg_select! {
+    windows => {
+        "windows"
+    }
+    unix => {
+        "unix"
+    }
+    _ => {
+        "none"
+    } // FIXME. Prevent wrapping back up to the next line
+}
+
+cfg_select! {
+    windows => "windows",
+    unix => "unix",
+    _ => "none", // FIXME. Prevent wrapping back up to the next line
+}


A pre-existing issue with trailing comments in rustfmt. I wanted to explicitly capture the current behavior in a test case.

ytmimi · 2026-03-22T05:10:36Z

src/tools/rustfmt/tests/target/cfg_select.rs

+// Can't format cfg_select! at all with style_edition <= 2021.
+// Things can be formatted with style_edition >= 2024
+cfg_select! {
+    feature = "debug-with-rustfmt-long-long-long-long-loooooooonnnnnnnnnnnnnnnggggggffffffffffffffff" =>
+    {
+        // abc
+        println!();
+    }
+    feature = "debug-with-rustfmt-long-long-long-long-loooooooonnnnnnnnnnnnnnnggggggffffffffffffffff" =>
+    {
+        // abc
+    }
+    all(anything(
+        "some other long long long long long thing long long long long long long long long long long long",
+        feature = "debug-with-rustfmt-long-long-long-long-loooooooonnnnnnnnnnnnnnnggggggffffffffffffffff"
+    )) => {
+        let x = 7;
+    }
+}


Through some testing I found that in style_edition <= 2021 this example won't be formatted at all because the predicate can't be formatted within max_width, but with style_edition >= 2024 that's not an issue.

ytmimi · 2026-03-22T05:17:06Z

src/tools/rustfmt/tests/target/cfg_select.rs

+// comments within the predicate are fine with style_edition=2024+
+cfg_select! {
+    any(
+        true, /* comment */
+        true, true, // true,
+        true,
+    ) => {}
+
+    not(
+        false // comment
+    ) => {}
+
+    any(
+        false // comment
+    ) => "any",
+}


Not the best place for a comment to begin with IMO, but I wanted to call out that comments within the predicate aren't correctly handled in style_edition <= 2021. This is how the current example is formatted:

cfg_select! { any( true, /* comment */ true, true, // true, true, ) => {} not(false // comment) => {} any(false // comment) => "any", }

Clearly that's wrong and produces invalid code, but fixing this seems like it's outisde the scope of this PR. since it only impacts style_edition <= 2021.

Maybe this is something we can address later?

I agree, I think we shouldn't try to bundle that in this PR. We probably should open an issue in rustfmt/r-l/r to track this as a known-bug though.

folkertdev

Extremely happy to see this!

View changes since this review

src/tools/rustfmt/tests/source/cfg_select.rs

jieyouxu · 2026-03-23T06:10:57Z

(I'll gradually review this; need to build up some background ctx first.)

jieyouxu

Thanks! The impl broadly looks good to me, just some minor nits and a few test coverage discussions.
@rustbot author

View changes since this review

jieyouxu · 2026-03-23T14:39:03Z

src/tools/rustfmt/src/parse/macros/cfg_select.rs

Question: just to check my understanding,

cfg_select! parsing needs to be implemented in rustfmt right now
because there's no good way to call rustc_attr_parsing::parse_cfg_select.

I assume this is because rustfmt can't / don't want to (transitively) depend on things beyond parsing/AST? I.e. rustc_hir?

jieyouxu · 2026-03-24T01:59:55Z

src/tools/rustfmt/src/parse/macros/cfg_select.rs

@@ -0,0 +1,108 @@
+use rustc_ast::tokenstream::TokenStream;


Suggestion: add a backlink to the reference re. cfg_select!:

//! See <https://doc.rust-lang.org/nightly/reference/conditional-compilation.html#the-cfg_select-macro> for grammar.

jieyouxu · 2026-03-30T02:20:11Z

src/tools/rustfmt/src/parse/macros/cfg_select.rs

+pub(crate) enum CfgSelectFormatPredicate {
+    Cfg(ast::MetaItemInner),
+    Wildcard(Span),
+}


Suggestion: maybe leave a quick TL;DR example for this:

/// LHS predicate of a `cfg_select!` arm. pub(crate) enum CfgSelectFormatPredicate { /// Example: the `unix` in `unix => {}`. Notably, outer or inner attributes are not permitted. Cfg(ast::MetaItemInner), /// `_` in `_ => {}`. Wildcard(Span), }

Re. outer/inner attributes, counter-example:

cfg_select! { #[cfg(true)] _ => { fn main() {} } }

jieyouxu · 2026-03-30T02:25:13Z

src/tools/rustfmt/src/parse/macros/cfg_select.rs

+    pub(crate) arrow: Token,
+    pub(crate) expr: Box<ast::Expr>,
+    pub(crate) trailing_comma: Option<Span>,
+}


Suggestion: maybe some comments for "visual" purposes.

/// Each `$predicate => $production` arm in `cfg_select!`. pub(crate) struct CfgSelectArm { /// The `$predicate` part. pub(crate) predicate: CfgSelectFormatPredicate, /// Span of `=>`. pub(crate) arrow: Token, /// The RHS `$production` expression. pub(crate) expr: Box<ast::Expr>, /// `cfg_select!` arms `$production`s can be optionally `,` terminated, like `match` arms. The `,` is not needed when `$production` is itself braced `{}`. pub(crate) trailing_comma: Option<Span>, }

jieyouxu · 2026-03-30T02:30:28Z

src/tools/rustfmt/src/parse/macros/cfg_select.rs

+impl Spanned for CfgSelectArm {
+    fn span(&self) -> Span {
+        self.predicate
+            .span()
+            .with_hi(if let Some(comma) = self.trailing_comma {
+                comma.hi()
+            } else {
+                self.expr.span.hi()
+            })
+    }
+}


Question: could you double-check if the following cross-crate macro expansion case

// another_crate.rs macro_rules! garbage { () => { fn main() {} } } // main.rs cfg_select! { _ => { garbage!(); } }

produces a span that can be used for formatting?

jieyouxu · 2026-03-30T02:48:31Z

src/tools/rustfmt/tests/target/cfg_select.rs

+// comments within the predicate are fine with style_edition=2024+
+cfg_select! {
+    any(
+        true, /* comment */
+        true, true, // true,
+        true,
+    ) => {}
+
+    not(
+        false // comment
+    ) => {}
+
+    any(
+        false // comment
+    ) => "any",
+}


I agree, I think we shouldn't try to bundle that in this PR. We probably should open an issue in rustfmt/r-l/r to track this as a known-bug though.

jieyouxu · 2026-03-30T02:49:26Z

src/tools/rustfmt/tests/target/cfg_select.rs

+// comments before and after the `=>` get dropped right now
+cfg_select! {
+    any(true, true, true, true,) => {}
+
+    not(false) => {}
+
+    any(false) => "any",
+}


Question: this should also a "known bug" right? Should we also include a follow-up issue to track this?

jieyouxu · 2026-03-30T02:53:51Z

src/tools/rustfmt/tests/target/cfg_select.rs

+    // Original `()` delimiters
+    cfg_select!( // opening brace comment
+
+);
+    std::cfg_select!( // opening brace comment
+
+);
+    core::cfg_select!( // opening brace comment
+
+);
+
+    // Original `[]` delimiters
+    cfg_select![ // opening brace comment
+
+];
+    std::cfg_select![ // opening brace comment
+
+];
+    core::cfg_select![ // opening brace comment
+
+];


Suggestion: could you leave this comment in the test too? (um, slightly funny for rustfmt tests because well, comments can be formatting-meaningful themselves)

jieyouxu · 2026-03-30T02:59:42Z

src/tools/rustfmt/tests/source/cfg_select.rs

Suggestion: there's two cases of additional test coverage that we may want to add:

Max-width and when to prefer , versus {}.
This was originally discussed in cfg_select: Support unbraced expressions #145233 where it was decided that we also permit unbraced RHS productions when the RHS is an expression, that <expr>, versus {...} is picked based on max width IIUC. Should we add a test cases for non-default max-width versus default max-width and its influence on whether , or {} is picked?

Nested cfg_select!. For better or worse, you can also write
cfg_select! { _ => cfg_select! [ _ => { // I don't know why you would write this, // but you can, so... 🤷 } ] }

jieyouxu · 2026-03-30T03:01:56Z

src/tools/rustfmt/tests/source/cfg_select.rs

+        (cfg_select! {
+            unix => Some(n),
+            _ => None,
+        }) => true,


Suggestion: should we add some coverage for how certain inline comments are handled?

match x { (/* 1 */ cfg_select! /* 2 */ { unix => Some(n), _ => None, } /* 3 */) => true, }

I'm less fussed about them, since I know certain inline comments gets dropped (long-standing known issues). But might still be good to have coverage to detect changes in case a fix is attempted.

rustbot · 2026-03-30T03:04:15Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

jieyouxu · 2026-03-30T08:30:16Z

src/tools/rustfmt/src/parse/macros/cfg_select.rs

Backlinks / context for myself (and future travellers):

Metadata

Library feature: cfg_select

std docs: https://doc.rust-lang.org/nightly/std/macro.cfg_select.html

Tracking issue: Tracking issue for cfg_select (formerly cfg_match) #115585

Style team discussion: Formatting for cfg_select style-team#201

Reference update PR: cfg_select! macro reference#2103

History

Initial style team discussions

The arms must be wrapped in braces, so rustfmt will have to ensure to not remove those.

Follow-up: unbraced expressions are permitted

PR: #145233

ytmimi added 4 commits March 21, 2026 23:42

feat: parse cfg_select! within rustfmt

cd3c8cc

`cfg_select!` parsing needs to be implemented in rustfmt right now because there's no good way to call `rustc_attr_parsing::parse_cfg_select`.

chore: make rewrite_match_body pub(crate) within rustfmt

2e45542

The plan is to leverage `rewrite_match_body` to help with `cfg_select!` formatting.

refactor: handle writing empty macro calls with () or [] delimiters

dd583bf

Since macro calls can use `{}`, `()`, or `[]` delimiters I'm making this change to handle cases where users write: ```rust cfg_select!(); cfg_select![]; ```

feat: implement cfg_select! formatting in rustfmt

15f3856

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue. labels Mar 22, 2026

rustbot assigned jieyouxu Mar 22, 2026

ytmimi commented Mar 22, 2026

View reviewed changes

folkertdev mentioned this pull request Mar 22, 2026

add rustfmt support for cfg_select #144323

Closed

folkertdev reviewed Mar 22, 2026

View reviewed changes

src/tools/rustfmt/tests/source/cfg_select.rs Show resolved Hide resolved

test: Add more cfg_select! test cases for other expansion sites

c267330

jieyouxu reviewed Mar 30, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is 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 Mar 30, 2026

jieyouxu reviewed Mar 30, 2026

View reviewed changes

Uh oh!

Conversation

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

Uh oh!

Uh oh!

rustbot commented Mar 22, 2026

Uh oh!

rustbot commented Mar 22, 2026

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!

ytmimi Mar 22, 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!

folkertdev 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!

Uh oh!

jieyouxu commented Mar 23, 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!

jieyouxu Mar 30, 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!

jieyouxu Mar 30, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu Mar 30, 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!

jieyouxu Mar 30, 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!

rustbot commented Mar 30, 2026

Uh oh!

Choose a reason for hiding this comment

Metadata

History

Initial style team discussions

Follow-up: unbraced expressions are permitted

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ytmimi commented Mar 22, 2026 •

edited by rustbot

Loading

ytmimi Mar 22, 2026 •

edited

Loading

folkertdev left a comment •

edited by rustbot

Loading

jieyouxu left a comment •

edited by rustbot

Loading

jieyouxu Mar 30, 2026 •

edited

Loading

jieyouxu Mar 30, 2026 •

edited

Loading

jieyouxu Mar 30, 2026 •

edited

Loading

jieyouxu Mar 30, 2026 •

edited

Loading