Skip to content

Allow &&, ||, and ! in cfg #3796

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

jhpratt
Copy link
Member

@jhpratt jhpratt commented Mar 30, 2025

About as simple as it gets.

Rendered

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

I think that a feature != "string" syntax as an alternative to not(feature = "string") would be a nice addition here. !(feature = "string") feels weirder to me than not(feature = "string"), honestly.

@jhpratt
Copy link
Member Author

jhpratt commented Mar 31, 2025

The problem I see with feature != "foo" is that it could plausibly be interpreted as "any feature other than foo".

@clarfonthey
Copy link

That interpretation feels like a stretch to me, but it's also understandable. Perhaps a feature("string") syntax might be a bit more natural with !feature("string"), and also extensible to feature("string", "other string").

@jhpratt
Copy link
Member Author

jhpratt commented Mar 31, 2025

Updated the following:

  • !feature = "foo" is no longer permitted. Negation can be followed by further negation, parentheses, a single identifier (so #[cfg(!unix)]), and any function-like predicate.
  • &/| added as alternative to &&/||
  • feature != "foo" added as alternative to !(feature = "foo")
  • added table with examples
  • linked to operator precedence for expressions for clarity

@RalfJung
Copy link
Member

RalfJung commented Apr 1, 2025

I think that a feature != "string" syntax as an alternative to not(feature = "string") would be a nice addition here. !(feature = "string") feels weirder to me than not(feature = "string"), honestly.

The problem here is that = in a cfg expression really means "contains", since feature (and also all other cfg variables) are really sets of strings, not just strings. So != would then mean "does not contain".

= is at least a different symbol from == so there's some hint that the semantics might be different, though it's not a very clear hint at all. != would exactly match expressions so there's not even a hint that the semantics are different.

@m-ou-se
Copy link
Member

m-ou-se commented Apr 1, 2025

I read #[cfg(feature = "asdf")] similar to how I read things like #[serde(default = "asdf")] or #[link_name = "asdf"]: The = syntax is used as a key-value pair, like in all attributes.

As mentioned above, feature = "asdf" is not at all like a Rust expression. It doesn't behave like a = expression (it's not an assignment). It doesn't behave like an == expression (it doesn't check equality either). There is no variable/concept named feature that is compared to anything.

It isn't even commutative: #[cfg("asdf" = feature)] doesn't work.

Using key = "value" syntax makes sense as it matches the syntax for all other attributes. But I don't think it makes much sense to take && and || from Rust expressions and trying to force them into attribute syntax. The result feels weirdly inconsistent, with things like !(feature = "asdf") not being the same as feature != "asdf", and so on.

If we do want to make cfg() take something that looks like an expression, we should also change the feature = "asdf" syntax to something that fits a Rust expression. E.g. #[cfg(has_feature("asdf") && has_target_feature("asdf"))].

@m-ou-se
Copy link
Member

m-ou-se commented Apr 1, 2025

Let's look at a real world example:

#[cfg(any(target_os = "android", target_os = "linux", target_os = "emscripten", target_os = "freebsd"))]

This proposal would allow rewriting that as:

#[cfg(target_os = "android" || target_os = "linux" || target_os = "emscripten" || target_os = "freebsd"))]

Which looks a bit nicer.

But instead of just replacing any() by a series of ||, I think we can do much better without giving up on the key = value syntax with something like:

#[cfg(target_os = "android" | "linux" | "emscripten" | "freebsd")]

I don't want to derail this thread with alternative proposals, but I do think that && and || is not getting us that much. If we go in that direction of making cfg() accept expression-like syntax, then we might be closing ourselves off from improvements to the key = value syntax that might serve the actual use cases better.

@RalfJung
Copy link
Member

RalfJung commented Apr 1, 2025

The result feels weirdly inconsistent, with things like !(feature = "asdf") not being the same as feature != "asdf"

How would they not be the same? The RFC, I think, intents for both of these to mean the same thing: the feature variable does not contain "asdf".

@m-ou-se
Copy link
Member

m-ou-se commented Apr 1, 2025

!(feature = "asdf") would work, while feature != "asdf" would be an error.

@RalfJung
Copy link
Member

RalfJung commented Apr 1, 2025

