Setting cfg(nightly) on nightly by default

I’ve thought about this, but I’m not actually sure how much benefit it will get us. Asking whether the feature foo is available I don’t think actually buys much more than asking if you’re on a nightly compiler due to the way our stability works out.

For example, if I know that feature foo is available, then I know that I’m on a nightly compiler, but that’s about it. There’s no guarantee about what foo looks like or whether it’s even the feature I originally wanted. The benefit that this may gain us is that you work with older nightlies which don’t have the feature, but by opting in to nightly behavior you’re probably always working on the bleeding edge and you’ve already admitted to updating code periodically as well.

I would say the ideal is that you should be able to compile older crates with OLDER features and still be on the latest nightly and compiling your own crates that depend on these older crates. Your own crates depend on newer features, while the older crates depend on the older features. That way you can still compile your program even if there are some breaking changes because of feature detection.

I’ve been fiddling with getting travis-ci to test against both nightly and beta, and it’s possible by adding to the .travis.yml:

language: rust
rust:
  - nightly
  - beta

However, I don’t have a way to detect the version, so I can’t toggle passing --features nightly to cargo. Once 1.0 is released, I’ll want to also add a stable test to my travis.yml. That’ll be much higher priority.

So, can we continue on this idea?

One can detect the version using the TRAVIS_RUST_VERSION environment variable, e.g. @bluss’ script, or my own travis-cargo (blog post).

One could also use a more detailed travis build matrix to set env vars along side rust versions:

matrix:
  include:
    - rust: nightly
      env: FEATURES=nightly
    - rust: beta
      env: FEATURES=''
script:
   - cargo build --features "$FEATURES"

(not sure if that actually works, but it’s probably close.)

1 Like

Was there any progress on this?

This can be done with rust_version.

1 Like

You can use rustc_version to be relatively targeted, for example enabling a feature on those nightlies that have it (example). It can of course break if there’s a change to the unstable feature.

Was this rejected, or it just hasn’t happened yet?

There are a lot of nightly features that can be reasonably used only when available, such as benchmarks. It seems like a shame that every crate has to define its own way to enable or detect them.

Especially benchmarks are a bad example, as cargo has the ability to fully isolate them in /benches and run them through cargo bench. Only run that on nightly until they are available on stable, then.

I don’t think cfg(nightly) is a good idea: crates should not silently change their compilation when detecting nightly vs stable; that makes nightly compilers less suitable to use for running stable projects.

3 Likes

I want to use specialization when available, and disable the specializations otherwise (my users get worse performance).

I want the right thing to happen by default without my users having to enable any feature flags, because otherwise all crates that use my crate might need to re-export my flags: crate A includes crate B which includes crate MyCrate. If crate A is compiled with nightly but crate B does not pass my crate the use_specialization flag, A and users of A cannot get the performance boost.

Ideally, I want to be able to not only detect stable, beta, nightly, but in the case of nightly, I would like to use a date / git hash as well to be able to work around nightly bugs / enable unstable features only for the builds in which they actually do work, that is, something like #![cfg_attr(nightly > 2016.01.20, feature(foo))]. Probably makes sense to allow all logical operators for the dates: #![cfg_attr(nightly > 2016.01.20 && nightly < 2016.03.20 && nightly != 2016.02.23, feature(foo))] .

EDIT: specialization, target_feature, u128/i128, intrinsics, … they are all features that it makes sense to use when available because they provide a performance boost, but I would like my crates to still compile on stable without any hassles.

1 Like

Another situation I’ve run into is with operators (AddAssign, BitAndAssign, BitOrAssign,…).

If I implement those for types I export then the minimum rust version is 1.6.0. However if you are using an older rust version my types and library might still work for you (as long as I don’t provide those features).

One can hack all of these into a build.rs that defines features automatically but I’d rather have something better ASAP since these problems are only going to get worse with time.

I’m not sure what the problems you allude to are. You can already do this detection fairly easily in build.rs: although it doesn’t detect nightlies, the approach used in strcursor's build.rs should work fine for that.

Note this gives names to the things you’re compiling against, making the actual code easier to understand. It’s far better than a rat’s nest of anonymous #[cfg(rust >= 1.10.0)] directives.

Also, I’m with bluss: specialising code based on stable features is one thing, but silently changing behaviour based on unstable features that may or may not change at any point in the future is just begging for trouble. Hell, I had a totally uncontentious method just up and vanish on me with effectively no warning and no replacement. I don’t want to have to not use nightly releases because some library has decided to ride the knife’s edge for my “convenience” except it’s now broken and I have to wait for it to be fixed.

2 Likes

I am doing it in build.rs using the rustc_version crate which does allow you to detect nightlies.

I'm not sure what the problems you allude to are. It's far better than a rat's nest of anonymous #[cfg(rust >= 1.10.0)] directives.

I think a "rat's nest of anonymous #[cfg(rust >= 1.10.0)] directives" would be significantly better, since right now users seeing the labels #[cfg(rust_greater_than_1_10_0)] need to dabble in the build.rs to see if those labels do what they say they do.

unstable features that may or may not change at any point in the future is just begging for trouble.

Hence why one should pin them to a nightly version or a range of nightly versions: #[cfg(nightly_rust_from_2016_10-05_to_2017_01_19_inclusive)] (or _till_present or similar).

