Allow &&, ||, and ! in cfg

[jhpratt]

View all comments

About as simple as it gets.

Rendered

[clarfonthey]

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]

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

[clarfonthey]

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]

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]

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]

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]

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]

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]

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

[RalfJung]

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]

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]

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]

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]

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]

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]

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]

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]

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]

@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]

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

[jmjoy]

Simple Boolean operations are one thing, but what I’m really hoping for is the ability to compare values, something that’s quite common in C macros, like:

#if FOO > 1000

But in Rust, there’s currently no way to express this, which leads to some rather awkward workarounds.

Right now, the values in Rust’s cfg are all strings, so it seems difficult to support something like:

#[cfg(FOO > 1000)]

[jhpratt]

@jmjoy That would fundamentally change what cfg is.

[joshtriplett]

There have been proposals for key-value features, and I'd expect such a proposal to talk about how to use them in cfg, but that's off-topic for this RFC.

[Jules-Bertholet]

It would be nice if this RFC also specified a new macro_rules! fragment specifier to match an arbitrary cfg predicate, for use by macros like cfg_if. Currently, you can use meta, which excludes only true and false, but with this RFC, that would become completely inadequate.

[traviscross]

It would be nice if this RFC also specified a new macro_rules! fragment specifier to match an arbitrary cfg predicate, for use by macros like cfg_if. Currently, you can use meta, which excludes only true and false, but with this RFC, that would become completely inadequate.

That's a good point. This document should, I think, discuss the effect of this on macro fragment specifiers and propose solutions, perhaps using our guidance in #3531, if appropriate.

@rfcbot concern discuss-effect-on-macro-fragment-specifiers

[jhpratt]

meta already works, though, as it matches the entire attribute inside the brackets, not just what's inside cfg(). Playground link for proof.

Sure, things like cfg_if and cfg_select need to work around some things because they match on the inner parts, but isn't that already the case?

[Jules-Bertholet]

Sure, things like cfg_if and cfg_select need to work around some things because they match on the inner parts, but isn't that already the case?

Right now, the only thing they “need to work around” is true/false:

• cfg_if simply doesn’t support them.
• cfg_select in the stdlib doesn’t use macro_rules!.
• Support #[rustc_align_static] inside thread_local! rust#146281 (a PR I wrote that modifies the std thread_local such that it parses #[cfg_attr(…)]) uses extra match arms just for true and false, which is annoying¹ but doable. Supporting this RFC’s syntax, in contrast, would require complex TT-munching, which is really unappealing, especially given that the macro has far too much of that going on already.

Footnotes

1. If this RFC were to be rejected, I would probably propose changing the meta specifier to accept true and false, in order to address this issue—it would be a backward-compatible change that would not require an edition. But if this RFC is accepted, a dedicated cfg specifier would make more sense, and there wouldn’t be much point in changing meta. ↩

[jhpratt]

How common are macros that need to handle the internal part of cfg? Ultimately that's what we're talking about here. The fact that meta, prior to true/false, matched the internal part perfectly was largely a historical artifact from before arbitrary tokens could be accepted. That restriction was lifted in April 2019.

Essentially, my argument is that this scenario is sufficiently uncommon that it is not worth the added language complexity for a rarely used macro fragment specifier.

[Jules-Bertholet]

How common are macros that need to handle the internal part of cfg?

I just cited three examples.

If you feel that defining an entirely new specifier is too much complexity, one alternative would be to re-use expr.

[jhpratt]

I just cited three examples.

I meant in general, everyday code. Defining macros like that is not common, presumably, hence my question. My belief is that manipulating the contents of cfg is quite rare, as nearly all cases will want to merely pass the contents on to the output, for which purpose meta or $(x:tt)* would suffice. All existing macro fragment specifiers are used with sufficient frequency (or are fundamentally required due to "follows" ambiguity) that having one here seems overly niche.

For what it's worth, meta does not appear to have been intended to be used in this manner in the first place; it seems to be a historical quirk that the internal syntax followed the same pattern as the attribute as a whole.

[Jules-Bertholet]

Actually, I realized that cfg_if doesn’t need a dedicated fragment specifier, it can just switch to tt: rust-lang/cfg-if#99. That wouldn’t work for cfg_match or rust-lang/rust#146281, however.

[tgross35]

See rust-lang/rust#145060 for previous discussion on meta matching true and false. Quoting @petrochenkov:

meta is not supposed to match syntax of cfg predicates, or other syntaxes that other attributes may define, it matches the stuff inside the attribute brackets #[...].

* `#[attr]` - ok, `attr` is `meta`
* `#[attr { a b c }]` - ok, `attr { a b c }` is `meta`
* `#[true]` - not ok, `true` is not `meta`

[Jules-Bertholet]

one alternative would be to re-use expr.

Increasingly I am thinking that this is probably the best solution. expr already accepts all the syntax in this RFC, the restriction on its use in cfg simply needs to be lifted.

[traviscross]

@rfcbot resolve discuss-effect-on-macro-fragment-specifiers

[Jules-Bertholet]

Increasingly I am thinking that this is probably the best solution. expr already accepts all the syntax in this RFC, the restriction on its use in cfg simply needs to be lifted.

rust-lang/rust#146961

[jhpratt]

Regarding precedence, would it be easier to pass the RFC if instead we required (feature = "foo"), erroring on the unparenthesized version? That would avoid the precedence carveout while being forward compatible with anything we want.

[traviscross]

What would be an example of something that would not be accepted under that rule but would be accepted with = having its normal precedence? I.e., is there any case where giving = its normal precedence would not already force the parentheses?

[jhpratt]

I believe they would be identical — that's what I meant in my previous comment. Eliminate the carveout and require parentheses.

[traviscross]

That would indeed resolve my concern.

[jhpratt]

@traviscross I've added a commit removing the precedence carveout while also adding a couple notes.

[traviscross]

Thanks. That resolves this concern:

@rfcbot resolve uncomfortable-with-precedence-carve-out

[traviscross]

Since the proposal has now changed, and as there are no outstanding filed concerns to track, for procedural reasons, I'm going to cancel the proposed FCP.

@rfcbot fcp cancel

I'm not going to restart immediately, since I personally want to see other work, such as @tmandry's upcoming cfg_version RFC, play out a bit first, but anyone else on the team who wants to propose a new FCP can of course do so.

[rust-rfcbot]

@traviscross proposal cancelled.

[jhpratt]

A year on from when FCP was initially proposed and 8 months out from the most recent comment, what does @rust-lang/lang think of this? There has been time to consider interaction with cfg_version, hopefully.