Skip to content

RFC: naming groups of configuration with cfg_alias #3804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Apr 23, 2025

This proposal introduces a way to name configuration predicates for easy reuse throughout a crate.

#![cfg_alias(
    x86_linux,
    all(any(target_arch = "x86", target_arch = "x86_64"), target_os = "linux")
)]

#[cfg(x86_linux)]
fn foo() { /* ... */ }
#[cfg(not(x86_linux))]
fn foo() { /* ... */ }

Previous discussion:

Rendered

Comment on lines 127 to 129
- The syntax `cfg_alias(name, predicate)` was chosen for similarity with
`cfg_attr(predicate, attributes)`. Alternatives include:
- `cfg_alias(name = predicate)`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the = form. cfg_attr doesn't seem like a compelling precedent when the predicate goes first with cfg_attr and second with cfg_alias.

I also happen to think the cfg_attr syntax is hard to read, we should avoid replicating that :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cfg_attr is definitely not very nice to read or write (though, I don't really know what would have been better here). Reading through it more, I am also starting to prefer = but one concern is that = in config is used for something like equality rather than assignment. Should there be any concern about that here?

I know that also came up in discussion at #3796

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the syntax to use =

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other downside: with =, it looks a bit weird when aliasing a single = option

#![cfg_alias(alias = target_os = "linux")]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that is a negative. I'll retract my preference then. I'm not crazy about the comma but it's closest to what we have..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could choose to require parens there.


[RFC3697]: https://github.com/rust-lang/rfcs/pull/3697

# Rationale and alternatives
Copy link
Member

@tmandry tmandry Apr 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inspired by the above, one alternative that comes to mind is declarative attribute macros that do the cfg matching for you. I think actually you have to declare two macros, one when the cfg you want is true and one when it is false, so that's a major drawback because it would require repeating the same clause twice.

However, attribute macros (possibly in combination with this feature) would allow a crate to "export" an alias.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point, I'll mention that. One other downside is that with a specific set of config tied to an attribute macro, it wouldn't be easily possible to combine with other config in all or any (could probably be done with the attribute macro's parameters).

Exporting would be quite convenient at times.

Copy link
Member

@tmandry tmandry Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point that the macro approach doesn't compose all that well. I wish there was an obvious way to support exporting these.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a more radical design, it might be possible to treat aliases as effectively a new kind of macro; something that shares the macro namespace but only expands within cfg. I think this would mean the new macro scoping can be used, so pub use some_alias can make an alias crate-public for another crate to import with use crate_with_alias::some_alias.

It sounds borderline too complex for an otherwise pretty simple feature, but being able to do that could be a nice help if public macros expand to code that contains #[cfg(...)].

With that, it would almost be possible to define the builtin cfg(windows)/cfg(unix) as something like cfg_alias(windows = target_os = "windows") in the prelude (not that we'd have any reason to actually do that).

Copy link

@mejrs mejrs Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect some kind of way to export an alias. I'd be disappointed with a design that wouldn't permit it.

Anyway, your comment about macros made me think of something like...

// library

#[macro_export]
macro_rules! has_atomics {
    () =>  { any(target = a, target = b, .. ) }
}

// crate
#[cfg(has_atomics!())]
fn blah(){}

That has the advantage of not needing a new kind of attribute or new syntax to define an alias. It also makes it obvious when an alias is being used.

Moreover this syntax implies that you can pass arguments.

Let me give an example where this would be useful to me personally. The Python C api has different ABI guarantees. Take PyObject_Vectorcall for example. This function was added in Python 3.9, but only in 3.12 it was added to the Stable ABI.

That means I define the bindings as:

extern "C" {
    #[cfg(any(Py_3_9, all(Py_3_12, not(Py_LIMITED_API))))]
    pub fn PyObject_Vectorcall(..) -> ...

This would be a lot simpler if the syntax is macro-like:

macro_rules! limited {
    ($added_in:ident) =>  { all($added_in, not(Py_LIMITED_API)) }
    ($added_in:ident, $stable_in:ident) =>  { any($stable_in, all($added_in, not(Py_LIMITED_API))) }
}

#[cfg(limited!(Py_3_9, Py_3_12))]
// ...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think @mejrs that this would be quite nice. I suspect it might be difficult to implement, though. Maybe what's needed is an experiment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for that.

@ehuss ehuss added the T-lang Relevant to the language team, which will review and decide on the RFC. label Apr 24, 2025
@oxalica
Copy link

oxalica commented Apr 24, 2025

#![cfg_alias(some_alias = predicate)]

I'm against syntax cfg_alias(ident = predicate) and prefer cfg_alias(ident, predicate), because of consistency concerns:

  1. We already have a very similar cfg_attr(predicate, meta) syntax. cfg_alias(ident, predicate) just flips it and identifier is also a valid meta. Using comma here is most natural and consistent design.
  2. path(ident = meta) is a completely new syntax that does not exist anywhere, and contradicts the status quo that = always follows an expression, adding cognitive complexity for human. See current "meta" grammar in reference. Not sure if it will introduce actual implementation complexity.
  3. As mentioned above, #![cfg_alias(name = target_os = "linux")] has two = with different meaning which is very confusing. Same situation can also occur when nesting cfg_alias inside other attributes or vice versa.

```rust
#![cfg_alias(todo = false)] // change `false` to `true` to enable WIP code

#[cfg(todo)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #3804 (comment) I had meant more that you could use #[cfg(todo)] as an attribute you could add/delete from sections of code, rather than toggling todo's value. So it would be kinda like how todo!() always panics, rather than disappearing if you toggle some flag.

e.g.:

#[cfg(todo)]
pub fn uses_some_api_that_isnt_finished() {
    api::cool_function_that_doesnt_exist_yet();
}

later, once that api is implemented, you can just delete the #[cfg(todo)] line to have your code no longer be skipped.

Comment on lines +72 to +73
`predicate` can be anything that usually works within `#[cfg(...)]`, including
`all`, `any`, and `not`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably drop ", including..." here as it makes it more rather than less ambiguous. Alternatively, it'd be OK to say e.g. "including (but not limited to) combining operators such as all, any, and not.

Comment on lines +95 to +98
```text
CfgAliasAttribute:
cfg_alias(IDENTIFIER `=` ConfigurationPredicate)
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a look at the grammar syntax in the Reference. Probably best to just use that.

Comment on lines +138 to +139
`cfg_alias` may also be used as a module-level attribute rather than
crate-level:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why module level? Once we eat the cost of making these scoped, I'm curious if there's a reason we shouldn't specify them for other scopes also.

In terms of how we specify the language, it actually makes the specification simpler to have fewer rather than more exceptions. (We can always of course still incrementally stabilize if there are reasons to do so.)

Of course, if we go with a different design, such as leaning into macros somehow, then we could sidestep this question.

Comment on lines +190 to +191
- It may be possible to have `#[cfg_alias(...)]` work as an outer macro and only
apply to a specific scope. This likely is not worth the complexity.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- It may be possible to have `#[cfg_alias(...)]` work as an outer macro and only
apply to a specific scope. This likely is not worth the complexity.

Since this has now been added to the RFC, this alternative can be removed.

Comment on lines +216 to +217
- Substitution vs. evaluation at define time (the question under the
reference-level explanation)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Substitution vs. evaluation at define time (the question under the
reference-level explanation)
- Substitution vs. evaluation at define time (the question under the
reference-level explanation).

@traviscross traviscross added the I-lang-radar Items that are on lang's radar and will need eventual work or consideration. label Apr 24, 2025
- It may be possible to have `#[cfg_alias(...)]` work as an outer macro and only
apply to a specific scope. This likely is not worth the complexity.

# Prior art

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cfg_aliases crate could be mentioned. https://crates.io/crates/cfg_aliases

The identifier is added to the `cfg` namespace. It must not conflict with:

- Any builtin configuration names
- Any configuration passed via `--cfg`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to clarify, if we passed --cfg 'foo="bar"', it means cfg_alias(foo, ...) will be also conflicting right?

// Enabled/disabled based on `cfg(baz)`
#[cfg(foo)]
fn qux() { /* ... */ }
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some details on hygiene. For instance, what happens if you write a cfg_alias on a macro_defining_a_fn!()? Does the macro "see" the alias set, ignoring hygiene? Or do you need to pass the identifier into the macro (e.g. macro_defining_a_fn!(the_alias), so that hygiene works? I would expect the latter.

Copy link
Contributor

@traviscross traviscross Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


- A `--cfg-alias` CLI option would provide a way for Cargo to interact with this
feature, such as defining config aliases in the workspace `Cargo.toml` for
reuse in multiple crates.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
reuse in multiple crates.
reuse in multiple crates.
- We could add a visibility to the syntax, allowing a crate to export a cfg alias for use by other crates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-lang-radar Items that are on lang's radar and will need eventual work or consideration. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants