Skip to content

Commit

Permalink
Rollup merge of rust-lang#117522 - Urgau:check-cfg-cli-own-lint, r=pe…
Browse files Browse the repository at this point in the history
…trochenkov

Remove `--check-cfg` checking of command line `--cfg` args

Back in rust-lang#100574 we added to the `unexpected_cfgs` lint the checking of `--cfg` CLI arguments and emitted unexpected names and values for them.

The implementation works as expected, but it's usability in particular when using it in combination with Cargo+`RUSTFLAGS` as people who set `RUSTFLAGS=--cfg=tokio_unstable` (or whatever) have `unexpected_cfgs` warnings on all of their crates is debatable. ~~To fix this issue this PR proposes that we split the CLI argument checking into it's own separate allow-by-default lint: `unexpected_cli_cfgs`.~~

~~This has the advantage of letting people who want CLI warnings have them (although not by default anymore), while still linting on every unexpected cfg name and values in the code.~~

After some discussion with the Cargo team ([Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/check-cfg.20and.20RUSTFLAGS.20interaction)) and member of the compiler team (see below), I propose that we follow the suggestion from `@epage:` never check `--cfg` arguments, but still reserve us the possibility to do it later.

We would still lint on unexpected cfgs found in the source code no matter the `--cfg` args passed. This mean reverting rust-lang#100574 but NOT rust-lang#99519.

r? `@petrochenkov`
  • Loading branch information
Noratrieb authored Nov 21, 2023
2 parents e62f7be + a5658e6 commit aff407e
Show file tree
Hide file tree
Showing 19 changed files with 32 additions and 130 deletions.
6 changes: 0 additions & 6 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,6 @@ lint_builtin_type_alias_generic_bounds = bounds on generic parameters are not en
lint_builtin_type_alias_where_clause = where clauses are not enforced in type aliases
.suggestion = the clause will not be checked when the type alias is used, and should be removed
lint_builtin_unexpected_cli_config_name = unexpected `{$name}` as condition name
.help = was set with `--cfg` but isn't in the `--check-cfg` expected names
lint_builtin_unexpected_cli_config_value = unexpected condition value `{$value}` for condition name `{$name}`
.help = was set with `--cfg` but isn't in the `--check-cfg` expected values
lint_builtin_unpermitted_type_init_label = this code causes undefined behavior when executed
lint_builtin_unpermitted_type_init_label_suggestion = help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
Expand Down
25 changes: 0 additions & 25 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ use crate::{
BuiltinMutablesTransmutes, BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns,
BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasGenericBounds,
BuiltinTypeAliasGenericBoundsSuggestion, BuiltinTypeAliasWhereClause,
BuiltinUnexpectedCliConfigName, BuiltinUnexpectedCliConfigValue,
BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit,
BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnsafe,
BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
Expand All @@ -60,7 +59,6 @@ use rustc_middle::ty::GenericArgKind;
use rustc_middle::ty::ToPredicate;
use rustc_middle::ty::TypeVisitableExt;
use rustc_middle::ty::{self, Ty, TyCtxt, VariantDef};
use rustc_session::config::ExpectedValues;
use rustc_session::lint::{BuiltinLintDiagnostics, FutureIncompatibilityReason};
use rustc_span::edition::Edition;
use rustc_span::source_map::Spanned;
Expand Down Expand Up @@ -2889,26 +2887,3 @@ impl EarlyLintPass for SpecialModuleName {
}
}
}

pub use rustc_session::lint::builtin::UNEXPECTED_CFGS;

declare_lint_pass!(UnexpectedCfgs => [UNEXPECTED_CFGS]);