Hell, I had a totally uncontentious method just up and vanish on me with effectively no warning and no replacement. I don't want to have to not use nightly releases because some library has decided to ride the knife's edge for my "convenience" except it's now broken and I have to wait for it to be fixed.

The power to do this is already there, that one should use this power responsibly is nothing new. Real attributes would make it easier/safer to use this to support older compilers or to use new features without increasing the minimum required rust version for everybody (which is a major API breaking change). And this also could allow rust/cargo to actually diagnose early if you are trying to compile a crate with a compiler that is not supported (top level #![cfg(rust >= 1.10.0)]).

Which is what was in that link. I was referring to how I was not detecting nightlies in the linked code, but that it should work.

I assume you didn't look at the link. Here is a relevant snippet:

if version_matches("1.4.0") {
    println!("cargo:rustc-cfg=has_string_into_boxed_string");
}

Note that this sets a cfg which states what's being gated here. A user seeing rust >= 1.4 has to guess what particular part of having Rust 1.4 or higher is actually relevant. Seeing has_string_into_boxed_string is unambiguous: the associated code depends on String::into_boxed_string which may not be present.

If you believe stating a version number is clearer than that, then we'll just have to agree to disagree.

You're seriously going to vet every nightly? And publish updates on a regular basis? And document which ones contain the performance gains and which don't?

I mean, if you are, then I can't say anything against that. I concede that in that respect (having libraries that automatically use nightly features not break on update), if the dev is going to keep up that level of maintenance, there isn't any actual practical drawback for users.

I fail to see how this is any safer. Easier, yes, though I still feel having named cfgs is better for readability and maintenance than raw numbers.

By the time you get to this check, Cargo has already decided what version of a crate it's going to try and use. We want this check in Cargo, not the compiler. That way, Cargo can exclude incompatible versions from selection entirely.

In theory it would be nice to have standard feature macros for testing whether a rust compiler implements a particular feature (instead of every project having to define their own).

If you believe stating a version number is clearer than that, then we'll just have to agree to disagree.

The problem is that a feature flag will never be enough: even if a compiler version implements a feature, that implementation might contain a bug that makes the feature unusable for a particular purpose / context. So one needs to easily be able to test for a compiler version.

C++ has both, and while C++ feature testing macros are nice in theory, they have actually made things worse in practice. Before, libraries used to test whether a particular compiler version or higher was available. Now libraries test whether a particular feature flag is available but if it is available, they still need to test against compiler versions to work around bugs in the reporting of the flags or in the implementations of the feature... (range-v3 config file is a wonderful example of the current situation).

You're seriously going to vet every nightly? And publish updates on a regular basis? And document which ones contain the performance gains and which don't?

Most open source C++ libraries that I've worked on do this without problems :confused: range-v3 is one example: when clang trunk/gcc trunk hits a bug, they workaround the bug for the range of trunk versions that contain the bug (because a lot of their users use tip-of-trunk compiler versions). The workarounds are disabled but not removed after new compiler versions with the fix are released because, what's the point, the work has already been done. I find it really hacky to have to do things like these in build.rs.

By the time you get to this check, Cargo has already decided what version of a crate it's going to try and use. We want this check in Cargo, not the compiler. That way, Cargo can exclude incompatible versions from selection entirely.

Yes, putting this check in cargo makes more sense, but a rust compiler should still reject compiling something that it cannot compile. Having this information at the top-level of a crate would allow the rust compiler to emit a better diagnostic, like "rustc version 1.4.0 is not enogh to compile this crate which requires version >=1.6.0".

But that's what you can already do! I mean, here's a snippet from scan-rules' current build.rs:

    /*
    See <https://github.com/rust-lang/rust/issues/26448#issuecomment-173794570>.
    */
    if version_matches("< 1.7.0") {
        println!("cargo:rustc-cfg=str_into_output_extra_broken");
    }

Also, you're dangerously close to a strawman argument: I never said the only way to do this should be compiler-defined feature flags. Even if that was a thing, you're quite right, it's still helpful to define your own, which is what you can already do.

I mean, yeah, but that just changes the optics of those errors (if it won't compile, it won't compile, just with a different message). It doesn't make code that wouldn't have otherwise compiled work (which adding this to Cargo could). Also, this is coming from someone who previously proposed more or less exactly what you're proposing now, so it's not like I think it's a bad idea... just a possible misapplication of effort.

Also, you're dangerously close to a strawman argument: I never said the only way to do this should be compiler-defined feature flags

Sorry, I did not meant to convey that you did, just that while feature flags look nicer, they are not enough anyways.

here's a snippet from scan-rules' current build.rs:

This is basically what I've been doing as well and it works, but I don't think it is nice. I thought that adding a way to do this (or something that covers 99% of the use cases) outside build.rs (in normal rust files) would make this nicer, but thinking twice about it, this kind of compiler-version-dependent conditional compilation is never nice, so maybe it is a pointless effort.

I’d like a nicer, more convenient syntax for it, too.

That’s partly why I’m arguing against it, here. Having to use build.rs is clunky, but maybe imposing a bit of separation and inconvenience isn’t a bad thing. I’ve seen some really horrific codebases using conditional compilation.

Plus, every time I try to imagine how to introduce named custom cfgs directly in Rust source, it starts to smell of Inner Platform Effect, at which point I go think about something else.

2 Likes

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.