replace cfg_if with cfg_select#1224
Conversation
abef64d to
31375b6
Compare
There was a problem hiding this comment.
Thank you for the interest, but I don't think there is any need to do this. We can switch to cfg_select as minimum versions across the repo are raised, but the status quo works sufficiently well that there's no reason for cfg_select to be the driving factor.
415dc77 to
fcf6ccc
Compare
|
@tgross35 this now no longer needs an msrv bump due to a local impl, but should make switching to the real cfg_select trivial when msrv reaches 1.95+. What do you think? |
|
This looks doable, thanks! I'm going to hold off until the patch for the formatting bug lands. |
|
What formatting bug? rust-lang/rust#158371 is specific to the builtin |
|
I thought this used the real |
| /// `libm` cannot have dependencies, so this is vendored directly from the `cfg-if` crate | ||
| /// (with some comments stripped for compactness). | ||
| macro_rules! cfg_if { | ||
| ( | ||
| if #[cfg( $($i_meta:tt)+ )] { $( $i_tokens:tt )* } | ||
| $( | ||
| else if #[cfg( $($ei_meta:tt)+ )] { $( $ei_tokens:tt )* } | ||
| )* | ||
| $( | ||
| else { $( $e_tokens:tt )* } | ||
| )? | ||
| ) => { | ||
| cfg_if! { | ||
| @__items () ; | ||
| (( $($i_meta)+ ) ( $( $i_tokens )* )), | ||
| $( | ||
| (( $($ei_meta)+ ) ( $( $ei_tokens )* )), | ||
| )* | ||
| $( | ||
| (() ( $( $e_tokens )* )), | ||
| )? | ||
| } | ||
| #[macro_export] | ||
| macro_rules! cfg_select { | ||
| ({ $($tt:tt)* }) => {{ |
There was a problem hiding this comment.
Could you add a note about why we have this? Can just be a // FIXME(msrv):
fcf6ccc to
da1c6f8
Compare
|
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. |
Now that cfg_select is stable, we can replace cfg_if. One difference is that cfg_select needs one branch to match. Therefore an empty default branch is sometimes needed.
EDIT: now using a local impl of cfg_select (thanks @folkertdev [1]) so as not to have to increase msrv.
[1]:#t-libs > compiler-builtins msrv policy? @ 💬