From ab8d6b3bb7aaf3a92d3b734cfd714942d1806e3e Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 10 Aug 2018 15:09:52 +0200 Subject: [PATCH 01/50] rfc, cfg-path-version: initial version. --- text/0000-cfg-path-version.md | 484 ++++++++++++++++++++++++++++++++++ 1 file changed, 484 insertions(+) create mode 100644 text/0000-cfg-path-version.md diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md new file mode 100644 index 00000000000..1a1f38b22ec --- /dev/null +++ b/text/0000-cfg-path-version.md @@ -0,0 +1,484 @@ +- Feature Name: `cfg_path_version` +- Start Date: 2018-08-10 +- RFC PR: _ +- Rust Issue: _ + +# Summary +[summary]: #summary + +Permit users to `#[cfg(..)]` whether: + ++ they are on a `nightly` compiler (`#[cfg(nightly)]`). ++ they are above a certain Rust version (`#[cfg(version("1.27"))]`). ++ a certain external path exists `#[cfg(path_exists(::std::mem::ManuallyDrop)]`. + +# Motivation +[motivation]: #motivation + +[stability_stagnation]: https://blog.rust-lang.org/2014/10/30/Stability.html +[what_is_rust2018]: https://blog.rust-lang.org/2018/07/27/what-is-rust-2018.html + +A core tenant of Rust's story is +[*"stability without stagnation"*][stability_stagnation]. +We have made great strides sticking to this story while continuously +improving the language and the community. This is especially the case with +the coming [Rust 2018 edition][what_is_rust2018]. + +However, while the situation for evolving the language is doing well, +the situation for library authors is not as good as it could be. +Today, crate authors often face a dilemma: - *"Shall I provide more features +and implementations for later versions of Rust, or should I stay compatible +with more versions of the compiler"*. + +[cargo_version_selection]: http://aturon.github.io/2018/07/25/cargo-version-selection/ + +While [much thought][cargo_version_selection] has been given to how we can +reduce "dependency hell" by enhancing cargo for: + ++ the **control** users have over their dependencies. ++ the **compatibility** of crates with each other. ++ reducing the **maintainability** burden of having to make sure that + versions work with each other. + +[RFC 2483]: https://github.com/rust-lang/rfcs/pull/2483 + +...not much focus has been given to how you can improve the situation can be +improved by enhancing conditional compilation to extend how many versions back +a crate supports. This becomes critically important if and when we gain LTS +channels as proposed by [RFC 2483]. + +[version_check]: https://crates.io/crates/version_check + +The current support for such conditional compilation is lacking. +While [it is possible][version_check] to check if you are on a nightly +compiler or to check if you are above a certain compiler version, +such facilities are not particularly ergonomic at the moment. +In particular, they require the setting up of a `build.rs` file and +declaring up-front which versions you are interested in knowing about. +These tools are also unable to check if a certain path exists and instead +force you to know which version they were introduced in. + +*We can do better.* In this RFC we aim to rectify this by giving library +authors the tools they need in the language itself. With the features +proposed in the [summary] we aim to make retaining *compatibility* and +supporting more compiler versions *pain-free* and to give authors a lot +of *control* over what is supported and what is not. + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +## `#[cfg(nightly)]` and `#[cfg(path_exists($path)]` + +Consider for a moment that we would like to use the `Iterator::flatten` +method of the standard library if it exists, but otherwise fall back to +`Itertools::flatten`. We can do that with the following snippet: + +```rust +#![cfg_attr(nightly, feature(iterator_flatten))] + +#[cfg(path_exists(::std::iter::Flatten))] +fn make_iter(limit: u8) -> impl Iterator { + (0..limit).map(move |x| (x..limit)).flatten() +} + +#[cfg(not(path_exists(::std::iter::Flatten)))] +fn make_iter() { + use itertools::Itertools; + (0..limit).map(move |x| (x..limit)).flatten() +} + +fn main() { + println!("{:?}", make_iter(10).collect::>()); +} +``` + +What this snippet does is the following: + +1. If you happen to be on a nightly compiler, but not otherwise, + the feature `itertator_flatten` will be enabled. + +2. If the path `::std::iter::Flatten` exists, the compiler will compile + the first version of `make_iter`. If the path does not exist, + the compiler will instead compile the second version of `make_iter`. + +The result of 1. and 2. is that your crate can opt into using `Iterator::flatten` +on nightly compilers but use `Itertools::flatten` on stable compilers meanwhile. +Once the standard library has stabilized `iter::Flatten`, future stable compilers +will start using the first version of the function. As a crate author, you +don't have to publish any new versions of your crate for the compiler to +switch to the libstd version when it is available in the future. + +[`proptest`]: https://github.com/altsysrq/proptest +[adding support]: https://github.com/AltSysrq/proptest/blob/67945c89e09f8223ae945cc8da029181822ce27e/src/num.rs#L66-L76 + +In this case we used the `nightly` and `path_exists` flags to handle a problem +that the addition of `Iterator::flatten` caused for us if we had used +`Itertools::flatten`. We can also use these mechanisms for strictly additive +cases as well. Consider for example the [`proptest`] crate [adding support] +for `RangeInclusive`: + +```rust +#[cfg_attr(nightly, feature(inclusive_range))] + +macro_rules! numeric_api { + ($typ:ident) => { + ... + + #[cfg(path_exists(::core::ops::RangeInclusive))] + impl Strategy for ::core::ops::RangeInclusive<$typ> { + type Tree = BinarySearch; + type Value = $typ; + + fn new_tree(&self, runner: &mut TestRunner) -> NewTree { + Ok(BinarySearch::new_clamped( + *self.start(), + $crate::num::sample_uniform_incl(runner, *self.start(), *self.end()), + *self.end())) + } + } + + ... + } +} + +macro_rules! unsigned_integer_bin_search { + ($typ:ident) => { + pub mod $typ { + use rand::Rng; + + use strategy::*; + use test_runner::TestRunner; + + int_any!($typ); + } + } +} + +unsigned_integer_bin_search!(u8); +unsigned_integer_bin_search!(u16); +... +``` + +This means that `proptest` can continue to evolve and add support for +`RangeInclusive` from the standard library and the `x..=y` syntax in the +language without having to release a new breaking change version. +Dependents of `proptest` simply need to be on a compiler version where +`::core::ops::RangeInclusive` is defined to take advantage of this. + +So far we have only used `path_exists(..)` to refer to paths in the standard +library. However, while it will be a less likely use case, you can use the flag +to test if a path exists in some library in the ecosystem. This can for example +be useful if you need to support lower minor versions of a library but also +add support for features in a higher minor version. + +## `#[cfg(version("1.27"))]` + +Until now, we have only improved our support for library features but never +any language features. By checking if we are on a certain minimum version of +Rust or any version above it, we can conditionally support such new features. +For example: + +```rust +#[cfg_attr(version("1.27"), must_use)] +fn double(x: i32) -> i32 { + 2 * x +} + +fn main() { + double(4); + // warning: unused return value of `double` which must be used + // ^--- This warning only happens if we are on Rust >= 1.27. +} +``` + +Another example is opting into the system allocator on Rust 1.28 and beyond: + +```rust +#[cfg(version("1.28")) +// or: #[cfg(path_exists(::std::alloc::System))] +use std::alloc::System; + +#[cfg_attr(version("1.28"), global_allocator)] +static GLOBAL: System = System; + +fn main() { + let mut v = Vec::new(); + // This will allocate memory using the system allocator. + // ^--- But only on Rust 1.28 and beyond! + v.push(1); +} +``` + +Note that you won't be able to make use of `#[cfg(version(..))` for these +particular features since they were introduced before this RFC's featuers +get stabilized. However, there will be features in the future to use this +mechanism on. + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +## `#[cfg(nightly)]` + +To the `cfg` attribute , a `nightly` flag is added. + +If and only if a Rust compiler considers itself to be on a nightly channel +it the `nightly` flag be considered active. + +## `#[cfg(version(""))]` + +To the `cfg` attribute, a `version` flag is added. +This flag requires that a string literal be specified in it inside parenthesis. +The string literal must have the format: + +``` +version_string ::= \d(.\d)? +``` + +If and only if a Rust compiler considers itself to have a version which is +greater or equal to the version in the `version_string` will the +`#[cfg(version("")]` flag be considered active. + +## `#[cfg(path_exists($path))]` + +To the `cfg` attribute, a `path_exists` flag is added. +This flag requires that a `path` fragment be specified in it inside parenthesis +but not inside a string literal. The `$path` must start with leading `::` +and may not refer to any parts of the own crate (e.g. with `::crate::foo`, +`::self::foo`, or `::super::foo` if such paths are legal). +This restriction exists to ensure that the user does not try to +conditionally compile against parts of their own crate because that crate +has not been compiled when the `path_exists` flag is checked on an item. + +If and only if the path referred to by `$path` does exist will the +`#[cfg(path_exists($path)]` flag be considered active. +In checking whether the path exists or not, the compiler will consider +feature gated items to exist if the gate has been enabled. +If a path refers to an item inside an inherent implementation, +the path will be considered to exist if any configuration of generic +parameters can lead to the item. To check whether an item exists for +an implementation with a specific sequence of concrete types applied to +a type constructor, it is possible to use the `::foo::bar::::item` syntax. + +## `cfg_attr` and `cfg!` + +Note that the above sections also apply to the attribute `#[cfg_attr(..)]` +as well as the special macro `cfg!(..)` in that `nightly`, `version(..)`, +and `path_exists(..)` are added to those as well. + +# Drawbacks +[drawbacks]: #drawbacks + +## It becomes harder to `crater` + +As we get more version based conditional compilation, it can get harder to +use the `crater` tool because regressions are not tested on an old compiler +that may not have certain paths defined. + +## Incremental garbage code and its collection + +It sometimes happens that feature gates never make it to stable and +that they instead get scrapped. This occurs infrequently. +However, when this does happen, code that is conditionally compiled under +`path_exists(::std::the::obsoleted::path)` will become garbage that just +sits around. Over time, this garbage can grow to a non-trivial amount. + +However, if we provide LTS channels in the style of [RFC 2483], +then there are opportunities to perform some "garbage collection" +of definitions that won't be used when the LTS version changes. + +# Rationale and alternatives +[alternatives]: #rationale-and-alternatives + +## `path_exists(..)` + +The primary rationale for the `path_exists` mechanism is that when you +want to support some library feature, it is some path you are thinking of +rather than what version it was added. For example, if you want to use +`ManuallyDrop`, you can just ask if it exists. The `version` is instead a +proxy for the feature. Instead of detecting if the path we want is available +or not via an indirection, we can just check if the path exists directly. +This way, a user does not have to look up the minimum version number for +the feature. + +You may think that `version(..)` subsumes `path_exists(..)`. +However, we argue that it does not. This is the case because at the time of +enabling the `nightly` feature that enables the path in the standard library, +we do not yet know what minimum version it will be supported under. +If we try to support it with `version(..)`, it is possible that we may +need to update the minimum version some small number of times. +However, doing so even once means that you will need to release new versions +of your crate. If you instead use `path_exists(..)` you won't need to use +it even once unless the name of the path changes in-between. + +### The bikeshed + +One might consider other names for the flag instead of `path_exist`. +Some contenders are: + ++ `has_path` ++ `has_item` ++ `path_reachable` ++ `item_reachable` ++ `item_exists` + +Currently `path_exists` is the choice because it clearly signals the intent. + +## `nightly` + +[dbg]: https://crates.io/crates/dbg + +One reason for the inclusion of `#[cfg(nightly)]` is that it is useful on its +own to conditionally compile based on nightly/not as opposed to providing +an `unstable` feature in `Cargo.toml`. An example of this is provided by the +[dbg] crate which currently uses [version_check] to provide this automation. + +However, as we've argued and demonstrated in the [guide-level-explanation], +the ability to `#[cfg(nightly)]` really shines when used in conjunction with +`#[cfg(path_exists($path))]`. + +## `version(..)` + +When it comes to `version(..)`, it is needed to support conditional compilation +of language features as opposed to library features as previously shown. +Also, as we've seen, `version(..)` does not subsume `path_exists(..)` but is +rather a complementary mechanism. + +One problem specific to `version(..)` is that it might get too `rustc` specific. +It might be difficult for other Rust implementations than `rustc` to work with +this version numbering as libraries will compile against `rustc`s release +numbering. However, it is possible for other implementations to follow +`rustc` in the numbering and what features it provides. This is probably not +too unreasonable as we can expect `rustc` to be the reference implementation +and that other ones will probably lag behind. Indeed, this is the experience +with `GHC` and alternative Haskell compilers. + +### The bikeshed + +Naturally, there are other possible names for the flag. For example: + ++ `rustc_version` ++ `compiler_version` ++ `min_version` + +We pick the current naming because we believe it is sufficiently clear +while also short and sweet. However, `min_version` is a good alternative +to consider because it telegraphs the `>=` nature of the flag. + +As for the `version_string` syntax, it could also be adjusted such that +you could write `version(">= 1.27")`. We could also support exact version +checking (`==`) as well as checking if the compiler is below a certain version +(`<=`). However, as a first iteration, `version("1.27")` is simple and covers +most use cases. + +## [version_check] as an alternative + +Using the crate `version_check` we may conditionally compile using a `build.rs` +file. For example, the [dbg] crate does this: + +```rust +// src/lib.rs: +// ----------------------------------------------------------------------------- + +#![cfg_attr(use_nightly, feature(core_intrinsics, specialization))] + +// Deal with specialization: +// On nightly: typeof(expr) doesn't need to be Debug. +#[allow(dead_code)] +#[doc(hidden)] +pub struct WrapDebug(pub T); +use std::fmt::{Debug, Formatter, Result}; + +#[cfg(use_nightly)] +impl Debug for WrapDebug { + default fn fmt(&self, f: &mut Formatter) -> Result { + use ::std::intrinsics::type_name; + write!(f, "[ of type {} is !Debug]", + unsafe { type_name::() }) + } +} + +... + +// build.rs: +// ----------------------------------------------------------------------------- + +//! +//! This build script detects if we are nightly or not +//! + +extern crate version_check; + +fn main() { + println!("cargo:rerun-if-changed=build.rs"); + if let Some(true) = version_check::is_nightly() { + println!("cargo:rustc-cfg=use_nightly"); + } +} +``` + +The [version_check] crate also supports testing for a minimum `version(..)` with: + +```rust +extern crate version_check; + +if let Some((true, _)) = version_check::is_min_version("1.13.0") { + println!("cargo:rustc-cfg=MIN_COMPILER_1_13"); +} +``` + +However, this is quite verbose in comparison and requires you to invent +ad-hoc and crate-specific names for your `#[cfg(..)]` flags such as +`MIN_COMPILER_1_13` that will not be the same for every crate. +You will also need to repeat this per version you want to support. +This causes the mechanism to scale poorly as compared to `version("1.27")` +which we argue is simple and intuitive. + +## Conditional compilation on feature gates + +An alternative to `version(..)` and `path_exists(..)` is to allow users +to query where a certain feature gate is stable or not. +However, it has been argued that allowing this would essentially stabilize +the names of the gates which we've historically not done. + +We also argue that `path_exists(..)` is more intuitive because it is more +natural to think of a feature in terms of how you would make use of it +(via its path) rather than the sometimes somewhat arbitrarily named feature gate. + +# Prior art +[prior-art]: #prior-art + +## Crates + +[rustc_version]: https://crates.io/crates/rustc_version + +As previously mentioned, the [version_check] crate provides precedent for +doing the desired conditional compilation in this RFC. There is also the +[rustc_version] crate. Together, these crates have 18 + 67 direct reverse +dependencies. This suggests that the feature is both desired and used. + +## Haskell + +Using the Glasgow Haskell Compiler (GHC), it is possible to conditionally +compile using it's provided preprocessor: + +```haskell +{-# LANGUAGE CPP #-} + +module Main where + +version :: String +#if __GLASGOW_HASKELL__ >= 706 +version = "Version 7.0.6" +#else +version = "Below." +#endif + +main :: IO () +main = putStrLn version +``` + +# Unresolved questions +[unresolved]: #unresolved-questions + +1. What should the flags be named? This can be deferred to after the RFC if + need be so that we may gain usage experience. \ No newline at end of file From 4670370e8f17bea3aa5d789c1daa32557418ad9e Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 10 Aug 2018 15:21:05 +0200 Subject: [PATCH 02/50] rfc, cfg-path-version: fix unbalanced parens. --- text/0000-cfg-path-version.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 1a1f38b22ec..cc22833b531 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -10,7 +10,7 @@ Permit users to `#[cfg(..)]` whether: + they are on a `nightly` compiler (`#[cfg(nightly)]`). + they are above a certain Rust version (`#[cfg(version("1.27"))]`). -+ a certain external path exists `#[cfg(path_exists(::std::mem::ManuallyDrop)]`. ++ a certain external path exists `#[cfg(path_exists(::std::mem::ManuallyDrop))]`. # Motivation [motivation]: #motivation From e0f0fea382c2d168d19625d2a8caa2c6b7888c22 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 10 Aug 2018 15:21:53 +0200 Subject: [PATCH 03/50] rfc, cfg-path-version: fix typo. --- text/0000-cfg-path-version.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index cc22833b531..7dd4e69b0f2 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -6,7 +6,7 @@ # Summary [summary]: #summary -Permit users to `#[cfg(..)]` whether: +Permit users to `#[cfg(..)]` on whether: + they are on a `nightly` compiler (`#[cfg(nightly)]`). + they are above a certain Rust version (`#[cfg(version("1.27"))]`). From d564d754489b60f3b8797c5fa973b6944610fc6b Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 10 Aug 2018 15:24:30 +0200 Subject: [PATCH 04/50] rfc, cfg-path-version: fix incorrect summary. --- text/0000-cfg-path-version.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 7dd4e69b0f2..36e3c58d3ed 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -9,7 +9,7 @@ Permit users to `#[cfg(..)]` on whether: + they are on a `nightly` compiler (`#[cfg(nightly)]`). -+ they are above a certain Rust version (`#[cfg(version("1.27"))]`). ++ they have a certain minimum Rust version (`#[cfg(version("1.27"))]`). + a certain external path exists `#[cfg(path_exists(::std::mem::ManuallyDrop))]`. # Motivation From 091e470fd6c90c23bfedefe8384c8971d02cbc34 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 04:10:01 +0200 Subject: [PATCH 05/50] rfc, cfg-path-version: fix typos. --- text/0000-cfg-path-version.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 36e3c58d3ed..0d026721035 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -18,7 +18,7 @@ Permit users to `#[cfg(..)]` on whether: [stability_stagnation]: https://blog.rust-lang.org/2014/10/30/Stability.html [what_is_rust2018]: https://blog.rust-lang.org/2018/07/27/what-is-rust-2018.html -A core tenant of Rust's story is +A core tenet of Rust's story is [*"stability without stagnation"*][stability_stagnation]. We have made great strides sticking to this story while continuously improving the language and the community. This is especially the case with @@ -210,7 +210,7 @@ fn main() { ``` Note that you won't be able to make use of `#[cfg(version(..))` for these -particular features since they were introduced before this RFC's featuers +particular features since they were introduced before this RFC's features get stabilized. However, there will be features in the future to use this mechanism on. From 8477b577cecc689ab486d148780e70202cb55d9a Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 13:37:14 +0200 Subject: [PATCH 06/50] rfc, cfg-path-version: use semver caret requirements. --- text/0000-cfg-path-version.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 0d026721035..245a25dea3f 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -224,19 +224,22 @@ To the `cfg` attribute , a `nightly` flag is added. If and only if a Rust compiler considers itself to be on a nightly channel it the `nightly` flag be considered active. -## `#[cfg(version(""))]` +## `#[cfg(version(""))]` To the `cfg` attribute, a `version` flag is added. This flag requires that a string literal be specified in it inside parenthesis. The string literal must have the format: ``` -version_string ::= \d(.\d)? +semver ::= \d(.\d)?(.\d)? ``` +[caret requirements]: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#caret-requirements + If and only if a Rust compiler considers itself to have a version which is -greater or equal to the version in the `version_string` will the -`#[cfg(version("")]` flag be considered active. +greater or equal to the version in the `semver` string will the +`#[cfg(version("")]` flag be considered active. +Greater or equal is defined in terms of [caret requirements]. ## `#[cfg(path_exists($path))]` From d44a484a9d7bcd05796d8354b314a497e6e0de69 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 14:17:16 +0200 Subject: [PATCH 07/50] rfc, cfg-path-version: bnf consistency. --- text/0000-cfg-path-version.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 245a25dea3f..c244ecc7961 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -231,7 +231,7 @@ This flag requires that a string literal be specified in it inside parenthesis. The string literal must have the format: ``` -semver ::= \d(.\d)?(.\d)? +semver : \d(.\d)?(.\d)? ; ``` [caret requirements]: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#caret-requirements From c06eb9550226468d86e1ca9fe408eb68c1e99f16 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 15:03:38 +0200 Subject: [PATCH 08/50] rfc, cfg-path-version: use version = '..' and motivate it. --- text/0000-cfg-path-version.md | 61 +++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index c244ecc7961..1e2e3d8d90a 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -9,7 +9,7 @@ Permit users to `#[cfg(..)]` on whether: + they are on a `nightly` compiler (`#[cfg(nightly)]`). -+ they have a certain minimum Rust version (`#[cfg(version("1.27"))]`). ++ they have a certain minimum Rust version (`#[cfg(version = "1.27")]`). + a certain external path exists `#[cfg(path_exists(::std::mem::ManuallyDrop))]`. # Motivation @@ -171,7 +171,7 @@ to test if a path exists in some library in the ecosystem. This can for example be useful if you need to support lower minor versions of a library but also add support for features in a higher minor version. -## `#[cfg(version("1.27"))]` +## `#[cfg(version = "1.27")]` Until now, we have only improved our support for library features but never any language features. By checking if we are on a certain minimum version of @@ -179,7 +179,7 @@ Rust or any version above it, we can conditionally support such new features. For example: ```rust -#[cfg_attr(version("1.27"), must_use)] +#[cfg_attr(version = "1.27", must_use)] fn double(x: i32) -> i32 { 2 * x } @@ -194,11 +194,11 @@ fn main() { Another example is opting into the system allocator on Rust 1.28 and beyond: ```rust -#[cfg(version("1.28")) +#[cfg(version = "1.28")] // or: #[cfg(path_exists(::std::alloc::System))] use std::alloc::System; -#[cfg_attr(version("1.28"), global_allocator)] +#[cfg_attr(version = "1.28", global_allocator)] static GLOBAL: System = System; fn main() { @@ -209,7 +209,7 @@ fn main() { } ``` -Note that you won't be able to make use of `#[cfg(version(..))` for these +Note that you won't be able to make use of `#[cfg(version = "..")]` for these particular features since they were introduced before this RFC's features get stabilized. However, there will be features in the future to use this mechanism on. @@ -224,7 +224,7 @@ To the `cfg` attribute , a `nightly` flag is added. If and only if a Rust compiler considers itself to be on a nightly channel it the `nightly` flag be considered active. -## `#[cfg(version(""))]` +## `#[cfg(version = "")]` To the `cfg` attribute, a `version` flag is added. This flag requires that a string literal be specified in it inside parenthesis. @@ -238,7 +238,7 @@ semver : \d(.\d)?(.\d)? ; If and only if a Rust compiler considers itself to have a version which is greater or equal to the version in the `semver` string will the -`#[cfg(version("")]` flag be considered active. +`#[cfg(version = "")]` flag be considered active. Greater or equal is defined in terms of [caret requirements]. ## `#[cfg(path_exists($path))]` @@ -265,7 +265,7 @@ a type constructor, it is possible to use the `::foo::bar::::item` syntax. ## `cfg_attr` and `cfg!` Note that the above sections also apply to the attribute `#[cfg_attr(..)]` -as well as the special macro `cfg!(..)` in that `nightly`, `version(..)`, +as well as the special macro `cfg!(..)` in that `nightly`, `version = ".."`, and `path_exists(..)` are added to those as well. # Drawbacks @@ -303,11 +303,11 @@ or not via an indirection, we can just check if the path exists directly. This way, a user does not have to look up the minimum version number for the feature. -You may think that `version(..)` subsumes `path_exists(..)`. +You may think that `version = ".."` subsumes `path_exists(..)`. However, we argue that it does not. This is the case because at the time of enabling the `nightly` feature that enables the path in the standard library, we do not yet know what minimum version it will be supported under. -If we try to support it with `version(..)`, it is possible that we may +If we try to support it with `version = ".."`, it is possible that we may need to update the minimum version some small number of times. However, doing so even once means that you will need to release new versions of your crate. If you instead use `path_exists(..)` you won't need to use @@ -339,14 +339,14 @@ However, as we've argued and demonstrated in the [guide-level-explanation], the ability to `#[cfg(nightly)]` really shines when used in conjunction with `#[cfg(path_exists($path))]`. -## `version(..)` +## `version = ".."` -When it comes to `version(..)`, it is needed to support conditional compilation +When it comes to `version = ".."`, it is needed to support conditional compilation of language features as opposed to library features as previously shown. -Also, as we've seen, `version(..)` does not subsume `path_exists(..)` but is +Also, as we've seen, `version = ".."` does not subsume `path_exists(..)` but is rather a complementary mechanism. -One problem specific to `version(..)` is that it might get too `rustc` specific. +One problem specific to `version = ".."` is that it might get too `rustc` specific. It might be difficult for other Rust implementations than `rustc` to work with this version numbering as libraries will compile against `rustc`s release numbering. However, it is possible for other implementations to follow @@ -355,7 +355,26 @@ too unreasonable as we can expect `rustc` to be the reference implementation and that other ones will probably lag behind. Indeed, this is the experience with `GHC` and alternative Haskell compilers. -### The bikeshed +### The bikeshed - Argument syntax + +We have two options with respect to how the `version` flag may be specified: + +1. `version = ""` +2. `version("")` + +The syntax in 2. is currently an error in `#[cfg(..)]` as you may witness with: + +```rust +// error[E0565]: unsupported literal +#[cfg(abracadabra("1.27"))] fn bar() {} +``` + +We could allow this syntax. However, we have chosen the syntax in 1. +This with consistency with flags such as `target_feature = "bmi"`. +Another reason to go with `version = ".."` is that `version("..")` +looks like a list. + +### The bikeshed - Attribute name Naturally, there are other possible names for the flag. For example: @@ -368,9 +387,9 @@ while also short and sweet. However, `min_version` is a good alternative to consider because it telegraphs the `>=` nature of the flag. As for the `version_string` syntax, it could also be adjusted such that -you could write `version(">= 1.27")`. We could also support exact version +you could write `version = ">= 1.27"`. We could also support exact version checking (`==`) as well as checking if the compiler is below a certain version -(`<=`). However, as a first iteration, `version("1.27")` is simple and covers +(`<=`). However, as a first iteration, `version = "1.27"` is simple and covers most use cases. ## [version_check] as an alternative @@ -419,7 +438,7 @@ fn main() { } ``` -The [version_check] crate also supports testing for a minimum `version(..)` with: +The [version_check] crate also supports testing for a minimum `version = ".."` with: ```rust extern crate version_check; @@ -433,12 +452,12 @@ However, this is quite verbose in comparison and requires you to invent ad-hoc and crate-specific names for your `#[cfg(..)]` flags such as `MIN_COMPILER_1_13` that will not be the same for every crate. You will also need to repeat this per version you want to support. -This causes the mechanism to scale poorly as compared to `version("1.27")` +This causes the mechanism to scale poorly as compared to `version = "1.27"` which we argue is simple and intuitive. ## Conditional compilation on feature gates -An alternative to `version(..)` and `path_exists(..)` is to allow users +An alternative to `version = ".."` and `path_exists(..)` is to allow users to query where a certain feature gate is stable or not. However, it has been argued that allowing this would essentially stabilize the names of the gates which we've historically not done. From eb3ec271b7a4426f84e2ac7d1b9cc75ebc8428d7 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 15:08:50 +0200 Subject: [PATCH 09/50] rfc, cfg-path-version: define 'nightly' in terms of #![feature(..)] --- text/0000-cfg-path-version.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 1e2e3d8d90a..2dfa22024e5 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -221,8 +221,8 @@ mechanism on. To the `cfg` attribute , a `nightly` flag is added. -If and only if a Rust compiler considers itself to be on a nightly channel -it the `nightly` flag be considered active. +If and only if a Rust compiler permits a user to specify `#![feature(..)]` +will the `nightly` flag be considered active. ## `#[cfg(version = "")]` From 65dd1392594e639a46612dfc2e21c4091bf5e860 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 15:13:41 +0200 Subject: [PATCH 10/50] rfc, cfg-path-version: note that path_exists only sees public things as existing. --- text/0000-cfg-path-version.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 2dfa22024e5..d772b67ea5b 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -252,8 +252,8 @@ This restriction exists to ensure that the user does not try to conditionally compile against parts of their own crate because that crate has not been compiled when the `path_exists` flag is checked on an item. -If and only if the path referred to by `$path` does exist will the -`#[cfg(path_exists($path)]` flag be considered active. +If and only if the path referred to by `$path` does exist and is public +will the `#[cfg(path_exists($path)]` flag be considered active. In checking whether the path exists or not, the compiler will consider feature gated items to exist if the gate has been enabled. If a path refers to an item inside an inherent implementation, From 144ff9316c7e6c78ee515a9b6aa0f86e0fb033e9 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 15:29:28 +0200 Subject: [PATCH 11/50] rfc, cfg-path-version: use path_exists = '..'. --- text/0000-cfg-path-version.md | 36 +++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index d772b67ea5b..8f9b2c18636 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -10,7 +10,7 @@ Permit users to `#[cfg(..)]` on whether: + they are on a `nightly` compiler (`#[cfg(nightly)]`). + they have a certain minimum Rust version (`#[cfg(version = "1.27")]`). -+ a certain external path exists `#[cfg(path_exists(::std::mem::ManuallyDrop))]`. ++ a certain external path exists `#[cfg(path_exists = ::std::mem::ManuallyDrop)]`. # Motivation [motivation]: #motivation @@ -67,7 +67,7 @@ of *control* over what is supported and what is not. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation -## `#[cfg(nightly)]` and `#[cfg(path_exists($path)]` +## `#[cfg(nightly)]` and `#[cfg(path_exists = $path)]` Consider for a moment that we would like to use the `Iterator::flatten` method of the standard library if it exists, but otherwise fall back to @@ -76,12 +76,12 @@ method of the standard library if it exists, but otherwise fall back to ```rust #![cfg_attr(nightly, feature(iterator_flatten))] -#[cfg(path_exists(::std::iter::Flatten))] +#[cfg(path_exists = ::std::iter::Flatten)] fn make_iter(limit: u8) -> impl Iterator { (0..limit).map(move |x| (x..limit)).flatten() } -#[cfg(not(path_exists(::std::iter::Flatten)))] +#[cfg(not(path_exists = ::std::iter::Flatten))] fn make_iter() { use itertools::Itertools; (0..limit).map(move |x| (x..limit)).flatten() @@ -124,7 +124,7 @@ macro_rules! numeric_api { ($typ:ident) => { ... - #[cfg(path_exists(::core::ops::RangeInclusive))] + #[cfg(path_exists = ::core::ops::RangeInclusive)] impl Strategy for ::core::ops::RangeInclusive<$typ> { type Tree = BinarySearch; type Value = $typ; @@ -165,7 +165,7 @@ language without having to release a new breaking change version. Dependents of `proptest` simply need to be on a compiler version where `::core::ops::RangeInclusive` is defined to take advantage of this. -So far we have only used `path_exists(..)` to refer to paths in the standard +So far we have only used `path_exists = ..` to refer to paths in the standard library. However, while it will be a less likely use case, you can use the flag to test if a path exists in some library in the ecosystem. This can for example be useful if you need to support lower minor versions of a library but also @@ -195,7 +195,7 @@ Another example is opting into the system allocator on Rust 1.28 and beyond: ```rust #[cfg(version = "1.28")] -// or: #[cfg(path_exists(::std::alloc::System))] +// or: #[cfg(path_exists = ::std::alloc::System)] use std::alloc::System; #[cfg_attr(version = "1.28", global_allocator)] @@ -241,7 +241,7 @@ greater or equal to the version in the `semver` string will the `#[cfg(version = "")]` flag be considered active. Greater or equal is defined in terms of [caret requirements]. -## `#[cfg(path_exists($path))]` +## `#[cfg(path_exists = $path)]` To the `cfg` attribute, a `path_exists` flag is added. This flag requires that a `path` fragment be specified in it inside parenthesis @@ -253,7 +253,7 @@ conditionally compile against parts of their own crate because that crate has not been compiled when the `path_exists` flag is checked on an item. If and only if the path referred to by `$path` does exist and is public -will the `#[cfg(path_exists($path)]` flag be considered active. +will the `#[cfg(path_exists = $path)]` flag be considered active. In checking whether the path exists or not, the compiler will consider feature gated items to exist if the gate has been enabled. If a path refers to an item inside an inherent implementation, @@ -266,7 +266,7 @@ a type constructor, it is possible to use the `::foo::bar::::item` syntax. Note that the above sections also apply to the attribute `#[cfg_attr(..)]` as well as the special macro `cfg!(..)` in that `nightly`, `version = ".."`, -and `path_exists(..)` are added to those as well. +and `path_exists = ..` are added to those as well. # Drawbacks [drawbacks]: #drawbacks @@ -282,7 +282,7 @@ that may not have certain paths defined. It sometimes happens that feature gates never make it to stable and that they instead get scrapped. This occurs infrequently. However, when this does happen, code that is conditionally compiled under -`path_exists(::std::the::obsoleted::path)` will become garbage that just +`path_exists = ::std::the::obsoleted::path` will become garbage that just sits around. Over time, this garbage can grow to a non-trivial amount. However, if we provide LTS channels in the style of [RFC 2483], @@ -292,7 +292,7 @@ of definitions that won't be used when the LTS version changes. # Rationale and alternatives [alternatives]: #rationale-and-alternatives -## `path_exists(..)` +## `path_exists = ..` The primary rationale for the `path_exists` mechanism is that when you want to support some library feature, it is some path you are thinking of @@ -303,14 +303,14 @@ or not via an indirection, we can just check if the path exists directly. This way, a user does not have to look up the minimum version number for the feature. -You may think that `version = ".."` subsumes `path_exists(..)`. +You may think that `version = ".."` subsumes `path_exists = ..`. However, we argue that it does not. This is the case because at the time of enabling the `nightly` feature that enables the path in the standard library, we do not yet know what minimum version it will be supported under. If we try to support it with `version = ".."`, it is possible that we may need to update the minimum version some small number of times. However, doing so even once means that you will need to release new versions -of your crate. If you instead use `path_exists(..)` you won't need to use +of your crate. If you instead use `path_exists = ..` you won't need to use it even once unless the name of the path changes in-between. ### The bikeshed @@ -337,13 +337,13 @@ an `unstable` feature in `Cargo.toml`. An example of this is provided by the However, as we've argued and demonstrated in the [guide-level-explanation], the ability to `#[cfg(nightly)]` really shines when used in conjunction with -`#[cfg(path_exists($path))]`. +`#[cfg(path_exists ? $path)]`. ## `version = ".."` When it comes to `version = ".."`, it is needed to support conditional compilation of language features as opposed to library features as previously shown. -Also, as we've seen, `version = ".."` does not subsume `path_exists(..)` but is +Also, as we've seen, `version = ".."` does not subsume `path_exists = ..` but is rather a complementary mechanism. One problem specific to `version = ".."` is that it might get too `rustc` specific. @@ -457,12 +457,12 @@ which we argue is simple and intuitive. ## Conditional compilation on feature gates -An alternative to `version = ".."` and `path_exists(..)` is to allow users +An alternative to `version = ".."` and `path_exists = ..` is to allow users to query where a certain feature gate is stable or not. However, it has been argued that allowing this would essentially stabilize the names of the gates which we've historically not done. -We also argue that `path_exists(..)` is more intuitive because it is more +We also argue that `path_exists = ..` is more intuitive because it is more natural to think of a feature in terms of how you would make use of it (via its path) rather than the sometimes somewhat arbitrarily named feature gate. From 13bdfac705894e448d41e9e0f434c5c64f77fa87 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 15:32:57 +0200 Subject: [PATCH 12/50] rfc, cfg-path-version: note on canary builds. --- text/0000-cfg-path-version.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 8f9b2c18636..623b33e7682 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -55,8 +55,9 @@ compiler or to check if you are above a certain compiler version, such facilities are not particularly ergonomic at the moment. In particular, they require the setting up of a `build.rs` file and declaring up-front which versions you are interested in knowing about. -These tools are also unable to check if a certain path exists and instead -force you to know which version they were introduced in. +These tools are also unable to check, without performing canary builds +of simple programs with `use ::std::some::path;`, if a certain path exists +and instead force you to know which version they were introduced in. *We can do better.* In this RFC we aim to rectify this by giving library authors the tools they need in the language itself. With the features From c54db7fa3501c6757353f2eefd521e43e3fc0b62 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 16:29:06 +0200 Subject: [PATCH 13/50] rfc, cfg-path-version: meta grammar changes.. --- text/0000-cfg-path-version.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 623b33e7682..06194486e2e 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -263,6 +263,30 @@ parameters can lead to the item. To check whether an item exists for an implementation with a specific sequence of concrete types applied to a type constructor, it is possible to use the `::foo::bar::::item` syntax. +The `#[cfg(path_exists = $path)]` syntax is not currently supported by the +meta grammar. To make it legal we extend the grammar from: + +```abnf +named_value : "=" lit ; + +meta_or_lit : meta | lit ; +meta_or_lit_list : meta_or_lit "," meta_or_lit_list ","? ; +meta_list : "(" meta_or_lit_list ")" ; +meta : path ( named_value | meta_list )? ; +``` + +into: + +```abnf +lit_or_path : path | lit ; +named_value : "=" lit_or_path ; + +meta_or_lit : meta | lit ; +meta_or_lit_list : meta_or_lit "," meta_or_lit_list ","? ; +meta_list : "(" meta_or_lit_list ")" ; +meta : path ( named_value | meta_list )? ; +``` + ## `cfg_attr` and `cfg!` Note that the above sections also apply to the attribute `#[cfg_attr(..)]` From a5f69b99b84265750d135101b8e3a7326fd96a51 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 16:31:52 +0200 Subject: [PATCH 14/50] rfc, cfg-path-version: optional dependencies are out of scope. --- text/0000-cfg-path-version.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 06194486e2e..f358653766d 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -527,5 +527,7 @@ main = putStrLn version # Unresolved questions [unresolved]: #unresolved-questions +The ability to have optional cargo dependencies is out of scope for this RFC. + 1. What should the flags be named? This can be deferred to after the RFC if need be so that we may gain usage experience. \ No newline at end of file From 8832ea4b571a20b0f3b81034d64f8e36e4c6c05c Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 16:49:44 +0200 Subject: [PATCH 15/50] rfc, cfg-path-version: note use case of working around compiler bugs. --- text/0000-cfg-path-version.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index f358653766d..037cc9a33b6 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -65,6 +65,9 @@ proposed in the [summary] we aim to make retaining *compatibility* and supporting more compiler versions *pain-free* and to give authors a lot of *control* over what is supported and what is not. +A minor use case this RFC supports is to work around compiler bugs by +checking if we are on a particular version. + # Guide-level explanation [guide-level-explanation]: #guide-level-explanation From fb30214ab0213eb057a72f90ca9bcb4f11d1a762 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 17:00:37 +0200 Subject: [PATCH 16/50] rfc, cfg-path-version: talk about the lint. --- text/0000-cfg-path-version.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 037cc9a33b6..9e4b0cb6918 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -533,4 +533,9 @@ main = putStrLn version The ability to have optional cargo dependencies is out of scope for this RFC. 1. What should the flags be named? This can be deferred to after the RFC if - need be so that we may gain usage experience. \ No newline at end of file + need be so that we may gain usage experience. + +2. Could we somehow have an allow-by-default lint that says + *"these paths don't exist"* which could be enabled on `cfg_attr(nightly)`? + This would be done to mitigate the accumulation of garbage code as + discussed in the [drawbacks]. \ No newline at end of file From 7201651f826e1b5374090bc30078c2b18ab77304 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 17:05:18 +0200 Subject: [PATCH 17/50] rfc, cfg-path-version: crater stuff not a drawback. --- text/0000-cfg-path-version.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 9e4b0cb6918..5174329ce38 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -299,11 +299,9 @@ and `path_exists = ..` are added to those as well. # Drawbacks [drawbacks]: #drawbacks -## It becomes harder to `crater` - -As we get more version based conditional compilation, it can get harder to -use the `crater` tool because regressions are not tested on an old compiler -that may not have certain paths defined. +One argument is that hypothetically, if the standard library removed +some unstable item, then we might "not notice" if everyone uses it through +`cfg(path_exists = ..)`. ## Incremental garbage code and its collection From 2fce9e6d54ee70181aa0bfc35f5e49f0c844434e Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 17:09:36 +0200 Subject: [PATCH 18/50] rfc, cfg-path-version: clarify re. old versions using this. --- text/0000-cfg-path-version.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 5174329ce38..5da990e3beb 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -215,8 +215,9 @@ fn main() { Note that you won't be able to make use of `#[cfg(version = "..")]` for these particular features since they were introduced before this RFC's features -get stabilized. However, there will be features in the future to use this -mechanism on. +get stabilized. This means that you can't for example add `version = "1.28"` +to your code and expect Rust 1.28 compilers to enable the code. +However, there will be features in the future to use this mechanism on. # Reference-level explanation [reference-level-explanation]: #reference-level-explanation From fdf3ed79dbd5ab165e1a75e8205874efe9a97293 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 17:15:57 +0200 Subject: [PATCH 19/50] rfc, cfg-path-version: implementation questions re. path_exists. --- text/0000-cfg-path-version.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 5da990e3beb..37168fadddd 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -537,4 +537,7 @@ The ability to have optional cargo dependencies is out of scope for this RFC. 2. Could we somehow have an allow-by-default lint that says *"these paths don't exist"* which could be enabled on `cfg_attr(nightly)`? This would be done to mitigate the accumulation of garbage code as - discussed in the [drawbacks]. \ No newline at end of file + discussed in the [drawbacks]. + +3. Is it technically feasible to implement `path_exists(..)`? + For example it could be hard if cfg-stripping runs before resolving things. From 6442367434c8d52993b60fc336f43817bf9fdf33 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 17:33:02 +0200 Subject: [PATCH 20/50] rfc, cfg-path-version: justify meta grammar changes. --- text/0000-cfg-path-version.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 37168fadddd..fe98be58cfc 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -340,6 +340,14 @@ However, doing so even once means that you will need to release new versions of your crate. If you instead use `path_exists = ..` you won't need to use it even once unless the name of the path changes in-between. +### Extended rationale for `= $path` + +To permit `path_exists = ::foo::bar` we had to extend the meta grammar. +To justify this change, we observe that crates such as `serde_derive` permit +users to write things like `#[serde(default = "some::function")]`. +By changing the grammar we can allow users to instead write: +`#[serde(default = some::function)]`. + ### The bikeshed One might consider other names for the flag instead of `path_exist`. From de60af497b82d38a5f5c48f2cd85ddec3e7659ce Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 17:41:44 +0200 Subject: [PATCH 21/50] rfc, cfg-path-version: justify preventing relative paths. --- text/0000-cfg-path-version.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index fe98be58cfc..56c3366edfe 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -340,6 +340,24 @@ However, doing so even once means that you will need to release new versions of your crate. If you instead use `path_exists = ..` you won't need to use it even once unless the name of the path changes in-between. +### Preventing relative paths + +The reason why we have enforced that all paths must start with `::` inside +`path_exists(..)` is that if we allow relative paths, and users write +`path_exists(self::foo)`, then they can construct situations such as: + +``` +#[cfg(path_exists(self::bar)] +fn foo() {} + +#[cfg(path_exists(self::foo)] +fn bar() {} +``` + +One way around this is to collect all items before `cfg`-stripping, +but this can cause problems with respect to stage separation. +Therefore, we prevent this from occurring with a simple syntactic check. + ### Extended rationale for `= $path` To permit `path_exists = ::foo::bar` we had to extend the meta grammar. From 72ee5fa4a169851dfc01d25a8458a99284b0aac3 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 11 Aug 2018 17:50:42 +0200 Subject: [PATCH 22/50] rfc, cfg-path-version: justify nightly instead of if_possible_feature. --- text/0000-cfg-path-version.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 56c3366edfe..bcb513d9432 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -392,6 +392,15 @@ However, as we've argued and demonstrated in the [guide-level-explanation], the ability to `#[cfg(nightly)]` really shines when used in conjunction with `#[cfg(path_exists ? $path)]`. +### Alternative `#![if_possible_feature()]` + +As an alternative to `#[cfg_attr(nightly, feature())]` +we could permit the user to write `#![if_possible_feature()]`. +The advantage of this is that it is quite direct with respect to intent. +However, adding this in terms of `nightly` already has precedent in +[version_check]. In addition, `nightly` also composes with other flags +using `any`, `not`, and `all`. + ## `version = ".."` When it comes to `version = ".."`, it is needed to support conditional compilation From 77bf1d644ecaa5d3c5a82de8f9c67d72c88905ab Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 12 Aug 2018 07:03:47 +0200 Subject: [PATCH 23/50] rfc, cfg-path-version: path_exists -> accessible + talk about fields. --- text/0000-cfg-path-version.md | 182 ++++++++++++++++++++++------------ 1 file changed, 119 insertions(+), 63 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index bcb513d9432..4e8a405e4b3 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -10,7 +10,7 @@ Permit users to `#[cfg(..)]` on whether: + they are on a `nightly` compiler (`#[cfg(nightly)]`). + they have a certain minimum Rust version (`#[cfg(version = "1.27")]`). -+ a certain external path exists `#[cfg(path_exists = ::std::mem::ManuallyDrop)]`. ++ a certain external path exists `#[cfg(accessible(::std::mem::ManuallyDrop))]`. # Motivation [motivation]: #motivation @@ -71,7 +71,7 @@ checking if we are on a particular version. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation -## `#[cfg(nightly)]` and `#[cfg(path_exists = $path)]` +## `#[cfg(nightly)]` and `#[cfg(accessible($path))]` Consider for a moment that we would like to use the `Iterator::flatten` method of the standard library if it exists, but otherwise fall back to @@ -80,12 +80,12 @@ method of the standard library if it exists, but otherwise fall back to ```rust #![cfg_attr(nightly, feature(iterator_flatten))] -#[cfg(path_exists = ::std::iter::Flatten)] +#[cfg(accessible(::std::iter::Flatten))] fn make_iter(limit: u8) -> impl Iterator { (0..limit).map(move |x| (x..limit)).flatten() } -#[cfg(not(path_exists = ::std::iter::Flatten))] +#[cfg(not(accessible(::std::iter::Flatten)))] fn make_iter() { use itertools::Itertools; (0..limit).map(move |x| (x..limit)).flatten() @@ -99,7 +99,7 @@ fn main() { What this snippet does is the following: 1. If you happen to be on a nightly compiler, but not otherwise, - the feature `itertator_flatten` will be enabled. + the feature `iterator_flatten` will be enabled. 2. If the path `::std::iter::Flatten` exists, the compiler will compile the first version of `make_iter`. If the path does not exist, @@ -115,7 +115,7 @@ switch to the libstd version when it is available in the future. [`proptest`]: https://github.com/altsysrq/proptest [adding support]: https://github.com/AltSysrq/proptest/blob/67945c89e09f8223ae945cc8da029181822ce27e/src/num.rs#L66-L76 -In this case we used the `nightly` and `path_exists` flags to handle a problem +In this case we used the `nightly` and `accessible` flags to handle a problem that the addition of `Iterator::flatten` caused for us if we had used `Itertools::flatten`. We can also use these mechanisms for strictly additive cases as well. Consider for example the [`proptest`] crate [adding support] @@ -128,7 +128,7 @@ macro_rules! numeric_api { ($typ:ident) => { ... - #[cfg(path_exists = ::core::ops::RangeInclusive)] + #[cfg(accessible(::core::ops::RangeInclusive))] impl Strategy for ::core::ops::RangeInclusive<$typ> { type Tree = BinarySearch; type Value = $typ; @@ -169,7 +169,7 @@ language without having to release a new breaking change version. Dependents of `proptest` simply need to be on a compiler version where `::core::ops::RangeInclusive` is defined to take advantage of this. -So far we have only used `path_exists = ..` to refer to paths in the standard +So far we have only used `accessible(..)` to refer to paths in the standard library. However, while it will be a less likely use case, you can use the flag to test if a path exists in some library in the ecosystem. This can for example be useful if you need to support lower minor versions of a library but also @@ -199,7 +199,7 @@ Another example is opting into the system allocator on Rust 1.28 and beyond: ```rust #[cfg(version = "1.28")] -// or: #[cfg(path_exists = ::std::alloc::System)] +// or: #[cfg(accessible(::std::alloc::System))] use std::alloc::System; #[cfg_attr(version = "1.28", global_allocator)] @@ -246,71 +246,75 @@ greater or equal to the version in the `semver` string will the `#[cfg(version = "")]` flag be considered active. Greater or equal is defined in terms of [caret requirements]. -## `#[cfg(path_exists = $path)]` +## `#[cfg(accessible($path))]` -To the `cfg` attribute, a `path_exists` flag is added. +To the `cfg` attribute, a `accessible` flag is added. This flag requires that a `path` fragment be specified in it inside parenthesis but not inside a string literal. The `$path` must start with leading `::` and may not refer to any parts of the own crate (e.g. with `::crate::foo`, `::self::foo`, or `::super::foo` if such paths are legal). This restriction exists to ensure that the user does not try to conditionally compile against parts of their own crate because that crate -has not been compiled when the `path_exists` flag is checked on an item. +has not been compiled when the `accessible` flag is checked on an item. If and only if the path referred to by `$path` does exist and is public -will the `#[cfg(path_exists = $path)]` flag be considered active. +will the `#[cfg(accessible($path))]` flag be considered active. In checking whether the path exists or not, the compiler will consider feature gated items to exist if the gate has been enabled. + If a path refers to an item inside an inherent implementation, the path will be considered to exist if any configuration of generic parameters can lead to the item. To check whether an item exists for an implementation with a specific sequence of concrete types applied to a type constructor, it is possible to use the `::foo::bar::::item` syntax. -The `#[cfg(path_exists = $path)]` syntax is not currently supported by the -meta grammar. To make it legal we extend the grammar from: +It is also possible to refer to fields of `struct`s, `enum`s, and `unions`. +Assuming that we have the following definitions in the `foobar` crate: -```abnf -named_value : "=" lit ; +```rust +pub struct Person { pub ssn: SSN, age: u16 } -meta_or_lit : meta | lit ; -meta_or_lit_list : meta_or_lit "," meta_or_lit_list ","? ; -meta_list : "(" meta_or_lit_list ")" ; -meta : path ( named_value | meta_list )? ; +pub enum Shape { + Triangle { pub sides: [Unit; 3] }, + ... +} + +pub union MaybeUninit { uninit: (), pub value: T } ``` -into: +We can then refer to them like so: -```abnf -lit_or_path : path | lit ; -named_value : "=" lit_or_path ; - -meta_or_lit : meta | lit ; -meta_or_lit_list : meta_or_lit "," meta_or_lit_list ","? ; -meta_list : "(" meta_or_lit_list ")" ; -meta : path ( named_value | meta_list )? ; +```rust +#[cfg(all( + accessible(foobar::Person::ssn), + accessible(foobar::Shape::Triangle::sides), + accessible(foobar::Shape::MaybeUninit::value) +))] +fn do_stuff() { + ... +} ``` ## `cfg_attr` and `cfg!` Note that the above sections also apply to the attribute `#[cfg_attr(..)]` as well as the special macro `cfg!(..)` in that `nightly`, `version = ".."`, -and `path_exists = ..` are added to those as well. +and `accessible(..)` are added to those as well. # Drawbacks [drawbacks]: #drawbacks One argument is that hypothetically, if the standard library removed some unstable item, then we might "not notice" if everyone uses it through -`cfg(path_exists = ..)`. +`#[cfg(accessible(..))]`. ## Incremental garbage code and its collection It sometimes happens that feature gates never make it to stable and that they instead get scrapped. This occurs infrequently. However, when this does happen, code that is conditionally compiled under -`path_exists = ::std::the::obsoleted::path` will become garbage that just -sits around. Over time, this garbage can grow to a non-trivial amount. +`#[cfg(accessible(::std::the::obsoleted::path))]` will become garbage that +just sits around. Over time, this garbage can grow to a non-trivial amount. However, if we provide LTS channels in the style of [RFC 2483], then there are opportunities to perform some "garbage collection" @@ -319,9 +323,9 @@ of definitions that won't be used when the LTS version changes. # Rationale and alternatives [alternatives]: #rationale-and-alternatives -## `path_exists = ..` +## `accessible(..)` -The primary rationale for the `path_exists` mechanism is that when you +The primary rationale for the `accessible` mechanism is that when you want to support some library feature, it is some path you are thinking of rather than what version it was added. For example, if you want to use `ManuallyDrop`, you can just ask if it exists. The `version` is instead a @@ -330,27 +334,27 @@ or not via an indirection, we can just check if the path exists directly. This way, a user does not have to look up the minimum version number for the feature. -You may think that `version = ".."` subsumes `path_exists = ..`. +You may think that `version = ".."` subsumes `accessible(..)`. However, we argue that it does not. This is the case because at the time of enabling the `nightly` feature that enables the path in the standard library, we do not yet know what minimum version it will be supported under. If we try to support it with `version = ".."`, it is possible that we may need to update the minimum version some small number of times. However, doing so even once means that you will need to release new versions -of your crate. If you instead use `path_exists = ..` you won't need to use +of your crate. If you instead use `accessible(..)` you won't need to use it even once unless the name of the path changes in-between. ### Preventing relative paths The reason why we have enforced that all paths must start with `::` inside -`path_exists(..)` is that if we allow relative paths, and users write -`path_exists(self::foo)`, then they can construct situations such as: +`accessible(..)` is that if we allow relative paths, and users write +`accessible(self::foo)`, then they can construct situations such as: ``` -#[cfg(path_exists(self::bar)] +#[cfg(accessible(self::bar)] fn foo() {} -#[cfg(path_exists(self::foo)] +#[cfg(accessible(self::foo)] fn bar() {} ``` @@ -358,28 +362,72 @@ One way around this is to collect all items before `cfg`-stripping, but this can cause problems with respect to stage separation. Therefore, we prevent this from occurring with a simple syntactic check. -### Extended rationale for `= $path` +### `#[cfg(accessible(..))` or `#[cfg(accessible = ..)` + +We need to decide between the syntax `accessible(..)` or `accessible = ..`. +The reason we've opted for the former rather than the latter is that the +former syntax looks more like a question/query whilst the latter looks more +like a statement of fact. -To permit `path_exists = ::foo::bar` we had to extend the meta grammar. -To justify this change, we observe that crates such as `serde_derive` permit -users to write things like `#[serde(default = "some::function")]`. -By changing the grammar we can allow users to instead write: -`#[serde(default = some::function)]`. +In addition, if we would like to enable `accessible = $path` we would need to +extend the meta grammar. We could justify that change in and of itself by +observing that crates such as `serde_derive` permit users to write things like +`#[serde(default = "some::function")]`. By changing the grammar we can allow +users to instead write: `#[serde(default = some::function)]`. +However, in this case, `accessible($path)` seems the optimal notation. + +If we would like to extend the meta grammar, we could do so by changing: + +```abnf +named_value : "=" lit ; + +meta_or_lit : meta | lit ; +meta_or_lit_list : meta_or_lit "," meta_or_lit_list ","? ; +meta_list : "(" meta_or_lit_list ")" ; +meta : path ( named_value | meta_list )? ; +``` + +into: + +```abnf +lit_or_path : path | lit ; +named_value : "=" lit_or_path ; + +meta_or_lit : meta | lit ; +meta_or_lit_list : meta_or_lit "," meta_or_lit_list ","? ; +meta_list : "(" meta_or_lit_list ")" ; +meta : path ( named_value | meta_list )? ; +``` ### The bikeshed -One might consider other names for the flag instead of `path_exist`. +One might consider other names for the flag instead of `accessible`. Some contenders are: -+ `has_path` -+ `has_item` ++ `path_accessible` ++ `can_use` ++ `path_exists` ++ `have_path` ++ `have` ++ `have_item` + `path_reachable` + `item_reachable` + `item_exists` -Currently `path_exists` is the choice because it clearly signals the intent. +Currently `accessible` is the choice because it clearly signals the intent +while also being short enough to remain ergonomic to use. +In particular, while `path_accessible` might be somewhat more unambiguous, +we argue that from the context of seeing `accessible(::std::foo::bar)` +it is clear that it is paths we are talking about because the argument +is a path and not something else. + +While `can_use` is also a strong contender, we reject this option because +it may imply to the user that only things that you may `use $path;` can +go in there. Meanwhile, you may `#[cfg(accessible(::foo::MyTrait::my_method))` +which is *not* possible as `use ::foo::MyTrait::my_method;`. This also applies +to other associated items and inherent methods as well as `struct` fields. -## `nightly` +## `#[cfg(nightly)` [dbg]: https://crates.io/crates/dbg @@ -390,7 +438,7 @@ an `unstable` feature in `Cargo.toml`. An example of this is provided by the However, as we've argued and demonstrated in the [guide-level-explanation], the ability to `#[cfg(nightly)]` really shines when used in conjunction with -`#[cfg(path_exists ? $path)]`. +`#[cfg(accessible($path))]`. ### Alternative `#![if_possible_feature()]` @@ -401,11 +449,11 @@ However, adding this in terms of `nightly` already has precedent in [version_check]. In addition, `nightly` also composes with other flags using `any`, `not`, and `all`. -## `version = ".."` +## `#[cfg(version = "..")` When it comes to `version = ".."`, it is needed to support conditional compilation of language features as opposed to library features as previously shown. -Also, as we've seen, `version = ".."` does not subsume `path_exists = ..` but is +Also, as we've seen, `version = ".."` does not subsume `accessible(..)` but is rather a complementary mechanism. One problem specific to `version = ".."` is that it might get too `rustc` specific. @@ -519,12 +567,12 @@ which we argue is simple and intuitive. ## Conditional compilation on feature gates -An alternative to `version = ".."` and `path_exists = ..` is to allow users +An alternative to `version = ".."` and `accessible(..)` is to allow users to query where a certain feature gate is stable or not. However, it has been argued that allowing this would essentially stabilize the names of the gates which we've historically not done. -We also argue that `path_exists = ..` is more intuitive because it is more +We also argue that `accessible(..)` is more intuitive because it is more natural to think of a feature in terms of how you would make use of it (via its path) rather than the sometimes somewhat arbitrarily named feature gate. @@ -566,13 +614,21 @@ main = putStrLn version The ability to have optional cargo dependencies is out of scope for this RFC. -1. What should the flags be named? This can be deferred to after the RFC if - need be so that we may gain usage experience. - -2. Could we somehow have an allow-by-default lint that says +1. Could we somehow have an allow-by-default lint that says *"these paths don't exist"* which could be enabled on `cfg_attr(nightly)`? This would be done to mitigate the accumulation of garbage code as discussed in the [drawbacks]. -3. Is it technically feasible to implement `path_exists(..)`? +2. Is it technically feasible to implement `accessible(..)`? For example it could be hard if cfg-stripping runs before resolving things. + + @eddyb has indicated that: + + > The good news is that we should be able to resolve that during macro + > expansion nowadays. The bad news is I don't know how hard early stability + > checking would be although, no, we should be able to easily add a + > `DefId -> Option` method somewhere, with enough information to + > check against feature-gates (assuming the set of `#![feature(...)]`s in + > the local crate is known at `cfg`-stripping time). + +3. Should we allow referring to fields of type definitions in `accessible(..)`? \ No newline at end of file From 8c9858a5f0f346cdb9401de1ada6b092bde8acc6 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 12 Aug 2018 07:10:22 +0200 Subject: [PATCH 24/50] rfc, cfg-path-version: refresh start date. --- text/0000-cfg-path-version.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 4e8a405e4b3..d307a5b8fb3 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -1,5 +1,5 @@ - Feature Name: `cfg_path_version` -- Start Date: 2018-08-10 +- Start Date: 2018-08-12 - RFC PR: _ - Rust Issue: _ From 09f23a83b81cbb7b833bb169b12475157e58bc33 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 12 Aug 2018 07:11:05 +0200 Subject: [PATCH 25/50] rfc, cfg-path-version: fix typo --- text/0000-cfg-path-version.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index d307a5b8fb3..ab3fe28d838 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -10,7 +10,7 @@ Permit users to `#[cfg(..)]` on whether: + they are on a `nightly` compiler (`#[cfg(nightly)]`). + they have a certain minimum Rust version (`#[cfg(version = "1.27")]`). -+ a certain external path exists `#[cfg(accessible(::std::mem::ManuallyDrop))]`. ++ a certain external path exists (`#[cfg(accessible(::std::mem::ManuallyDrop))]`). # Motivation [motivation]: #motivation From 7ca8a5fe344a8d74ba4b6fd398918c5fd0235b58 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 12 Aug 2018 07:17:48 +0200 Subject: [PATCH 26/50] rfc, cfg-path-version: reword a bit --- text/0000-cfg-path-version.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index ab3fe28d838..937cec1a6ae 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -10,7 +10,8 @@ Permit users to `#[cfg(..)]` on whether: + they are on a `nightly` compiler (`#[cfg(nightly)]`). + they have a certain minimum Rust version (`#[cfg(version = "1.27")]`). -+ a certain external path exists (`#[cfg(accessible(::std::mem::ManuallyDrop))]`). ++ a certain external path is accessible + (`#[cfg(accessible(::std::mem::ManuallyDrop))]`). # Motivation [motivation]: #motivation From c4d7c2c508c9dddf9722c385c50beaec871ce2c7 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 12 Aug 2018 07:58:17 +0200 Subject: [PATCH 27/50] rfc, cfg-path-version: remove abnf highlighting --- text/0000-cfg-path-version.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 937cec1a6ae..5356233ee28 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -379,7 +379,7 @@ However, in this case, `accessible($path)` seems the optimal notation. If we would like to extend the meta grammar, we could do so by changing: -```abnf +``` named_value : "=" lit ; meta_or_lit : meta | lit ; @@ -390,7 +390,7 @@ meta : path ( named_value | meta_list )? ; into: -```abnf +``` lit_or_path : path | lit ; named_value : "=" lit_or_path ; From 9cedd5a4fece23a457dd260aa56ee8bcd57dc023 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 14 Aug 2018 10:49:58 +0200 Subject: [PATCH 28/50] rfc, cfg-path-version: fix typo. --- text/0000-cfg-path-version.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 5356233ee28..fccdf630f65 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -87,7 +87,7 @@ fn make_iter(limit: u8) -> impl Iterator { } #[cfg(not(accessible(::std::iter::Flatten)))] -fn make_iter() { +fn make_iter(limit: u8) -> impl Iterator { use itertools::Itertools; (0..limit).map(move |x| (x..limit)).flatten() } From 3abb76f7824017248e2512f612c1ab8652eaa992 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 14 Aug 2018 11:10:19 +0200 Subject: [PATCH 29/50] rfc, cfg-path-version: talk about SomeFeature --- text/0000-cfg-path-version.md | 39 +++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index fccdf630f65..b9804b97396 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -321,6 +321,45 @@ However, if we provide LTS channels in the style of [RFC 2483], then there are opportunities to perform some "garbage collection" of definitions that won't be used when the LTS version changes. +## Combining `nightly` and `accessible(..)` + +Consider that a popular library writes: + +```rust +#![cfg_attr(nightly, feature(some_feature))] +#[cfg(accessible(::std::foo:SomeFeature))] +use std::foo::SomeFeature; + +#[cfg(not(accessible(::std::foo:SomeFeature)))] +struct SomeFeature { ... } +``` + +One potential hazard when writing this migrating construct is that +once `SomeFeature` finally gets stabilized, it may have been shipped +in a modified form. Such modification may include changing the names +of `SomeFeature`'s methods, their type signatures, or what trait +implementations exist for `SomeFeature`. + +This problem only occurs when you combine `nightly` and `accessible(..)` +or indeed `nightly` and `version = ".."`. However, there is a risk of breaking +code that worked on one stable release of Rust in one or more versions after. + +A few mitigating factors to consider are: + ++ It is possible to check if the methods of `SomeFeature` are `accessible` + or not by using their paths. This reduces the risk somewhat. + ++ If a crate author runs continuous integration (CI) builds that include + testing the crate on a nightly toolchain, breakage can be detected + well before any crates are broken and a patch release of the crate + can be made which either removes the nightly feature or adjusts the + usage of it. The remaining problem is that dependent crates may have + `Cargo.lock` files that have pinned the patch versions of the crate. + ++ Users should take care not to use this mechanism unless they are fairly + confident that no consequential changes will be made to the library. + A risk still exists, but it is opt-in. + # Rationale and alternatives [alternatives]: #rationale-and-alternatives From 4601f1c4c4619d12cd17061a97b0a8a5683efe93 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 19 Aug 2018 11:07:58 +0200 Subject: [PATCH 30/50] rfc, cfg-path-version: add `usable` to bikeshed. --- text/0000-cfg-path-version.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index b9804b97396..da04368a503 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -445,6 +445,7 @@ One might consider other names for the flag instead of `accessible`. Some contenders are: + `path_accessible` ++ `usable` + `can_use` + `path_exists` + `have_path` @@ -461,8 +462,8 @@ we argue that from the context of seeing `accessible(::std::foo::bar)` it is clear that it is paths we are talking about because the argument is a path and not something else. -While `can_use` is also a strong contender, we reject this option because -it may imply to the user that only things that you may `use $path;` can +While `can_use` and `usable` are also strong contenders, we reject these options +because they may imply to the user that only things that you may `use $path;` can go in there. Meanwhile, you may `#[cfg(accessible(::foo::MyTrait::my_method))` which is *not* possible as `use ::foo::MyTrait::my_method;`. This also applies to other associated items and inherent methods as well as `struct` fields. @@ -671,4 +672,4 @@ The ability to have optional cargo dependencies is out of scope for this RFC. > check against feature-gates (assuming the set of `#![feature(...)]`s in > the local crate is known at `cfg`-stripping time). -3. Should we allow referring to fields of type definitions in `accessible(..)`? \ No newline at end of file +3. Should we allow referring to fields of type definitions in `accessible(..)`? From 4cad91d0501fdb4d99257cb9054c62c496285aa9 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 19 Aug 2018 12:08:40 +0200 Subject: [PATCH 31/50] rfc, cfg-path-version: version = '..' => version(..) --- text/0000-cfg-path-version.md | 68 +++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index da04368a503..05018a5ec01 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -9,7 +9,7 @@ Permit users to `#[cfg(..)]` on whether: + they are on a `nightly` compiler (`#[cfg(nightly)]`). -+ they have a certain minimum Rust version (`#[cfg(version = "1.27")]`). ++ they have a certain minimum Rust version (`#[cfg(version(1.27.0))]`). + a certain external path is accessible (`#[cfg(accessible(::std::mem::ManuallyDrop))]`). @@ -176,7 +176,7 @@ to test if a path exists in some library in the ecosystem. This can for example be useful if you need to support lower minor versions of a library but also add support for features in a higher minor version. -## `#[cfg(version = "1.27")]` +## `#[cfg(version(1.27.0))]` Until now, we have only improved our support for library features but never any language features. By checking if we are on a certain minimum version of @@ -184,7 +184,7 @@ Rust or any version above it, we can conditionally support such new features. For example: ```rust -#[cfg_attr(version = "1.27", must_use)] +#[cfg_attr(version(1.27), must_use)] fn double(x: i32) -> i32 { 2 * x } @@ -199,11 +199,11 @@ fn main() { Another example is opting into the system allocator on Rust 1.28 and beyond: ```rust -#[cfg(version = "1.28")] +#[cfg(version(1.28))] // or: #[cfg(accessible(::std::alloc::System))] use std::alloc::System; -#[cfg_attr(version = "1.28", global_allocator)] +#[cfg_attr(version(1.28), global_allocator)] static GLOBAL: System = System; fn main() { @@ -214,9 +214,9 @@ fn main() { } ``` -Note that you won't be able to make use of `#[cfg(version = "..")]` for these +Note that you won't be able to make use of `#[cfg(version(..))]` for these particular features since they were introduced before this RFC's features -get stabilized. This means that you can't for example add `version = "1.28"` +get stabilized. This means that you can't for example add `version(1.28)` to your code and expect Rust 1.28 compilers to enable the code. However, there will be features in the future to use this mechanism on. @@ -230,7 +230,7 @@ To the `cfg` attribute , a `nightly` flag is added. If and only if a Rust compiler permits a user to specify `#![feature(..)]` will the `nightly` flag be considered active. -## `#[cfg(version = "")]` +## `#[cfg(version())]` To the `cfg` attribute, a `version` flag is added. This flag requires that a string literal be specified in it inside parenthesis. @@ -244,7 +244,7 @@ semver : \d(.\d)?(.\d)? ; If and only if a Rust compiler considers itself to have a version which is greater or equal to the version in the `semver` string will the -`#[cfg(version = "")]` flag be considered active. +`#[cfg(version()]` flag be considered active. Greater or equal is defined in terms of [caret requirements]. ## `#[cfg(accessible($path))]` @@ -299,7 +299,7 @@ fn do_stuff() { ## `cfg_attr` and `cfg!` Note that the above sections also apply to the attribute `#[cfg_attr(..)]` -as well as the special macro `cfg!(..)` in that `nightly`, `version = ".."`, +as well as the special macro `cfg!(..)` in that `nightly`, `version(..)`, and `accessible(..)` are added to those as well. # Drawbacks @@ -341,7 +341,7 @@ of `SomeFeature`'s methods, their type signatures, or what trait implementations exist for `SomeFeature`. This problem only occurs when you combine `nightly` and `accessible(..)` -or indeed `nightly` and `version = ".."`. However, there is a risk of breaking +or indeed `nightly` and `version(..)`. However, there is a risk of breaking code that worked on one stable release of Rust in one or more versions after. A few mitigating factors to consider are: @@ -374,11 +374,11 @@ or not via an indirection, we can just check if the path exists directly. This way, a user does not have to look up the minimum version number for the feature. -You may think that `version = ".."` subsumes `accessible(..)`. +You may think that `version(..)` subsumes `accessible(..)`. However, we argue that it does not. This is the case because at the time of enabling the `nightly` feature that enables the path in the standard library, we do not yet know what minimum version it will be supported under. -If we try to support it with `version = ".."`, it is possible that we may +If we try to support it with `version(..)`, it is possible that we may need to update the minimum version some small number of times. However, doing so even once means that you will need to release new versions of your crate. If you instead use `accessible(..)` you won't need to use @@ -490,14 +490,14 @@ However, adding this in terms of `nightly` already has precedent in [version_check]. In addition, `nightly` also composes with other flags using `any`, `not`, and `all`. -## `#[cfg(version = "..")` +## `#[cfg(version(..))` -When it comes to `version = ".."`, it is needed to support conditional compilation +When it comes to `version(..)`, it is needed to support conditional compilation of language features as opposed to library features as previously shown. -Also, as we've seen, `version = ".."` does not subsume `accessible(..)` but is +Also, as we've seen, `version(..)` does not subsume `accessible(..)` but is rather a complementary mechanism. -One problem specific to `version = ".."` is that it might get too `rustc` specific. +One problem specific to `version(..)` is that it might get too `rustc` specific. It might be difficult for other Rust implementations than `rustc` to work with this version numbering as libraries will compile against `rustc`s release numbering. However, it is possible for other implementations to follow @@ -508,22 +508,27 @@ with `GHC` and alternative Haskell compilers. ### The bikeshed - Argument syntax -We have two options with respect to how the `version` flag may be specified: +We have roughly two options with respect to how the `version` flag may be specified: 1. `version = ""` -2. `version("")` +2. `version()` The syntax in 2. is currently an error in `#[cfg(..)]` as you may witness with: ```rust // error[E0565]: unsupported literal -#[cfg(abracadabra("1.27"))] fn bar() {} +#[cfg(abracadabra(1.27))] fn bar() {} + ^^^^ ``` -We could allow this syntax. However, we have chosen the syntax in 1. -This with consistency with flags such as `target_feature = "bmi"`. -Another reason to go with `version = ".."` is that `version("..")` -looks like a list. +However, we have decided to go with the syntax in 2. because the `cfg` flags +that use the `flag = ".."` syntax are all static as opposed to dynamic. +In other words, the semantics of `cfg(x = "y")` is equivalent to +*"`rustc --print=cfg` contains a `x="y"` line"*. +Therefore, and since `version(..)` entails a dynamic check as opposed +to simple string equality, we use the syntax `version(..)`. + +However, one reason to pick syntax 1. is that `version("..")` looks like a list. ### The bikeshed - Attribute name @@ -537,11 +542,12 @@ We pick the current naming because we believe it is sufficiently clear while also short and sweet. However, `min_version` is a good alternative to consider because it telegraphs the `>=` nature of the flag. -As for the `version_string` syntax, it could also be adjusted such that -you could write `version = ">= 1.27"`. We could also support exact version +As for the `` syntax, it could also be adjusted such that +you could write `version(>= 1.27)`. We could also support exact version checking (`==`) as well as checking if the compiler is below a certain version -(`<=`). However, as a first iteration, `version = "1.27"` is simple and covers -most use cases. +(`<=`). There are also the "tilde requirements" and "wildcard requirements" +that Cargo features that we could add. However, as a first iteration, +`version(1.27.0)` is simple and covers most use cases. ## [version_check] as an alternative @@ -589,7 +595,7 @@ fn main() { } ``` -The [version_check] crate also supports testing for a minimum `version = ".."` with: +The [version_check] crate also supports testing for a minimum `version(..)` with: ```rust extern crate version_check; @@ -603,12 +609,12 @@ However, this is quite verbose in comparison and requires you to invent ad-hoc and crate-specific names for your `#[cfg(..)]` flags such as `MIN_COMPILER_1_13` that will not be the same for every crate. You will also need to repeat this per version you want to support. -This causes the mechanism to scale poorly as compared to `version = "1.27"` +This causes the mechanism to scale poorly as compared to `version(1.27)` which we argue is simple and intuitive. ## Conditional compilation on feature gates -An alternative to `version = ".."` and `accessible(..)` is to allow users +An alternative to `version(..)` and `accessible(..)` is to allow users to query where a certain feature gate is stable or not. However, it has been argued that allowing this would essentially stabilize the names of the gates which we've historically not done. From 457da191c020af85fd00aea0e1365286d97105f1 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 19 Aug 2018 13:32:08 +0200 Subject: [PATCH 32/50] rfc, cfg-path-version: fix eddybs review comments. --- text/0000-cfg-path-version.md | 37 +++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 05018a5ec01..3fbaff9297d 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -521,14 +521,35 @@ The syntax in 2. is currently an error in `#[cfg(..)]` as you may witness with: ^^^^ ``` -However, we have decided to go with the syntax in 2. because the `cfg` flags -that use the `flag = ".."` syntax are all static as opposed to dynamic. -In other words, the semantics of `cfg(x = "y")` is equivalent to -*"`rustc --print=cfg` contains a `x="y"` line"*. -Therefore, and since `version(..)` entails a dynamic check as opposed -to simple string equality, we use the syntax `version(..)`. - -However, one reason to pick syntax 1. is that `version("..")` looks like a list. +[attr_grammar]: https://github.com/rust-lang/rust/blob/097c40cf6e1defc2fc49d521374254ee27f5f1fb/src/libsyntax/parse/attr.rs#L141-L149 + +However, the attribute grammar is [technically][attr_grammar]: + +```rust +attribute : "#" "!"? "[" path attr_inner? "]" ; +attr_inner : "[" token_stream "]" + | "(" token_stream ")" + | "{" token_stream "}" + | "=" token_tree + ; +``` + +Note in particular that `#[my_attribute()]` is a legal production +in the grammar wherefore we can support `#[cfg(version(1.27.0))]` if we so wish. + +[@eddyb]: https://github.com/eddyb + +Given that syntax 2. is possible, we have decided to use it because as [@eddyb] +has noted, the `cfg` flags that use the `flag = ".."` syntax are all static as +opposed to dynamic. In other words, the semantics of `cfg(x = "y")` is that of +checking for a membership test within a fixed set determined ahead of time. +This set is also available through `rustc --print=cfg`. + +What a user may infer from how other `cfg(flag = "..")` flags work is that +`version = ".."` checks for an *exact* version. But as we've seen before, +this interpretation is not the one in this RFC. + +However, one reason to pick syntax 1. is that `version(..)` looks like a list. ### The bikeshed - Attribute name From e7840f455513c9bac10256b3c16d802f442fe670 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 19 Aug 2018 14:00:00 +0200 Subject: [PATCH 33/50] rfc, cfg-path-version: remove #[cfg(nightly)] from proposal. --- text/0000-cfg-path-version.md | 215 ++++++++++++++++++---------------- 1 file changed, 116 insertions(+), 99 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 3fbaff9297d..4592cf86622 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -8,7 +8,6 @@ Permit users to `#[cfg(..)]` on whether: -+ they are on a `nightly` compiler (`#[cfg(nightly)]`). + they have a certain minimum Rust version (`#[cfg(version(1.27.0))]`). + a certain external path is accessible (`#[cfg(accessible(::std::mem::ManuallyDrop))]`). @@ -51,9 +50,8 @@ channels as proposed by [RFC 2483]. [version_check]: https://crates.io/crates/version_check The current support for such conditional compilation is lacking. -While [it is possible][version_check] to check if you are on a nightly -compiler or to check if you are above a certain compiler version, -such facilities are not particularly ergonomic at the moment. +While [it is possible][version_check] to check if you are above a certain +compiler version, such facilities are not particularly ergonomic at the moment. In particular, they require the setting up of a `build.rs` file and declaring up-front which versions you are interested in knowing about. These tools are also unable to check, without performing canary builds @@ -72,14 +70,14 @@ checking if we are on a particular version. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation -## `#[cfg(nightly)]` and `#[cfg(accessible($path))]` +## `#[cfg(accessible($path))]` Consider for a moment that we would like to use the `Iterator::flatten` method of the standard library if it exists, but otherwise fall back to `Itertools::flatten`. We can do that with the following snippet: ```rust -#![cfg_attr(nightly, feature(iterator_flatten))] +#![cfg_attr(feature = "unstable", feature(iterator_flatten))] #[cfg(accessible(::std::iter::Flatten))] fn make_iter(limit: u8) -> impl Iterator { @@ -99,31 +97,33 @@ fn main() { What this snippet does is the following: -1. If you happen to be on a nightly compiler, but not otherwise, - the feature `iterator_flatten` will be enabled. +1. If the cargo feature `unstable` is enabled and assuming the compiler is + capable of feature gates, but not otherwise, the feature `iterator_flatten` + will be enabled. 2. If the path `::std::iter::Flatten` exists, the compiler will compile the first version of `make_iter`. If the path does not exist, the compiler will instead compile the second version of `make_iter`. -The result of 1. and 2. is that your crate can opt into using `Iterator::flatten` -on nightly compilers but use `Itertools::flatten` on stable compilers meanwhile. -Once the standard library has stabilized `iter::Flatten`, future stable compilers -will start using the first version of the function. As a crate author, you -don't have to publish any new versions of your crate for the compiler to -switch to the libstd version when it is available in the future. +The result of 1. and 2. is that your crate can opt into using +`Iterator::flatten` when `feature = "unstable"` is enabled, +but use `Itertools::flatten` on stable compilers meanwhile. +Once the standard library has stabilized `iter::Flatten`, +future stable compilers will start using the first version of the function. +As a crate author, you don't have to publish any new versions of your crate for +the compiler to switch to the libstd version when it is available in the future. [`proptest`]: https://github.com/altsysrq/proptest [adding support]: https://github.com/AltSysrq/proptest/blob/67945c89e09f8223ae945cc8da029181822ce27e/src/num.rs#L66-L76 -In this case we used the `nightly` and `accessible` flags to handle a problem -that the addition of `Iterator::flatten` caused for us if we had used -`Itertools::flatten`. We can also use these mechanisms for strictly additive -cases as well. Consider for example the [`proptest`] crate [adding support] -for `RangeInclusive`: +In this case we used the `feature = "unstable"` and `accessible` flags +to handle a problem that the addition of `Iterator::flatten` caused for us +if we had used `Itertools::flatten`. We can also use these mechanisms for +strictly additive cases as well. Consider for example the [`proptest`] crate +[adding support] for `RangeInclusive`: ```rust -#[cfg_attr(nightly, feature(inclusive_range))] +#[cfg_attr(feature = "unstable", feature(inclusive_range))] macro_rules! numeric_api { ($typ:ident) => { @@ -223,13 +223,6 @@ However, there will be features in the future to use this mechanism on. # Reference-level explanation [reference-level-explanation]: #reference-level-explanation -## `#[cfg(nightly)]` - -To the `cfg` attribute , a `nightly` flag is added. - -If and only if a Rust compiler permits a user to specify `#![feature(..)]` -will the `nightly` flag be considered active. - ## `#[cfg(version())]` To the `cfg` attribute, a `version` flag is added. @@ -298,9 +291,9 @@ fn do_stuff() { ## `cfg_attr` and `cfg!` -Note that the above sections also apply to the attribute `#[cfg_attr(..)]` -as well as the special macro `cfg!(..)` in that `nightly`, `version(..)`, -and `accessible(..)` are added to those as well. +Note that the above sections also apply to the attribute `#[cfg_attr(..)]` as +well as the special macro `cfg!(..)` in that `version(..)` and `accessible(..)` +are added to those as well. # Drawbacks [drawbacks]: #drawbacks @@ -321,45 +314,6 @@ However, if we provide LTS channels in the style of [RFC 2483], then there are opportunities to perform some "garbage collection" of definitions that won't be used when the LTS version changes. -## Combining `nightly` and `accessible(..)` - -Consider that a popular library writes: - -```rust -#![cfg_attr(nightly, feature(some_feature))] -#[cfg(accessible(::std::foo:SomeFeature))] -use std::foo::SomeFeature; - -#[cfg(not(accessible(::std::foo:SomeFeature)))] -struct SomeFeature { ... } -``` - -One potential hazard when writing this migrating construct is that -once `SomeFeature` finally gets stabilized, it may have been shipped -in a modified form. Such modification may include changing the names -of `SomeFeature`'s methods, their type signatures, or what trait -implementations exist for `SomeFeature`. - -This problem only occurs when you combine `nightly` and `accessible(..)` -or indeed `nightly` and `version(..)`. However, there is a risk of breaking -code that worked on one stable release of Rust in one or more versions after. - -A few mitigating factors to consider are: - -+ It is possible to check if the methods of `SomeFeature` are `accessible` - or not by using their paths. This reduces the risk somewhat. - -+ If a crate author runs continuous integration (CI) builds that include - testing the crate on a nightly toolchain, breakage can be detected - well before any crates are broken and a patch release of the crate - can be made which either removes the nightly feature or adjusts the - usage of it. The remaining problem is that dependent crates may have - `Cargo.lock` files that have pinned the patch versions of the crate. - -+ Users should take care not to use this mechanism unless they are fairly - confident that no consequential changes will be made to the library. - A risk still exists, but it is opt-in. - # Rationale and alternatives [alternatives]: #rationale-and-alternatives @@ -376,7 +330,7 @@ the feature. You may think that `version(..)` subsumes `accessible(..)`. However, we argue that it does not. This is the case because at the time of -enabling the `nightly` feature that enables the path in the standard library, +enabling the `feature = "unstable"` feature that enables the path in libstd, we do not yet know what minimum version it will be supported under. If we try to support it with `version(..)`, it is possible that we may need to update the minimum version some small number of times. @@ -468,28 +422,6 @@ go in there. Meanwhile, you may `#[cfg(accessible(::foo::MyTrait::my_method))` which is *not* possible as `use ::foo::MyTrait::my_method;`. This also applies to other associated items and inherent methods as well as `struct` fields. -## `#[cfg(nightly)` - -[dbg]: https://crates.io/crates/dbg - -One reason for the inclusion of `#[cfg(nightly)]` is that it is useful on its -own to conditionally compile based on nightly/not as opposed to providing -an `unstable` feature in `Cargo.toml`. An example of this is provided by the -[dbg] crate which currently uses [version_check] to provide this automation. - -However, as we've argued and demonstrated in the [guide-level-explanation], -the ability to `#[cfg(nightly)]` really shines when used in conjunction with -`#[cfg(accessible($path))]`. - -### Alternative `#![if_possible_feature()]` - -As an alternative to `#[cfg_attr(nightly, feature())]` -we could permit the user to write `#![if_possible_feature()]`. -The advantage of this is that it is quite direct with respect to intent. -However, adding this in terms of `nightly` already has precedent in -[version_check]. In addition, `nightly` also composes with other flags -using `any`, `not`, and `all`. - ## `#[cfg(version(..))` When it comes to `version(..)`, it is needed to support conditional compilation @@ -682,12 +614,7 @@ main = putStrLn version The ability to have optional cargo dependencies is out of scope for this RFC. -1. Could we somehow have an allow-by-default lint that says - *"these paths don't exist"* which could be enabled on `cfg_attr(nightly)`? - This would be done to mitigate the accumulation of garbage code as - discussed in the [drawbacks]. - -2. Is it technically feasible to implement `accessible(..)`? +1. Is it technically feasible to implement `accessible(..)`? For example it could be hard if cfg-stripping runs before resolving things. @eddyb has indicated that: @@ -699,4 +626,94 @@ The ability to have optional cargo dependencies is out of scope for this RFC. > check against feature-gates (assuming the set of `#![feature(...)]`s in > the local crate is known at `cfg`-stripping time). -3. Should we allow referring to fields of type definitions in `accessible(..)`? +2. Should we allow referring to fields of type definitions in `accessible(..)`? + +# Possible future work +[possible future work]: #possible-future-work + +## `#[cfg(nightly)]` + +In a previous iteration of this RFC, a `#[cfg(nightly)]` flag was included. +However, this flag has since been removed from the RFC. +We may still add such a feature in the future if we wish. +Therefore, we have outlined what `nightly` would have meant +and some upsides and drawbacks to adding it. + +## Technical specification + +To the `cfg` attribute, a `nightly` flag is added. + +If and only if a Rust compiler permits a user to specify `#![feature(..)]` +will the `nightly` flag be considered active. + +## Drawbacks: Combining `nightly` and `accessible(..)` + +Consider that a popular library writes: + +```rust +#![cfg_attr(nightly, feature(some_feature))] +#[cfg(accessible(::std::foo:SomeFeature))] +use std::foo::SomeFeature; + +#[cfg(not(accessible(::std::foo:SomeFeature)))] +struct SomeFeature { ... } +``` + +One potential hazard when writing this migrating construct is that +once `SomeFeature` finally gets stabilized, it may have been shipped +in a modified form. Such modification may include changing the names +of `SomeFeature`'s methods, their type signatures, or what trait +implementations exist for `SomeFeature`. + +This problem only occurs when you combine `nightly` and `accessible(..)` +or indeed `nightly` and `version(..)`. However, there is a risk of breaking +code that worked on one stable release of Rust in one or more versions after. + +A few mitigating factors to consider are: + ++ It is possible to check if the methods of `SomeFeature` are `accessible` + or not by using their paths. This reduces the risk somewhat. + ++ If a crate author runs continuous integration (CI) builds that include + testing the crate on a nightly toolchain, breakage can be detected + well before any crates are broken and a patch release of the crate + can be made which either removes the nightly feature or adjusts the + usage of it. The remaining problem is that dependent crates may have + `Cargo.lock` files that have pinned the patch versions of the crate. + ++ Users should take care not to use this mechanism unless they are fairly + confident that no consequential changes will be made to the library. + A risk still exists, but it is opt-in. + +However, at the end, compared to `feature = "unstable"`, +which reverse dependencies may opt out of, `nightly` can't be opted out of +(unless we add a mechanism to Cargo to perform such an override, +but this would be anti-modular). +This is the fundamental reason that for the time being, +we have not included `nightly` in the proposal. + +## Upsides + +[dbg]: https://crates.io/crates/dbg + +One reason for the inclusion of `#[cfg(nightly)]` is that it is useful on its +own to conditionally compile based on nightly/not as opposed to providing +an `unstable` feature in `Cargo.toml`. An example of this is provided by the +[dbg] crate which currently uses [version_check] to provide this automation. + +## Alternative `#![if_possible_feature()]` + +As an alternative to `#[cfg_attr(nightly, feature())]` +we could permit the user to write `#![if_possible_feature()]`. +The advantage of this is that it is quite direct with respect to intent. +However, adding this in terms of `nightly` already has precedent in +[version_check]. In addition, `nightly` also composes with other flags +using `any`, `not`, and `all`. + +This alternative also suffers from the problems previously noted. + +## Naming of the attribute + +If this flag were to be proposed again, it would probably be proposed under +a different name than `nightly`. Instead, a more apt name with respect to intent +would be `unstable_features`. From 1cf87f690e18266b3c06977a546742823d8238ac Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 19 Aug 2018 17:03:51 +0200 Subject: [PATCH 34/50] rfc, inferred-type-aliases: discuss semantics of version(..) in depth. --- text/0000-cfg-path-version.md | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 4592cf86622..313924cdc82 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -628,6 +628,51 @@ The ability to have optional cargo dependencies is out of scope for this RFC. 2. Should we allow referring to fields of type definitions in `accessible(..)`? +3. In the [reference-level-explanation], we note that: + > If and only if a Rust compiler considers itself to have a version which is + > greater or equal to the version in the `semver` string will the + > `#[cfg(version()]` flag be considered active. + + However, it is currently not well specified what "considers itself" exactly + means. To be more precise, if querying a mid-cycle nightly compiler with + `rustc --version` results in `rustc 1.29.0-nightly (31f1bc7b4 2018-07-15)`, + but 1.29.0 has not been released on the stable channel, + will then `version(1.29.0)` be active for this nightly or will it not? + + The reason this question matters is because on one 1.29.0-nightly compiler, + a feature may not have been stabilized. Some days later, but before 1.29.0 + hits a beta or stable compiler, a feature does get stabilized. + + To resolve this question, there are broadly 3 approaches: + + 1. Answer the question in the affirmative. + This entails that some breakage might sometimes occur when + using a nightly compiler. + + 2. Answer it in the negative by changing the date when the version constant + is bumped in the compiler. That is, a version would only be bumped when + releasing new stable or beta compilers and nightly compilers would always + be versioned as the latest stable/beta. This also means that given + `#[stable(feature = "foobar", since = "1.42.0")]` for some feature + `foobar`, the feature would not be available first when the feature + actually reaches stable/beta. + + 3. As 2. but separate versions reported by `rustc --version` and to + `version(..)`. This would for example mean that if the last + stable compiler is `1.42.0`, then that would be used by `version(..)` + while `rustc --version` would report `1.43.0-nightly`. + This approach could be technically achieved by for example + maintaining one version constant that tracks the last stable/beta + compiler as `x.y.z` and then `--version` would report + `x.(y + 1).0-nightly`. + + Two arguments in favour of either 2. or 3. is that they would be more + principled as we have not really stabilized something until it reaches + stable or beta. + + We consider this unresolved question to be a matter of implementation detail + which may be resolved during implementation. + # Possible future work [possible future work]: #possible-future-work From 210e909bd862a7c7e247aca5b7e1ebcd8f2cc936 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 31 Aug 2018 23:26:42 +0200 Subject: [PATCH 35/50] rfc, cfg-path-version: fix bugs. --- text/0000-cfg-path-version.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 313924cdc82..ca255b98e13 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -280,9 +280,9 @@ We can then refer to them like so: ```rust #[cfg(all( - accessible(foobar::Person::ssn), - accessible(foobar::Shape::Triangle::sides), - accessible(foobar::Shape::MaybeUninit::value) + accessible(::foobar::Person::ssn), + accessible(::foobar::Shape::Triangle::sides), + accessible(::foobar::Shape::MaybeUninit::value) ))] fn do_stuff() { ... From 592de812c3c79ee369e8fbcd7e64d8d3f2d4a386 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 31 Aug 2018 23:48:40 +0200 Subject: [PATCH 36/50] rfc, cfg-path-version: talk more about relative paths. --- text/0000-cfg-path-version.md | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index ca255b98e13..da565dcedbe 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -344,7 +344,7 @@ The reason why we have enforced that all paths must start with `::` inside `accessible(..)` is that if we allow relative paths, and users write `accessible(self::foo)`, then they can construct situations such as: -``` +```rust #[cfg(accessible(self::bar)] fn foo() {} @@ -356,6 +356,31 @@ One way around this is to collect all items before `cfg`-stripping, but this can cause problems with respect to stage separation. Therefore, we prevent this from occurring with a simple syntactic check. +One mechanism we could use to make relative paths work is to use a different +resolution algorithm for `accessible(..)` than for `use`. We would first +syntactically reject `self::$path`, `super::$path`, and `crate::$path`. +The resolution algorithm would then need to deal with situations such as: + +```rust +#[cfg(accessible(bar)] +fn foo() {} + +#[cfg(accessible(foo)] +fn bar() {} +``` + +by simply not considering local items and assuming that `bar` and `foo` are +crates. While that would make `accessible($path)` a bit more ergonomic by +shaving off two characters, chances are, assuming the `uniform_paths` system, +that it would lead to surprises for some users who think that `bar` and `foo` +refer to the local crate. This is problematic because it is not immediately +evident for the user which is which since a different crate is needed to observe +the difference. + +Also do note that requiring absolute paths with leading `::` is fully +forward-compatible with not requiring leading `::`. If we experience that +this restriction is a problem in the future, we may remove the restriction. + ### `#[cfg(accessible(..))` or `#[cfg(accessible = ..)` We need to decide between the syntax `accessible(..)` or `accessible = ..`. From 7aa9cefddfbfe8586f2cee103dbf0658cfadd042 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 1 Sep 2018 00:03:37 +0200 Subject: [PATCH 37/50] rfc, cfg-path-version: more rigorous bikeshed wrt. accessible. --- text/0000-cfg-path-version.md | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index da565dcedbe..3a74a48032e 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -423,17 +423,20 @@ meta : path ( named_value | meta_list )? ; One might consider other names for the flag instead of `accessible`. Some contenders are: ++ `reachable` + `path_accessible` + `usable` + `can_use` + `path_exists` -+ `have_path` ++ `have_path` (or `has_path`) + `have` + `have_item` + `path_reachable` + `item_reachable` + `item_exists` +#### `accessible` + Currently `accessible` is the choice because it clearly signals the intent while also being short enough to remain ergonomic to use. In particular, while `path_accessible` might be somewhat more unambiguous, @@ -441,12 +444,36 @@ we argue that from the context of seeing `accessible(::std::foo::bar)` it is clear that it is paths we are talking about because the argument is a path and not something else. +#### `reachable` + +The word `reachable` is also a synonym of `accessible` and is one character +shorter. However, it tends to have a different meaning in code wherefore we +have chosen to go with `accessible` instead as the more intuitive option. + +#### `usable` + While `can_use` and `usable` are also strong contenders, we reject these options because they may imply to the user that only things that you may `use $path;` can go in there. Meanwhile, you may `#[cfg(accessible(::foo::MyTrait::my_method))` which is *not* possible as `use ::foo::MyTrait::my_method;`. This also applies to other associated items and inherent methods as well as `struct` fields. +#### `has_path` + +Another strong contender is `has_path` or `have_path`. + +However, this variant is vague with respect to what "having" something means. +In other words, it does not say whether it refers to being accessible and public, +or whether it is usable, and so on. As we previously noted, having `path` in the +name is also somewhat redundant because it is clear that `::std::bar` is a path. +Another small wrinkle is that it is unclear whether it should be `have` or `has`. +That choice depends on what one things the subject is. For example, if one +considers a module to be an "it", then it should probably be `has`. + +One upside to `has_path` is that it has precedent from the `clang` compiler. +For example, a user may write: `#if __has_feature(cxx_rvalue_references)` +or `__has_feature(c_generic_selections)`. + ## `#[cfg(version(..))` When it comes to `version(..)`, it is needed to support conditional compilation From 63b4eaca216a8e30a03086cad05631e85a763a95 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 1 Sep 2018 00:16:05 +0200 Subject: [PATCH 38/50] rfc, cfg-path-version: clang as prior art. --- text/0000-cfg-path-version.md | 40 ++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 3a74a48032e..f02f3071729 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -464,8 +464,11 @@ Another strong contender is `has_path` or `have_path`. However, this variant is vague with respect to what "having" something means. In other words, it does not say whether it refers to being accessible and public, -or whether it is usable, and so on. As we previously noted, having `path` in the +or whether it is usable, and so on. + +As we previously noted, having `path` in the name is also somewhat redundant because it is clear that `::std::bar` is a path. + Another small wrinkle is that it is unclear whether it should be `have` or `has`. That choice depends on what one things the subject is. For example, if one considers a module to be an "it", then it should probably be `has`. @@ -474,6 +477,9 @@ One upside to `has_path` is that it has precedent from the `clang` compiler. For example, a user may write: `#if __has_feature(cxx_rvalue_references)` or `__has_feature(c_generic_selections)`. +Another benefit is that `has_` gives us the opportunity to introduce a family +of `has_path`, `has_feature`, and `has_$thing` if we so wish. + ## `#[cfg(version(..))` When it comes to `version(..)`, it is needed to support conditional compilation @@ -661,6 +667,38 @@ main :: IO () main = putStrLn version ``` +## Clang + +[clang_check]: https://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros + +The `clang` compiler gives you a [suite of feature checking macros][clang_check] +with which you can for example check whether a certain feature, extension, +or attribute is supported. An example of this is: + +```cpp +#if __has_feature(cxx_rvalue_references) + +// This code will only be compiled with the -std=c++11 and -std=gnu++11 +// options, because rvalue references are only standardized in C++11. + +#endif +``` + +This would be analogous to checking for the existence of a feature gate in Rust. + +[clang_include]: https://clang.llvm.org/docs/LanguageExtensions.html#include-file-checking-macros + +Clang also supports checking whether an [include][clang_include] will succeed. +For example, you may write: + +```cpp +#if __has_include("myinclude.h") && __has_include() +#include "myinclude.h" +#endif +``` + +This is similar in spirit to `accessible($path)`. + # Unresolved questions [unresolved]: #unresolved-questions From f0dc1e648b1393ea573c80fa2809208088d021a6 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 1 Sep 2018 00:29:02 +0200 Subject: [PATCH 39/50] rfc, cfg-path-version: discuss rust_feature. --- text/0000-cfg-path-version.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index f02f3071729..b3b5b019b70 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -766,6 +766,35 @@ The ability to have optional cargo dependencies is out of scope for this RFC. # Possible future work [possible future work]: #possible-future-work +## `#[cfg(rust_feature(..))]` + +[GAT]: https://github.com/rust-lang/rust/issues/44265 + +One possible extension we might want to do in the future is to allow users to +check whether a certain `rustc` feature gate is enabled or not. +For example, we might write `#[cfg(rustc_feature(generic_associated_types))]` +to check whether the [GAT] feature is supported in the compiler or not. + +The main benefit of such an approach is that it is more direct than checking +for a particular version. Also note that `clang` uses this approach as noted +in the [prior art][prior-art]. + +However, there are some drawbacks as well: + +1. The names of feature gates are not always aptly named and usually do not + follow a coherent naming system. As a frequent author of RFCs, the author + of this one knows that they do not have a principled approach to naming + RFCs. The feature name that is then used in the compiler is usually drawn + directly from the RFC, so we would either need to accept the random naming + of feature gates, or we would need to impose some system. + +2. Permitting dependence on the names of feature gates on stable would + require us to be more principled with feature gates. + For example, `rustc`, or any other Rust compiler, would be unable to + remove gates or drastically change their implementations without changing + their names. Being more principled could potentially add an undue burden + on the library and compiler teams. + ## `#[cfg(nightly)]` In a previous iteration of this RFC, a `#[cfg(nightly)]` flag was included. From c6d47e724c984eb0342483c8b1abd5aef06856a0 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 1 Sep 2018 00:33:05 +0200 Subject: [PATCH 40/50] rfc, cfg-path-version: elaborate on reachable. --- text/0000-cfg-path-version.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index b3b5b019b70..27edef035a0 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -447,8 +447,13 @@ is a path and not something else. #### `reachable` The word `reachable` is also a synonym of `accessible` and is one character -shorter. However, it tends to have a different meaning in code wherefore we -have chosen to go with `accessible` instead as the more intuitive option. +shorter. However, it tends to have a different meaning in code. Examples include: + ++ `std::hint::unreachable_unchecked` ++ `std::unreachable` + +All in all, we have chosen to go with `accessible` instead as the +more intuitive option. #### `usable` From 0b12c54d6f0c48ee8fbe9f147f604bd7c4141549 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 1 Sep 2018 00:37:40 +0200 Subject: [PATCH 41/50] rfc, cfg-path-version: add error-chain#101 as an example. --- text/0000-cfg-path-version.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 27edef035a0..eb10f0316b0 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -64,8 +64,11 @@ proposed in the [summary] we aim to make retaining *compatibility* and supporting more compiler versions *pain-free* and to give authors a lot of *control* over what is supported and what is not. -A minor use case this RFC supports is to work around compiler bugs by -checking if we are on a particular version. +[rust-lang-nursery/error-chain#101]: https://github.com/rust-lang-nursery/error-chain/issues/101 + +Another use case this RFC supports is to work around compiler bugs by +checking if we are on a particular version. An example where this occurred +is documented in [rust-lang-nursery/error-chain#101]. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation From 16ef0b2ee71c2fc4d83927fec02490e6d0306550 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 1 Sep 2018 01:02:17 +0200 Subject: [PATCH 42/50] rfc, cfg-path-version: discuss has_attr as future work. --- text/0000-cfg-path-version.md | 43 +++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index eb10f0316b0..e80645f112d 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -778,8 +778,8 @@ The ability to have optional cargo dependencies is out of scope for this RFC. [GAT]: https://github.com/rust-lang/rust/issues/44265 -One possible extension we might want to do in the future is to allow users to -check whether a certain `rustc` feature gate is enabled or not. +One possible extension we might want to do in the future is to allow users +to check whether a certain `rustc` feature gate is enabled or not. For example, we might write `#[cfg(rustc_feature(generic_associated_types))]` to check whether the [GAT] feature is supported in the compiler or not. @@ -803,6 +803,35 @@ However, there are some drawbacks as well: their names. Being more principled could potentially add an undue burden on the library and compiler teams. +## `#[cfg(has_attr($attribute))]` + +One possible extension would be to introduce a `has_attr(..)` feature. +`has_attr` would check if the specified attribute would be usable on the +item the `cfg` (or `cfg_attr`) directive is attached to. For instance: + +```rust +#[cfg_attr(have_attr(must_use), must_use)] +fn double(x: i32) -> i32 { + 2 * x +} +``` + +This would allow code to detect the availability of an attribute before using it, +while not failing if the attribute did not exist. + +Using `has_attr` in a `cfg` block may be useful for conditionally compiling +code that only makes sense if a given attribute exists (e.g. `global_allocator`), +while using `has_attr` in a `cfg_attr` block may be useful for adding an attribute to an item if supported but still support compilers that don't +support that attribute. + +As previously discussed, currently, the names of feature gates do not tend to +appear in code targeting stable versions of Rust. Allowing code to detect the +availability of specified feature gates by name would require committing to stable names for these features, and would require that those names refer to +a fixed set of functionality. This would require additional curation. +However, as attribute names already have to be standardized, +`has_attr(..)` would not suffer the same problems wherefore +it may be the better solution. + ## `#[cfg(nightly)]` In a previous iteration of this RFC, a `#[cfg(nightly)]` flag was included. @@ -811,14 +840,14 @@ We may still add such a feature in the future if we wish. Therefore, we have outlined what `nightly` would have meant and some upsides and drawbacks to adding it. -## Technical specification +### Technical specification To the `cfg` attribute, a `nightly` flag is added. If and only if a Rust compiler permits a user to specify `#![feature(..)]` will the `nightly` flag be considered active. -## Drawbacks: Combining `nightly` and `accessible(..)` +### Drawbacks: Combining `nightly` and `accessible(..)` Consider that a popular library writes: @@ -864,7 +893,7 @@ but this would be anti-modular). This is the fundamental reason that for the time being, we have not included `nightly` in the proposal. -## Upsides +### Upsides [dbg]: https://crates.io/crates/dbg @@ -873,7 +902,7 @@ own to conditionally compile based on nightly/not as opposed to providing an `unstable` feature in `Cargo.toml`. An example of this is provided by the [dbg] crate which currently uses [version_check] to provide this automation. -## Alternative `#![if_possible_feature()]` +### Alternative `#![if_possible_feature()]` As an alternative to `#[cfg_attr(nightly, feature())]` we could permit the user to write `#![if_possible_feature()]`. @@ -884,7 +913,7 @@ using `any`, `not`, and `all`. This alternative also suffers from the problems previously noted. -## Naming of the attribute +### Naming of the attribute If this flag were to be proposed again, it would probably be proposed under a different name than `nightly`. Instead, a more apt name with respect to intent From 333388276db048b181629ebee2c9c6184826641f Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 1 Sep 2018 01:10:17 +0200 Subject: [PATCH 43/50] rfc, cfg-path-version: fix word wrap. --- text/0000-cfg-path-version.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index e80645f112d..100c4c6e703 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -821,7 +821,8 @@ while not failing if the attribute did not exist. Using `has_attr` in a `cfg` block may be useful for conditionally compiling code that only makes sense if a given attribute exists (e.g. `global_allocator`), -while using `has_attr` in a `cfg_attr` block may be useful for adding an attribute to an item if supported but still support compilers that don't +while using `has_attr` in a `cfg_attr` block may be useful for adding an +attribute to an item if supported but still support compilers that don't support that attribute. As previously discussed, currently, the names of feature gates do not tend to From e0de3bbcdffe93e94265a4d1d107248803281979 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 29 Nov 2018 20:39:57 +0100 Subject: [PATCH 44/50] rfc, cfg-path-version: highlight the risks of unstable. --- text/0000-cfg-path-version.md | 45 +++++++++++++++++------------------ 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 100c4c6e703..a23d388c190 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -75,13 +75,12 @@ is documented in [rust-lang-nursery/error-chain#101]. ## `#[cfg(accessible($path))]` -Consider for a moment that we would like to use the `Iterator::flatten` -method of the standard library if it exists, but otherwise fall back to -`Itertools::flatten`. We can do that with the following snippet: +Consider for a moment that we would like to use the `Iterator::flatten` method +of the standard library if it exists (because it has become soon in a certain +Rust version), but otherwise fall back to `Itertools::flatten`. +We can do that with the following snippet: ```rust -#![cfg_attr(feature = "unstable", feature(iterator_flatten))] - #[cfg(accessible(::std::iter::Flatten))] fn make_iter(limit: u8) -> impl Iterator { (0..limit).map(move |x| (x..limit)).flatten() @@ -100,33 +99,33 @@ fn main() { What this snippet does is the following: -1. If the cargo feature `unstable` is enabled and assuming the compiler is - capable of feature gates, but not otherwise, the feature `iterator_flatten` - will be enabled. - -2. If the path `::std::iter::Flatten` exists, the compiler will compile +1. If the path `::std::iter::Flatten` exists, the compiler will compile the first version of `make_iter`. If the path does not exist, the compiler will instead compile the second version of `make_iter`. -The result of 1. and 2. is that your crate can opt into using -`Iterator::flatten` when `feature = "unstable"` is enabled, -but use `Itertools::flatten` on stable compilers meanwhile. -Once the standard library has stabilized `iter::Flatten`, -future stable compilers will start using the first version of the function. -As a crate author, you don't have to publish any new versions of your crate for -the compiler to switch to the libstd version when it is available in the future. +The result of 1. is that your crate will use `Iterator::flatten` on newer +versions of Rust and `Itertools::flatten` on older compilers. +The result of this is that as a crate author, you don't have to publish any +new versions of your crate for the compiler to switch to the libstd version +when people use a newer version of Rust. [`proptest`]: https://github.com/altsysrq/proptest [adding support]: https://github.com/AltSysrq/proptest/blob/67945c89e09f8223ae945cc8da029181822ce27e/src/num.rs#L66-L76 -In this case we used the `feature = "unstable"` and `accessible` flags -to handle a problem that the addition of `Iterator::flatten` caused for us -if we had used `Itertools::flatten`. We can also use these mechanisms for -strictly additive cases as well. Consider for example the [`proptest`] crate -[adding support] for `RangeInclusive`: +Once the standard library has stabilized `iter::Flatten`, +future stable compilers will start using the first version of the function. + +In this case we used the `accessible` flag to handle a problem that the addition +of `Iterator::flatten` caused for us if we had used `Itertools::flatten`. +We can also use these mechanisms for strictly additive cases as well. +Consider for example the [`proptest`] crate [adding support] for `RangeInclusive`: ```rust -#[cfg_attr(feature = "unstable", feature(inclusive_range))] +// #[cfg_attr(feature = "unstable", feature(inclusive_range))] +// ^-- If you include this line; then `cargo build --features unstable` +// would cause nightly compilers to activate the feature gate. +// Note that this has some inherent risks similar to those for +// `#[cfg(nightly)]` (as discussed later in this RFC). macro_rules! numeric_api { ($typ:ident) => { From 96ec9e7f0a52d1b73d91789258a03b2cbec737bc Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 29 Nov 2018 20:50:19 +0100 Subject: [PATCH 45/50] rfc, cfg-path-version: consider attributes & macros. --- text/0000-cfg-path-version.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index a23d388c190..a9682d71833 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -291,6 +291,10 @@ fn do_stuff() { } ``` +Finally, bang macros, derive macros, attributes of all sorts including +built-in, user provided, as well as latent derive helper attributes, +will be considered when determining if a path is accessible. + ## `cfg_attr` and `cfg!` Note that the above sections also apply to the attribute `#[cfg_attr(..)]` as From 40bc0f5c95635b518f6a9b542f60d19c58105662 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 11 Jan 2019 02:37:32 +0100 Subject: [PATCH 46/50] rfc, cfg-path-version: fix wording in motivation. --- text/0000-cfg-path-version.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index a9682d71833..eec863cd147 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -42,10 +42,9 @@ reduce "dependency hell" by enhancing cargo for: [RFC 2483]: https://github.com/rust-lang/rfcs/pull/2483 -...not much focus has been given to how you can improve the situation can be -improved by enhancing conditional compilation to extend how many versions back -a crate supports. This becomes critically important if and when we gain LTS -channels as proposed by [RFC 2483]. +...not much focus has been given to how conditional compilation can be improved +to extend how many versions back a crate supports. This becomes critically +important if and when we gain LTS channels as proposed by [RFC 2483]. [version_check]: https://crates.io/crates/version_check From 4618306ac7c9cc2fc3134ecb37b516568f6a2b7f Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 11 Jan 2019 02:51:52 +0100 Subject: [PATCH 47/50] rfc, cfg-path-version: fix formal grammar to correspond to guide. --- text/0000-cfg-path-version.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index eec863cd147..1fc80bbf123 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -227,11 +227,12 @@ However, there will be features in the future to use this mechanism on. ## `#[cfg(version())]` To the `cfg` attribute, a `version` flag is added. -This flag requires that a string literal be specified in it inside parenthesis. -The string literal must have the format: +This flag has the following grammar (where `\d` is any digit in `0` to `9`): -``` -semver : \d(.\d)?(.\d)? ; +```rust +flag : "version" "(" semver ")" ; +semver : digits ("." digits ("." digits)?)? ; +digits : \d+ ; ``` [caret requirements]: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#caret-requirements From ca5a92bba64f6e47c13e4a56e5771c0fbe5b4596 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 11 Jan 2019 03:55:16 +0100 Subject: [PATCH 48/50] rfc, cfg-path-version: mention target_has_atomic in rationale. --- text/0000-cfg-path-version.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 1fc80bbf123..2e63cd3b945 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -344,6 +344,10 @@ However, doing so even once means that you will need to release new versions of your crate. If you instead use `accessible(..)` you won't need to use it even once unless the name of the path changes in-between. +Another use case `accessible(..)` supports that `version(..)` doesn't is checking +support for atomic types, e.g. `accessible(::std::sync::atomic::AtomicU8)`. +This subsumes the proposed `#[cfg(target_has_atomic = "..")]` construct. + ### Preventing relative paths The reason why we have enforced that all paths must start with `::` inside From 5b4b21a42d8186706764e1ed760b5c1381f3fb7b Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 17 Feb 2019 10:18:33 +0100 Subject: [PATCH 49/50] rfc, cfg-path-version: clarify stability policy re. accessible(..). --- text/0000-cfg-path-version.md | 30 +++++++++++++++++++++++++++++- text/1105-api-evolution.md | 10 ++++++++++ text/1122-language-semver.md | 10 ++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/text/0000-cfg-path-version.md b/text/0000-cfg-path-version.md index 2e63cd3b945..5d9db04ace4 100644 --- a/text/0000-cfg-path-version.md +++ b/text/0000-cfg-path-version.md @@ -244,7 +244,10 @@ Greater or equal is defined in terms of [caret requirements]. ## `#[cfg(accessible($path))]` -To the `cfg` attribute, a `accessible` flag is added. +To the `cfg` attribute, an `accessible` flag is added. + +### Syntactic form + This flag requires that a `path` fragment be specified in it inside parenthesis but not inside a string literal. The `$path` must start with leading `::` and may not refer to any parts of the own crate (e.g. with `::crate::foo`, @@ -253,17 +256,40 @@ This restriction exists to ensure that the user does not try to conditionally compile against parts of their own crate because that crate has not been compiled when the `accessible` flag is checked on an item. +### Basic semantics + If and only if the path referred to by `$path` does exist and is public will the `#[cfg(accessible($path))]` flag be considered active. + +### `#![feature(..)]` gating + In checking whether the path exists or not, the compiler will consider feature gated items to exist if the gate has been enabled. +**NOTE:** In the section on `#[cfg(nightly)]` and in the +[guide level explanation][guide-level-explanation] we note that there are +some risks when combining `cfg(feature = "unstable")` and `accessible(..)` to +add conditional support for an unstable feature that is expected to stabilize. +With respect to such usage: + +1. User-facing documentation, regarding `accessible(..)` should highlight risky + scenarios, including with examples, with respect to possible breakage. + +2. Our stability policy is updated to state that breakage caused due to misuse + of `accessible(..)` is _allowed_ breakage. Consequently, rust teams will not + delay releases or un-stabilize features because they broke a crate using + `accessible(..)` to gate on those features. + +### Inherent implementations + If a path refers to an item inside an inherent implementation, the path will be considered to exist if any configuration of generic parameters can lead to the item. To check whether an item exists for an implementation with a specific sequence of concrete types applied to a type constructor, it is possible to use the `::foo::bar::::item` syntax. +### Fields + It is also possible to refer to fields of `struct`s, `enum`s, and `unions`. Assuming that we have the following definitions in the `foobar` crate: @@ -291,6 +317,8 @@ fn do_stuff() { } ``` +### Macros + Finally, bang macros, derive macros, attributes of all sorts including built-in, user provided, as well as latent derive helper attributes, will be considered when determining if a path is accessible. diff --git a/text/1105-api-evolution.md b/text/1105-api-evolution.md index b8b37f1d2dd..20799e4d906 100644 --- a/text/1105-api-evolution.md +++ b/text/1105-api-evolution.md @@ -759,6 +759,16 @@ type parameter to `foo` can break code, even if a default is provided. This could be easily addressed by adding a notation like `...` to leave additional parameters unspecified: `foo::(x, y)`. +## [Amendment] Misuse of `accessible(..)` + +[RFC 2523]: https://github.com/rust-lang/rfcs/blob/master/text/2523-cfg-path-version.md + +[RFC 2523] introduces `#[cfg(accessible($path)]`. Based on the accessibility of +a to-the-current-crate external `$path`, the flag allows conditional compilation. +When combined with `#[cfg(feature = "unstable")]`, this has certain breakage risks. +Such breakage due to misuse, as outlined in the RFC, is considered acceptable and +not covered by our stability promises. Please see the RFC for more details. + # Drawbacks and Alternatives The main drawback to the approach laid out here is that it makes the stability diff --git a/text/1122-language-semver.md b/text/1122-language-semver.md index ed0985d606d..7fb81c15082 100644 --- a/text/1122-language-semver.md +++ b/text/1122-language-semver.md @@ -229,6 +229,16 @@ future as well. The `-Z` flags are of course explicitly unstable, but some of the `-C`, rustdoc, and linker-specific flags are expected to evolve over time (see e.g. [#24451]). +## [Amendment] Misuse of `accessible(..)` + +[RFC 2523]: https://github.com/rust-lang/rfcs/blob/master/text/2523-cfg-path-version.md + +[RFC 2523] introduces `#[cfg(accessible($path)]`. Based on the accessibility of +a to-the-current-crate external `$path`, the flag allows conditional compilation. +When combined with `#[cfg(feature = "unstable")]`, this has certain breakage risks. +Such breakage due to misuse, as outlined in the RFC, is considered acceptable and +not covered by our stability promises. Please see the RFC for more details. + # Drawbacks The primary drawback is that making breaking changes are disruptive, From e488a3aad953ff8462253506b14a64820dd41872 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 26 Sep 2019 04:35:15 +0200 Subject: [PATCH 50/50] RFC 2523 --- text/{0000-cfg-path-version.md => 2523-cfg-path-version.md} | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) rename text/{0000-cfg-path-version.md => 2523-cfg-path-version.md} (99%) diff --git a/text/0000-cfg-path-version.md b/text/2523-cfg-path-version.md similarity index 99% rename from text/0000-cfg-path-version.md rename to text/2523-cfg-path-version.md index 5d9db04ace4..aabd8943121 100644 --- a/text/0000-cfg-path-version.md +++ b/text/2523-cfg-path-version.md @@ -1,7 +1,7 @@ -- Feature Name: `cfg_path_version` +- Feature Name: `cfg_version` and `cfg_accessible` - Start Date: 2018-08-12 -- RFC PR: _ -- Rust Issue: _ +- RFC PR: [rust-lang/rfcs#2523](https://github.com/rust-lang/rfcs/pull/2523) +- Rust Issue: [rust-lang/rust#64796](https://github.com/rust-lang/rust/issues/64796) and [rust-lang/rust#64797](https://github.com/rust-lang/rust/issues/64797) # Summary [summary]: #summary