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

[Centril]

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

Steps:

• [x] Implement the RFC (cc @rust-lang/compiler -- can anyone write up mentoring instructions?)
• [ ] Adjust documentation (see instructions on rustc-guide)
• [ ] Stabilization PR (see instructions on rustc-guide)

Unresolved questions:

• [x] 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.

  • Resolved in https://github.com/rust-lang/rust/issues/64796#issuecomment-625474439

• [ ] 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 (https://github.com/rust-lang/rust/issues/79010)

[pickfire]

@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]

@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)

[pickfire]

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

@rustbot claim

[pickfire]

Can anyone please help to write up the mentoring instructions. Based on what I know, this should be similar to https://github.com/rust-lang/rust/issues/64797#issuecomment-540350338

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 https://github.com/rust-lang/rust/blob/b56b23988de532744fd05301f87c329b612700e3/src/libsyntax/attr/builtin.rs#L566-L568.

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?

[mibac138]

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

[pickfire]

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

[mibac138]

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

[pickfire]

@mibac138 Do you want me to publish my progress?

[mibac138]

@pickfire sure, that'd be great!

[pickfire]

@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.

[petrochenkov]

Status update: https://github.com/rust-lang/rust/pull/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.

[roblabla]

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.

[nikomatsakis]

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?

[nikomatsakis]

Nominating for lang-team meeting to discuss

[dekellum]

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?

[Nemo157]

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.

[dekellum]

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.

[roblabla]

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.

[dekellum]

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]

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.

[joshtriplett]

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.

[pickfire]

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.

[dekellum]

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.

[roblabla]

@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.

[joshtriplett]

We discussed the unresolved question (as explained in https://github.com/rust-lang/rust/issues/64796#issue-498617312) 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).

[dekellum]

That's lame.

[jhpratt]

That's lame.

Please be constructive.

[pickfire]

@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]

@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".

[pickfire]

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.