Skip to content

Decide whether to separate cfg(version("..")) and cfg_has_version #141401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
traviscross opened this issue May 22, 2025 · 7 comments
Closed

Decide whether to separate cfg(version("..")) and cfg_has_version #141401

traviscross opened this issue May 22, 2025 · 7 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. F-cfg_version `#![feature(cfg_version)]` T-lang Relevant to the language team

Comments

@traviscross
Copy link
Contributor

traviscross commented May 22, 2025

In this stabilization,

it was observed that stabilizing cfg(version("..")) doesn't help people immediately, because they have to wait for their MSRV to exceed the first version in which cfg(version("..")) is supported. To mitigate this, a new mechanism, cfg(has_cfg_version) was proposed.

Since then, @joshtriplett has raised good points about how this mechanism would be difficult to use. See this comment:

We need to decide whether we want to consider these questions together, or whether we'd accept a stabilization of cfg(version("..")) that sets this question aside.

cc @rust-lang/lang @est31 @jieyouxu @ehuss

Tracking:

@traviscross traviscross added T-lang Relevant to the language team C-discussion Category: Discussion or questions that doesn't represent real issues. labels May 22, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 22, 2025
@traviscross
Copy link
Contributor Author

Proposal: For the reasons outlined by @joshtriplett, I propose that we separate these questions and reopen the stabilization PR for cfg(version("..")) in its original form. We can separately and later consider the question of cfg(has_cfg_version).

@rfcbot poll

@rfcbot
Copy link
Collaborator

rfcbot commented May 22, 2025

Team member @traviscross has asked teams: T-lang, for consensus on:

@jieyouxu jieyouxu added F-cfg_version `#![feature(cfg_version)]` and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 22, 2025
@traviscross
Copy link
Contributor Author

I'm going to check @scottmcm's box, since he signaled unambiguously in the meeting support for stabilizing cfg(version("..")) alone, and @joshtriplett's given his statements in the PR, but feel free to adjust of course.

@joshtriplett
Copy link
Member

joshtriplett commented May 22, 2025

FWIW, I do think there's potential value in making it easier to adopt cfg(version(...)) more quickly. I don't believe has_cfg_version as proposed solves that, due to the issues I wrote about, and I think it'll make cfg(version(...)) harder and/or more confusing to adopt (given that there will be a simple path that ignores old MSRV and a complicated path that doesn't).

In other words: please take my comments as concerns with the concrete proposal that was made, rather than any objection to the underlying value of wishing to better support the transition for people whose MSRVs are older than cfg(version(...)) support.

@traviscross traviscross added I-lang-nominated Nominated for discussion during a lang team meeting. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang labels May 22, 2025
@tmandry
Copy link
Member

tmandry commented May 28, 2025

We discussed this in the lang call today. I was hoping that cfg_has_version would be a simple and uncontroversial addition. It turned out to be simple to implement, but not entirely uncontroversial. I'm therefore willing to check my box and not block cfg(version("..")) as promised.

In tandem with the stabilization of cfg(version("..")), I plan to write up a simple RFC for cfg_has_version. My hope is to get them in as close together as possible to simplify the user story.

@rfcbot reviewed

@traviscross traviscross added I-lang-radar Items that are on lang's radar and will need eventual work or consideration. and removed I-lang-nominated Nominated for discussion during a lang team meeting. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang labels May 28, 2025
@traviscross
Copy link
Contributor Author

traviscross commented May 28, 2025

@est31: That's your cue. Once you're ready, we'd welcome a new v2 stabilization PR for cfg(version("..")) with a report that discusses the considerations related to cfg_has_version and notes that is being handled separately.

@traviscross traviscross removed the I-lang-radar Items that are on lang's radar and will need eventual work or consideration. label May 28, 2025
@est31
Copy link
Member

est31 commented May 29, 2025

Thanks. I will file a stabilization PR in the coming days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. F-cfg_version `#![feature(cfg_version)]` T-lang Relevant to the language team
Projects
None yet
Development

No branches or pull requests

7 participants