about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2022-03-18 21:50:46 +0100
committerGitHub <noreply@github.com>2022-03-18 21:50:46 +0100
commitd15006ceca3854e507341dd8b7171a451351b4cd (patch)
treebb4bbd1871d59189f449465b21fc9dc284df1db2
parentc183d4a510a0007923354bec8ec2ea4d23d92f7d (diff)
parentf57cc8ca5cd344a6111650a8ba4744b4e4859da8 (diff)
downloadrust-d15006ceca3854e507341dd8b7171a451351b4cd.tar.gz
rust-d15006ceca3854e507341dd8b7171a451351b4cd.zip
Rollup merge of #94295 - Urgau:cfg-always-eval-all-predicate, r=petrochenkov
Always evaluate all cfg predicate in all() and any()

This pull-request adjust the handling of the `all()` and `any()` to always evaluate every cfg predicate because not doing so result in accepting incorrect `cfg`:

```rust
#[cfg(any(unix, foo::bar))] // Should error on foo::bar, but does not on unix platform (but does on non unix platform)
fn foo1() {}

#[cfg(all(foo, foo::bar))] // Should error on foo::bar, but does not
fn foo2() {}

#[cfg(all(foo::bar, foo))] // Correctly error on foo::bar
fn foo3() {}

#[cfg(any(foo::bar, foo))] // Correctly error on foo::bar
fn foo4() {}
```
This pull-request take the side to directly turn it into a hard error instead of having a future incompatibility lint because the combination to get this incorrect behavior is unusual and highly probable that some code have this without noticing.

A [search](https://cs.github.com/?scopeName=All+repos&scope=&q=lang%3Arust+%2Fany%5C%28%5Ba-zA-Z%5D%2C+%5Ba-zA-Z%5D%2B%3A%3A%5Ba-zA-Z%5D%2B%2F) on Github reveal no such instance nevertheless a Crater run should probably be done before merging this.

This was discover in https://github.com/rust-lang/rust/pull/94175 when trying to lint on the second predicate. Also note that this seems to have being introduce with Rust 1.27.0: https://rust.godbolt.org/z/KnfqKv15f.

r? `@petrochenkov`
-rw-r--r--compiler/rustc_attr/src/builtin.rs12
-rw-r--r--src/test/ui/cfg/cfg-path-error.rs19
-rw-r--r--src/test/ui/cfg/cfg-path-error.stderr26
3 files changed, 55 insertions, 2 deletions
diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs
index 613320087d2..8a134bf7f96 100644
--- a/compiler/rustc_attr/src/builtin.rs
+++ b/compiler/rustc_attr/src/builtin.rs
@@ -603,10 +603,18 @@ pub fn eval_condition(
             match cfg.name_or_empty() {
                 sym::any => mis
                     .iter()
-                    .any(|mi| eval_condition(mi.meta_item().unwrap(), sess, features, eval)),
+                    // We don't use any() here, because we want to evaluate all cfg condition
+                    // as eval_condition can (and does) extra checks
+                    .fold(false, |res, mi| {
+                        res | eval_condition(mi.meta_item().unwrap(), sess, features, eval)
+                    }),
                 sym::all => mis
                     .iter()
-                    .all(|mi| eval_condition(mi.meta_item().unwrap(), sess, features, eval)),
+                    // We don't use all() here, because we want to evaluate all cfg condition
+                    // as eval_condition can (and does) extra checks
+                    .fold(true, |res, mi| {
+                        res & eval_condition(mi.meta_item().unwrap(), sess, features, eval)
+                    }),
                 sym::not => {
                     if mis.len() != 1 {
                         struct_span_err!(
diff --git a/src/test/ui/cfg/cfg-path-error.rs b/src/test/ui/cfg/cfg-path-error.rs
new file mode 100644
index 00000000000..5bf80bd74b8
--- /dev/null
+++ b/src/test/ui/cfg/cfg-path-error.rs
@@ -0,0 +1,19 @@
+// check-fail
+
+#[cfg(any(foo, foo::bar))]
+//~^ERROR `cfg` predicate key must be an identifier
+fn foo1() {}
+
+#[cfg(any(foo::bar, foo))]
+//~^ERROR `cfg` predicate key must be an identifier
+fn foo2() {}
+
+#[cfg(all(foo, foo::bar))]
+//~^ERROR `cfg` predicate key must be an identifier
+fn foo3() {}
+
+#[cfg(all(foo::bar, foo))]
+//~^ERROR `cfg` predicate key must be an identifier
+fn foo4() {}
+
+fn main() {}
diff --git a/src/test/ui/cfg/cfg-path-error.stderr b/src/test/ui/cfg/cfg-path-error.stderr
new file mode 100644
index 00000000000..84b44b2b0c2
--- /dev/null
+++ b/src/test/ui/cfg/cfg-path-error.stderr
@@ -0,0 +1,26 @@
+error: `cfg` predicate key must be an identifier
+  --> $DIR/cfg-path-error.rs:3:16
+   |
+LL | #[cfg(any(foo, foo::bar))]
+   |                ^^^^^^^^
+
+error: `cfg` predicate key must be an identifier
+  --> $DIR/cfg-path-error.rs:7:11
+   |
+LL | #[cfg(any(foo::bar, foo))]
+   |           ^^^^^^^^
+
+error: `cfg` predicate key must be an identifier
+  --> $DIR/cfg-path-error.rs:11:16
+   |
+LL | #[cfg(all(foo, foo::bar))]
+   |                ^^^^^^^^
+
+error: `cfg` predicate key must be an identifier
+  --> $DIR/cfg-path-error.rs:15:11
+   |
+LL | #[cfg(all(foo::bar, foo))]
+   |           ^^^^^^^^
+
+error: aborting due to 4 previous errors
+