-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Stabilize cfg_boolean_literals
#138632
base: master
Are you sure you want to change the base?
Stabilize cfg_boolean_literals
#138632
Conversation
r? @davidtwco rustbot has assigned @davidtwco. Use |
rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead. cc @rust-lang/rust-analyzer Some changes occurred in compiler/rustc_attr_parsing |
@clubby789: If you could, please link in the stabilization report to each test that we would want to see to review the behavior of this feature and the edges of it, and describe what each is intended to demonstrate if that's not already included as a comment in the test itself. |
Added a couple of new tests with the edge cases described, and descriptions of the test coverage to the report |
@rfcbot fcp merge |
Team member @tmandry has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
In testing, I notice that this works, as expected: //@ check-pass
//@ compile-flags: --cfg r#false --check-cfg=cfg(r#false)
fn main() {
#[cfg(not(r#false))]
compile_error!("");
} And this works: //@ check-pass
//@ compile-flags: --cfg false --check-cfg=cfg(r#false)
fn main() {
#[cfg(not(r#false))]
compile_error!("");
} But that this doesn't work: //@ check-pass
//@ compile-flags: --cfg false --check-cfg=cfg(false)
fn main() {
#[cfg(not(r#false))]
compile_error!("");
} What's the reason that it's OK for |
We should definitely ensure that We may wish to add a warning when you apply either of those to a keyword, since in code that would need to be matched with (None of these should ever affect the behavior of |
We discussed this today in the lang triage meeting. We believe this test should pass, exactly as it does for //@ check-pass
//@ revisions: r0x0 r0x1 r1x0 r1x1
//@[r0x0] compile-flags: --cfg false --check-cfg=cfg(false)
//@[r0x1] compile-flags: --cfg false --check-cfg=cfg(r#false)
//@[r1x0] compile-flags: --cfg r#false --check-cfg=cfg(false)
//@[r1x1] compile-flags: --cfg r#false --check-cfg=cfg(r#false)
#![deny(unexpected_cfgs)]
fn main() {
#[cfg(not(r#false))]
compile_error!("");
} Please add a test to this effect. |
In While in And since a path can be a identifier |
@Urgau: Do you think that |
I think we could justify the boolean literals being treated differently and accept them in
We could try to be more consistent in |
Yes. Since we already accept |
Made a PR to allow this |
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.
Stabilisation impl LGTM, r=me after t-lang FCP concludes
Closes #131204
@rustbot labels +T-lang +I-lang-nominated
This will end up conflicting with the test in #138293 so whichever doesn't land first will need updating
--
Stabilization Report
General design
What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized?
RFC 3695, none.
What behavior are we committing to that has been controversial? Summarize the major arguments pro/con.
None
Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those?
None
Has a call-for-testing period been conducted? If so, what feedback was received?
Yes; only positive feedback was received.
Implementation quality
Summarize the major parts of the implementation and provide links into the code (or to PRs)
Implemented in #131034.
Summarize existing test coverage of this feature
#[cfg()]
,cfg!()
and#[cfg_attr()]
--cfg=true/false
on the command line being accessible viar#true/r#false
#[doc(cfg(..))]
feature--check-cfg=cfg(true/false)
--cfg false
on the command line doesn't change the meaning ofcfg(false)
:tests/ui/cfg/cmdline-false.rs
cfg(true)
andcfg(false)
on the same item result in it being disabled:tests/ui/cfg/both-true-false.rs
What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking?
The above mentioned issue; it should not block as it interacts with another unstable feature.
What FIXMEs are still in the code for that feature and why is it ok to leave them there?
None
Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization
Which tools need to be adjusted to support this feature. Has this work been done?
rustdoc
's unstable#[doc(cfg(..)]
has been updated to respect it.cargo
has been updated with a forward compatibility lint to enable supporting it in cargo once stabilized.Type system and execution rules
What updates are needed to the reference/specification? (link to PRs when they exist)
A few lines to be added to the reference for configuration predicates, specified in the RFC.