Add unstable cfg_if! macro to libcore #59443
Conversation
rust-highfive
added
the
S-waiting-on-review
| /// | ||
| /// # fn main() {} | ||
| /// ``` | ||
| #[macro_export(local_inner_macros)] |
|
Thanks @gnzlbg! I’m in favor of this PR, and there’s precedent for accepting unstable APIs without an RFC. However this adds a new macro to the libcore prelude, so it deserves special attention from @rust-lang/libs. In any case, we’ll at least do a FCP before stabilizing. Given there there is no concrete proposal so far for stabilizing
On the contrary I think we can reduce maintenance burden by having fewer copies of the macro. Could you add commits to this PR to remove the ones in It’d also be nice to either add a new test or point to an existing test to show that when an unstable macro is in the prelude, it can be shadowed by a stable macro with the same name from another crate without the crate using the macro opting into any feature gate. (The case relevant here is using crates.io’s Finally, it looks like the multiple existing implementations of this macro are not identical. Could you say if the differences are significant, and why pick this one? |
Is that also ok if I do this once this lands in the next bootstrap compiler? Otherwise one probably still needs to keep the older macros around, at least for bootstraping right ? |
Older ones are probably copy-pastes from older versions of the cfg-if crate (at least that's the case for the libstd and stdsimd ones). I am not sure what bugs the different releases of cfg-if have fixed, but this implementation is the one from its latest release. |
I'll add such a test to this PR :) |
I think not: all crates in the repo are compiled against libcore in the repo. It’s only standard library crates, in stage 0, that are compiled against the bootstrap compiler. So they may need |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@SimonSapin should be correct in that all the existing @SimonSapin as to the litany of definitions most of them probably stem from me as I've copied it to various places over time as I got fed up with the jungle of
Each of the macros may represent some snapshot in time amongst all those, but the general gist is that it's basically been the same over time, the surface syntax never changing except for a final |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
☔ The latest upstream changes (presumably #59478) made this pull request unmergeable. Please resolve the merge conflicts. |
|
I would argue that a cfg_match! {
#[cfg(unix)] => { fn foo() { /* unix specific functionality */ } },
#[cfg(target_pointer_width = "32")] => { fn foo() { /* non-unix, 32-bit functionality */ } },
_ => { fn foo() { /* fallback implementation */ } }
} |
|
@eaglgenes101 Oooh! I had thought of suggesting that also. Cool. One thing I dislike about |
|
@eaglgenes101 is there a crate implementing such a macro? How many crates
are using it?
Note that this PR does not stabilize anything, it just exposes to users
something that a big part of the ecosystem is using. There will be an FCP /
mini-RFC before stabilization, and until that happens there is enough time
for any interested party to come up with a better solution, and convince
the ecosystem to use it. Adding a cfg match later on is not incompatible
with adding cfg if before that happens.
|
|
Regarding the const PLATFORM_NAME: &str = cfg_match! {
#[cfg(unix)] => "*nix",
#[cfg(windows)] => "Win",
_ => compile_error!("Unsupported platform!"),
};That would reduce sooo much duplication and risk for mistakes by not having to repeat |
|
ping from triage @gnzlbg you have failing tests to resolve. Any updates? |
Dylan-DPC-zz
added
S-waiting-on-author
jonas-schievink
added
A-macros
|
Ping from triage @gnzlbg, this needs merge conflicts to be resolved. I'm not sure what the status of this is – does this still need an RFC/FCP, or is |
|
I've opened #61720 to separate out the question of adding an unstable macro from the internal refactorings done here. |
That question is still unresolved. |
|
Also, resolving this question would require for somebody to implement a So this is kind of blocked on whether we can improve the macro system to support |
jonas-schievink
added
S-blocked
|
@rust-lang/libs should decide what to do here. Should this be closed until either someone builds a good |
jonas-schievink
added
S-waiting-on-team
|
I would personally prefer that we not take this route. I don't think a libs team decision should be guided by internal implementation details, and for example there are now currently no duplications of the |
|
Does |
|
It does not get it because libcore doesn't have a definition. It also doesn't currently use it. |
|
Indeed, only std_detect needs it but that can be fetched from crates.io. |
FWIW, there have been at least a few occasions where I would have used (I'm not saying this should necessarily change anything here, but this is one of those things that falls into the category of "really nice if it's available, but maybe not worth the trouble of bringing in another dep in less complex scenarios.") |
|
@BurntSushi When I opened this the reason to not go through an RFC was that the design here was small and constrained enough that a tracking issue / FCP might be enough. Some people have raised the issue that a |
This PR adds the
cfg_if!macro to libcore and exposes it behind thecfg_iffeature-gate.Is this macro useful ? It has >140 reverse dependencies on crates.io.
Pros of exposing it from
libcore: inlibcorewe can't use the crates.io version unless we stabilize theno_corefeature, which is something we might not want to do. The macro is already there - in fact, it's there twice (once in internal_macros, once as part of core::arch) - so we might as well expose it once, so that everybody can just easily use it. Note that even though libstd can technically usecfg_iffrom crates.io, to minimize external dependencies, the macro is copy-pasted another 3-4 times (std_arch, std_detect, hashbrown, libstd-itself, ...).Cons of exposing it from
libcore: extra maintenance burden, if we were to stabilizeno_core,libcorecould just use a dependency from crates.io, the cost of copy-pasting it N times and maintaining the copies is less work than exposing it from libcore. Also, it might well be that some day we might want to solve this problem using language features (e.g. allowing people to programatically create their owncfgs, etc.). If that ever happens, we can just deprecate it.r? @SimonSapin cc @rust-lang/libs