about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJubilee <46493976+workingjubilee@users.noreply.github.com>2023-10-28 01:07:37 -0700
committerGitHub <noreply@github.com>2023-10-28 01:07:37 -0700
commit87a564d271999dafcc9bcfdcca33fd5fa60b0e58 (patch)
treece26f707f807146ed11cb953d7f435b70f6461f7
parent9f631d0c233790cd7aeef53f2e0409cc038c8ba4 (diff)
parent84a1a689ccc8eb6b8be48f3385bc9b0aeb5c71a6 (diff)
downloadrust-87a564d271999dafcc9bcfdcca33fd5fa60b0e58.tar.gz
rust-87a564d271999dafcc9bcfdcca33fd5fa60b0e58.zip
Rollup merge of #117025 - Urgau:cleanup-improve-check-cfg-impl, r=petrochenkov
Cleanup and improve `--check-cfg` implementation

This PR removes some indentation in the code, as well as preventing some bugs/misusages and fix a nit in the doc.

r? ```@petrochenkov``` (maybe)
-rw-r--r--compiler/rustc_interface/src/interface.rs385
-rw-r--r--src/doc/unstable-book/src/compiler-flags/check-cfg.md2
-rw-r--r--tests/ui/check-cfg/invalid-arguments.any_values.stderr2
-rw-r--r--tests/ui/check-cfg/invalid-arguments.rs4
-rw-r--r--tests/ui/check-cfg/invalid-arguments.unterminated.stderr2
5 files changed, 193 insertions, 202 deletions
diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs
index 87badcbdf15..60e060a9dc8 100644
--- a/compiler/rustc_interface/src/interface.rs
+++ b/compiler/rustc_interface/src/interface.rs
@@ -140,7 +140,7 @@ pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec<String>) -> Check
             let filename = FileName::cfg_spec_source_code(&s);
 
             macro_rules! error {
-                ($reason: expr) => {
+                ($reason:expr) => {
                     handler.early_error(format!(
                         concat!("invalid `--check-cfg` argument: `{}` (", $reason, ")"),
                         s
@@ -148,217 +148,202 @@ pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec<String>) -> Check
                 };
             }
 
-            let expected_error =
-                || error!("expected `cfg(name, values(\"value1\", \"value2\", ... \"valueN\"))`");
-
-            match maybe_new_parser_from_source_str(&sess, filename, s.to_string()) {
-                Ok(mut parser) => match parser.parse_meta_item() {
-                    Ok(meta_item) if parser.token == token::Eof => {
-                        if let Some(args) = meta_item.meta_item_list() {
-                            if meta_item.has_name(sym::names) {
-                                // defaults are flipped for the old syntax
-                                if old_syntax == None {
-                                    check_cfg.exhaustive_names = false;
-                                    check_cfg.exhaustive_values = false;
-                                }
-                                old_syntax = Some(true);
-
-                                check_cfg.exhaustive_names = true;
-                                for arg in args {
-                                    if arg.is_word() && arg.ident().is_some() {
-                                        let ident = arg.ident().expect("multi-segment cfg key");
-                                        check_cfg
-                                            .expecteds
-                                            .entry(ident.name.to_string())
-                                            .or_insert(ExpectedValues::Any);
-                                    } else {
-                                        error!("`names()` arguments must be simple identifiers");
-                                    }
-                                }
-                            } else if meta_item.has_name(sym::values) {
-                                // defaults are flipped for the old syntax
-                                if old_syntax == None {
-                                    check_cfg.exhaustive_names = false;
-                                    check_cfg.exhaustive_values = false;
-                                }
-                                old_syntax = Some(true);
-
-                                if let Some((name, values)) = args.split_first() {
-                                    if name.is_word() && name.ident().is_some() {
-                                        let ident = name.ident().expect("multi-segment cfg key");
-                                        let expected_values = check_cfg
-                                            .expecteds
-                                            .entry(ident.name.to_string())
-                                            .and_modify(|expected_values| match expected_values {
-                                                ExpectedValues::Some(_) => {}
-                                                ExpectedValues::Any => {
-                                                    // handle the case where names(...) was done
-                                                    // before values by changing to a list
-                                                    *expected_values =
-                                                        ExpectedValues::Some(FxHashSet::default());
-                                                }
-                                            })
-                                            .or_insert_with(|| {
-                                                ExpectedValues::Some(FxHashSet::default())
-                                            });
-
-                                        let ExpectedValues::Some(expected_values) = expected_values
-                                        else {
-                                            bug!("`expected_values` should be a list a values")
-                                        };
-
-                                        for val in values {
-                                            if let Some(LitKind::Str(s, _)) =
-                                                val.lit().map(|lit| &lit.kind)
-                                            {
-                                                expected_values.insert(Some(s.to_string()));
-                                            } else {
-                                                error!(
-                                                    "`values()` arguments must be string literals"
-                                                );
-                                            }
-                                        }
-
-                                        if values.is_empty() {
-                                            expected_values.insert(None);
-                                        }
-                                    } else {
-                                        error!(
-                                            "`values()` first argument must be a simple identifier"
-                                        );
-                                    }
-                                } else if args.is_empty() {
-                                    check_cfg.exhaustive_values = true;
-                                } else {
-                                    expected_error();
-                                }
-                            } else if meta_item.has_name(sym::cfg) {
-                                old_syntax = Some(false);
-
-                                let mut names = Vec::new();
-                                let mut values: FxHashSet<_> = Default::default();
-
-                                let mut any_specified = false;
-                                let mut values_specified = false;
-                                let mut values_any_specified = false;
-
-                                for arg in args {
-                                    if arg.is_word() && let Some(ident) = arg.ident() {
-                                        if values_specified {
-                                            error!("`cfg()` names cannot be after values");
-                                        }
-                                        names.push(ident);
-                                    } else if arg.has_name(sym::any)
-                                        && let Some(args) = arg.meta_item_list()
-                                    {
-                                        if any_specified {
-                                            error!("`any()` cannot be specified multiple times");
-                                        }
-                                        any_specified = true;
-                                        if !args.is_empty() {
-                                            error!("`any()` must be empty");
-                                        }
-                                    } else if arg.has_name(sym::values)
-                                        && let Some(args) = arg.meta_item_list()
-                                    {
-                                        if names.is_empty() {
-                                            error!(
-                                                "`values()` cannot be specified before the names"
-                                            );
-                                        } else if values_specified {
-                                            error!(
-                                                "`values()` cannot be specified multiple times"
-                                            );
-                                        }
-                                        values_specified = true;
-
-                                        for arg in args {
-                                            if let Some(LitKind::Str(s, _)) =
-                                                arg.lit().map(|lit| &lit.kind)
-                                            {
-                                                values.insert(Some(s.to_string()));
-                                            } else if arg.has_name(sym::any)
-                                                && let Some(args) = arg.meta_item_list()
-                                            {
-                                                if values_any_specified {
-                                                    error!(
-                                                        "`any()` in `values()` cannot be specified multiple times"
-                                                    );
-                                                }
-                                                values_any_specified = true;
-                                                if !args.is_empty() {
-                                                    error!("`any()` must be empty");
-                                                }
-                                            } else {
-                                                error!(
-                                                    "`values()` arguments must be string literals or `any()`"
-                                                );
-                                            }
-                                        }
-                                    } else {
-                                        error!(
-                                            "`cfg()` arguments must be simple identifiers, `any()` or `values(...)`"
-                                        );
-                                    }
+            let expected_error = || -> ! {
+                error!("expected `cfg(name, values(\"value1\", \"value2\", ... \"valueN\"))`")
+            };
+
+            let Ok(mut parser) = maybe_new_parser_from_source_str(&sess, filename, s.to_string())
+            else {
+                expected_error();
+            };
+
+            let meta_item = match parser.parse_meta_item() {
+                Ok(meta_item) if parser.token == token::Eof => meta_item,
+                Ok(..) => expected_error(),
+                Err(err) => {
+                    err.cancel();
+                    expected_error();
+                }
+            };
+
+            let Some(args) = meta_item.meta_item_list() else {
+                expected_error();
+            };
+
+            if meta_item.has_name(sym::names) {
+                // defaults are flipped for the old syntax
+                if old_syntax == None {
+                    check_cfg.exhaustive_names = false;
+                    check_cfg.exhaustive_values = false;
+                }
+                old_syntax = Some(true);
+
+                check_cfg.exhaustive_names = true;
+                for arg in args {
+                    if arg.is_word() && arg.ident().is_some() {
+                        let ident = arg.ident().expect("multi-segment cfg key");
+                        check_cfg
+                            .expecteds
+                            .entry(ident.name.to_string())
+                            .or_insert(ExpectedValues::Any);
+                    } else {
+                        error!("`names()` arguments must be simple identifiers");
+                    }
+                }
+            } else if meta_item.has_name(sym::values) {
+                // defaults are flipped for the old syntax
+                if old_syntax == None {
+                    check_cfg.exhaustive_names = false;
+                    check_cfg.exhaustive_values = false;
+                }
+                old_syntax = Some(true);
+
+                if let Some((name, values)) = args.split_first() {
+                    if name.is_word() && name.ident().is_some() {
+                        let ident = name.ident().expect("multi-segment cfg key");
+                        let expected_values = check_cfg
+                            .expecteds
+                            .entry(ident.name.to_string())
+                            .and_modify(|expected_values| match expected_values {
+                                ExpectedValues::Some(_) => {}
+                                ExpectedValues::Any => {
+                                    // handle the case where names(...) was done
+                                    // before values by changing to a list
+                                    *expected_values = ExpectedValues::Some(FxHashSet::default());
                                 }
+                            })
+                            .or_insert_with(|| ExpectedValues::Some(FxHashSet::default()));
+
+                        let ExpectedValues::Some(expected_values) = expected_values else {
+                            bug!("`expected_values` should be a list a values")
+                        };
+
+                        for val in values {
+                            if let Some(LitKind::Str(s, _)) = val.lit().map(|lit| &lit.kind) {
+                                expected_values.insert(Some(s.to_string()));
+                            } else {
+                                error!("`values()` arguments must be string literals");
+                            }
+                        }
+
+                        if values.is_empty() {
+                            expected_values.insert(None);
+                        }
+                    } else {
+                        error!("`values()` first argument must be a simple identifier");
+                    }
+                } else if args.is_empty() {
+                    check_cfg.exhaustive_values = true;
+                } else {
+                    expected_error();
+                }
+            } else if meta_item.has_name(sym::cfg) {
+                old_syntax = Some(false);
+
+                let mut names = Vec::new();
+                let mut values: FxHashSet<_> = Default::default();
+
+                let mut any_specified = false;
+                let mut values_specified = false;
+                let mut values_any_specified = false;
 
-                                if values.is_empty() && !values_any_specified && !any_specified {
-                                    values.insert(None);
-                                } else if !values.is_empty() && values_any_specified {
+                for arg in args {
+                    if arg.is_word() && let Some(ident) = arg.ident() {
+                        if values_specified {
+                            error!("`cfg()` names cannot be after values");
+                        }
+                        names.push(ident);
+                    } else if arg.has_name(sym::any)
+                        && let Some(args) = arg.meta_item_list()
+                    {
+                        if any_specified {
+                            error!("`any()` cannot be specified multiple times");
+                        }
+                        any_specified = true;
+                        if !args.is_empty() {
+                            error!("`any()` must be empty");
+                        }
+                    } else if arg.has_name(sym::values)
+                        && let Some(args) = arg.meta_item_list()
+                    {
+                        if names.is_empty() {
+                            error!("`values()` cannot be specified before the names");
+                        } else if values_specified {
+                            error!("`values()` cannot be specified multiple times");
+                        }
+                        values_specified = true;
+
+                        for arg in args {
+                            if let Some(LitKind::Str(s, _)) =
+                                arg.lit().map(|lit| &lit.kind)
+                            {
+                                values.insert(Some(s.to_string()));
+                            } else if arg.has_name(sym::any)
+                                && let Some(args) = arg.meta_item_list()
+                            {
+                                if values_any_specified {
                                     error!(
-                                        "`values()` arguments cannot specify string literals and `any()` at the same time"
+                                        "`any()` in `values()` cannot be specified multiple times"
                                     );
                                 }
-
-                                if any_specified {
-                                    if !names.is_empty()
-                                        || !values.is_empty()
-                                        || values_any_specified
-                                    {
-                                        error!("`cfg(any())` can only be provided in isolation");
-                                    }
-
-                                    check_cfg.exhaustive_names = false;
-                                } else {
-                                    for name in names {
-                                        check_cfg
-                                            .expecteds
-                                            .entry(name.to_string())
-                                            .and_modify(|v| match v {
-                                                ExpectedValues::Some(v)
-                                                    if !values_any_specified =>
-                                                {
-                                                    v.extend(values.clone())
-                                                }
-                                                ExpectedValues::Some(_) => *v = ExpectedValues::Any,
-                                                ExpectedValues::Any => {}
-                                            })
-                                            .or_insert_with(|| {
-                                                if values_any_specified {
-                                                    ExpectedValues::Any
-                                                } else {
-                                                    ExpectedValues::Some(values.clone())
-                                                }
-                                            });
-                                    }
+                                values_any_specified = true;
+                                if !args.is_empty() {
+                                    error!("`any()` must be empty");
                                 }
                             } else {
-                                expected_error();
+                                error!(
+                                    "`values()` arguments must be string literals or `any()`"
+                                );
                             }
-                        } else {
-                            expected_error();
                         }
+                    } else {
+                        error!(
+                            "`cfg()` arguments must be simple identifiers, `any()` or `values(...)`"
+                        );
                     }
-                    Ok(..) => expected_error(),
-                    Err(err) => {
-                        err.cancel();
-                        expected_error();
+                }
+
+                if values.is_empty() && !values_any_specified && !any_specified {
+                    values.insert(None);
+                } else if !values.is_empty() && values_any_specified {
+                    error!(
+                        "`values()` arguments cannot specify string literals and `any()` at the same time"
+                    );
+                }
+
+                if any_specified {
+                    if names.is_empty()
+                        && values.is_empty()
+                        && !values_specified
+                        && !values_any_specified
+                    {
+                        check_cfg.exhaustive_names = false;
+                    } else {
+                        error!("`cfg(any())` can only be provided in isolation");
+                    }
+                } else {
+                    for name in names {
+                        check_cfg
+                            .expecteds
+                            .entry(name.to_string())
+                            .and_modify(|v| match v {
+                                ExpectedValues::Some(v) if !values_any_specified => {
+                                    v.extend(values.clone())
+                                }
+                                ExpectedValues::Some(_) => *v = ExpectedValues::Any,
+                                ExpectedValues::Any => {}
+                            })
+                            .or_insert_with(|| {
+                                if values_any_specified {
+                                    ExpectedValues::Any
+                                } else {
+                                    ExpectedValues::Some(values.clone())
+                                }
+                            });
                     }
-                },
-                Err(errs) => {
-                    drop(errs);
-                    expected_error();
                 }
+            } else {
+                expected_error();
             }
         }
 
diff --git a/src/doc/unstable-book/src/compiler-flags/check-cfg.md b/src/doc/unstable-book/src/compiler-flags/check-cfg.md
index 7a3ef5e9e2b..0e15c79076f 100644
--- a/src/doc/unstable-book/src/compiler-flags/check-cfg.md
+++ b/src/doc/unstable-book/src/compiler-flags/check-cfg.md
@@ -139,7 +139,7 @@ fn do_mumble_frotz() {}
 
 ```bash
 # This turns on checking for feature values, but not for condition names.
-rustc --check-cfg 'configure(feature, values("zapping", "lasers"))' \
+rustc --check-cfg 'cfg(feature, values("zapping", "lasers"))' \
       --check-cfg 'cfg(any())' \
       --cfg 'feature="zapping"' -Z unstable-options
 ```
diff --git a/tests/ui/check-cfg/invalid-arguments.any_values.stderr b/tests/ui/check-cfg/invalid-arguments.any_values.stderr
new file mode 100644
index 00000000000..f9a9c4a6e13
--- /dev/null
+++ b/tests/ui/check-cfg/invalid-arguments.any_values.stderr
@@ -0,0 +1,2 @@
+error: invalid `--check-cfg` argument: `cfg(any(),values())` (`values()` cannot be specified before the names)
+
diff --git a/tests/ui/check-cfg/invalid-arguments.rs b/tests/ui/check-cfg/invalid-arguments.rs
index 79bef89c957..a56f48e0af9 100644
--- a/tests/ui/check-cfg/invalid-arguments.rs
+++ b/tests/ui/check-cfg/invalid-arguments.rs
@@ -6,7 +6,7 @@
 // revisions: multiple_values_any not_empty_any not_empty_values_any
 // revisions: values_any_missing_values values_any_before_ident ident_in_values_1
 // revisions: ident_in_values_2 unknown_meta_item_1 unknown_meta_item_2 unknown_meta_item_3
-// revisions: mixed_values_any mixed_any giberich
+// revisions: mixed_values_any mixed_any any_values giberich unterminated
 //
 // compile-flags: -Z unstable-options
 // [anything_else]compile-flags: --check-cfg=anything_else(...)
@@ -29,6 +29,8 @@
 // [unknown_meta_item_3]compile-flags: --check-cfg=cfg(foo,values(test()))
 // [mixed_values_any]compile-flags: --check-cfg=cfg(foo,values("bar",any()))
 // [mixed_any]compile-flags: --check-cfg=cfg(any(),values(any()))
+// [any_values]compile-flags: --check-cfg=cfg(any(),values())
 // [giberich]compile-flags: --check-cfg=cfg(...)
+// [unterminated]compile-flags: --check-cfg=cfg(
 
 fn main() {}
diff --git a/tests/ui/check-cfg/invalid-arguments.unterminated.stderr b/tests/ui/check-cfg/invalid-arguments.unterminated.stderr
new file mode 100644
index 00000000000..80161a6aa0f
--- /dev/null
+++ b/tests/ui/check-cfg/invalid-arguments.unterminated.stderr
@@ -0,0 +1,2 @@
+error: invalid `--check-cfg` argument: `cfg(` (expected `cfg(name, values("value1", "value2", ... "valueN"))`)
+