Ah. That seems entirely consistent to me, it's = and not == after all.

I like target_os = "android" | "linux" | "emscripten" | "freebsd" but it doesn't when you need to look at more than one key.

@m-ou-se
Copy link
Member

m-ou-se commented Apr 1, 2025

My point is that we need tot look at real world examples of code we're trying to simplify.

For example, quite often, any() is used with the same key, such as when selecting for a list of different operating systems. We should optimize the syntax for patterns that occur in actual code.

Large cfg() attributes can be very confusing. But replacing any() and all() by || and && isn't making it much better. Large boolean expressions will still be confusing, especially if their syntax is a hybrid of expressions and (key=value) attribute syntax with inconsistent precedence and operator meaning.

@m-ou-se
Copy link
Member

m-ou-se commented Apr 1, 2025

Rust parses feature = "asdf" && unix as feature = ("asdf" && unix), not as (feature = "asdf") && unix.

One might argue that = in attributes is different, but how would we expect an attribute proc macro to parse this today?

#[my_attribute_macro(expr = a && b, expr2 = c || d)]

I would assume it interprets that as two key = value pairs with the values being a && b and c || d. (It's up to the proc macro to decide, this is what you get if you parse it with a Rust expression parser.)

So even in an attribute, I'd argue that && should have higher precedence than the key=value operator. But that's the opposite of what this RFC proposes.

@programmerjake
Copy link
Member

maybe embrace that they are sets and allow using "abcd" in feature instead of feature = "abcd"? though then that makes me think we should have named it features.

@kpreid
Copy link
Contributor

kpreid commented Apr 1, 2025

Large cfg() attributes can be very confusing. But replacing any() and all() by || and && isn't making it much better.

I would go so far as to argue that any() and all() are better for formatting. Binary operators used repeatedly have an awkward indentation problem. Currently, rustfmt produces:

#[cfg(any(
    feature = "foo",
    feature = "bar",
    feature = "baz",
    feature = "quux",
    feature = "magic",
    test
))]

Whereas a similar boolean expression is less regular and longer:

something(cfg(
    feature == "foo"
        || feature == "bar"
        || feature == "baz"
        || feature == "quux"
        || feature == "magic"
        || test,
));

(And there are probably even messier cases to present with && and ! but I don't feel up to producing an illustrative example right now.) I won’t say that I haven’t wished for conventional binary operators in simple cases, but switching to them completely would have significant disadvantages (as would having a mix of both as standard style, simply due to having two things to learn and the choice of which one to use).

On the other hand, supporting feature = "foo" | "bar" has the inherent advantage of making the expression shorter and less repetitive, while still aligning with existing Rust syntax (patterns). It does raise the question of “why can’t I just write cfg(windows | unix)”, which, if also supported, would have a precedence problem — i.e. you have to either allow or prohibit cfg(feature = "foo" | bar), and if allowed, bar could be misinterpreted as a merely-unquoted value string.

@kennytm
Copy link
Member

kennytm commented Apr 1, 2025

So even in an attribute, I'd argue that && should have higher precedence than the key=value operator. But that's the opposite of what this RFC proposes.

GitHub really needs a 😢 emoji.

To be compatible with this existing syntax, it means all key = "value" cfg has to be wrapped in parenthesis:

#[cfg((target_os="linux") && (target_arch="x86_64") && (target_env="gnu") && !debug_assertions)]

@kennytm
Copy link
Member

kennytm commented Apr 1, 2025

I would go so far as to argue that any() and all() are better for formatting. Binary operators used repeatedly have an awkward indentation problem.

...

Whereas a similar boolean expression is less regular and longer:

...

(And there are probably even messier cases to present with && and ! but I don't feel up to producing an illustrative example right now.) I won’t say that I haven’t wished for conventional binary operators in simple cases, but switching to them completely would have significant disadvantages (as would having a mix of both as standard style, simply due to having two things to learn and the choice of which one to use).

Yes the formatting is strange, but I'd argue that the binary operator makes the cfg more readable for reviewers because you can be sure it is a chain of any rather than all at the first glance (spot the difference:

Existing syntax
#[cfg(any(
    feature = "foo",
    feature = "bar",
    feature = "baz",
    feature = "quux",
    feature = "magic",
    test
))]
#[cfg(all(
    feature = "foo",
    feature = "bar",
    feature = "baz",
    feature = "quux",
    feature = "magic",
    test
))]
This RFC
#[cfg(
    (feature = "foo")
        || (feature = "bar")
        || (feature = "baz")
        || (feature = "quux")
        || (feature = "magic")
        || test
)]
#[cfg(
    (feature = "foo")
        && (feature = "bar")
        && (feature = "baz")
        && (feature = "quux")
        && (feature = "magic")
        && test
)]
Alternative proposal
#[cfg(any(
    feature = "foo" 
        | "bar" 
        | "baz" 
        | "quux" 
        | "magic",
    test,
))]
#[cfg(all(
    feature = "foo" 
        & "bar" 
        & "baz" 
        & "quux" 
        & "magic",
    test,
))]