impl EarlyLintPass for UnexpectedCfgs {
fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &ast::Crate) {
let cfg = &cx.sess().parse_sess.config;
let check_cfg = &cx.sess().parse_sess.check_config;
for &(name, value) in cfg {
match check_cfg.expecteds.get(&name) {
Some(ExpectedValues::Some(values)) if !values.contains(&value) => {
let value = value.unwrap_or(kw::Empty);
cx.emit_lint(UNEXPECTED_CFGS, BuiltinUnexpectedCliConfigValue { name, value });
}
None if check_cfg.exhaustive_names => {
cx.emit_lint(UNEXPECTED_CFGS, BuiltinUnexpectedCliConfigName { name });
}
_ => { /* expected */ }
}
}
}
}
1 change: 0 additions & 1 deletion compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ early_lint_methods!(
IncompleteInternalFeatures: IncompleteInternalFeatures,
RedundantSemicolons: RedundantSemicolons,
UnusedDocComment: UnusedDocComment,
UnexpectedCfgs: UnexpectedCfgs,
]
]
);
Expand Down
15 changes: 0 additions & 15 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,21 +553,6 @@ pub enum BuiltinSpecialModuleNameUsed {
Main,
}

#[derive(LintDiagnostic)]
#[diag(lint_builtin_unexpected_cli_config_name)]
#[help]
pub struct BuiltinUnexpectedCliConfigName {
pub name: Symbol,
}

#[derive(LintDiagnostic)]
#[diag(lint_builtin_unexpected_cli_config_value)]
#[help]
pub struct BuiltinUnexpectedCliConfigValue {
pub name: Symbol,
pub value: Symbol,
}

// deref_into_dyn_supertrait.rs
#[derive(LintDiagnostic)]
#[diag(lint_supertrait_as_deref_target)]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3439,6 +3439,7 @@ declare_lint_pass! {
UNCONDITIONAL_PANIC,
UNCONDITIONAL_RECURSION,
UNDEFINED_NAKED_FUNCTION_ABI,
UNEXPECTED_CFGS,
UNFULFILLED_LINT_EXPECTATIONS,
UNINHABITED_STATIC,
UNKNOWN_CRATE_TYPES,
Expand Down
3 changes: 3 additions & 0 deletions src/doc/unstable-book/src/compiler-flags/check-cfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ and `cfg!(name = "value")` call. It will check that the `"value"` specified is p
list of expected values. If `"value"` is not in it, then `rustc` will report an `unexpected_cfgs`
lint diagnostic. The default diagnostic level for this lint is `Warn`.

The command line `--cfg` arguments are currently *NOT* checked but may very well be checked in
the future.

To enable checking of values, but to provide an empty set of expected values, use these forms:

