Tracking issue for RFC 2523, #[cfg(accessible(::path::to::thing))]

[Centril]

This is a tracking issue for #[cfg(accessible(::path::to::thing))] (rust-lang/rfcs#2523).

Steps

• Implement the RFC: partially done in expand: Implement something similar to #[cfg(accessible(path))] #69870
• Adjust documentation (see instructions on rustc-guide)
• Stabilization PR (see instructions on rustc-guide)

Status

From this comment

• the surface of the feature was implemented in expand: Implement something similar to #[cfg(accessible(path))] #69870 as an attribute #[cfg_accessible(path)] item. The attribute can configure or unconfigure the item and wait until the predicate "path is accessible" becomes determinate.
• the predicate itself is not implemented, it either returns truth if the path is certainly available, or indeterminacy if we need to try again later, or reports an error otherwise. So the attribute is not usable in practice yet.
• desugaring of #[cfg(accessible)] into #[cfg_accessible] is not implemented, we need to consider doing or not doing it only when everything else is implemented.

Unresolved questions:

None so far.

[pickfire]

@Centril I am interested in doing this, I have never contributed to rust code and would like to try. Any guidance?

[Centril]

@pickfire Cool!

I believe the logic here can be divided into 2 parts roughly:

1. Syntax:

   1. Add a new sym::accessible in https://doc.rust-lang.org/nightly/nightly-rustc/src/syntax_pos/symbol.rs.html#22.

   2. Feature gate accessible in GATED_CFGS. Also add cfg_accessible to active.rs: https://doc.rust-lang.org/nightly/nightly-rustc/src/syntax/feature_gate/active.rs.html#530. This will also require a new sym::cfg_accessible.

   3. Introduce a match arm for sym::accessible in cfg_matches. This should look mostly like the case for sym::not.

      Here you need to extract an &ast::Path and delegate the interpretation to a function of the rough shape fn is_accessible(sess: &ParseSess, path: &Path) -> bool { ... }

2. Implement fn is_accessible.

   1. First do some validation. We want to emit errors if !path.is_global(). Use sess.span_diagnostic.struct_span_err.

   2. At this point we have a cycle between libsyntax (conditional compilation code where cfg_matches lives) and e.g. librustc_resolve where the latter has the information we need. To fix this we will need to enrich ParseSess or a modified CrateConfig (as a struct which would include the type alias) with a mechanism to ask if the path is accessible or not. Alternatively you could use some extern "Rust" { fn is_path_accessible(...) -> bool; } to achieve the same idea.

      Points to remember (and test for) when implementing this:

      • A path must not only exist but also be publicly exported from the target crate.

      • Feature gates should be considered in this; check whether the required feature gate is active if the feature is #[unstable(...)].

      • The bit in https://github.com/rust-lang/rfcs/blob/master/text/2523-cfg-path-version.md#inherent-implementations probably requires interactions with librustc_typeck (tho that can be fixed in a later stage of implementation)?

      • The various subsections in the reference section for accessible will need tests to make sure they work right.

      • Here we probably will want to assume that crate metadata for the ::target exists so if target is not a dependency of the current crate then we will answer false. If we don't do this I believe we would run into time-travel / cycle problems.

      • https://doc.rust-lang.org/nightly/nightly-rustc/rustc_resolve/struct.Resolver.html#method.per_ns is probably going to be used somewhere.

      • A question I thought of just now: If ::target::thing is deprecated, does that result in a warning in #[cfg(::target::thing)]?

      At this point I probably know too little about metadata and resolution to provide mentoring notes so I hope @eddyb, @petrochenkov, or maybe @mw can help you out.

[petrochenkov]

Oh no, this RFC passed.

[petrochenkov]

We can reliably answer the accessible(PATH) query for module-relative paths from other crates.

For type-relative paths everything looks harder.

• First, it breaks staging, type-based resolution isn't accessible during cfg-expansion.
  For paths from other crates we can perhaps break staging by moving querification closer to the compilation start.
• Second, even for paths from other crates to resolve a type-relative path we need to know what traits are in scope at the cfg point in this crate.
  This means merging more of the late resolution into early resolution.
• Additionally we need to know what trait aliases are in scope and be able to go from aliases to traits at cfg-expansion time. That means either duplicating the logic because the current trait alias subsitution works entirely at HIR/type level, or rewriting trait aliases to work on AST.

[Centril]

For type-relative paths everything looks harder.

Agreed!

• Second, even for paths from other crates to resolve a type-relative path we need to know what traits are in scope at the cfg point in this crate.

Can you elaborate on this point a bit? What specifically induces this requirement re. this crate and are there syntactic restrictions on the path we can impose to prevent the requirement from arising? (For example by preventing Foo::Bar::<T> syntactically.)