@SabrinaJewson
Copy link

cfg(any()) and cfg(all()) are very useful, often in macros, as always-false and always-true cfg values respectively. This RFC does not need to supplant all existing use cases of all and any, but it does seem weird to replace most things except this one edge case.

@Jules-Bertholet
Copy link
Contributor

cfg(any()) and cfg(all()) are very useful, often in macros, as always-false and always-true cfg values respectively.

rust-lang/rust#131204

@joshtriplett
Copy link
Member

@m-ou-se wrote:

If we do want to make cfg() take something that looks like an expression, we should also change the feature = "asdf" syntax to something that fits a Rust expression. E.g. #[cfg(has_feature("asdf") && has_target_feature("asdf"))].

Agreed completely. I don't think it would be ambiguous here to reuse "feature" without adding "has_": cfg(feature("asdf")). And this would also be consistent with other parts of cfg syntax.

@kennytm
Copy link
Member

kennytm commented Apr 2, 2025

I don't think it would be ambiguous here to reuse "feature" without adding "has_": cfg(feature("asdf")). And this would also be consistent with other parts of cfg syntax.

Technically this change of syntax will conflict with the cfg_version feature but since it is unstable it is fine...? (plus can't use version="x" on stable at all, it somehow triggers cfg_version too)

#![feature(cfg_version)]
fn main() {
	dbg!(cfg!(version("1.69.0")));
	dbg!(cfg!(version("2.0.0")));
	dbg!(cfg!(version = "1.69.0"));
	dbg!(cfg!(version = "2.0.0"));
}
$ rustc +nightly --cfg 'version="2.0.0"' 1.rs
$ ./1
[1.rs:3:2] cfg!(version("1.69.0")) = true
[1.rs:4:2] cfg!(version("2.0.0")) = false
[1.rs:5:2] cfg!(version = "1.69.0") = false
[1.rs:6:2] cfg!(version = "2.0.0") = true

@jdonszelmann
Copy link

jdonszelmann commented Apr 3, 2025

I feel like many problems raised (by various people) here can be partially mitigated by changing feature = "something" into feature("something") and equivalently for the other key-values such as target_arch("x86_64")

I didn't come up with this, it felt like a slightly nicer way of writing @m-ou-se's has_feature which I like less, and now after having written the rest of this comment and reading back the thread I see that @clarfonthey proposed this quite literally.

Having said that, let this comment be a motivation for why I think this mitigates so many of the concerns. The advantages I see are:

  1. The confusion about the meaning of = (whether it means == or "assignment =" or a third kind of =) becomes irrelevant.
  2. Because we use parentheses, we automatically disambiguate any use of && and ||. In other words, #[my_attribute_macro(expr = a && b, expr2 = c || d)] is instead written as #[my_attribute_macro(expr(a) && b && expr2(c) || d)] Though of course, whether && has precedence over || is still here
  3. Whether to permit feature != "a" is irrelevant. it'd always be !feature("a")
  4. it's consistent with any , all and not which are already parenthesized.
  5. If we choose to permit alternation within an attribute (i.e. feature = "a" | "b" | "c" which with my proposal becomes feature("a" | "b" | "c")) this becomes clearly unambiguous from feature("a") | b | c

@jhpratt
Copy link
Member Author

jhpratt commented Apr 3, 2025

I'll defer to the team here. I'm happy to write up whatever changes are desired, which at this point sounds like it may be feature("foo"), key(val1 | val2 | val3), etc. If this is the case, I have one question: should we permit mixing and matching or only support all-or-nothing?

@tmccombs
Copy link

tmccombs commented Apr 4, 2025

I like the proposal in #3796 (comment), but the feature = "abc" syntax still needs to be supported for backwords compatibility, so we need to define how that interacts with the new syntax. Do we

  1. Disallow the = syntax in combination with && or ||
  2. Require any use of = to be within parentheses, ex. (feature="abc") && (target_os="linux")
  3. As originally described in this RFC
  4. Something else

@camsteffen
Copy link

I propose that we add key(val | val) syntax without adding &&, ||, !. That's where the bigger win is, and it would be less churn.

In a world of boolean logic, the existence of any/all/not functions is indubitably justified, and they deserve our respect.

@jhpratt
Copy link
Member Author

jhpratt commented Apr 5, 2025

@camsteffen Is churn the primary (or even sole) concern? I think the 👍s on this PR show that there's unquestionably the desire for logical operators — it feels obvious imo.

@jdonszelmann
Copy link

I actually like @camsteffen's proposal a lot. It makes many scenarios simpler right now without changing ther original feature = name syntax. And the existing situation isn't so bad. And, if we feel like adding boolean operators later that still works!

@camsteffen
Copy link

@jhpratt Not just churn, but the fact of having two ways to express something and a split in the ecosystem of which one is used. The motivation may be to make things simpler by matching other syntax, but I think the reality is that it's increasing complexity with another thing to learn. I would be more open to it if I could snap my fingers and change all the code in the world to the new syntax. But I also agree with @kpreid that any/all is better with regard to formatting.

@Jules-Bertholet
Copy link
Contributor

Jules-Bertholet commented Apr 7, 2025

I would go so far as to argue that any() and all() are better for formatting. Binary operators used repeatedly have an awkward indentation problem…

Yes the formatting is strange, but I'd argue that the binary operator makes the cfg more readable for reviewers…

[…]

Alternative proposal

#[cfg(any(
    feature = "foo" 
        | "bar" 
        | "baz" 
        | "quux" 
        | "magic",
    test,
))]
#[cfg(all(
    feature = "foo" 
        & "bar" 
        & "baz" 
        & "quux" 
        & "magic",
    test,
))]