```bash
Expand Down
10 changes: 3 additions & 7 deletions tests/ui/check-cfg/exhaustive-names-values.empty_cfg.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: unexpected `cfg` condition name: `unknown_key`
--> $DIR/exhaustive-names-values.rs:12:7
--> $DIR/exhaustive-names-values.rs:11:7
|
LL | #[cfg(unknown_key = "value")]
| ^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -8,7 +8,7 @@ LL | #[cfg(unknown_key = "value")]
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition value: `value`
--> $DIR/exhaustive-names-values.rs:16:7
--> $DIR/exhaustive-names-values.rs:15:7
|
LL | #[cfg(test = "value")]
| ^^^^----------
Expand All @@ -17,9 +17,5 @@ LL | #[cfg(test = "value")]
|
= note: no expected value for `test`

warning: unexpected `empty_cfg` as condition name
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected names

warning: 3 warnings emitted
warning: 2 warnings emitted

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: unexpected `cfg` condition name: `unknown_key`
--> $DIR/exhaustive-names-values.rs:12:7
--> $DIR/exhaustive-names-values.rs:11:7
|
LL | #[cfg(unknown_key = "value")]
| ^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -8,7 +8,7 @@ LL | #[cfg(unknown_key = "value")]
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition value: `value`
--> $DIR/exhaustive-names-values.rs:16:7
--> $DIR/exhaustive-names-values.rs:15:7
|
LL | #[cfg(test = "value")]
| ^^^^----------
Expand All @@ -17,9 +17,5 @@ LL | #[cfg(test = "value")]
|
= note: no expected value for `test`

warning: unexpected `empty_names_values` as condition name
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected names

warning: 3 warnings emitted
warning: 2 warnings emitted

12 changes: 4 additions & 8 deletions tests/ui/check-cfg/exhaustive-names-values.feature.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: unexpected `cfg` condition name: `unknown_key`
--> $DIR/exhaustive-names-values.rs:12:7
--> $DIR/exhaustive-names-values.rs:11:7
|
LL | #[cfg(unknown_key = "value")]
| ^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -8,7 +8,7 @@ LL | #[cfg(unknown_key = "value")]
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition value: `value`
--> $DIR/exhaustive-names-values.rs:16:7
--> $DIR/exhaustive-names-values.rs:15:7
|
LL | #[cfg(test = "value")]
| ^^^^----------
Expand All @@ -18,16 +18,12 @@ LL | #[cfg(test = "value")]
= note: no expected value for `test`

warning: unexpected `cfg` condition value: `unk`
--> $DIR/exhaustive-names-values.rs:20:7
--> $DIR/exhaustive-names-values.rs:19:7
|
LL | #[cfg(feature = "unk")]
| ^^^^^^^^^^^^^^^
|
= note: expected values for `feature` are: `std`

warning: unexpected condition value `` for condition name `feature`
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected values

warning: 4 warnings emitted
warning: 3 warnings emitted

12 changes: 4 additions & 8 deletions tests/ui/check-cfg/exhaustive-names-values.full.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: unexpected `cfg` condition name: `unknown_key`
--> $DIR/exhaustive-names-values.rs:12:7
--> $DIR/exhaustive-names-values.rs:11:7
|
LL | #[cfg(unknown_key = "value")]
| ^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -8,7 +8,7 @@ LL | #[cfg(unknown_key = "value")]
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition value: `value`
--> $DIR/exhaustive-names-values.rs:16:7
--> $DIR/exhaustive-names-values.rs:15:7
|
LL | #[cfg(test = "value")]
| ^^^^----------
Expand All @@ -18,16 +18,12 @@ LL | #[cfg(test = "value")]
= note: no expected value for `test`

warning: unexpected `cfg` condition value: `unk`
--> $DIR/exhaustive-names-values.rs:20:7
--> $DIR/exhaustive-names-values.rs:19:7
|
LL | #[cfg(feature = "unk")]
| ^^^^^^^^^^^^^^^
|
= note: expected values for `feature` are: `std`

warning: unexpected `full` as condition name
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected names

warning: 4 warnings emitted
warning: 3 warnings emitted

3 changes: 1 addition & 2 deletions tests/ui/check-cfg/exhaustive-names-values.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Check warning for unexpected cfg in the code and in the CLI
// arguments (here the revision cfg).
// Check warning for unexpected cfg in the code.
//
// check-pass
// revisions: empty_names_values empty_cfg feature full
Expand Down
6 changes: 1 addition & 5 deletions tests/ui/check-cfg/exhaustive-names.empty_names.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,5 @@ LL | #[cfg(unknown_key = "value")]
= help: expected names are: `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `empty_names` as condition name
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected names

warning: 2 warnings emitted
warning: 1 warning emitted

