Skip to content

Commit 69eeb6a

Browse files
authored
Implement RFC3695: Allow boolean literals as cfg predicates (#14649)
### What does this PR try to resolve? This PR implements rust-lang/rfcs#3695: allow boolean literals as cfg predicates, i.e. `cfg(true)` and `cfg(false)`. ### How should we test and review this PR? The PR should be reviewed commit by commit and tested by looking at the tests and using `[target.'cfg(<true/false>)']` in `Cargo.toml`/`.cargo/config.toml`. ### Additional information I had to bump `cargo-platform` to `0.3.0` has we are changing `CfgExpr` enum in a semver incompatible change. We currently have (thks to #14671) a forward compatibility warning against `cfg(true/false)` as identifiers instead of keywords. ~~I choose a use a `Cargo.toml` feature (for the manifest) as well as a unstable CLI flag for the `.cargo/config.toml` part.~~ ~~Given the very small (two occurrences on Github Search) for [`cfg(true)`](https://github.com/search?q=lang%3Atoml+%2F%28%3F-i%29%5C%5Btarget%5C.%5B%27%22%5Dcfg.*true%2F&type=code) and [`cfg(false)`](https://github.com/search?q=lang%3Atoml+%2F%28%3F-i%29%5C%5Btarget%5C.%5B%27%22%5Dcfg.*false%2F&type=code), I choose to gate the feature under a error and not a warning.~~
2 parents 8ddf367 + 9563738 commit 69eeb6a

File tree

7 files changed

+205
-41
lines changed

7 files changed

+205
-41
lines changed

Cargo.lock

+3-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ cargo-credential = { version = "0.4.2", path = "credential/cargo-credential" }
3030
cargo-credential-libsecret = { version = "0.4.13", path = "credential/cargo-credential-libsecret" }
3131
cargo-credential-macos-keychain = { version = "0.4.13", path = "credential/cargo-credential-macos-keychain" }
3232
cargo-credential-wincred = { version = "0.4.13", path = "credential/cargo-credential-wincred" }
33-
cargo-platform = { path = "crates/cargo-platform", version = "0.2.0" }
33+
cargo-platform = { path = "crates/cargo-platform", version = "0.3.0" }
3434
cargo-test-macro = { version = "0.4.2", path = "crates/cargo-test-macro" }
3535
cargo-test-support = { version = "0.7.1", path = "crates/cargo-test-support" }
3636
cargo-util = { version = "0.2.20", path = "crates/cargo-util" }

crates/cargo-platform/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "cargo-platform"
3-
version = "0.2.0"
3+
version = "0.3.0"
44
edition.workspace = true
55
license.workspace = true
66
rust-version.workspace = true

crates/cargo-platform/src/cfg.rs

+11-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ pub enum CfgExpr {
1111
All(Vec<CfgExpr>),
1212
Any(Vec<CfgExpr>),
1313
Value(Cfg),
14+
True,
15+
False,
1416
}
1517

1618
/// A cfg value.
@@ -147,6 +149,8 @@ impl CfgExpr {
147149
CfgExpr::All(ref e) => e.iter().all(|e| e.matches(cfg)),
148150
CfgExpr::Any(ref e) => e.iter().any(|e| e.matches(cfg)),
149151
CfgExpr::Value(ref e) => cfg.contains(e),
152+
CfgExpr::True => true,
153+
CfgExpr::False => false,
150154
}
151155
}
152156
}
@@ -174,6 +178,8 @@ impl fmt::Display for CfgExpr {
174178
CfgExpr::All(ref e) => write!(f, "all({})", CommaSep(e)),
175179
CfgExpr::Any(ref e) => write!(f, "any({})", CommaSep(e)),
176180
CfgExpr::Value(ref e) => write!(f, "{}", e),
181+
CfgExpr::True => write!(f, "true"),
182+
CfgExpr::False => write!(f, "false"),
177183
}
178184
}
179185
}
@@ -229,7 +235,11 @@ impl<'a> Parser<'a> {
229235
self.eat(&Token::RightParen)?;
230236
Ok(CfgExpr::Not(Box::new(e)))
231237
}
232-
Some(Ok(..)) => self.cfg().map(CfgExpr::Value),
238+
Some(Ok(..)) => self.cfg().map(|v| match v {
239+
Cfg::Name(n) if n == "true" => CfgExpr::True,
240+
Cfg::Name(n) if n == "false" => CfgExpr::False,
241+
v => CfgExpr::Value(v),
242+
}),
233243
Some(Err(..)) => Err(self.t.next().unwrap().err().unwrap()),
234244
None => Err(ParseError::new(
235245
self.t.orig,

crates/cargo-platform/src/lib.rs

+10-21
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ impl Platform {
9898
))
9999
},
100100
}
101+
CfgExpr::True | CfgExpr::False => {},
101102
}
102103
}
103104

