Skip to content

Commit

Permalink
Rollup merge of #94295 - Urgau:cfg-always-eval-all-predicate, r=petro…
Browse files Browse the repository at this point in the history
…chenkov

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 #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`
  • Loading branch information
matthiaskrgr authored Mar 18, 2022
2 parents c183d4a + f57cc8c commit d15006c
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 2 deletions.
12 changes: 10 additions & 2 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
19 changes: 19 additions & 0 deletions src/test/ui/cfg/cfg-path-error.rs
Original file line number Diff line number Diff line change
@@ -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() {}
26 changes: 26 additions & 0 deletions src/test/ui/cfg/cfg-path-error.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit d15006c

Please sign in to comment.