6 changes: 1 addition & 5 deletions tests/ui/check-cfg/exhaustive-names.exhaustive_names.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,5 @@ LL | #[cfg(unknown_key = "value")]
= help: expected names are: `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `exhaustive_names` as condition name
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected names

warning: 2 warnings emitted
warning: 1 warning emitted

6 changes: 1 addition & 5 deletions tests/ui/check-cfg/exhaustive-values.empty_cfg.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,5 @@ LL | #[cfg(test = "value")]
= note: no expected value for `test`
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `empty_cfg` as condition name
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected names

warning: 2 warnings emitted
warning: 1 warning emitted

10 changes: 1 addition & 9 deletions tests/ui/check-cfg/mix.cfg.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,6 @@ LL | #[cfg_attr(uu, test)]
|
= help: expected names are: `cfg`, `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `names_values`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`

warning: unexpected condition value `bar` for condition name `feature`
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected values

warning: unexpected `unknown_name` as condition name
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected names

warning: unexpected `cfg` condition name: `widnows`
--> $DIR/mix.rs:43:10
|
Expand Down Expand Up @@ -188,5 +180,5 @@ LL | cfg!(all(feature = "zebra", feature = "zebra", feature = "zebra"));
|
= note: expected values for `feature` are: `foo`

warning: 28 warnings emitted
warning: 26 warnings emitted

10 changes: 1 addition & 9 deletions tests/ui/check-cfg/mix.names_values.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,6 @@ LL | #[cfg_attr(uu, test)]
|
= help: expected names are: `cfg`, `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `names_values`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`

warning: unexpected condition value `bar` for condition name `feature`
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected values

warning: unexpected `unknown_name` as condition name
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected names

warning: unexpected `cfg` condition name: `widnows`
--> $DIR/mix.rs:43:10
|
Expand Down Expand Up @@ -188,5 +180,5 @@ LL | cfg!(all(feature = "zebra", feature = "zebra", feature = "zebra"));
|
= note: expected values for `feature` are: `foo`

warning: 28 warnings emitted
warning: 26 warnings emitted

10 changes: 3 additions & 7 deletions tests/ui/check-cfg/unexpected-cfg-value.cfg.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: unexpected `cfg` condition value: `sedre`
--> $DIR/unexpected-cfg-value.rs:11:7
--> $DIR/unexpected-cfg-value.rs:9:7
|
LL | #[cfg(feature = "sedre")]
| ^^^^^^^^^^-------
Expand All @@ -10,16 +10,12 @@ LL | #[cfg(feature = "sedre")]
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition value: `rand`
--> $DIR/unexpected-cfg-value.rs:18:7
--> $DIR/unexpected-cfg-value.rs:16:7
|
LL | #[cfg(feature = "rand")]
| ^^^^^^^^^^^^^^^^
|
= note: expected values for `feature` are: `full`, `serde`

warning: unexpected condition value `rand` for condition name `feature`
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected values

warning: 3 warnings emitted
warning: 2 warnings emitted

6 changes: 2 additions & 4 deletions tests/ui/check-cfg/unexpected-cfg-value.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
// Check warning for invalid configuration value in the code and
// in the cli
// Check for unexpected configuration value in the code.
//
// check-pass
// revisions: values cfg
// compile-flags: --cfg=feature="rand" -Z unstable-options
// compile-flags: --check-cfg=cfg(values,cfg)
// compile-flags: -Z unstable-options
// [values]compile-flags: --check-cfg=values(feature,"serde","full")
// [cfg]compile-flags: --check-cfg=cfg(feature,values("serde","full"))

Expand Down
10 changes: 3 additions & 7 deletions tests/ui/check-cfg/unexpected-cfg-value.values.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: unexpected `cfg` condition value: `sedre`
--> $DIR/unexpected-cfg-value.rs:11:7
--> $DIR/unexpected-cfg-value.rs:9:7
|
LL | #[cfg(feature = "sedre")]
| ^^^^^^^^^^-------
Expand All @@ -10,16 +10,12 @@ LL | #[cfg(feature = "sedre")]
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition value: `rand`
--> $DIR/unexpected-cfg-value.rs:18:7
--> $DIR/unexpected-cfg-value.rs:16:7
|
LL | #[cfg(feature = "rand")]
| ^^^^^^^^^^^^^^^^
|
= note: expected values for `feature` are: `full`, `serde`

warning: unexpected condition value `rand` for condition name `feature`
|
= help: was set with `--cfg` but isn't in the `--check-cfg` expected values

warning: 3 warnings emitted
warning: 2 warnings emitted

0 comments on commit aff407e

Please sign in to comment.