[cfg_match] Adjust syntax

[c410-f3r]

A year has passed since the creation of #115585 and the feature, as expected, is not moving forward. Let's change that.

This PR proposes changing the arm's syntax from cfg(SOME_CONDITION) => { ... } to SOME_CODITION => {}.

match_cfg! {
   unix => {
        fn foo() { /* unix specific functionality */ }
    }
    target_pointer_width = "32" => {
        fn foo() { /* non-unix, 32-bit functionality */ }
    }
    _ => {
        fn foo() { /* fallback implementation */ }
    }
}

Why? Because after several manual migrations in #116342 it became clear, at least for me, that cfg prefixes are unnecessary, verbose and redundant.

Again, everything is just a proposal to move things forward. If the shown syntax isn't ideal, feel free to close this PR or suggest other alternatives.

[rustbot]

r? @davidtwco

rustbot has assigned @davidtwco.
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

[c410-f3r]

@rustbot modify labels: +T-libs-api -T-compiler

[davidtwco]

r? libs-api

[danielhenrymantilla]

Let's take this opportunity to simplify the implementations significantly:

Suggested change

	pub macro cfg_match {
	// with a final wildcard
	(
	$(($initial_meta:meta) => { $($initial_tokens:tt)* })+
	_ => { $($extra_tokens:tt)* }
	) => {
	cfg_match! {
	@__items ();
	$((($initial_meta) ($($initial_tokens)*)),)+
	(() ($($extra_tokens)*)),
	}
	},
	// without a final wildcard
	(
	$(($extra_meta:meta) => { $($extra_tokens:tt)* })*
	) => {
	cfg_match! {
	@__items ();
	$((($extra_meta) ($($extra_tokens)*)),)*
	}
	},
	// Internal and recursive macro to emit all the items
	//
	// Collects all the previous cfgs in a list at the beginning, so they can be
	// negated. After the semicolon is all the remaining items.
	(@__items ($($_:meta,)*);) => {},
	(
	@__items ($($no:meta,)*);
	(($($yes:meta)?) ($($tokens:tt)*)),
	$($rest:tt,)*
	) => {
	// Emit all items within one block, applying an appropriate #[cfg]. The
	// #[cfg] will require all `$yes` matchers specified and must also negate
	// all previous matchers.
	#[cfg(all(
	$($yes,)?
	not(any($($no),*))
	))]
	cfg_match! { @__identity $($tokens)* }
	// Recurse to emit all other items in `$rest`, and when we do so add all
	// our `$yes` matchers to the list of `$no` matchers as future emissions
	// will have to negate everything we just matched as well.
	cfg_match! {
	@__items ($($no,)* $($yes,)?);
	$($rest,)*
	}
	},
	// Internal macro to make __apply work out right for different match types,
	// because of how macros match/expand stuff.
	(@__identity $($tokens:tt)*) => {
	$($tokens)*
	}
	pub macro cfg_match {
	(
	_ => { $($output:tt)* }
	) => {
	$($output)*
	},
	(
	$cfg:meta => $output:tt
	$($( $rest:tt )+)?
	) => {
	#[cfg($cfg)]
	cfg_match! { _ => $output }
	$(
	#[cfg(not($cfg))]
	cfg_match! { $($rest)+ }
	)?
	},

• This is also suggesting that the wrapping parens be removed as well, at this point:

  cfg_match! {
      windows => { ... }
      feature = "foo" => { ... }
  }

---

Optional extra features

Expanding to expressions

A third rule could also be added, btw:

({ $($tt:tt)* }) => {{
    cfg_match! { $($tt)* }
}};

This would make it so cfg_match!({ ... }) would be expanding in the context of a { ... } block, so that the =>-rhs of the invocation body would then be able to expand to expressions anywhere (rather than just in end-of-block position):

let os_suffix = cfg_match!({
    windows => {
        " XP"
    }
    _ => {
        ""
    }
});

Consistency with macro rules

Requiring , after the => { ... }, like macro arms

By changing

    (
        $cfg:meta => $output:tt
        $($( $rest:tt )+)?
    ) => {

into:

    (
        $cfg:meta => $output:tt
        $(, $($( $rest:tt )+)?)?
    ) => {

• (and slap a $(,)? after the _ rule, see below)

Accepting => ( ... ) (requiring previous section)

(to avoid falling into the typical trap of writing => { print!(...); 42 }, like it happens with macros too)

Just add an alternative _ rule:

    (
        _ => { $($output:tt)* } $(,)?
    ) => {
        $($output)*
    },

+   (
+       _ => ( $($output:tt)* ) $(,)?
+   ) => {
+       $($output)*
+   },

[c410-f3r]

Wow, you are always impressing me, @danielhenrymantilla! Thank you very much!

The suggested implementation is indeed really elegant.

cfg_match! {
    windows => { let _ = 1; }
    feature = "foo" => { let _ = 2; }
    unix => { let _ = 3; }
    _ => { let _ = 4; }
}

The current algorithm roughly expands the above snippet into a combination of different configuration targets.

#[cfg(all(windows, not(any(feature = "foo", unix))))]
cfg_match! { @__identity let _ = 1; }

#[cfg(all(feature = "foo", not(any(windows, unix))))]
cfg_match! { @__identity let _ = 2; }

#[cfg(all(unix, not(any(windows, feature = "foo"))))]
cfg_match! { @__identity let _ = 3; }

#[cfg(all(not(any(windows, feature = "foo", unix))))]
cfg_match! { @__identity let _ = 4; }

But now everything expands into a nested exclusionary structure.

cfg_match! {
    #[cfg(windows)]
    let _ = 1;
    
    #[cfg(not(windows))]
    cfg_match! {
        #[cfg(feature = "foo")]
        let _ = 2;
    
        #[cfg(not(feature = "foo"))]
        cfg_match! {
            #[cfg(unix)]
            let _ = 3;
        
            #[cfg(not(unix))]
            cfg_match! {
                let _ = 4;
            }
        }
    }
}

I am just leaving the comma (,) and parentheses => ( ... ) matters on hold until more individuals express their options about possible further inclusions or modifications regarding the syntax, if any.

[joshtriplett]

👍 for the change, conceptually. Either way we're going to want to teach rustdoc some way to understand it and present the alternatives, but that can come later.

[c410-f3r]

Looks like that's it then! Let's move forward with everything.

[c410-f3r]

r? libs-api

[joshtriplett]

This looks great. Let's ship it.

[joshtriplett]

Part of me is tempted to add one extra macro rule for the old syntax, and then not add all the duplicated cfg(bootstrap)/cfg(not(bootstrap)) blocks for every usage. Then, once the new implementation ships in the bootstrap, we could just convert the macro invocations over without duplicating them for bootstrap. And then, once that conversion ships, we could remove the macro rule for the old syntax.

That'd avoid the duplication, but it'd require making the change over the course of several bootstraps, and I'm not sure that's worth it.

I'm fine with the current approach. r=me when ready, with or without the proposed refactor and ,/(...) handling.

[c410-f3r]

Thank you for the review, @joshtriplett.

For what it is worth, my personal preference falls under the current duplicated strategy because of it is already in place 🦥

[joshtriplett]

Thank you for the review, @joshtriplett.

For what it is worth, my personal preference falls under the current duplicated strategy because of it is already in place 🦥

That was what I was suggesting. I don't think it's worth the additional delay to avoid the duplication.