Rust: 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 (cc @rust-lang/compiler -- can anyone write up mentoring instructions?)
• [ ] 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 #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 https://github.com/rust-lang/rust/issues/64797#issuecomment-540606496 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 https://github.com/rust-lang/rust/issues/64797#issuecomment-571131376 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 - https://github.com/rust-lang/rust/pull/69870.

[petrochenkov]

Status:

• the surface of the feature was implemented in #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.
  
  

[joshtriplett]

This came up in the @rust-lang/cargo meeting today. If you've specified conditionals on a dependency, such as with [target.'cfg(some_platform)'.dependencies], you might not want to duplicate that information into corresponding cfg directives when using that dependency. It'd be nice to, instead, write #[cfg(accessible(dep_name))], which would be true whenever that dep was available.

Would it be potentially reasonable, and avoid the trait-path corner cases, to only allow #[cfg(accessible(single_token))] for now? You could still write use std::foo::bar; #[cfg(accessible(bar))] item;, but you couldn't access methods that way (trait method or otherwise), so type-specific resolution shouldn't be an issue. Would that work?

[petrochenkov]

Reporting an error on paths that can potentially point to associated items shouldn't be much harder than reporting an error on multi-segment paths, so the single-segment restriction would mostly be artificial.
It can be done as an intermediate step though, if the second item in https://github.com/rust-lang/rust/issues/64797#issuecomment-625803760 (which is the current blocker) is going to be (partially) implemented by someone unfamiliar with resolve.

[m-ou-se]

You could still write use std::foo::bar; #[cfg(accessible(bar))] item;

Wouldn't that use fail directly if that bar wasn't accessible?

[coolreader18]

If I'm understanding correctly, that's the point; it would avoid the question of trait/type paths and just implement a sort of cfg alias as an MVP.

[joshtriplett]

@petrochenkov wrote:

Reporting an error on paths that can potentially point to associated items shouldn't be much harder than reporting an error on multi-segment paths, so the single-segment restriction would mostly be artificial.
It can be done as an intermediate step though, if the second item in #64797 (comment) (which is the current blocker) is going to be (partially) implemented by someone unfamiliar with resolve.

I read that message, and what I was wondering was whether that second item would be substantially simpler to implement and stabilize if it doesn't have to take trait methods or any other type-dependent resolution into account.

[petrochenkov]

@joshtriplett
It already doesn't have to account for type-dependent resolution, it just has to report an error if type-dependent resolutions are potentially possible.

---

Accounting for type-dependent resolutions would require emitting some new kind of where clauses resolved during type checking, like

#[cfg(accessigle(Type::dependent))]
struct S { ... }

=>

// Name `S` is still defined, but referring to it is an error if the where predicate ends up being false during type checking, similarly to false bounds with `#![feature(trivial_bounds)]`.
struct S where exists(Type::dependent) {
    ...
}

[thomcc]

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:

• This one is attempting to replicate #[cfg(accessible(libc::getloadavg))]

#[cfg(any(
    target_os = "macos",
    target_os = "ios",
    target_os = "linux", // android?
    target_os = "netbsd",
    target_os = "openbsd",
    // target_os = "freebsd", ??
    // TODO: verify
))]

• This one is attempting to replicate #[cfg(accessible(libc::sysctlbyname))]

#[cfg(any(
    target_os = "macos",
    target_os = "ios",
    target_os = "freebsd"
    // does anybody else have this?
))]

I briefly almost considered having doing an autoconf-style "do a test build to check existence", expose --cfg have_<c function>.

But more generally: When you consider this in conjunction with performing bindgen, it gets pretty powerful, and makes it vastly easier to support C apis that gain functionality over time (for example, I'd love to use that inside rusqlite as well).

[coolreader18]

Yup, that's my sorta use case as well: the _os module in RustPython has a bunch of wrappers around libc functions, and it'd be really great if we could just do #[cfg(accessible(libc::foo))] fn os_foo(x: i32) -> i32 { libc::foo(x) }, rather than copying cfg()s from libc that might become inaccurate later. Also for libc constants: it'd be really nice if we could just create a map of String->i32 of constants just by passing the names of all the ones that _might_ exist to a macro.