rust-lang/rust

Tracking issue for RFC 2523, `#[cfg(version(..))]`

Centril opened this issue Β· 110 comments

This is a tracking issue for #[cfg(version(..))] (rust-lang/rfcs#2523).

Steps:

Unresolved questions:

  • What is cfg(version(...)) relative to in terms of nightly compilers?

    We could check against what rustc --version says, e.g. nightly being 1.40.0, beta being 1.39.0, and stable being 1.38.0. We could also have cfg(version(...)) at most be relative to the beta compiler. See the RFC for a longer discussion.

  • Should we also support version = "..." so that crates having a MSRV below when version(...) was stabilized can use the flag?

  • Dependency updates cause language changes (#79010)

@csmoe Are you working on this? If not I might want to try out this task first since @petrochenkov mentioned that this task is easier than cfg(accessible = ::path).

csmoe commented

@pickfire I didn't have much bandwidth on this, seems you have already made some progress on cfg(accessible = ::path), so feel free to check this :)
(Don't forget to assign yourself with @rustbot claim)

@csmoe Nice, thanks a lot for telling me that. I did not know such thing exist.

@rustbot claim

Can anyone please help to write up the mentoring instructions. Based on what I know, this should be similar to #64797 (comment)

1. Syntax:
   
   1. Add a new `sym::accessible` in https://doc.rust-lang.org/nightly/nightly-rustc/src/syntax_pos/symbol.rs.html#22.
   2. Feature gate `accessible` in [`GATED_CFGS`](https://doc.rust-lang.org/nightly/nightly-rustc/syntax/feature_gate/builtin_attrs/constant.GATED_CFGS.html). Also add `cfg_accessible` to `active.rs`: https://doc.rust-lang.org/nightly/nightly-rustc/src/syntax/feature_gate/active.rs.html#530. This will also require a new `sym::cfg_accessible`.
   3. Introduce a match arm for `sym::accessible` in [`cfg_matches`](https://doc.rust-lang.org/nightly/nightly-rustc/syntax/attr/fn.cfg_matches.html). This should look mostly like [the case for `sym::not`](https://doc.rust-lang.org/nightly/nightly-rustc/src/syntax/attr/builtin.rs.html#591).
      Here you need to extract an [`&ast::Path`](https://doc.rust-lang.org/nightly/nightly-rustc/syntax/ast/struct.Path.html) and delegate the interpretation to a function of the rough shape `fn is_accessible(sess: &ParseSess, path: &Path) -> bool { ... }`

2. Implement `fn is_accessible`.
   
   1. First do some validation. We want to emit errors if [`!path.is_global()`](https://doc.rust-lang.org/nightly/nightly-rustc/syntax/ast/struct.Path.html#method.is_global). Use [`sess.span_diagnostic.struct_span_err`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/struct.Handler.html#method.struct_span_err).

I believed step 1 and 2 should be the same. For step 3, based on what I know requires me to modify somewhere around

MetaItemKind::List(..) => {
error(cfg.span, "unexpected parentheses after `cfg` predicate key")
}
.

Based on fn is_accessible, the next thing to be done should be to implement fn min_version and then somehow reference version_check on how to do the min_version logic by checking the CFG_VERSION environment variable?

Hi, @pickfire. Are you still working on this?

No, I was stuck working on this and don't know how to proceed.

Ok, thanks for quick response. I'll take my shot at this.
@rustbot claim

@mibac138 Do you want me to publish my progress?

@pickfire sure, that'd be great!

@mibac138 Err, no need. You have already done more than me, I don't know how to change the eval_condition. Oh, it didn't seemed that hard after looking at your PR but I didn't know how to work on that.

Status update: #71314 implemented this RFC with a slightly different syntax - #[cfg(version("1.2.3"))] instead of #[cfg(version(1.2.3))].

The reason is that 1.2.3 doesn't fit into some existing attribute-parsing infrastructure in the compiler (MetaItems), so it requires separate work.

Additionally, the version_check crate currently used to parse versions wants a string as an input, so we need to stringify 1.2.3 before passing it, which we can only do imprecisely (up to whitespaces).
So we can turn 1 . 2 .3 into e.g. "1.2.3" during stringification and it may be a problem depending on what exactly we are supposed to accept as a valid version.

The RFC had an unresolved question about how this feature would interact with nightly/beta. Should #[cfg(version("1.45.0"))] return true or false for nightly/beta 1.45.0?

Currently, the implementation returns true. E.G. for rustc 1.45.0-nightly, the following code prints Yes:

#![feature(cfg_version)]
fn main() {
    test();
}

#[cfg(version("1.45.0"))]
fn test() {
    println!("Yes")
}

#[cfg(not(version("1.45.0")))]
fn test() {
    println!("No")
}

IMO, this is a mistake. The main use-case for cfg(version), at least for me, is to allow use of new features that are known to be stable at a given version. For instance, in the core-error crate, I was planning to use cfg(version) to implement my custom core::error::Error trait on the various error structures of libcore/liballoc automatically, based on cfg(version). Unfortunately, this is not possible as it will certainly cause breakage in the in-between nightly versions that are reported as implementing a version, whilst not having all the types associated with it.

That sounds like good reasoning to me, but I've not been following this RFC that closely. @joshtriplett -- I feel like you were "liaison'ing" for this before we had a term for it, do you have a take?

Nominating for lang-team meeting to discuss

Currently, the implementation returns true

Which is most useful, the way it is, AFAICT.

Isn't there a way to combine this with cfg test(s) for "train", e.g. beta or nightly and unstable feature gates?

Isn't there a way to combine this with cfg test(s) for "train", e.g. beta or nightly and unstable feature gates?

Code written for the stable compiler should not have to know about beta or nightly.

I'm not surprised with that as a goal, but I can think of cases where for practical reasons I nead to dev on 1.45 nightly with expectation that it will then work on 1.45 stable. I'm surprised if I'm the only one.

Isn't there a way to combine this with cfg test(s) for "train", e.g. beta or nightly and unstable feature gates?

Not currently, and I don't think it'd be a good idea anyways. Rust 1.45.0-nightly can refer to many different nightlies with many different implemented features. The original RFC had an additional cfg(nightly) proposal for this reason, but it had to be dropped because it was controversial IIRC.

IMO to target nightly and beta, cfg_accessible is a better alternative when possible (e.g. for all the features that expose a type or function). Otherwise, the only way to write robust code that spans multiple nightlies would be to match on build dates.

I guess "robust code" is the important phrase in your comment, because in practice, the way it is currently implemented (what you documented above) works just fine (if not robustly) for 9 out of 10 cases where I've personally needed it.

Edit: And I should add I think it works "just fine" on the nightlies because of a somewhat non-robust assumption that future users download a nightly after whichever nightly I'm currently developing on. :)

est31 commented

First off, note that, while rarely, sometimes features are unstabilized before they get released, as problems are discovered. In this instance, downloading newer nightlies will break your compilation.

Matching on build dates isn't as robust as it seems. Custom builds of rustc master also have custom build dates that base on the commit date. If any custom patches are introduced, they affect the build date. So it's no robust piece of information to base checks on.

From what I could gather, pinning nightlies is a common thing to do in corporate environments, and some even do custom patches. In these scenarios, having 1.45-nightly count as 1.45 would be a problem, and build date based checks even more. Imagine you are adding a patch to fix a bug, only to get even more bugs because you increased the date to something very recent. One could argue that these environments likely already use nightly features so can expect breakage, but this would be breakage relating only stable features, and part of the Rust stability story is making the potential for breakage still opt-in even in nightly (that's what -Z options and #![feature] are for).

I'm not even sure whether 1.45-beta should count as 1.45, but I'm leaning towards yes, because there are two more reasons speaking in favour. First, beta has a smaller user base and it's less likely that someone is pinning betas instead of using stable. Second, beta is meant to be a staging environment and thus should help discover any bugs that newly stabilized features have. However, even here, every now and then (un-)stabilizations are being backported to beta so it can still be considered unfinished. But a) this is less common today than it used to be and b) due to people not pinning betas I don't consider this as too much of a problem.

Anyways, no matter the decision, it can always be changed in the future, in both directions. There is no compatibility hazard here unlike in many other stabilization scenarios.

TLDR: I'd advocate for never treating nightly as the release version, while treating beta and stable as it.

That sounds like good reasoning to me, but I've not been following this RFC that closely. @joshtriplett -- I feel like you were "liaison'ing" for this before we had a term for it, do you have a take?

I agree that nightly 1.45 shouldn't pass a cfg(version("1.45.0")) check. Beta should, though.

This kind of complication is a big part of why I'd like to see cfg(accessible(...)) used much more often, and cfg(version(...)) used only as a last resort.

This kind of complication is a big part of why I'd like to see cfg(accessible(...)) used much more often, and cfg(version(...)) used only as a last resort.

Yes, but cfg(accessible(...)) have some complications right now IIRC, such as not able to match all different kind of paths? Correct me if I am wrong.

I agree that nightly 1.45 shouldn't pass a cfg(version("1.45.0")) check. Beta should, though.

This is unproductive rehash/relitigation of same issues raised during RFC. Making attempts to optimize this for "maximum robustness" WRT to nightly, is likely to mean people will just keep using rustc --version in build.rs, and without any additional nightly considerations. That doesn't sound like it really matches your goals.

@dekellum Does cfg_accessible not work for your use-case? It'd help to know what you're using cfg(version) to gate your code for.

We discussed the unresolved question (as explained in #64796 (comment)) in today's lang team triage meeting.

We had general consensus on the following behavior:

Version cfg(version("1.48")) cfg(version("1.49")) cfg(version("1.50"))
1.48 stable true false false
1.49 beta true true false
1.50 nightly true true false

In other words, stable and beta compilers will both report being at least version V for any version up to and including their own version, while nightly compilers will report being at least version V for any version up to but not including their own version.

Some rationale:

  • Using features that only exist on nightly is likely to require the use of feature gates or other special cases.
  • Crates that have extra features on nightly commonly handle nightly via a separate feature flag.
  • Most crates don't have great support for nightlies older than the last stable, because they typically assume "nightly" means "latest bleeding-edge". This seems likely to at least somewhat improve that situation, while reporting the nightly version seems likely to make that worse (because crates would assume that cfg(version("1.50")) means "supports all the features of stable 1.50", which will break the nightlies leading up to 1.50).
  • The primary use case of this feature is for crates that want to support older compilers, not for crates supporting nightly (for which we don't have any good way of distinguishing between builds of nightly with the same version number).

That's lame.

That's lame.

Please be constructive.

@joshtriplett How do we teach others this? This looks like weird condition that people might not expect. I think we should keep the version for nightly with it coming with a warning when people are checking current version on nightly. We could also do this but I think a warning is better than just false which may make people go looking why does it not work.

est31 commented

@pickfire I'd explain it this way: during a nightly cycle, the features that comprise the new release are being assembled on nightly. The release is considered unfinished. The #[cfg(version)] is based on the last finished release and ignores all unfinished releases. The story of nightly is: "we don't know yet what it's going to include". The story of beta is "we know what it will include and ask for QC and testing". The story of stable is "this is the tested and stable release of Rust for wide community use".

Yes, I understand that here after reading it in the above posts, but how does someone who did not read this comments understand our reasoning? Is there anyway to let other people using the command know? Preferably without reading any docs since they may read the wrong docs or misread some tiny details like this.

So from what I will say, rather than implicitly disable it, we explicitly warn about the use of version on nightly regardless we allow/disallow the use of version on nightly, so they don't even need to read the docs for this. Still, I prefer to have both warning while allowing the use of version on nightly.

Another piece of rationale that I found convincing was that declaring that "version" is testing the most recent beta version (and not the "nightly version") enables you to do something, but declaring that version is the nightly version doesn't give any new capabilities.

  • If we say that version is testing the "beta version", then you can build crates that successfully build with any version of the compiler (including random nightly builds that may have "evolved" the feature) (the only exception is when we rollback a stabilization). Given the fact that we often make small changes in the runup to stabilization (as the feature is getting attention), this seems important.

  • If you want to incorporate (but gate) support for features that are "not yet stabilized", you can still do that via cfg-accessible, custom cfg flags, or even custom cargo features.

@pickfire Warning on cfg(version) would introduce noise for code that's trying to use cfg(version) to decide if it should use a feature or not.

This feature seems to require that rustc's versioning scheme (along with the language features supported in each version) will be part of the Rust language forever, unless alternative implementations are supposed to use their own version number for this. I don't think the RFC is sufficiently clear on what's meant by this. The reference-level explanation in the RFC is especially light on details and I would really like to see it expanded before this is stabilized.

The RFC mentions that feature-based detection is not being pursued because it would stabilize the names of features, but if the implemented "language version" is instead used it would mean that the full set of features at every version (every 6 weeks!) would need to be stabilized (and, eventually, specified). This seems significantly less elegant (and also like more work) to me than just stabilizing feature names, and it would mean that alternative Rust implementation could only add new features in lock-step with rustc (since you couldn't claim to be version X as long as you're still missing one tiny feature from that version).

In case the intention is that an implementation would just return its implementation version, regardless of which features are available, and regardless of which versioning scheme is used, then this feature looks fairly similar to user-agent detection in browsers, except with the crucial missing ability to tell apart different implementations, making the feature useless for writing Rust code that is portable between implementations.

@jonas-schievink I'm hoping that cfg(accessible) satisfies much of the demand for feature detection, and I think we may potentially provide other feature detection in the future. cfg(version) is intended as a last resort for subtle things that don't correspond to a specific feature. People can already do the same detection via build scripts, so we're not giving access to a capability people didn't already have.

cfg(version) is defined in terms of versions of rustc; other similar features, such as MSRV, are also defined in terms of versions of rustc. An alternative implementation of rustc should only advertise cfg(version) for a given version if it's entirely compatible with the features of that rustc version; if not, crates expecting such a version will break on such an alternative implementation.

Havvy commented

Rust the language and rustc the compiler have the same release cadence and release versions. They're still technically two different version numbers.

I think the answer is that this is testing the version of Rust the language, and that a new version of Rust the language is released every six weeks. That seems ok to me. That pace of evolution may slow down at some point, or it may not.

It's true that, as a result of this feature, if there is some alternative rust compiler out there (let's call it alt-rustc), it would have to know which version of "rust the language" it supports, so that it can emulate the correct set of available features. In other words, if it advertises itself as Rust 1.22, then it ought to support everything that Rust 1.22 supported.

It's not true that it would necessarily have to know or have recorded at which version each feature became available (although we do have that information in our annotations) -- but it does have to know which version of the spec it fully supports.

And of course it may offer additional features beyond those that were available in Rust 1.22, which is why cfg(accessible) remains a superior (and more convenient) option.

That sounds very reasonable.

I would like to push back on #64796 (comment); I think that is the wrong decision. 1.N nightly (any date) should have cfg(version("1.N")) as true.

Context: I maintain some widely used crates, many that support a broad range of compiler versions (back to 1.15.0, 1.13.0, a few 1.0.0), and many that have done version detection in build.rs for years, including both stable and unstable things. One example: https://github.com/serde-rs/serde/blob/v1.0.110/serde/build.rs

I found the following rationales in the discussion, none of which are a compelling justification to me:

  • Note that, while rarely, sometimes features are unstabilized before they get released, as problems are discovered. In this instance, downloading newer nightlies will break your compilation.

    But that breakage happens either way. If something is stabilized in nightly and I publish a use of it behind a version cfg using the anticipated version it's stabilized in, and then it's unstabilized, that code will break. I want it to break in nightly asap. Waiting until beta for the version cfg to tick over and it to break isn't better. Also that harms things like crater runs on the release candidates. Also ecosystem coverage of beta in CI is still not great so an author who uses only nightly and/or stable locally wouldn't even find out about the unstabilization until the stable release is out and their crate doesn't work in it because the feature was unstabilized.

  • Using features that only exist on nightly is likely to require the use of feature gates or other special cases.
    Crates that have extra features on nightly commonly handle nightly via a separate feature flag.

    Yes. That looks like #![cfg_attr(all(feature = "..", version("1.N")), feature(..))] where 1.N is the version whose nightly landed the current form of the unstable thing you are enabling.

  • Most crates don't have great support for nightlies older than the last stable, because they typically assume "nightly" means "latest bleeding-edge". This seems likely to at least somewhat improve that situation, while reporting the nightly version seems likely to make that worse (because crates would assume that cfg(version("1.50")) means "supports all the features of stable 1.50", which will break the nightlies leading up to 1.50).

    Having been involved in this discussion for 4 years (remember a crate called serde_macros which was nightly only?), providing any more than accidental support for old nightlies is a lost cause. If someone is pinning nightly and using unstable features, they need to pin their crates too. If someone is pulling in crate updates and using unstable features, they need to be prepared to pull in nightlies too. The thing that this rationale refers to as being made worse is a non-issue in my experience with my crates.


I don't see rationales in the other direction so adding one:

  • If today an exciting stabilization lands in 1.N nightly and I want my crate to be in good shape to use it as soon as 1.N is stable, how do I even develop under the proposed approach?

    • Do I fork the project and do all implementation work involving the new feature off of master for the next 3 weeks until beta, without putting version cfg anywhere, just so I can compile and test the code, then after 3 weeks when beta is out I insert all the required version cfgs everywhere and do a massive merge?
    • Or do I develop on master using a version("1.N") gate, but I need to comment out all of the feature gates locally in order to compile and test my changes, meaning I get zero coverage of the new code in CI for 3 weeks?
    • Or do I develop on master using a version("1.N-1") gate and turn off my beta build in CI for the next 3 weeks?

    The developer experience here is bad.

If someone is pinning nightly and using unstable features, they need to pin their crates too. If someone is pulling in crate updates and using unstable features, they need to be prepared to pull in nightlies too.

Note that this includes never updating stable dependencies either, if you have a pinned nightly from early on in 1.50-nightly, and a dependency of yours starts using a feature stabilized in 1.50-stable behind a cfg(version) check, that feature may have been stabilized after the nightly you are using (or worse, stabilized before it, but with a subtle change in behaviour after stabilization before beta). Currently even if you are building on a pinned nightly you can assume that updating any stable crates that guarantee a MSRV before your nightly is a non-breaking change.

est31 commented

If something is stabilized in nightly and I publish a use of it behind a version cfg using the anticipated version it's stabilized in, and then it's unstabilized, that code will break. I want it to break in nightly asap.

Pushing code you don't test for isn't really a good development practice.

If today an exciting stabilization lands in 1.N nightly and I want my crate to be in good shape to use it as soon as 1.N is stable, how do I even develop under the proposed approach?

I have wondered about this myself as well and pondered about proposing a nightly-only cargo/rustc -Zassume-complete-nightly option that enables the version(1.N) check for 1.N nightly. Generally it's beneficial to have bug testing of features on nightly as much as possible as getting fixes into beta is harder than onto master. However, ultimately I felt like it didn't have enough weight as it doesn't prevent testing per se, only enabling it via version(1.N). Ideally, a feature is tested before it's stable where version(...) doesn't help you anyways.

Given your statement though, I could totally live with a -Zassume-complete-nightly option as a compromise.

Currently people can do #[cfg(hello)] and then do cargo rustc -- --cfg hello or similar to test stuff. Once beta is cut, it won't be hard to grep for hello and replace it with version(1.N). Admittedly it would be even easier if you could just do version(1.N) right away.

The entire point of this feature is to support older compiler versions. The point of contention is whether the feature should include older nightly versions or not. I think people pin nightly as often as they pin stable, and I think we should do our best to discourage people from enabling RUSTC_BOOTSTRAP.

Thanks @dtolnay for the pushback =)

One important factor in this discussion seems to be #[cfg(accessible)]. I think there was some assumption that the "right" way to test for an individual feature being available would be to use cfg(accessible), but afaik that feature hasn't even been prototyped. I'm not sure how to think about that.

I'm going to nominate for us to rediscuss, regardless.

Actually the ideal would probably be for some smaller set of folks (including @dtolnay) to setup a meeting and hash it out.

#[cfg(accessible)] has been prototyped and is available in nightly. It is accessible as #[cfg_accessible], see the implementation PR and the tracking issue.

It apparently has some limitations, though I haven't played with it enough to know what exactly the limitations are.

Oh, great! I stand corrected!

We discussed this in last week's lang-team meeting and I was supposed to write a summary, but I didn't get to it until now. Eep. I don't think we came up with a remarkably new twist, except to note that the correct workflow for a library, in part, depends on whether you expect people to be using pinned nightly results. I don't think it was really anything that hadn't already been said in this thread, to be honest.

(One side note is that we did have an example of the sort of language feature that cfg(accessible) couldn't be used to test for, which was a const fn stabilization, such as adding the ability to use loops in a const fn.)

So, I am re-reading @dtolnay's comment and I think that this part stands out to me:

Having been involved in this discussion for 4 years (remember a crate called serde_macros which was nightly only?), providing any more than accidental support for old nightlies is a lost cause. If someone is pinning nightly and using unstable features, they need to pin their crates too. If someone is pulling in crate updates and using unstable features, they need to be prepared to pull in nightlies too. The thing that this rationale refers to as being made worse is a non-issue in my experience with my crates.

I think my feeling has shifted. In part, I am not willing to second guess @dtolnay's experience in what is the best workflow for managing library development and nightly crates and so forth.

I think I would be happy to reverse the decision, and to have #[cfg(version)] test the nightly version, but I would like to see a kind of comprehensive guide to how do to develop against nightly features. This should cover both advice for library authors and advice for folks who are using nightly -- e.g., if you're going to pin nightly, you need to coordinate that with upgrading of Cargo.lock, and so forth. It seems clear that this is not the smoothest experience right now but that's ok. I imagine it would also talk about cfg(accessible) vs cfg(version) and when you should use each one.

I feel like that guide ought to be written before we stabilize either feature. It seems like the knowledge is there and it wouldn't be that hard for us to write it. That said, while I long to volunteer my time towards doing so, I know that's a bad idea, so I'm curious if there's anybody out there who'd like to take a stab at it, and run it by e.g. @dtolnay for feedback.

Also, obviously, other folks on the @rust-lang/lang team may disagree with my assessment here, this is just me speaking for me.

Related to const fn, though admittedly tangential to this thread, is the "preferred" way to handle that just rewriting the entire method (or using something like the const_fn crate to do it for you), or is there some better way I'm not aware of?

I don't have much new to add either. I see the downsides in both solutions, but ultimately the ecosystem would adapt to either one.

  • In favor of my design, the point I find most compelling is the discussion of how to do nightly development at the bottom of #64796 (comment). #64796 (comment) brings up some ways that we could make the lang team's original design work but none of those seem like a satisfying experience to me; I fear we would give up a lot of eyes on features in the most critical window between when the stabilization lands in nightly and when the stable release ships. I don't have numbers but my sense is that features often see a spike in usage in this window as people get their crates ready for the release, and that's a big opportunity to discover last-minute blockers or polish.

  • In favor of the lang team's original design, the point I find most compelling is the ability to do "back in time" compilation where, in a world where everything uses cfg(version()) perfectly, we can take current code and compile it on an old nightly, for example to bisect bugs. Obviously I find this less compelling than the previous. The fact that the code being compiled changes as we go back versions already reduces the usefulness of this in attempting bisects. The way I'd prefer this use case to be addressed instead is via tooling which can update you to crate versions "as of" a given date.

@dtolnay @nikomatsakis What if we introduce a compiler option that causes the compiler to identify as the previous beta (or the previous stable), and introduced some kind of integration that makes it easy for people pinning a specific nightly version to use that option? That way, people working on a pinned nightly will get the behavior that doesn't break with features introduced after their pinned nightly version and before the next beta/stable, while people working on latest nightly will get the behavior they expect?

I like that.

est31 commented

Two examples of companies using something like pinned nightlies: Fuchsia , Dropbox . Admittedly though, Fuchsia seems to update versions regularly (something like once per week) and the dropbox comment is a bit old by now. Furthermore if you want to pin nightlies, you can always use nightlies that were released short before beta cutoff, they should have the full feature set of their specific release available. So I'd guess it'd be okay to have an option as @joshtriplett suggests.

I think having an option seems fine but I'm not quite sure where/how it's supposed to be set by people.

est31 commented

@nikomatsakis I'd imagine it being part of RUSTFLAGS or ~/.cargo/config

est31 commented

I fear we would give up a lot of eyes on features in the most critical window between when the stabilization lands in nightly and when the stable release ships.

I think that's a compelling argument. I have reconsidered my position and now think it's ok to to flip the default and just having that flag that @joshtriplett suggests.

Also, I've said this in the thread already I think, but how cfg(version) behaves on nightly Rust is not a stable feature, and can thus easily be changed in the future.

I've filed a PR to switch the default: #81468

Also I think now would be a good time to wonder about stabilization.

est31 commented

The "Dependency updates cause language changes" checkbox can be marked now as #81259 has been merged.

#81468 has been merged too, so the default is now switched to make cfg(version(c)) always eval to true, even if c is equal to the nightly compiler's version.

I'd like to get it stable so that I can use new language features without hurting my MSRV. I don't see any remaining issues.

Does this need a stabilization report? Should I try to write one?

Yes, this does need a stabilization report. @est31 if you'd be willing to write one, I'd be happy to kick off FCP.

est31 commented

Stabilization report

I propose that the cfg_version feature be stabilized. The feature guards the cfg(version) predicate, which takes a version number like cfg(version("1.50.0")) and evaluates to true if the compiler's/language's version is newer or equal to the contained version number. Notation wise, "1.50" is allowed too, but any postfixes like -stable or -beta.2 are not allowed.

#[cfg(version("1.50.0"))]
fn foo() {}

#[cfg(version("1.27.0"))]
fn bar() {}

The predicate is one of the two predicates proposed by RFC 2523, the other being cfg(accessible(path)), tracked by #64797. The main alteration from the RFC was to surround the version number with "" to better fit into the compiler's internal data structures.

The implementation issues a warning if invalid version syntax is contained in the string literal. This is done to provide forward compatibility. Literal-free invocations like version(1.50) do cause errors just as version(foo) does.

version on the nightly channel

In the post-RFC discussion there has been a conversation about which version the nightly compiler should use as its supported version. Unlike beta or stable compilers, nightly compilers can still gain new features, and un-stabilizations are possible. So technically it's possible that newer code can rely on a feature that someone's pinned or not updated nightly does not support yet. On the other hand, during the stabilized-in-nightly phase, features often attain their largest extent of exposure before being shipped as part of a stable release.

The proposed compromise is to treat as a nightly's version as complete, but to add the -Z assume-incomplete-release flag added by #81468 . Users who pin their nightlies can enable it to back-date the supported version to the prior release. Intent is to keep it perma-unstable as it targets nightly users only. As this question only affects nightly compilers, the decision can also be revisited in the future.

Implementation history

Documentation

Tests in the compiler

See also

See also the cfg(accessible) feature (issue #64797).

@est31 Thank you for writing up a detailed stabilization report.

Based on that report, this feature seems ready in isolation.

However, when last we discussed this feature, we also had the concern that stabilizing a compiler version check before stabilizing accessible would have a negative effect on the ecosystem, by pushing people towards version checks rather than feature checks. I feel that that concern still holds.

I'm furthermore concerned that once we have version, there will be even less motivation to work on accessible.

@joshtriplett I agree that it would be better if we could stabilize accessible first. However, I do think this is a very useful feature, and there is real harm done to the ecosystem by not providing it. I also believe that, once accessible is made available, we will be able to successfully encourage the ecosystem best practices to adapt (over time). accessible is currently at the pre-implementation phase, whereas this feature has been implemented and had time to bake on nightly for months. With that in mind, I personally would be in favor of moving forwards with stabilization. Feel free to register a concern if you disagree-- I absolutely understand wanting to make sure that the right best-practices develop within the ecosystem.

@rfcbot merge

Edit: correction, #[cfg(accessible)] was partially implemented as #[cfg_accessible] in #69870. I've updated the tracking issue (#64797) to reflect that.

Team member @cramertj has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

est31 commented

On the "should one wait for #[cfg(accessible(...))]" question I have the following opinion.

  • Generally both features are distinct in their abilities as well as in how they work. cfg(version(...)) can be used to gate language features that don't expose any items. It can also hide features that are early gated in the compiler, while cfg(accessible(...)) is sugar for an attr macro and some language features aren't accepted when passed as input to attr macros. cfg(accessible(...)) on the other hand is useful outside of minimum Rust stuff for deciding whether a function from -sys crates is available for example. Ideally one would have both.

  • I think that using cfg(version(...)) in place of cfg(accessible(...)) is better than vice versa. It's better to use cfg(version(...)) to use some iterator combinator while having some fallback code that's compatible with older compilers, than it is to use cfg(accessible(...)) for an unrelated library feature that happened to be stabilized in the same version your language feature was stabilized in that you want to use. Ideally of course, one would always use cfg(accessible(...)) where it fits and cfg(version(...)) where it doesn't. But a world with cfg(version(...)) only seems much nicer to me than a world with cfg(accessible(...)) only.

  • I think there is a natural desire to use cfg(accessible(...)) over cfg(version(...)). Remembering when some item got stabilized is kinda tough while it's easy to type out its path. I don't think that if cfg(version(...)) got stabilized first, inertia would make people keep using it too much once cfg(accessible(...)) gets stabilized too. Obviously, for having compat with really old compilers, people might still prefer cfg(version(...)) in places where one would use cfg(accessible(...)), but I think that would mostly only affect a transition period in which crates with such an old MSRV exist (1). If one waited with stabilizing cfg(version(...)), it would mean that these crates have to use crutches instead (build.rs printing to stdout). Don't think that's a better situation.

  • One should very much prefer feature detection over version detection in ecosystems where multiple implementations exist for a thing. E.g. matching user agent versions is bad in JS to detect which implementation to load because there are multiple browser vendors. Same goes for make implementations etc. As for Rust implementations, there is only one serious compiler, and only one serious standard library. In general one should worry about such issues before they become serious, but in this instance I think it's ok to worry about it later.

  • Ultimately, I believe that the need for version gating exists today, and cfg(version(...)) is good enough at least at the current moment of Rust's evolution. Work on cfg(accessible(...)) shouldn't stop though.

(1): yes, serde still has a super old MSRV. However, serde itself won't ever be able to use cfg(version) anyways as long as it doesn't bump its MSRV. Note that even serde_derive has bumped its MSRV. If we assume that serde never bumps its MSRV, I don't think that many serde-like crates will pop up in the future, instead it'll be serde_derive like crates that bump their MSRV every couple of years (once per edition?).

Will these be usable in Cargo.toml so you can have version specific dependencies, similar to the platform specific ones?

In the RFC example, itertools is used as a fallback when the std version of flatten isnt available, but when it is available on std it'd be nice to avoid bringing in the dependency.

@rfcbot reviewed

I look forward to having this feature available. It may also help with some questions around the async libraries, as it enables one to have methods that move to libstd and "disappear" from the futures crate at the same time.

I agree that it would be nice to make progress on accessible as well-- my impression when last we spoke was that accessible is largely implemented? I may be misremembering.

However, in general, I don't like holding up things that are ready because of other things that are not yet ready.

I agree that it would be nice to make progress on accessible as well-- my impression when last we spoke was that accessible is largely implemented? I may be misremembering.

I recall there are quite some caveats, see #69870 (comment) for maybe the state of it IIRC that is the last merge request.

@rfcbot concern ecosystem-impact-of-version-without-accessible

I'm going to go ahead and register this as a concern, precisely because accessible is harder than version. Other ecosystems (e.g. C) had version-detection much earlier than they had feature-detection, and the effects of that are quite visible in the ecosystem. I'm quite concerned that version will become the "good enough" mechanism for detection, and accessible won't end up happening.

@est31 To be clear, I do think we need both, precisely because accessible only works for library changes, not for language changes. I'd love to have a variant of accessible that works for language changes as well, but there are multiple ways we could do that.

Note: this concern is predicated on the understanding that accessible is feasible to implement, and "just" needs further implementation work. If that turns out to not be the case, and there's some critical blocking issue that prevents implementing accessible as specified, then I'd like to see it re-specified in a fashion that'd be more feasible to implement, but I'd be willing to drop this concern because I don't think stabilizing version should wait on further design work.

est31 commented

Other ecosystems (e.g. C) had version-detection much earlier than they had feature-detection

Which feature detection do you mean? The one that runs alongside configure scripts, launching compiler processes with tiny test programs and checking whether they return an error? That is already possible with rust and build.rs, and already in use in the ecosystem. I'd say that both cfg(accessible) and cfg(version) are better as they avoid the overhead of launching an entire compiler process.

I think the concern is valid, but I think we should then invest some energy in recruiting and mentoring someone to move cfg(accessible) along.

Given the presence of crates like rustversion that make it trivial to gate functionality by a version, surely the ship has already sailed on containing that? Unless your thought is that including it in Rust proper will make it more widely used (which will likely be the case, admittedly).

@est31 By "feature detection", I'm talking about things like clang's __has_builtin, __has_feature, __has_extension, __has_attribute, __has_include, and similar. https://clang.llvm.org/docs/LanguageExtensions.html

If those had been available much earlier, code could have widely used them, rather than checking compiler version macros.

@jhpratt Yes, that's exactly my concern.

@nikomatsakis Complete agreement with that.

est31 commented

@joshtriplett thanks! Those attributes seem way easier to implement than #[cfg(accessible)]... the current plan for implementing that feature involves having to reimplement lots of path resolution features (right now the code is minimal, only made to support macros and modules). And even if that's implemented, there's the issue of type dependent paths, for which the current plan is to not implement them at all... Examples for type dependent paths would be the two recent stabilizations of bool::then or Iterator::reduce.

#[cfg(lang_feature)] (analogous to __has_feature) would be much nicer to implement. If we made #[cfg(lib_feature)] require to start with a crate name like std, proc_macro or such, it would be possible to implement it. #[cfg(lib_feature(proc_macro::proc_macro_diagnostic))].

@est31 Long-term, the generalized version of accessible would be helpful, for use with other crates as well. But short-term, since cfg(version(...)) is just about the version of Rust, a cfg mechanism like lib_feature that only works with std/alloc/core/etc would be entirely enough to unblock this in my opinion.

Nominating for T-lang discussion; it may be a good idea to schedule a design meeting, but we can start with a bit of triage discussion. The key question to resolve is whether @joshtriplett's concern that stabilizing cfg(version(...)) will lock us into version-based, rather than feature-based, support.

It is not entirely clear to me what is being passed into lib_feature/lang_feature as discussed in the last few comments; if those are feature names it does seem unfortunate to stabilize those. It might make sense to have a dedicated set of names that we add as part of the stabilization process; this would avoid problems where parts of an existing feature are stabilized, for example. It does increase the stabilization overhead though.

est31 commented

It is not entirely clear to me what is being passed into lib_feature/lang_feature as discussed in the last few comments; if those are feature names it does seem unfortunate to stabilize those. It might make sense to have a dedicated set of names that we add as part of the stabilization process; this would avoid problems where parts of an existing feature are stabilized, for example. It does increase the stabilization overhead though.

I've been wondering about writing an RFC for lib_feature. Part of the RFC's work would be to research about the current behaviour, whether features exist that are half stabilized, half not stabilized, how often this happens in practice, and whether a policy of having to split off features before partially stabilizing them makes sense or not. As for the feature names, yeah maybe there needs to be some review of them. Maybe it would make sense to stabilize them piece by piece. cpp apparently has a similar process to Rust by having proposals denote the feature name. This is a list of features I could find. Last, right now features outside of core are prefixed like proc_macro_something, but it would be useful if that prefix got dropped and turned into a pseudo-path, so proc_macro::something instead, as then the compiler can explicitly look in the proc_macro crate for presence of that, and doesn't have to execute a scan first whether someone has said extern crate proc_macro or not. Generally it'd be something for the RFC's discussion to determine.

It would be nice if the lang team could give a fundamental feedback whether it would be a good idea to open an RFC on this, or whether it's not a good path forward.

Rather than using library "feature names", I personally think for a first pass it would suffice to have a cfg(lib_has(std::some::path)), where the path inside lib_has ignores any local name bindings and just checks std/alloc/core/prelude. That would completely sidestep name resolution difficulties and feature naming issues. That'd be quite sufficient to allow feature-detection of the standard library.

Could it just use the normal pathing system and people can write ::std if they absolutely want to be sure they're checking the standard library?

est31 commented

Yeah, that wouldn't need lib_has actually. It could be exposed directly as part of cfg(accessible) with an absolute path. After the ::, at least on edition 2018 and beyond, there has to follow a crate name. If it doesn't find the crate as part of the prelude or std/alloc/etc it can just error, regardless of whether anything beyond is accessible or not. Even on edition 2015 you are not allowed to shadow crates from the prelude.

Stabilization report

I propose that the cfg_version feature be stabilized. The feature guards the cfg(version) predicate, which takes a version number like cfg(version("1.50.0")) and evaluates to true if the compiler's/language's version is newer or equal to the contained version number. Notation wise, "1.50" is allowed too, but any postfixes like -stable or -beta.2 are not allowed.

#[cfg(version("1.50.0"))]
fn foo() {}

#[cfg(version("1.27.0"))]
fn bar() {}

The predicate is one of the two predicates proposed by RFC 2523, the other being cfg(accessible(path)), tracked by #64797. The main alteration from the RFC was to surround the version number with "" to better fit into the compiler's internal data structures.

The implementation issues a warning if invalid version syntax is contained in the string literal. This is done to provide forward compatibility. Literal-free invocations like version(1.50) do cause errors just as version(foo) does.

version on the nightly channel

In the post-RFC discussion there has been a conversation about which version the nightly compiler should use as its supported version. Unlike beta or stable compilers, nightly compilers can still gain new features, and un-stabilizations are possible. So technically it's possible that newer code can rely on a feature that someone's pinned or not updated nightly does not support yet. On the other hand, during the stabilized-in-nightly phase, features often attain their largest extent of exposure before being shipped as part of a stable release.

The proposed compromise is to treat as a nightly's version as complete, but to add the -Z assume-incomplete-release flag added by #81468 . Users who pin their nightlies can enable it to back-date the supported version to the prior release. Intent is to keep it perma-unstable as it targets nightly users only. As this question only affects nightly compilers, the decision can also be revisited in the future.

Implementation history

Documentation

Tests in the compiler

See also

See also the cfg(accessible) feature (issue #64797).

I propose to add the specific nightly compiler commit to the attribute, as in:

#[cfg(version("1.54.0-nightly (ed597e7e1 2021-06-08)"))]

Such that nightly builds earlier than that (or after than that) would consider the code annotated with that attribute as "conditionally disabled code" even if it is exactly 1.54.0-nightly, but not the same commit? Not sure would that be too specific; but that might solve the problem.

Setting aside the fact that I don't believe this is the appropriate time or place to discuss that (a new issue should be opened imo), that would effectively preclude an alternative implementation of Rust from ever existing. Matching the behavior of an exact SHA and date would not be worth the effort to say the least.

That is true though

Setting aside the fact that I don't believe this is the appropriate time or place to discuss that (a new issue should be opened imo), that would effectively preclude an alternative implementation of Rust from ever existing.

Not inherently; it would just allow Rust code to detect the exact compiler version it's running on. That might be useful for code that intends to be pinned to a specific compiler version.

I expect alternative implementations to target stable rust, not nightly. So a feature to pin a particular nightly would not need to be implemented by alternative rust compilers (or they could even be implemented with alternative semantics).

Regarding:

Should we also support version = "..."

Yes, please! I have a bunch of projects (and contribute to a bunch of others) that want to support at least the version available in Debian stable. However additional features would be sometimes useful.

Unfortunately, this would not work as-is. Let's say I want to do this:

#cfg(version= "1.60")
struct Foo;
// ...

If I write such code it'll behave strangely:

  • It compiles just fine for old Rust versions (e.g. 1.41)
  • It errors in newer versions (e.g. 1.48)
  • It starts to compile in even newer version again (e.g. let's say it's stabilized in 1.58)
  • It turns on the additional functionality in the given version (1.60)

To an uninformed observer this would look like certain versions had regression.

To solve this I propose renaming version to rust_version in the same release this gets stabilized in. (In other words renaming would be mandatory part of the stabilization commit.) I find that name more readable anyway and it's in line with similar cargo feature. An alternative is just to get this in ASAP but that incentivizes rushed decisions. (Maybe not too big issue given this is a very simple feature?)

est31 commented

@Kixunil yeah the same behaviour occurs with edition = "..." in Cargo.toml which is ignored by older cargos, then newer cargos error about it, until cargo 1.31.0 onwards accepts it.

I think the best solution to the problem is to just wait it out, because even if this feature exists, projects will increase their MSRV eventually to one that supports #[cfg(version)]. I think that's better than adding the trademarked rust term to even more places in the language (this can be problematic for forks for example, or people who don't want to/can't abide by the trademark policy the foundation sets).

@est31 WTF, "Rust" is name of the programming language so if you make a fork it's reasonable to keep the name of the language. Does the trademark disallow people forking?

IMO version is not clear - is it crate version? Dep version? std version? cargo version? Rust version? Renaming kills two birds with one stone. (Or if we want to avoid trademark discussion, lang-version is fine.)

I'd agree that lang_version is better than just version.

est31 commented

Does the trademark disallow people forking?

The copyright doesn't but the trademark rights are separate and it's up to the owner of the trademark to set the policy. When the trademarks were owned by Mozilla, they were under basically the same rules as the Firefox trademark. Some distros were forced to create different branding for their Firefox builds even though they only added a few patches. Not familiar enough with what the foundation's been doing to tell whether there has been any change of the trademark policy. The website still treats the trademarks as if they were owned by Mozilla. Even if, such policies can change any minute.

Also, generally it's a good idea to rename a project when forking it to avoid confusion, even if you are not required to do so.

I feel that fairness owes it that if possible, it would be best to not refer to the name of the language in proper language features.

IMO version is not clear - is it crate version? Dep version? std version? cargo version? Rust version? Renaming kills two birds with one stone. (Or if we want to avoid trademark discussion, lang-version is fine.)

I like lang_version.

@rfcbot cancel

Hi all. This has been in FCP for a very long time and so I am going to cancel the FCP (my personal philosophy is that FCPs should not last this long). I believe it is currently blocked waiting on progress around some kind of subset of cfg-accessible, as described by @joshtriplett here (or perhaps in comments that @joshtriplett is referencing there).

If somebody would like to move this issue forward: The next step would be to open a separate issue regarding that proposal and link it from here, and perhaps we can try to drive that work forward with mentorship or other means?

@nikomatsakis proposal cancelled.

It looks like cfg(accessible(...)) may be at a state where we could stabilize enough of it to be generally useful, and what's available is enough that I'd consider it to unblock this.

#64797

alex commented

#64797 appears to have stalled out somewhat. I'd like to make an argument for why I believe it'd be appropriate to consider stabilizing this even before cfg(accessible(...)) is stabilized:

  1. There are many use cases for version checking beyond core library types being available. A particularly common one I've seen is version checking to see whether a particular piece of functionality can be used in const contexts.
  2. 3rd party crates for this functionality are already quite common version_check and autocfg both have more than 100 million downloads from crates.io. Under many circumstances, availability of adequate 3rd party crates would ameliorate the need for an in-language solution, however in this case they have a major downside: they require the use of build scripts, which impose a non-trivial compilation time overhead. Build speed is of course a major area of interest -- in a survey of a crate with 56 items in Cargo.lock, about half the build scripts are solely to do version checks.
  3. Because this functionality ultimately is aimed at enabling folks to support multiple versions of Rust more easily, its users won't be able to adopt it until its been stable for quite a while. Therefore getting it out sooner is a big benefit.

I hit a bug in Rustc today, where rustc fails to derive Send and Sync automatically in some cyclic type relation context. This is fixed with 1.72, so my use case here would be to #[cfg(version(..))] an unsafe Send block to make sure code compiles with older Rustc.

This is important for the project I'm working on as it's Mesa, the OpenGL/Vulkan/OpenCL/etc.. implementation for the Linux desktop and we need to be mindful of what Rustc version to support.

Edit: Apparently it was Sync stabilized in 1.72 for mpsc::Sender

mqudsi commented

There are many use cases for version checking beyond core library types being available.

+1 to this, because you have to take the entire ecosystem and tooling into account and not just the language itself. For example, in rsconf 0.2, I added an api that uses rustc --check-cfg syntax to inform the compiler of a cfg managed by build.rs, but now building under older versions of rust that were aware of the check-cfg syntax but didn't have support for it enabled without -Z check-cfg are broken (if you error on warnings). This isn't a language level change but a tooling change, and so I would have to use #[cfg(version(1.80))] on the println!("cargo:rustc-check-cfg...") statement to get this to not warn under certain rust versions.

Nominating for lang-team discussion. This was marked as blocked on #64797 two years ago but #64797 still has open design questions and does not appear ready for stabilization anytime soon whereas this feature has no open design questions and is fully implemented. Since #64797 does not address some important use cases such as conditionally using compiler or language features and use of version_check continues to grow (now at >229M all-time downloads and upward-trending daily downloads), I think it makes sense to revisit stabilization of this feature πŸ™‚

@rustbot label +I-nominated

mqudsi commented

Not to add to the pressure or anything, but for a feature with the raison d’Γͺtre being backwards compatibility this one won't be a viable alternative to the version_check crate until the msrv of the codebase hits whatever version cfg(version) was stabilized under. So it really makes sense to try and get this out the gate on its own without blocking on other tangentially related issues.

@mqudsi yes, though if the cfg gets renamed to lang_version as was suggested then people can already start using it in old crates for features that are only available in whichever version it gets stabilized. (Because unknown cfgs are allowed and evaluate to false.)

We discussed this in the lang call today. We had a question:

What's the situation on stabilizing a minimum version of cfg(accessible(..)), i.e. one that just works for things in core, std, etc.?

We understood there were issues with a more general version of the feature, and we understand there are some issues with paths in attributes, but we were unclear on whether stabilizing the most minimal cfg(accessible(..)) that we could was within the realm of possibility.

If it is within the realm of possibility, the feeling was generally that we'd prefer to do that, and that such a minimum version would resolve the concern here and allow for the stabilization of cfg(version(..)).

On the other hand, if it's not within the realm of short-term possibility for some reason, then people were open to reevaluating the approach here and whether other options might be possible for moving this forward.

@rustbot labels -I-lang-nominated

Going to unnominate this for now, but please renominate when the answer to the question above is available.