• Additionally we need to know what trait aliases are in scope and be able to go from aliases to traits at cfg-expansion time.

Hmm; I don't think I follow how why we need to go from aliases to their bodies here... can you elaborate on this as well?

[petrochenkov]

@Centril

Can you elaborate on this point a bit? What specifically induces this requirement re. this crate and are there syntactic restrictions on the path we can impose to prevent the requirement from arising?

use std::io::Write;

// The cfg result depends on whether the `io::Write` trait is in scope here
#[cfg(::std::fs::File::write)]

There's no syntactic way to detect this.

I don't think I follow how why we need to go from aliases to their bodies here... can you elaborate on this as well?

trait Alias = std::io::Write;

// The cfg result depends on whether the `Alias` refers to `io::Write`
#[cfg(::std::fs::File::write)]

[eddyb]

For type-relative paths, I don't think we should do anything more than inherent impls (because otherwise it's really cfg on a trait bound at best and I doubt that was ever intended by the RFC).

For example by preventing Foo::Bar::<T> syntactically.

There should be no generic arguments on such paths anyway, because there is no machinery to interpret them in any meaningful way.

---

I'd suggest implementing only module-relative paths to begin with, and do more investigations for type-relative paths, since I suspect they're not as useful.
Note that "module-relative" includes Trait::associated_item, since Trait is module-like for name resolution, so it would work for usecases like "did Iterator::foo get added upstream".

[petrochenkov]

The unpleasant part is that for some paths we may give an incorrect answer if we don't consider type-relative resolutions, e.g. using module-relative resolution we may return false for

cfg(MyEnum::MaybeNonExistent)

or

cfg(MyTrait::maybe_non_existent)

even if both paths exist if we apply type-relative resolution.

For imports those would also return "false", but in that case it's a compilation error (unresolved import), rather than a silent un-configuration.
We can report an error if the path in cfg is potentially type-resolved, that kinda reduces the usefulness of the feature though.

[petrochenkov]

Module-relative resolutions also have complications.
Consider this case, for example:

#[cfg(::proc_macro::TokenStream)]
struct S;

macro_rules! m { () => { extern crate proc_macro; } }

m!();

If we expand the cfg immediately like all cfgs are currently expanded, then we'll return an incorrect answer - false.

That means the path-based cfgs need to be enqueued until they are ready/resolved and only then expanded.
(That's exactly the behavior of attribute macros, and it comes with some costs - some restrictions apply to entities like macros or imports if they are expanded from macros. Same restrictions don't apply to cfgs, which are expanded immediately.)

The conclusion is that supporting paths from other crates is not easier than supporting local paths.
On the bright side, that means supporting local paths is not harder than extern paths! (Too bad local paths are useless in this context. EDIT: At least on 2018 edition, on 2015 edition everything is a local path.)

[pickfire]

@Centril Thanks for the explanation and the quick response, I do quite understand what you meant but not quite for the others.

Based on my understanding, #[cfg(accessible(::type::alias))] still probably does not work for this?

[pickfire]

Introduce a match arm for sym::accessible in cfg_matches. This should look mostly like the case for sym::not.

Here you need to extract an &ast::Path and delegate the interpretation to a function of the rough shape fn is_accessible(sess: &ParseSess, path: &Path) -> bool { ... }

@Centril Does that mean that I need to add something like

diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
index 8b967048848..9b6373bfb42 100644
--- a/src/libsyntax/ast.rs
+++ b/src/libsyntax/ast.rs
@@ -514,6 +514,10 @@ pub enum MetaItemKind {
     ///
     /// E.g., `feature = "foo"` as in `#[feature = "foo"]`.
     NameValue(Lit),
+    /// Value meta item.
+    ///
+    /// E.g., `path` as in `accessible(path)`.
+    Value(Lit),
 }

 /// A block (`{ .. }`).

[mx00s]

Will this support paths to modules, e.g. #[cfg(not(accessible(std::todo)))]? That's a silly example because todo! can be shadowed by a new (polyfill) implementation.

I don't have any more realistic use cases in mind yet; just curious.

[nagisa]

How would

#[cfg(accessible(peach::Peach))]
mod banana {
    crate struct Banana;
}

#[cfg(accessible(banana::Banana))]
mod peach {
    crate struct Peach;
}

work? Given comments from @petrochenkov can we restrict this feature for now (and maybe indefinitely) to support external paths only and nothing related to type-relative resolution?

EDIT: I’ve been pointed to the RFC portion that mentions paths must refer to external crates.

I think that at very least makes type-relative trait resolution that was raised as a concern in #64797 (comment) a non-problem. For a use somecrate::Trait; somecrate::Type::method is effectively a use somecrate::Trait; <somecrate::Type as Trait>::method where Trait in the path is crate-local pretty much.

[petrochenkov]

FWIW, I work on implementing a prototype for this in the most conservative variant (https://github.com/petrochenkov/rust/tree/cfgacc), but I've been constantly distracted by job and holiday trips since mid December.

The prototype will work as a macro attribute rather than a cfg mode, will make no distinction between local and external paths and will report a "not sure whether this exist or not" error on any potential type-relative associated item.
(The @nagisa's example #64797 (comment) should be automatically treated as a cycle in macro resolution/expansion and produce an error.)

[petrochenkov]

FWIW, I work on implementing a prototype for this in the most conservative variant

PR submitted - #69870.

[petrochenkov]

Status:

• the surface of the feature was implemented in expand: Implement something similar to #[cfg(accessible(path))] #69870 as an attribute #[cfg_accessible(path)] item. The attribute can configure or unconfigure the item and wait until the predicate "path is accessible" becomes determinate.
• the predicate itself is not implemented, it either returns truth if the path is certainly available, or indeterminacy if we need to try again later, or reports an error otherwise. So the attribute is not usable in practice yet.
• desugaring of #[cfg(accessible)] into #[cfg_accessible] is not implemented, we need to consider doing or not doing it only when everything else is implemented.

[thomcc]

Yeah, not seems extremely important to me, and hard to work around its absence. Lack of any/all could definitely be worked around, though, just it would be tedious.

[m-ou-se]

A really amazing use case for this IMO is -sys crates For example: libc, currently i have code like this in a current project that does some unixey nonsense:

I'm not sure I agree that's a good use case for this feature. Using cfg(accessible) for that quickly results in hard-to-maintain code that changes behavior depending on whether a symbol exists, without any checks for typos or signatures. There would be all kind of very implicit contracts between crates, similar to what we'd have if we relied on duck-typing everywhere instead of well defined traits and types.

[thomcc]

What would your suggested alternative be?

Currently, the only alternative I know of is to maintain a large list of cfgs for which targets support (e.g. getloadavg or sysctlbyname in the example), and I'm not sure I think that's that's any more maintainable.

[m-ou-se]

Another thing I'm worried about this feature enabling or even encouraging code that depends on future versions of the standard library that don't even exist yet.

For example: If we're experimenting with an unstable type X in the standard library, someone might reasonably write code like this to allow for a seamless transition for the future if/when std::X gets stabilized:

#[cfg(accessible(std::X))]
pub type X = std::X;

#[cfg(not(accessible(std::X)))]
pub struct X { .. }

The problem with this 'future compatible' code is that it assumes the future stable std::X type will be compatible with their own struct X. If we decide to change the unstable API in some breaking ways before stabilization (which we regularly do), stabilization would break this code, even though they were not using nightly Rust or unstable #![feature]s.

We could simply blame the author of this code for using cfg(accessible) like this, but if this is part of a popular library, all their users now have a reason to not upgrade to a new version of Rust, since we broke their dependency in the new Rust version.

[thomcc]

That's a good point, and might be a point in favor of something like a #[cfg(version(...))] instead, which seems much harder to abuse in that way (short of making a called shot about when the feature will land, which seems unlikely to happen frequently enough to matter).

That said, I believe this kind of version checking is regarded as undesirable for various reasons as well (although it's widely used in crates with long MSRV requirements).

[m-ou-se]

What would your suggested alternative be?

Currently, the only alternative I know of is to maintain a large list of cfgs for which targets support (e.g. getloadavg or sysctlbyname in the example), and I'm not sure I think that's that's any more maintainable.

Ideally, something that interacts well with our type and trait system, with where-clauses. E.g. an impl GetLoadAvg for System, such that a function with a where System: GetLoadAverage can call System::getloadavg(). Then it can also be type checked properly on all systems, instead of only on those where the cfg()s are enabled. That'd also prevent a lot of the cases where we accidentally break tier 3 platforms.

I realize that this requires some new language features, and probably some more to make it anywhere close to ergonomic. I really dislike conditional compilation, as it results in similar surprises and headaches as e.g. C++ templates (which are only type checked when instantiated) or type issues in dynamically typed scripting languages. I think conditional compilation is unavoidable in some cases, but I don't think we should encourage more of it.

I'm not sure if I see cfg(accessible) as something to just improve existing #[cfg]s, or as something that encourages even more (and more fragile) #[cfg]s.

[thomcc]

I also have noted in the past that cfg(accessible) can seem like the right solution to a problem, but passing the wrong argument to accessible might end up with the code not running when expected. It's certainly a problem.

I also agree something like what I suggested probably wouldn't be the right fit for use inside libstd, but I don't think I take as hard of a line against conditional compilation. It does make code harder to maintain, but it also comes with a number of benefits (compile time, for example, even if just in terms of not needing to pull in things like windows-sys or objc when building for linux).

I am in favor of ways to avoid the need for it, though, especially inside the stdlib.

(I'm not sure I follow how your suggestion avoids the current situation, but am willing to accept that it may be slightly handwaved given the "this requires some new language features" bit)

[joshtriplett]

@m-ou-se I think we should document the implications of potential accessible patterns, and in particular, document the pub use std::... approach as not necessarily compatible with semver.

[jplatte]

As some anecdata that this kind of thing is always hard and requires getting many details right:

1. A number of years ago, when working with C++, I wanted to use its std::optional type but it wasn't available in all GCC / clang versions people were using to build / work on this codebase. I used the shiny new __has_include mechanism, plus a fallback for when that wasn't available or none of the optional headers were there (https://github.com/mapbox/Optional), to get hold of an optional implementation. (similar to this example on cppreference.com)

This worked great as long as you constrained yourself to the common API of all of them, over different compiler versions. Unfortunately, there was no way to statically detect when you went outside that realm and that was quite easy (IIRC, the value() method for getting the inner value or throwing an exception if there isn't one, was called get() in one of the implementations).

Now cfg(accessible) is a more fine-grained than __has_include, but are people really going to remember to check for accessibility of any method they want to call that was stabilized later than the type it is defined on? I suppose MSRV CI jobs help getting this right.

Another related thought: The same way __has_include didn't allow checking for the existence of a particular method, cfg(accessible) doesn't allow checking for other details that could be changed after initial stabilization, for example

• "is Foo::new (already) a const fn?"
• "does <Bar<T>>::new (still) require T: Ord?"

I'm not sure how relevant this will be in practice, just some food for thought :)

2. Much more recently, I was looking for which Rust nightly introduced a weird bug I was experiencing, which was degrading docs. At a certain point, I suddenly got compilation errors. This wasn't another nightly bug, it was lock_api doing version detection and making a constructor const based on that. I suppose this could have been circumvented if nightlies always reported their version as (stable + 1), same as betas, but I'm sure that would bring a set of new problems with it.

[shepmaster]

stabilization would break this code

Would adding a #[cfg(stable)] help in conjunction?

[Nemo157]

cfg(accessible) is supposed to obey stability itself (but doesn't currently), but that doesn't help with code written assuming an unstable API will be stabilized without any changes. That part of the RFC also specified:

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.
— https://github.com/rust-lang/rfcs/blob/master/text/2523-cfg-path-version.md#feature-gating

[thomcc]

It being technically allowed doesn't really mean we can ignore the concern, since if it breaks a large number of crates it still becomes a concern, as it can prevent people from updating.

[alex]

Is this currently awaiting a more complete implementation (as the status at the top suggests), or resolution of design questions (as the most recent questions suggest)?

[est31]

@alex a bit of column A and a bit of column B I think... Certainly there are some design questions still open, but there is also implementation work to be done in order to get this finished. The ideas by @m-ou-se are also interesting, although IMO they are a longer term project. One shouldn't let perfect be the enemy of good.

Regarding the discussion about the danger of people depending on unstable APIs in cfg_accessible before they get stabilized in nightly, what about adding a lint that warns if some API is unstable? The lint would be on by default on nightly only, and be deny by default, so people who run their nightly in CI will get the warning, but people on stable will not get it. Nightly is the first channel to get new stabilizations so there is no concern of the lint firing on beta and stable while it's stable on nightly already. It is not perfect, as there might be people who don't run nightly in CI at all, or don't deny warnings there, but at least it would catch some cases of forward compatibility concerns. One could also make the lint be part of the forward compatibility system, on nightly, so that downstream users are warned that their upstream is doing bad things.

[jonhoo]

This is obviously not the intended use-case, but one neat side-effect of getting this stabilized would be the ability to annotate code such that you'll get a warning when a feature stabilizes. For example, imagine you currently have some code that wants to use Box<MaybeUninit>::assume_init, but can't because it's not yet stable. So, it instead has the implementation of that method re-implemented with a // TODO saying to move to assume_init when it stabilizes. That TODO will likely not be noticed when the method stabilizes. With cfg(accessible) you could add something like:

#[cfg(accessible(Box<MaybeUninit<T>>::assume_init))]
let todo_adopt_assume_init = ();

which will trigger an unused variable warning when assume_init stabilizes! I'm sure someone will use compiler_error! for this, but will hopefully reconsider since it'll cause builds to start failing when they upgrade Rust (a case for compiler_warning! perhaps?).

Now, what exactly goes in side accessible(...) for something like this I'm not sure, but the pattern is neat!