A little more indentation makes this look much better:

#[cfg(any(
    feature = "foo" 
            | "bar" 
            | "baz" 
            | "quux"
            | "magic",
    test,
))]
#[cfg(all(
    feature = "foo" 
            & "bar" 
            & "baz" 
            & "quux" 
            & "magic",
    test,
))]

@kennytm
Copy link
Member

kennytm commented Apr 7, 2025

@Jules-Bertholet the default rustfmt will not use vertical alignment.


@camsteffen Rust isn't afraid of having multiple ways to express the same concept, we have for x in it {} vs while let Some(x) = t.next() {}; async fn f() -> R vs fn f() -> impl Future<Output=R>; fn g(a: impl Read) vs fn g<R: Read>(a: R) vs fn g<R>(a: R) where R: Read+, etc. Even your proposal it going to introduce #[cfg(feature = "name")] vs #[cfg(feature("name"))]. IMO "splitting the ecosystem" is FUD and is no ground to reject a proposal for Rust.

This proposed syntax #[cfg(key("val1" | "val2" | "val3"))] works well when everything involved has the same "key", but won't help when multiple "keys" are involved, e.g.:

Exhibit A

Exhibit B

Existing code

#[cfg(all(
    doc,
    any(
        all(target_arch = "wasm32", not(target_os = "wasi")),
        all(target_vendor = "fortanix", target_env = "sgx")
    )
))]
#[cfg(any(
    unix,
    windows,
    target_os = "psp",
    target_os = "solid_asp3",
    all(target_vendor = "fortanix", target_env = "sgx"),
))]
This RFC
#[cfg(doc && (
    (target_arch = "wasm32") && !(target_os = "wasi") 
        || (target_vendor = "fortanix") && (target_env = "sgx")
))]
#[cfg(unix 
    || windows 
    || (target_os = "psp") 
    || (target_os = "solid_asp3")
    || (target_vendor = "fortanix") && (target_env = "sgx")
)]
key("value") only
#[cfg(all(
    doc,
    any(
        all(target_arch("wasm32"), not(target_os("wasi"))),
        all(target_vendor("fortanix"), target_env("sgx"))
    )
))]
#[cfg(any(
    unix,
    windows,
    target_os("psp" | "solid_asp3"),
    all(target_vendor("fortanix"), target_env("sgx")),
))]
This RFC + key("value")
#[cfg(doc && (
    target_arch("wasm32") && !target_os("wasi") 
        || target_vendor("fortanix") && target_env("sgx")
))]
#[cfg(unix 
    || windows 
    || target_os("psp" | "solid_asp3")
    || target_vendor("fortanix") && target_env("sgx")
)]