@@ -115,30 +116,18 @@ impl Platform {
115116
check_cfg_expr(e, warnings, path);
116117
}
117118
}
119+
CfgExpr::True | CfgExpr::False => {}
118120
CfgExpr::Value(ref e) => match e {
119121
Cfg::Name(name) | Cfg::KeyPair(name, _) => {
120122
if !name.raw && KEYWORDS.contains(&name.as_str()) {
121-
if name.as_str() == "true" || name.as_str() == "false" {
122-
warnings.push(format!(
123-
"[{}] future-incompatibility: the meaning of `cfg({e})` will change in the future\n \
124-
| Cargo is erroneously allowing `cfg(true)` and `cfg(false)`, but both forms are interpreted as false unless manually overridden with `--cfg`.\n \
125-
| In the future these will be built-in defines that will have the corresponding true/false value.\n \
126-
| It is recommended to avoid using these configs until they are properly supported.\n \
127-
| See <https://github.com/rust-lang/rust/issues/131204> for more information.\n \
128-
|\n \
129-
| help: use raw-idents instead: `cfg(r#{name})`",
130-
path.display()
131-
));
132-
} else {
133-
warnings.push(format!(
134-
"[{}] future-incompatibility: `cfg({e})` is deprecated as `{name}` is a keyword \
135-
and not an identifier and should not have have been accepted in this position.\n \
136-
| this was previously accepted by Cargo but is being phased out; it will become a hard error in a future release!\n \
137-
|\n \
138-
| help: use raw-idents instead: `cfg(r#{name})`",
139-
path.display()
140-
));
141-
}
123+
warnings.push(format!(
124+
"[{}] future-incompatibility: `cfg({e})` is deprecated as `{name}` is a keyword \
125+
and not an identifier and should not have have been accepted in this position.\n \
126+
| this was previously accepted by Cargo but is being phased out; it will become a hard error in a future release!\n \
127+
|\n \
128+
| help: use raw-idents instead: `cfg(r#{name})`",
129+
path.display()
130+
));
142131
}
143132
}
144133
},

crates/cargo-platform/tests/test_cfg.rs

