rfc, cfg-path-version: path_exists -> accessible + talk about fields.

rust-lang · Centril · Sep 26, 2019 · Aug 10, 2018 · Aug 10, 2018 · Aug 10, 2018
commit 77bf1d644ecaa5d3c5a82de8f9c67d72c88905ab
diff --git 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<Item = u8> {
     (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 = "<string>")]` 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::<T>::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<Unit> {
+    Triangle { pub sides: [Unit; 3] },
+    ...
+}
+
+pub union MaybeUninit<T> { 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,56 +334,100 @@ 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() {}
 ```
 
 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(<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<Stability>` 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(..)`?