Improve --check-cfg implementation#94175
Conversation
This comment has been minimized.
This comment has been minimized.
aa754d7 to
f36c077
Compare
|
I've address all review comments, except for #94175 (comment) and #94175 (comment) where I left a comment. I also would like to apologies for my English mistakes, it's not my native language. I will try my best to make sure these mistakes doesn't happen again. @rustbot ready |
|
Fixed #94175 (comment) and added a note when no value is expected ecd3a65. @rustbot ready |
|
r=me with the remaining comments addressed and commits squashed appropriately. |
ecd3a65 to
6e3f642
Compare
|
Fixed last remaining comments and squashed to 5 commits. This is ready for r=you. @rustbot ready |
|
@bors r+ |
|
📌 Commit 6e3f642 has been approved by |
|
@bors r+ |
|
📌 Commit a556a2a has been approved by |
|
⌛ Testing commit a556a2a with merge d0a34dc40eba51c4cd4173f8fb56b4c562209c90... |
|
💔 Test failed - checks-actions |
|
@bors retry (crates.io down) |
…rochenkov
Improve `--check-cfg` implementation
This pull-request is a mix of improvements regarding the `--check-cfg` implementation:
- Simpler internal representation (usage of `Option` instead of separate bool)
- Add --check-cfg to the unstable book (based on the RFC)
- Improved diagnostics:
* List possible values when the value is unexpected
* Suggest if possible a name or value that is similar
- Add more tests (well known names, mix of combinations, ...)
r? `@petrochenkov`
…rochenkov
Improve `--check-cfg` implementation
This pull-request is a mix of improvements regarding the `--check-cfg` implementation:
- Simpler internal representation (usage of `Option` instead of separate bool)
- Add --check-cfg to the unstable book (based on the RFC)
- Improved diagnostics:
* List possible values when the value is unexpected
* Suggest if possible a name or value that is similar
- Add more tests (well known names, mix of combinations, ...)
r? `@petrochenkov`
…rochenkov
Improve `--check-cfg` implementation
This pull-request is a mix of improvements regarding the `--check-cfg` implementation:
- Simpler internal representation (usage of `Option` instead of separate bool)
- Add --check-cfg to the unstable book (based on the RFC)
- Improved diagnostics:
* List possible values when the value is unexpected
* Suggest if possible a name or value that is similar
- Add more tests (well known names, mix of combinations, ...)
r? ``@petrochenkov``
Rollup of 9 pull requests Successful merges: - rust-lang#91795 (resolve/metadata: Stop encoding macros as reexports) - rust-lang#93714 (better ObligationCause for normalization errors in `can_type_implement_copy`) - rust-lang#94175 (Improve `--check-cfg` implementation) - rust-lang#94212 (Stop manually SIMDing in `swap_nonoverlapping`) - rust-lang#94242 (properly handle fat pointers to uninhabitable types) - rust-lang#94308 (Normalize main return type during mono item collection & codegen) - rust-lang#94315 (update auto trait lint for `PhantomData`) - rust-lang#94316 (Improve string literal unescaping) - rust-lang#94327 (Avoid emitting full macro body into JSON errors) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…, 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 rust-lang#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`
This pull-request is a mix of improvements regarding the
--check-cfgimplementation:Optioninstead of separate bool)r? @petrochenkov