+7
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ macro_rules! e {
3939
(any($($t:tt),*)) => (CfgExpr::Any(vec![$(e!($t)),*]));
4040
(all($($t:tt),*)) => (CfgExpr::All(vec![$(e!($t)),*]));
4141
(not($($t:tt)*)) => (CfgExpr::Not(Box::new(e!($($t)*))));
42+
(true) => (CfgExpr::True);
43+
(false) => (CfgExpr::False);
4244
(($($t:tt)*)) => (e!($($t)*));
4345
($($t:tt)*) => (CfgExpr::Value(c!($($t)*)));
4446
}
@@ -122,6 +124,9 @@ fn cfg_expr() {
122124
good(" foo=\"3\" ", e!(foo = "3"));
123125
good("foo = \"3 e\"", e!(foo = "3 e"));
124126

127+
good("true", e!(true));
128+
good("false", e!(false));
129+
125130
good("all()", e!(all()));
126131
good("all(a)", e!(all(a)));
127132
good("all(a, b)", e!(all(a, b)));
@@ -249,6 +254,8 @@ fn check_cfg_attributes() {
249254
ok("windows");
250255
ok("any(not(unix), windows)");
251256
ok("foo");
257+
ok("true");
258+
ok("false");
252259

253260
ok("target_arch = \"abc\"");
254261
ok("target_feature = \"abc\"");

tests/testsuite/cfg.rs

+172-14
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,7 @@ fn cfg_raw_idents() {
545545
p.cargo("check")
546546
.with_stderr_data(str![[r#"
547547
[LOCKING] 1 package to latest compatible version
548+
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
548549
[CHECKING] foo v0.1.0 ([ROOT]/foo)
549550
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
550551
@@ -638,24 +639,181 @@ fn cfg_keywords() {
638639

639640
p.cargo("check")
640641
.with_stderr_data(str![[r#"
641-
[WARNING] [[ROOT]/foo/Cargo.toml] future-incompatibility: the meaning of `cfg(true)` will change in the future
642-
| Cargo is erroneously allowing `cfg(true)` and `cfg(false)`, but both forms are interpreted as false unless manually overridden with `--cfg`.
643-
| In the future these will be built-in defines that will have the corresponding true/false value.
644-
| It is recommended to avoid using these configs until they are properly supported.
645-
| See <https://github.com/rust-lang/rust/issues/131204> for more information.
646-
|
647-
| [HELP] use raw-idents instead: `cfg(r#true)`
648-
[WARNING] [.cargo/config.toml] future-incompatibility: the meaning of `cfg(false)` will change in the future
649-
| Cargo is erroneously allowing `cfg(true)` and `cfg(false)`, but both forms are interpreted as false unless manually overridden with `--cfg`.
650-
| In the future these will be built-in defines that will have the corresponding true/false value.
651-
| It is recommended to avoid using these configs until they are properly supported.
652-
| See <https://github.com/rust-lang/rust/issues/131204> for more information.
653-
|
654-
| [HELP] use raw-idents instead: `cfg(r#false)`
655642
[LOCKING] 1 package to latest compatible version
643+
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
656644
[CHECKING] foo v0.1.0 ([ROOT]/foo)
657645
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
658646
659647
"#]])
660648
.run();
661649
}
650+
651+
#[cargo_test]
652+
fn cfg_booleans() {
653+
let p = project()
654+
.file(
655+
"Cargo.toml",
656+
r#"
657+
[package]
658+
name = "a"
659+
version = "0.0.1"
660+
edition = "2015"
661+
authors = []
662+
663+
[target.'cfg(true)'.dependencies]
664+
b = { path = 'b' }
665+
666+
[target.'cfg(false)'.dependencies]
667+
c = { path = 'c' }
668+
"#,
669+
)
670+
.file("src/lib.rs", "")
671+
.file("b/Cargo.toml", &basic_manifest("b", "0.0.1"))
672+
.file("b/src/lib.rs", "")
673+
.file("c/Cargo.toml", &basic_manifest("c", "0.0.1"))
674+
.file("c/src/lib.rs", "")
675+
.build();
676+
677+
p.cargo("check")
678+
.with_stderr_data(str![[r#"
679+
[LOCKING] 2 packages to latest compatible versions
680+
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
681+
[CHECKING] a v0.0.1 ([ROOT]/foo)
682+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
683+
684+
"#]])
685+
.run();
686+
}
687+
688+
#[cargo_test]
689+
fn cfg_booleans_config() {
690+
let p = project()
691+
.file(
692+
"Cargo.toml",
693+
r#"
694+
[package]
695+
name = "a"
696+
version = "0.0.1"
697+
edition = "2015"
698+
"#,
699+
)
700+
.file("src/lib.rs", "")
701+
.file(
702+
".cargo/config.toml",
703+
r#"
704+
[target.'cfg(true)']
705+
rustflags = []
706+
"#,
707+
)
708+
.build();
709+
710+
p.cargo("check")
711+
.with_stderr_data(str![[r#"
712+
[CHECKING] a v0.0.1 ([ROOT]/foo)
713+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
714+
715+
"#]])
716+
.run();
717+
}
718+
719+
#[cargo_test]
720+
fn cfg_booleans_not() {
721+
let p = project()
722+
.file(
723+
"Cargo.toml",
724+
r#"
725+
[package]
726+
name = "a"
727+
version = "0.0.1"
728+
edition = "2015"
729+
authors = []
730+
731+
[target.'cfg(not(false))'.dependencies]
732+
b = { path = 'b' }
733+
"#,
734+
)
735+
.file("src/lib.rs", "")
736+
.file("b/Cargo.toml", &basic_manifest("b", "0.0.1"))
737+
.file("b/src/lib.rs", "")
738+
.build();
739+
740+
p.cargo("check")
741+
.with_stderr_data(str![[r#"
742+
[LOCKING] 1 package to latest compatible version
743+
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
744+
[CHECKING] a v0.0.1 ([ROOT]/foo)
745+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
746+
747+
"#]])
748+
.run();
749+
}
750+
751+
#[cargo_test]
752+
fn cfg_booleans_combinators() {
753+
let p = project()
754+
.file(
755+
"Cargo.toml",
756+
r#"
757+
[package]
758+
name = "a"
759+
version = "0.0.1"
760+
edition = "2015"
761+
authors = []
762+
763+
[target.'cfg(all(any(true), not(false), true))'.dependencies]
764+
b = { path = 'b' }
765+
"#,
766+
)
767+
.file("src/lib.rs", "")
768+
.file("b/Cargo.toml", &basic_manifest("b", "0.0.1"))
769+
.file("b/src/lib.rs", "")
770+
.build();
771+
772+
p.cargo("check")
773+
.with_stderr_data(str![[r#"
774+
[LOCKING] 1 package to latest compatible version
775+
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
776+
[CHECKING] a v0.0.1 ([ROOT]/foo)
777+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
778+
779+
"#]])
780+
.run();
781+
}
782+
783+
#[cargo_test]
784+
fn cfg_booleans_rustflags_no_effect() {
785+
let p = project()
786+
.file(
787+
"Cargo.toml",
788+
r#"
789+
[package]
790+
name = "a"
791+
version = "0.0.1"
792+
edition = "2015"
793+
authors = []
794+
795+
[target.'cfg(true)'.dependencies]
796+
b = { path = 'b' }
797+
798+
[target.'cfg(false)'.dependencies]
799+
c = { path = 'c' }
800+
"#,
801+
)
802+
.file("src/lib.rs", "")
803+
.file("b/Cargo.toml", &basic_manifest("b", "0.0.1"))
804+
.file("b/src/lib.rs", "")
805+
.file("c/Cargo.toml", &basic_manifest("c", "0.0.1"))
806+
.file("c/src/lib.rs", "")
807+
.build();
808+
809+
p.cargo("check")
810+
.env("RUSTFLAGS", "--cfg false")
811+
.with_stderr_data(str![[r#"
812+
[LOCKING] 2 packages to latest compatible versions
813+
[CHECKING] b v0.0.1 ([ROOT]/foo/b)
814+
[CHECKING] a v0.0.1 ([ROOT]/foo)
815+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
816+
817+
"#]])
818+
.run();
819+
}

0 commit comments

Comments
 (0)