@RalfJung
Copy link
Member

RalfJung commented Apr 7, 2025 via email

@SOF3
Copy link

SOF3 commented Apr 8, 2025

Can we just have && and || without the ! part? ! is already too overused, and it gets worse when applied in #![] attributes. I don't want to see this in my code:

#![cfg(!windows)]

@jhpratt
Copy link
Member Author

jhpratt commented Apr 8, 2025

@SOF3 Realistically you wouldn't because there isn't much of a use case for that specifically. ! meaning "not" is nearly universal in programming languages.

@RalfJung
Copy link
Member

RalfJung commented Apr 8, 2025

For those that are curious about the "nearly" part: the large family of ML-based languages including OCaml use ! to mean "load from a reference" (that's a bit like * in Rust but not quite). "not" is written ~ in those languages (I think this is intended to be close to ¬, the typical logical symbol for negation).

@kornelski
Copy link
Contributor

I really like this proposal. Not because it shortens some expressions, but because it's a step towards reducing the amount of unique microsyntaxes inside the Rust language.

@kornelski
Copy link
Contributor

kornelski commented Apr 14, 2025

As for the = bikeshed, how about feature.name/target_os.wasi, pretending that it's a struct with bool fields for all the options.

@programmerjake
Copy link
Member

As for the = bikeshed, how about feature.name/target_os.wasi, pretending that it's a struct with bool fields for all the options.

what happens when values aren't a simple number/identifier? e.g. --cfg=foo="abc def": https://rust.godbolt.org/z/bKfKGorbW

@kennytm
Copy link
Member

kennytm commented Apr 14, 2025

As for the = bikeshed, how about feature.name/target_os.wasi, pretending that it's a struct with bool fields for all the options.

A feature name may consist of [_0-9\p{XID_Start}][-+.\p{XID_Continue}]*, and even the crates.io-restricted set is still [_0-9a-zA-Z][-+_0-9a-zA-Z]*. It would be very confusing for the parser to allow #[cfg(feature.c++20)].

Furthermore a custom config --cfg 'thing="!@#$%^&"' allows any string literal as the value (including empty string --cfg 'thing=""', which is distinct from --cfg 'thing'). I don't think the quotation marks should be omitted.

#[cfg(feature = "c++20")]
#[cfg(feature("c++20"))]
#[cfg(feature."c++20")]

@programmerjake
Copy link
Member

I don't think the quotation marks should be omitted.

we could do something like TOML paths where you can omit the quotation marks if it's a valid identifier or number (equivalent to the corresponding quoted version) or you can have a string literal with any random text at all (except for commas since I think it's split into separate values using commas...).

so, with --cfg=a="123" --cfg=b="abc" --cfg=c="123-4,abc" that works with all of:

  • cfg(a.123)
  • cfg(a."123")
  • cfg(b.abc)
  • cfg(b."abc")
  • cfg(c."123-4")
  • cfg(c.abc)
  • cfg(c."abc")

as well as all the existing cfg(a = "123") syntax.

@kornelski
Copy link
Contributor

kornelski commented Apr 14, 2025

Without inventing new syntax (like c."abc") that isn't in Rust, access to non-identifier "fields" could be:

thing["#$%@!"]

For convenience, the thing.ident syntax could be supported too when the name is a simple identifier, similarly to JS where feature.foo and feature["foo"] are equivalent.

has_feature("foo") is also fine.

@RalfJung
Copy link
Member

key("value") is not inventing new syntax.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.