-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: Overhaul the #[cfg(..)]
pattern syntax
#194
Conversation
Looks good. Did you want to treat the initial |
Forgive the unterminated bracket. I'm on my phone and can't edit the message ;) |
My gut would be neither to avoid confusion, though defaulting to |
And |
proposed names read better with the function-like syntax and are consistent | ||
with `Iterator::all` and `Iterator::any`. | ||
|
||
Issue [#2119](https://github.com/rust-lang/rust/issuse/2119) proposed the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, link should point to rust/issues/2119 (s/issuse/issues)
will change from "include `foo` if *either of* `a` and `b` are present in the | ||
compilation environment" to "include `foo` if *both of* `a` and `b` are present | ||
in the compilation environment". To ease the transition it will be an error to | ||
have multiple `#[cfg(...)]` attributes on a single item for some reasonable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's entirely crazy for multiple cfg
s to be an error indefinitely.
👍 on the proposal, but I think we can make some tweaks to aid the transition. First, multiple However, if either of these cases are encountered in the wild, a warning will be emitted, with a note describing how to rewrite it. This warning will not be a lint, it will instead be built in to the At some point in the future, we can switch this to an error. And then at a later time, if we decide it's appropriate, we can then turn multiple |
cc rust-lang/rust#2119, the very very old bug on this |
@kballard sounds reasonable to me. |
6357402
to
e0acdf4
Compare
warning. After some reasonable period of time, the special case will be | ||
removed. | ||
|
||
In addition, `#[cfg(a, b, c)]` will be accepted with a warning and be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this give three ways to write all
? #[cfg(a, b)]
#[cfg(a)] #[cfg(b)]
and #[cfg(all(a, b))]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the first and second behaviors will trigger deprecation warnings.
Edit: Actually, the second case will in the short term be equivalent to #[cfg(any(a, b)]
, again with a warning.
This was discussed in today's meeting and the decision was to merge it, thanks @sfackler! |
fixed another bunch of clippy warnings
…event-mananger Deprecate component `eventManager`
No description provided.