From c36798357d353a8df1bf430ea3261741cfc51121 Mon Sep 17 00:00:00 2001 From: yukang Date: Sun, 28 Jan 2024 11:13:02 +0800 Subject: [PATCH 01/15] Suggest name value cfg when only value is used for check-cfg --- .../rustc_lint/src/context/diagnostics.rs | 50 +++++++++++++++---- .../cfg-value-for-cfg-name-multiple.rs | 12 +++++ .../cfg-value-for-cfg-name-multiple.stderr | 21 ++++++++ tests/ui/check-cfg/cfg-value-for-cfg-name.rs | 18 +++++++ .../check-cfg/cfg-value-for-cfg-name.stderr | 22 ++++++++ 5 files changed, 113 insertions(+), 10 deletions(-) create mode 100644 tests/ui/check-cfg/cfg-value-for-cfg-name-multiple.rs create mode 100644 tests/ui/check-cfg/cfg-value-for-cfg-name-multiple.stderr create mode 100644 tests/ui/check-cfg/cfg-value-for-cfg-name.rs create mode 100644 tests/ui/check-cfg/cfg-value-for-cfg-name.stderr diff --git a/compiler/rustc_lint/src/context/diagnostics.rs b/compiler/rustc_lint/src/context/diagnostics.rs index 312874db3f54e..31205f2b2fd97 100644 --- a/compiler/rustc_lint/src/context/diagnostics.rs +++ b/compiler/rustc_lint/src/context/diagnostics.rs @@ -187,6 +187,23 @@ pub(super) fn builtin( BuiltinLintDiagnostics::UnexpectedCfgName((name, name_span), value) => { let possibilities: Vec = sess.parse_sess.check_config.expecteds.keys().copied().collect(); + + let mut names_possibilities: Vec<_> = if value.is_none() { + // We later sort and display all the possibilities, so the order here does not matter. + #[allow(rustc::potential_query_instability)] + sess.parse_sess + .check_config + .expecteds + .iter() + .filter_map(|(k, v)| match v { + ExpectedValues::Some(v) if v.contains(&Some(name)) => Some(k), + _ => None, + }) + .collect() + } else { + Vec::new() + }; + let is_from_cargo = std::env::var_os("CARGO").is_some(); let mut is_feature_cfg = name == sym::feature; @@ -261,17 +278,30 @@ pub(super) fn builtin( } is_feature_cfg |= best_match == sym::feature; - } else if !possibilities.is_empty() { - let mut possibilities = - possibilities.iter().map(Symbol::as_str).collect::>(); - possibilities.sort(); - let possibilities = possibilities.join("`, `"); + } else { + if !names_possibilities.is_empty() { + names_possibilities.sort(); + for cfg_name in names_possibilities.iter() { + db.span_suggestion( + name_span, + "found config with similar value", + format!("{cfg_name} = \"{name}\""), + Applicability::MaybeIncorrect, + ); + } + } + if !possibilities.is_empty() { + let mut possibilities = + possibilities.iter().map(Symbol::as_str).collect::>(); + possibilities.sort(); + let possibilities = possibilities.join("`, `"); - // The list of expected names can be long (even by default) and - // so the diagnostic produced can take a lot of space. To avoid - // cloging the user output we only want to print that diagnostic - // once. - db.help_once(format!("expected names are: `{possibilities}`")); + // The list of expected names can be long (even by default) and + // so the diagnostic produced can take a lot of space. To avoid + // cloging the user output we only want to print that diagnostic + // once. + db.help_once(format!("expected names are: `{possibilities}`")); + } } let inst = if let Some((value, _value_span)) = value { diff --git a/tests/ui/check-cfg/cfg-value-for-cfg-name-multiple.rs b/tests/ui/check-cfg/cfg-value-for-cfg-name-multiple.rs new file mode 100644 index 0000000000000..edde6244ed1a9 --- /dev/null +++ b/tests/ui/check-cfg/cfg-value-for-cfg-name-multiple.rs @@ -0,0 +1,12 @@ +// #120427 +// This test checks that when a single cfg has a value for user's specified name +// +// check-pass +// compile-flags: -Z unstable-options +// compile-flags: --check-cfg=cfg(foo,values("my_value")) --check-cfg=cfg(bar,values("my_value")) + +#[cfg(my_value)] +//~^ WARNING unexpected `cfg` condition name: `my_value` +fn x() {} + +fn main() {} diff --git a/tests/ui/check-cfg/cfg-value-for-cfg-name-multiple.stderr b/tests/ui/check-cfg/cfg-value-for-cfg-name-multiple.stderr new file mode 100644 index 0000000000000..b88ee71a156a2 --- /dev/null +++ b/tests/ui/check-cfg/cfg-value-for-cfg-name-multiple.stderr @@ -0,0 +1,21 @@ +warning: unexpected `cfg` condition name: `my_value` + --> $DIR/cfg-value-for-cfg-name-multiple.rs:8:7 + | +LL | #[cfg(my_value)] + | ^^^^^^^^ + | + = help: expected names are: `bar`, `debug_assertions`, `doc`, `doctest`, `foo`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `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` + = help: to expect this configuration use `--check-cfg=cfg(my_value)` + = note: see for more information about checking conditional configuration + = note: `#[warn(unexpected_cfgs)]` on by default +help: found config with similar value + | +LL | #[cfg(foo = "my_value")] + | ~~~~~~~~~~~~~~~~ +help: found config with similar value + | +LL | #[cfg(bar = "my_value")] + | ~~~~~~~~~~~~~~~~ + +warning: 1 warning emitted + diff --git a/tests/ui/check-cfg/cfg-value-for-cfg-name.rs b/tests/ui/check-cfg/cfg-value-for-cfg-name.rs new file mode 100644 index 0000000000000..7a0c345b7ca76 --- /dev/null +++ b/tests/ui/check-cfg/cfg-value-for-cfg-name.rs @@ -0,0 +1,18 @@ +// #120427 +// This test checks that when a single cfg has a value for user's specified name +// suggest to use `#[cfg(target_os = "linux")]` instead of `#[cfg(linux)]` +// +// check-pass +// compile-flags: -Z unstable-options +// compile-flags: --check-cfg=cfg() + +#[cfg(linux)] +//~^ WARNING unexpected `cfg` condition name: `linux` +fn x() {} + +// will not suggest if the cfg has a value +#[cfg(linux = "os-name")] +//~^ WARNING unexpected `cfg` condition name: `linux` +fn y() {} + +fn main() {} diff --git a/tests/ui/check-cfg/cfg-value-for-cfg-name.stderr b/tests/ui/check-cfg/cfg-value-for-cfg-name.stderr new file mode 100644 index 0000000000000..c044755142419 --- /dev/null +++ b/tests/ui/check-cfg/cfg-value-for-cfg-name.stderr @@ -0,0 +1,22 @@ +warning: unexpected `cfg` condition name: `linux` + --> $DIR/cfg-value-for-cfg-name.rs:9:7 + | +LL | #[cfg(linux)] + | ^^^^^ help: found config with similar value: `target_os = "linux"` + | + = help: expected names are: `debug_assertions`, `doc`, `doctest`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `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` + = help: to expect this configuration use `--check-cfg=cfg(linux)` + = note: see for more information about checking conditional configuration + = note: `#[warn(unexpected_cfgs)]` on by default + +warning: unexpected `cfg` condition name: `linux` + --> $DIR/cfg-value-for-cfg-name.rs:14:7 + | +LL | #[cfg(linux = "os-name")] + | ^^^^^^^^^^^^^^^^^ + | + = help: to expect this configuration use `--check-cfg=cfg(linux, values("os-name"))` + = note: see for more information about checking conditional configuration + +warning: 2 warnings emitted + From 0213c87e12460e8f7101f96cc5daf4c3d20d6b9f Mon Sep 17 00:00:00 2001 From: Yukang Date: Tue, 30 Jan 2024 10:18:52 +0800 Subject: [PATCH 02/15] limit the names_possiblilities to less than 3 Co-authored-by: Urgau <3616612+Urgau@users.noreply.github.com> --- compiler/rustc_lint/src/context/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_lint/src/context/diagnostics.rs b/compiler/rustc_lint/src/context/diagnostics.rs index 31205f2b2fd97..07efc98f1fbe5 100644 --- a/compiler/rustc_lint/src/context/diagnostics.rs +++ b/compiler/rustc_lint/src/context/diagnostics.rs @@ -279,7 +279,7 @@ pub(super) fn builtin( is_feature_cfg |= best_match == sym::feature; } else { - if !names_possibilities.is_empty() { + if !names_possibilities.is_empty() && names_possibilities.len() <= 3 { names_possibilities.sort(); for cfg_name in names_possibilities.iter() { db.span_suggestion( From ca243e750118ae679c7c3413f21e5e772bf694da Mon Sep 17 00:00:00 2001 From: yukang Date: Tue, 30 Jan 2024 16:54:40 +0800 Subject: [PATCH 03/15] add testcase for more than 3 cfg names --- .../check-cfg/cfg-value-for-cfg-name-duplicate.rs | 12 ++++++++++++ .../cfg-value-for-cfg-name-duplicate.stderr | 13 +++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tests/ui/check-cfg/cfg-value-for-cfg-name-duplicate.rs create mode 100644 tests/ui/check-cfg/cfg-value-for-cfg-name-duplicate.stderr diff --git a/tests/ui/check-cfg/cfg-value-for-cfg-name-duplicate.rs b/tests/ui/check-cfg/cfg-value-for-cfg-name-duplicate.rs new file mode 100644 index 0000000000000..a6e68e1b7101c --- /dev/null +++ b/tests/ui/check-cfg/cfg-value-for-cfg-name-duplicate.rs @@ -0,0 +1,12 @@ +// #120427 +// This test checks we won't suggest more than 3 span suggestions for cfg names +// +// check-pass +// compile-flags: -Z unstable-options +// compile-flags: --check-cfg=cfg(foo,values("value")) --check-cfg=cfg(bar,values("value")) --check-cfg=cfg(bee,values("value")) --check-cfg=cfg(cow,values("value")) + +#[cfg(value)] +//~^ WARNING unexpected `cfg` condition name: `value` +fn x() {} + +fn main() {} diff --git a/tests/ui/check-cfg/cfg-value-for-cfg-name-duplicate.stderr b/tests/ui/check-cfg/cfg-value-for-cfg-name-duplicate.stderr new file mode 100644 index 0000000000000..82d471d715b83 --- /dev/null +++ b/tests/ui/check-cfg/cfg-value-for-cfg-name-duplicate.stderr @@ -0,0 +1,13 @@ +warning: unexpected `cfg` condition name: `value` + --> $DIR/cfg-value-for-cfg-name-duplicate.rs:8:7 + | +LL | #[cfg(value)] + | ^^^^^ + | + = help: expected names are: `bar`, `bee`, `cow`, `debug_assertions`, `doc`, `doctest`, `foo`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `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` + = help: to expect this configuration use `--check-cfg=cfg(value)` + = note: see for more information about checking conditional configuration + = note: `#[warn(unexpected_cfgs)]` on by default + +warning: 1 warning emitted + From c10a52e2c57605dcd1fa5373345cebae97a2304e Mon Sep 17 00:00:00 2001 From: klensy Date: Tue, 16 Jan 2024 14:45:22 +0300 Subject: [PATCH 04/15] tidy: wrap regexes with lazy_static yes, once_cell better, but ... this reduces from ==31349== Total: 1,365,199,543 bytes in 4,774,213 blocks ==31349== At t-gmax: 10,975,708 bytes in 66,093 blocks ==31349== At t-end: 2,880,947 bytes in 12,332 blocks ==31349== Reads: 5,210,008,956 bytes ==31349== Writes: 1,280,920,127 bytes to ==47796== Total: 821,467,407 bytes in 3,955,595 blocks ==47796== At t-gmax: 10,976,209 bytes in 66,100 blocks ==47796== At t-end: 2,944,016 bytes in 12,490 blocks ==47796== Reads: 4,788,959,023 bytes ==47796== Writes: 975,493,639 bytes miropt-test-tools: remove regex usage this removes regex usage and slightly refactors ext stripping in one case --- Cargo.lock | 3 --- src/tools/miropt-test-tools/Cargo.toml | 1 - src/tools/miropt-test-tools/src/lib.rs | 19 ++++++++++++------- src/tools/tidy/src/style.rs | 6 ++++-- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cab14a7a202b2..4e0cf834355aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2490,9 +2490,6 @@ dependencies = [ [[package]] name = "miropt-test-tools" version = "0.1.0" -dependencies = [ - "regex", -] [[package]] name = "native-tls" diff --git a/src/tools/miropt-test-tools/Cargo.toml b/src/tools/miropt-test-tools/Cargo.toml index 8589a44cf1bab..09b4c7d16dce3 100644 --- a/src/tools/miropt-test-tools/Cargo.toml +++ b/src/tools/miropt-test-tools/Cargo.toml @@ -4,4 +4,3 @@ version = "0.1.0" edition = "2021" [dependencies] -regex = "1.0" diff --git a/src/tools/miropt-test-tools/src/lib.rs b/src/tools/miropt-test-tools/src/lib.rs index 7d60033c3e824..4317f23a82219 100644 --- a/src/tools/miropt-test-tools/src/lib.rs +++ b/src/tools/miropt-test-tools/src/lib.rs @@ -100,14 +100,19 @@ pub fn files_for_miropt_test( } else { // Allow-list for file extensions that can be produced by MIR dumps. // Other extensions can be added here, as needed by new dump flags. - let ext_re = regex::Regex::new(r#"(\.(mir|dot))$"#).unwrap(); - let cap = ext_re.captures_iter(test_name).next().unwrap_or_else(|| { - panic!("in {testfile:?}:\nEMIT_MIR has an unrecognized extension: {test_name}") - }); - let extension = cap.get(1).unwrap().as_str(); + static ALLOWED_EXT: &[&str] = &["mir", "dot"]; + let Some((test_name_wo_ext, test_name_ext)) = test_name.rsplit_once('.') else { + panic!( + "in {testfile:?}:\nEMIT_MIR has an unrecognized extension: {test_name}, expected one of {ALLOWED_EXT:?}" + ) + }; + if !ALLOWED_EXT.contains(&test_name_ext) { + panic!( + "in {testfile:?}:\nEMIT_MIR has an unrecognized extension: {test_name}, expected one of {ALLOWED_EXT:?}" + ) + } - expected_file = - format!("{}{}{}", test_name.trim_end_matches(extension), suffix, extension,); + expected_file = format!("{}{}.{}", test_name_wo_ext, suffix, test_name_ext); from_file = test_name.to_string(); assert!(test_names.next().is_none(), "two mir pass names specified for MIR dump"); to_file = None; diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index 8b0e80a94b0b3..bb5a796f19e2b 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -127,8 +127,10 @@ fn should_ignore(line: &str) -> bool { // Matches test annotations like `//~ ERROR text`. // This mirrors the regex in src/tools/compiletest/src/runtest.rs, please // update both if either are changed. - let re = Regex::new("\\s*//(\\[.*\\])?~.*").unwrap(); - re.is_match(line) || ANNOTATIONS_TO_IGNORE.iter().any(|a| line.contains(a)) + lazy_static::lazy_static! { + static ref ANNOTATION_RE: Regex = Regex::new("\\s*//(\\[.*\\])?~.*").unwrap(); + } + ANNOTATION_RE.is_match(line) || ANNOTATIONS_TO_IGNORE.iter().any(|a| line.contains(a)) } /// Returns `true` if `line` is allowed to be longer than the normal limit. From a9ba38323e1a0b781a02a4b86ea3cc00bdc6168c Mon Sep 17 00:00:00 2001 From: klensy Date: Tue, 16 Jan 2024 15:07:08 +0300 Subject: [PATCH 05/15] update ignore crate $ cargo update -p ignore --precise=0.4.22 Updating crates.io index Updating aho-corasick v1.0.2 -> v1.1.2 Updating bstr v1.5.0 -> v1.9.0 Updating globset v0.4.10 -> v0.4.14 Updating ignore v0.4.20 -> v0.4.22 Updating log v0.4.19 -> v0.4.20 Updating memchr v2.5.0 -> v2.7.1 Adding regex-automata v0.4.3 Updating walkdir v2.3.3 -> v2.4.0 some notable change is https://github.com/BurntSushi/ripgrep/pull/2692 reduces memory usage from ==47796== Total: 821,467,407 bytes in 3,955,595 blocks ==47796== At t-gmax: 10,976,209 bytes in 66,100 blocks ==47796== At t-end: 2,944,016 bytes in 12,490 blocks ==47796== Reads: 4,788,959,023 bytes ==47796== Writes: 975,493,639 bytes to ==66633== Total: 791,565,538 bytes in 3,503,144 blocks ==66633== At t-gmax: 10,914,511 bytes in 65,997 blocks ==66633== At t-end: 395,531 bytes in 941 blocks ==66633== Reads: 4,249,388,949 bytes ==66633== Writes: 814,119,580 bytes bump regex to dedupe one regex-syntax $ cargo update -p regex Updating crates.io index Updating regex v1.8.4 -> v1.10.2 Removing regex-syntax v0.7.2 --- Cargo.lock | 77 +++++++++++++++++++++++++----------------------------- 1 file changed, 36 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e0cf834355aa..4ec95ae5decce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -49,18 +49,9 @@ dependencies = [ [[package]] name = "aho-corasick" -version = "0.7.20" +version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cc936419f96fa211c1b9166887b38e5e40b19958e5b895be7c1f93adec7071ac" -dependencies = [ - "memchr", -] - -[[package]] -name = "aho-corasick" -version = "1.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43f6cb1bf222025340178f382c426f13757b2960e89779dfcb319c32542a5a41" +checksum = "b2969dcb958b36655471fc61f7e416fa76033bdd4bfed0678d8fee1e2d07a1f0" dependencies = [ "memchr", ] @@ -319,13 +310,12 @@ dependencies = [ [[package]] name = "bstr" -version = "1.5.0" +version = "1.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a246e68bb43f6cd9db24bea052a53e40405417c5fb372e3d1a8a7f770a564ef5" +checksum = "c48f0051a4b4c5e0b6d365cd04af53aeaa209e3cc15ec2cdb69e73cc87fbd0dc" dependencies = [ "memchr", - "once_cell", - "regex-automata 0.1.10", + "regex-automata 0.4.3", "serde", ] @@ -596,7 +586,7 @@ dependencies = [ name = "clippy_dev" version = "0.0.1" dependencies = [ - "aho-corasick 1.0.2", + "aho-corasick", "clap", "indoc", "itertools", @@ -1597,15 +1587,15 @@ checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" [[package]] name = "globset" -version = "0.4.10" +version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "029d74589adefde59de1a0c4f4732695c32805624aec7b68d91503d4dba79afc" +checksum = "57da3b9b5b85bd66f31093f8c408b90a74431672542466497dcbdfdc02034be1" dependencies = [ - "aho-corasick 0.7.20", + "aho-corasick", "bstr", - "fnv", "log", - "regex", + "regex-automata 0.4.3", + "regex-syntax 0.8.2", ] [[package]] @@ -1944,17 +1934,16 @@ checksum = "cb56e1aa765b4b4f3aadfab769793b7087bb03a4ea4920644a6d238e2df5b9ed" [[package]] name = "ignore" -version = "0.4.20" +version = "0.4.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dbe7873dab538a9a44ad79ede1faf5f30d49f9a5c883ddbab48bce81b64b7492" +checksum = "b46810df39e66e925525d6e38ce1e7f6e1d208f72dc39757880fcb66e2c58af1" dependencies = [ + "crossbeam-deque", "globset", - "lazy_static", "log", "memchr", - "regex", + "regex-automata 0.4.3", "same-file", - "thread_local", "walkdir", "winapi-util", ] @@ -2274,9 +2263,9 @@ dependencies = [ [[package]] name = "log" -version = "0.4.19" +version = "0.4.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b06a4cde4c0f271a446782e3eff8de789548ce57dbc8eca9292c27f4a42004b4" +checksum = "b5e6163cb8c49088c2c36f57875e58ccd8c87c7427f7fbd50ea6710b2f3f2e8f" [[package]] name = "lzma-sys" @@ -2378,9 +2367,9 @@ dependencies = [ [[package]] name = "memchr" -version = "2.5.0" +version = "2.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" +checksum = "523dc4f511e55ab87b694dc30d0f820d60906ef06413f93d4d7a1385599cc149" dependencies = [ "compiler_builtins", "rustc-std-workspace-core", @@ -3150,13 +3139,14 @@ dependencies = [ [[package]] name = "regex" -version = "1.8.4" +version = "1.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d0ab3ca65655bb1e41f2a8c8cd662eb4fb035e67c3f78da1d61dffe89d07300f" +checksum = "380b951a9c5e80ddfd6136919eef32310721aa4aacd4889a8d39124b026ab343" dependencies = [ - "aho-corasick 1.0.2", + "aho-corasick", "memchr", - "regex-syntax 0.7.2", + "regex-automata 0.4.3", + "regex-syntax 0.8.2", ] [[package]] @@ -3178,16 +3168,21 @@ dependencies = [ ] [[package]] -name = "regex-syntax" -version = "0.6.29" +name = "regex-automata" +version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" +checksum = "5f804c7828047e88b2d32e2d7fe5a105da8ee3264f01902f796c8e067dc2483f" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax 0.8.2", +] [[package]] name = "regex-syntax" -version = "0.7.2" +version = "0.6.29" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "436b050e76ed2903236f032a59761c1eb99e1b0aead2c257922771dab1fc8c78" +checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" [[package]] name = "regex-syntax" @@ -5937,9 +5932,9 @@ checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" [[package]] name = "walkdir" -version = "2.3.3" +version = "2.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "36df944cda56c7d8d8b7496af378e6b16de9284591917d307c9b4d313c44e698" +checksum = "d71d857dc86794ca4c280d616f7da00d2dbfd8cd788846559a6813e6aa4b54ee" dependencies = [ "same-file", "winapi-util", From d34b0fa495bfbc3bd7de14a691822706074fcbb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 26 Jan 2024 18:57:58 +0000 Subject: [PATCH 06/15] Add test for method on unbounded type parameter receiver --- .../traits/method-on-unbounded-type-param.rs | 15 ++++ .../method-on-unbounded-type-param.stderr | 83 +++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 tests/ui/traits/method-on-unbounded-type-param.rs create mode 100644 tests/ui/traits/method-on-unbounded-type-param.stderr diff --git a/tests/ui/traits/method-on-unbounded-type-param.rs b/tests/ui/traits/method-on-unbounded-type-param.rs new file mode 100644 index 0000000000000..8505eb41e98bb --- /dev/null +++ b/tests/ui/traits/method-on-unbounded-type-param.rs @@ -0,0 +1,15 @@ +fn f(a: T, b: T) -> std::cmp::Ordering { + a.cmp(&b) //~ ERROR E0599 +} +fn g(a: T, b: T) -> std::cmp::Ordering { + (&a).cmp(&b) //~ ERROR E0599 +} +fn h(a: &T, b: T) -> std::cmp::Ordering { + a.cmp(&b) //~ ERROR E0599 +} +trait T {} +impl T for X {} +fn main() { + let x: Box = Box::new(0); + x.cmp(&x); //~ ERROR E0599 +} diff --git a/tests/ui/traits/method-on-unbounded-type-param.stderr b/tests/ui/traits/method-on-unbounded-type-param.stderr new file mode 100644 index 0000000000000..0b650995de5c5 --- /dev/null +++ b/tests/ui/traits/method-on-unbounded-type-param.stderr @@ -0,0 +1,83 @@ +error[E0599]: `T` is not an iterator + --> $DIR/method-on-unbounded-type-param.rs:2:7 + | +LL | fn f(a: T, b: T) -> std::cmp::Ordering { + | - method `cmp` not found for this type parameter +LL | a.cmp(&b) + | ^^^ `T` is not an iterator + | + = note: the following trait bounds were not satisfied: + `T: Iterator` + which is required by `&mut T: Iterator` +help: consider restricting the type parameter to satisfy the trait bound + | +LL | fn f(a: T, b: T) -> std::cmp::Ordering where T: Iterator { + | +++++++++++++++++ + +error[E0599]: the method `cmp` exists for reference `&T`, but its trait bounds were not satisfied + --> $DIR/method-on-unbounded-type-param.rs:5:10 + | +LL | (&a).cmp(&b) + | ^^^ method cannot be called on `&T` due to unsatisfied trait bounds + | + = note: the following trait bounds were not satisfied: + `T: Ord` + which is required by `&T: Ord` + `&T: Iterator` + which is required by `&mut &T: Iterator` + `T: Iterator` + which is required by `&mut T: Iterator` +help: consider restricting the type parameters to satisfy the trait bounds + | +LL | fn g(a: T, b: T) -> std::cmp::Ordering where T: Iterator, T: Ord { + | +++++++++++++++++++++++++ + +error[E0599]: the method `cmp` exists for reference `&T`, but its trait bounds were not satisfied + --> $DIR/method-on-unbounded-type-param.rs:8:7 + | +LL | a.cmp(&b) + | ^^^ method cannot be called on `&T` due to unsatisfied trait bounds + | + = note: the following trait bounds were not satisfied: + `T: Ord` + which is required by `&T: Ord` + `&T: Iterator` + which is required by `&mut &T: Iterator` + `T: Iterator` + which is required by `&mut T: Iterator` +help: consider restricting the type parameters to satisfy the trait bounds + | +LL | fn h(a: &T, b: T) -> std::cmp::Ordering where T: Iterator, T: Ord { + | +++++++++++++++++++++++++ + +error[E0599]: the method `cmp` exists for struct `Box`, but its trait bounds were not satisfied + --> $DIR/method-on-unbounded-type-param.rs:14:7 + | +LL | trait T {} + | ------- + | | + | doesn't satisfy `dyn T: Iterator` + | doesn't satisfy `dyn T: Ord` +... +LL | x.cmp(&x); + | ^^^ method cannot be called on `Box` due to unsatisfied trait bounds + --> $SRC_DIR/alloc/src/boxed.rs:LL:COL + ::: $SRC_DIR/alloc/src/boxed.rs:LL:COL + | + = note: doesn't satisfy `Box: Iterator` + | + = note: doesn't satisfy `Box: Ord` + | + = note: the following trait bounds were not satisfied: + `dyn T: Iterator` + which is required by `Box: Iterator` + `dyn T: Ord` + which is required by `Box: Ord` + `Box: Iterator` + which is required by `&mut Box: Iterator` + `dyn T: Iterator` + which is required by `&mut dyn T: Iterator` + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0599`. From 20b1c2aafca8d8861d193d3476ffc8d09c0479e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 26 Jan 2024 19:02:05 +0000 Subject: [PATCH 07/15] Account for unbounded type param receiver in suggestions When encountering ```rust fn f(a: T, b: T) -> std::cmp::Ordering { a.cmp(&b) //~ ERROR E0599 } ``` output ``` error[E0599]: no method named `cmp` found for type parameter `T` in the current scope --> $DIR/method-on-unbounded-type-param.rs:2:7 | LL | fn f(a: T, b: T) -> std::cmp::Ordering { | - method `cmp` not found for this type parameter LL | a.cmp(&b) | ^^^ method cannot be called on `T` due to unsatisfied trait bounds | = help: items from traits can only be used if the type parameter is bounded by the trait help: the following traits define an item `cmp`, perhaps you need to restrict type parameter `T` with one of them: | LL | fn f(a: T, b: T) -> std::cmp::Ordering { | +++++ LL | fn f(a: T, b: T) -> std::cmp::Ordering { | ++++++++++ ``` Fix #120186. --- .../rustc_hir_typeck/src/method/suggest.rs | 41 ++++++++++++------- .../method-on-unbounded-type-param.stderr | 16 ++++---- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 6c9501e93fa11..de42f011cf42a 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -554,6 +554,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { "`count` is defined on `{iterator_trait}`, which `{rcvr_ty}` does not implement" )); } + } else if !unsatisfied_predicates.is_empty() && matches!(rcvr_ty.kind(), ty::Param(_)) { + // We special case the situation where we are looking for `_` in + // `::method` because otherwise the machinery will look for blanket + // implementations that have unsatisfied trait bounds to suggest, leading us to claim + // things like "we're looking for a trait with method `cmp`, both `Iterator` and `Ord` + // have one, in order to implement `Ord` you need to restrict `TypeParam: FnPtr` so + // that `impl Ord for T` can apply", which is not what we want. We have a type + // parameter, we want to directly say "`Ord::cmp` and `Iterator::cmp` exist, restrict + // `TypeParam: Ord` or `TypeParam: Iterator`"". That is done further down when calling + // `self.suggest_traits_to_import`, so we ignore the `unsatisfied_predicates` + // suggestions. } else if !unsatisfied_predicates.is_empty() { let mut type_params = FxIndexMap::default(); @@ -1325,7 +1336,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } self.note_derefed_ty_has_method(&mut err, source, rcvr_ty, item_name, expected); - return Some(err); + Some(err) } fn note_candidates_on_method_error( @@ -2918,19 +2929,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // this isn't perfect (that is, there are cases when // implementing a trait would be legal but is rejected // here). - unsatisfied_predicates.iter().all(|(p, _, _)| { - match p.kind().skip_binder() { - // Hide traits if they are present in predicates as they can be fixed without - // having to implement them. - ty::PredicateKind::Clause(ty::ClauseKind::Trait(t)) => { - t.def_id() == info.def_id - } - ty::PredicateKind::Clause(ty::ClauseKind::Projection(p)) => { - p.projection_ty.def_id == info.def_id - } - _ => false, - } - }) && (type_is_local || info.def_id.is_local()) + (type_is_local || info.def_id.is_local()) && !self.tcx.trait_is_auto(info.def_id) && self .associated_value(info.def_id, item_name) @@ -2978,6 +2977,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { item.visibility(self.tcx).is_public() || info.def_id.is_local() }) .is_some() + && (matches!(rcvr_ty.kind(), ty::Param(_)) + || unsatisfied_predicates.iter().all(|(p, _, _)| { + match p.kind().skip_binder() { + // Hide traits if they are present in predicates as they can be fixed without + // having to implement them. + ty::PredicateKind::Clause(ty::ClauseKind::Trait(t)) => { + t.def_id() == info.def_id + } + ty::PredicateKind::Clause(ty::ClauseKind::Projection(p)) => { + p.projection_ty.def_id == info.def_id + } + _ => false, + } + })) }) .collect::>(); for span in &arbitrary_rcvr { diff --git a/tests/ui/traits/method-on-unbounded-type-param.stderr b/tests/ui/traits/method-on-unbounded-type-param.stderr index 0b650995de5c5..f12b4d6cabdaf 100644 --- a/tests/ui/traits/method-on-unbounded-type-param.stderr +++ b/tests/ui/traits/method-on-unbounded-type-param.stderr @@ -1,18 +1,18 @@ -error[E0599]: `T` is not an iterator +error[E0599]: no method named `cmp` found for type parameter `T` in the current scope --> $DIR/method-on-unbounded-type-param.rs:2:7 | LL | fn f(a: T, b: T) -> std::cmp::Ordering { | - method `cmp` not found for this type parameter LL | a.cmp(&b) - | ^^^ `T` is not an iterator + | ^^^ method cannot be called on `T` due to unsatisfied trait bounds | - = note: the following trait bounds were not satisfied: - `T: Iterator` - which is required by `&mut T: Iterator` -help: consider restricting the type parameter to satisfy the trait bound + = help: items from traits can only be used if the type parameter is bounded by the trait +help: the following traits define an item `cmp`, perhaps you need to restrict type parameter `T` with one of them: | -LL | fn f(a: T, b: T) -> std::cmp::Ordering where T: Iterator { - | +++++++++++++++++ +LL | fn f(a: T, b: T) -> std::cmp::Ordering { + | +++++ +LL | fn f(a: T, b: T) -> std::cmp::Ordering { + | ++++++++++ error[E0599]: the method `cmp` exists for reference `&T`, but its trait bounds were not satisfied --> $DIR/method-on-unbounded-type-param.rs:5:10 From 9ccc77036a144cc0d172c28e48c330d544ae5471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 30 Jan 2024 19:13:11 +0000 Subject: [PATCH 08/15] fix rebase --- .../method-on-unbounded-type-param.stderr | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/tests/ui/traits/method-on-unbounded-type-param.stderr b/tests/ui/traits/method-on-unbounded-type-param.stderr index f12b4d6cabdaf..245c25bbc6f67 100644 --- a/tests/ui/traits/method-on-unbounded-type-param.stderr +++ b/tests/ui/traits/method-on-unbounded-type-param.stderr @@ -53,20 +53,11 @@ LL | fn h(a: &T, b: T) -> std::cmp::Ordering where T: Iterator, T: Ord { error[E0599]: the method `cmp` exists for struct `Box`, but its trait bounds were not satisfied --> $DIR/method-on-unbounded-type-param.rs:14:7 | -LL | trait T {} - | ------- - | | - | doesn't satisfy `dyn T: Iterator` - | doesn't satisfy `dyn T: Ord` +LL | trait T {} + | ------- doesn't satisfy `dyn T: Iterator` or `dyn T: Ord` ... -LL | x.cmp(&x); - | ^^^ method cannot be called on `Box` due to unsatisfied trait bounds - --> $SRC_DIR/alloc/src/boxed.rs:LL:COL - ::: $SRC_DIR/alloc/src/boxed.rs:LL:COL - | - = note: doesn't satisfy `Box: Iterator` - | - = note: doesn't satisfy `Box: Ord` +LL | x.cmp(&x); + | ^^^ method cannot be called on `Box` due to unsatisfied trait bounds | = note: the following trait bounds were not satisfied: `dyn T: Iterator` From 5c414094ac8d41038819dd982403f4e3de05d93c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 30 Jan 2024 18:10:12 +0000 Subject: [PATCH 09/15] Account for non-overlapping unmet trait bounds in suggestion When a method not found on a type parameter could have been provided by any of multiple traits, suggest each trait individually, instead of a single suggestion to restrict the type parameter with *all* of them. Before: ``` error[E0599]: the method `cmp` exists for reference `&T`, but its trait bounds were not satisfied --> $DIR/method-on-unbounded-type-param.rs:5:10 | LL | (&a).cmp(&b) | ^^^ method cannot be called on `&T` due to unsatisfied trait bounds | = note: the following trait bounds were not satisfied: `T: Ord` which is required by `&T: Ord` `&T: Iterator` which is required by `&mut &T: Iterator` `T: Iterator` which is required by `&mut T: Iterator` help: consider restricting the type parameters to satisfy the trait bounds | LL | fn g(a: T, b: T) -> std::cmp::Ordering where T: Iterator, T: Ord { | +++++++++++++++++++++++++ ``` After: ``` error[E0599]: the method `cmp` exists for reference `&T`, but its trait bounds were not satisfied --> $DIR/method-on-unbounded-type-param.rs:5:10 | LL | (&a).cmp(&b) | ^^^ method cannot be called on `&T` due to unsatisfied trait bounds | = note: the following trait bounds were not satisfied: `T: Ord` which is required by `&T: Ord` `&T: Iterator` which is required by `&mut &T: Iterator` `T: Iterator` which is required by `&mut T: Iterator` = help: items from traits can only be used if the type parameter is bounded by the trait help: the following traits define an item `cmp`, perhaps you need to restrict type parameter `T` with one of them: | LL | fn g(a: T, b: T) -> std::cmp::Ordering { | +++++ LL | fn g(a: T, b: T) -> std::cmp::Ordering { | ++++++++++ ``` Fix #108428. --- .../rustc_hir_typeck/src/method/suggest.rs | 58 +++++++------------ .../box/unit/unique-object-noncopyable.stderr | 3 + tests/ui/box/unit/unique-pinned-nocopy.stderr | 3 - .../derives/derive-assoc-type-not-impl.stderr | 3 - ...method-unsatisfied-assoc-type-predicate.rs | 2 +- ...od-unsatisfied-assoc-type-predicate.stderr | 6 ++ .../trait-bounds/issue-30786.stderr | 12 ++++ tests/ui/methods/method-call-err-msg.stderr | 5 +- .../method-on-unbounded-type-param.stderr | 22 +++++-- 9 files changed, 62 insertions(+), 52 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index de42f011cf42a..f7aa5209e94a7 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -540,6 +540,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut bound_spans: SortedMap> = Default::default(); let mut restrict_type_params = false; + let mut suggested_derive = false; let mut unsatisfied_bounds = false; if item_name.name == sym::count && self.is_slice_ty(rcvr_ty, span) { let msg = "consider using `len` instead"; @@ -927,20 +928,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .enumerate() .collect::>(); - for ((span, add_where_or_comma), obligations) in type_params.into_iter() { - restrict_type_params = true; - // #74886: Sort here so that the output is always the same. - let obligations = obligations.into_sorted_stable_ord(); - err.span_suggestion_verbose( - span, - format!( - "consider restricting the type parameter{s} to satisfy the \ - trait bound{s}", - s = pluralize!(obligations.len()) - ), - format!("{} {}", add_where_or_comma, obligations.join(", ")), - Applicability::MaybeIncorrect, - ); + if !matches!(rcvr_ty.peel_refs().kind(), ty::Param(_)) { + for ((span, add_where_or_comma), obligations) in type_params.into_iter() { + restrict_type_params = true; + // #74886: Sort here so that the output is always the same. + let obligations = obligations.into_sorted_stable_ord(); + err.span_suggestion_verbose( + span, + format!( + "consider restricting the type parameter{s} to satisfy the trait \ + bound{s}", + s = pluralize!(obligations.len()) + ), + format!("{} {}", add_where_or_comma, obligations.join(", ")), + Applicability::MaybeIncorrect, + ); + } } bound_list.sort_by(|(_, a), (_, b)| a.cmp(b)); // Sort alphabetically. @@ -988,7 +991,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { "the following trait bounds were not satisfied:\n{bound_list}" )); } - self.suggest_derive(&mut err, unsatisfied_predicates); + suggested_derive = self.suggest_derive(&mut err, unsatisfied_predicates); unsatisfied_bounds = true; } @@ -1211,7 +1214,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } - if rcvr_ty.is_numeric() && rcvr_ty.is_fresh() || restrict_type_params { + if rcvr_ty.is_numeric() && rcvr_ty.is_fresh() || restrict_type_params || suggested_derive { } else { self.suggest_traits_to_import( &mut err, @@ -1221,7 +1224,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { args.map(|args| args.len() + 1), source, no_match_data.out_of_scope_traits.clone(), - unsatisfied_predicates, static_candidates, unsatisfied_bounds, expected.only_has_type(self), @@ -2481,7 +2483,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Option>, Option>, )], - ) { + ) -> bool { let mut derives = self.note_predicate_source_and_get_derives(err, unsatisfied_predicates); derives.sort(); derives.dedup(); @@ -2506,6 +2508,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Applicability::MaybeIncorrect, ); } + !derives_grouped.is_empty() } fn note_derefed_ty_has_method( @@ -2708,11 +2711,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { inputs_len: Option, source: SelfSource<'tcx>, valid_out_of_scope_traits: Vec, - unsatisfied_predicates: &[( - ty::Predicate<'tcx>, - Option>, - Option>, - )], static_candidates: &[CandidateSource], unsatisfied_bounds: bool, return_type: Option>, @@ -2977,20 +2975,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { item.visibility(self.tcx).is_public() || info.def_id.is_local() }) .is_some() - && (matches!(rcvr_ty.kind(), ty::Param(_)) - || unsatisfied_predicates.iter().all(|(p, _, _)| { - match p.kind().skip_binder() { - // Hide traits if they are present in predicates as they can be fixed without - // having to implement them. - ty::PredicateKind::Clause(ty::ClauseKind::Trait(t)) => { - t.def_id() == info.def_id - } - ty::PredicateKind::Clause(ty::ClauseKind::Projection(p)) => { - p.projection_ty.def_id == info.def_id - } - _ => false, - } - })) }) .collect::>(); for span in &arbitrary_rcvr { diff --git a/tests/ui/box/unit/unique-object-noncopyable.stderr b/tests/ui/box/unit/unique-object-noncopyable.stderr index 49547872d1a7b..8ea6edb48a7fc 100644 --- a/tests/ui/box/unit/unique-object-noncopyable.stderr +++ b/tests/ui/box/unit/unique-object-noncopyable.stderr @@ -12,6 +12,9 @@ LL | let _z = y.clone(); which is required by `Box: Clone` `dyn Foo: Clone` which is required by `Box: Clone` + = help: items from traits can only be used if the trait is implemented and in scope + = note: the following trait defines an item `clone`, perhaps you need to implement it: + candidate #1: `Clone` error: aborting due to 1 previous error diff --git a/tests/ui/box/unit/unique-pinned-nocopy.stderr b/tests/ui/box/unit/unique-pinned-nocopy.stderr index d2bf72249c451..69428604b197f 100644 --- a/tests/ui/box/unit/unique-pinned-nocopy.stderr +++ b/tests/ui/box/unit/unique-pinned-nocopy.stderr @@ -10,9 +10,6 @@ LL | let _j = i.clone(); = note: the following trait bounds were not satisfied: `R: Clone` which is required by `Box: Clone` - = help: items from traits can only be used if the trait is implemented and in scope - = note: the following trait defines an item `clone`, perhaps you need to implement it: - candidate #1: `Clone` help: consider annotating `R` with `#[derive(Clone)]` | LL + #[derive(Clone)] diff --git a/tests/ui/derives/derive-assoc-type-not-impl.stderr b/tests/ui/derives/derive-assoc-type-not-impl.stderr index 61268ffc7f852..13ba80243a5eb 100644 --- a/tests/ui/derives/derive-assoc-type-not-impl.stderr +++ b/tests/ui/derives/derive-assoc-type-not-impl.stderr @@ -15,9 +15,6 @@ note: trait bound `NotClone: Clone` was not satisfied | LL | #[derive(Clone)] | ^^^^^ unsatisfied trait bound introduced in this `derive` macro - = help: items from traits can only be used if the trait is implemented and in scope - = note: the following trait defines an item `clone`, perhaps you need to implement it: - candidate #1: `Clone` help: consider annotating `NotClone` with `#[derive(Clone)]` | LL + #[derive(Clone)] diff --git a/tests/ui/generic-associated-types/method-unsatisfied-assoc-type-predicate.rs b/tests/ui/generic-associated-types/method-unsatisfied-assoc-type-predicate.rs index add4d58f86a0d..92ce4a0970f36 100644 --- a/tests/ui/generic-associated-types/method-unsatisfied-assoc-type-predicate.rs +++ b/tests/ui/generic-associated-types/method-unsatisfied-assoc-type-predicate.rs @@ -5,7 +5,7 @@ trait X { type Y; } -trait M { +trait M { //~ NOTE fn f(&self) {} } diff --git a/tests/ui/generic-associated-types/method-unsatisfied-assoc-type-predicate.stderr b/tests/ui/generic-associated-types/method-unsatisfied-assoc-type-predicate.stderr index 1dd463f996ce6..61512dd46582d 100644 --- a/tests/ui/generic-associated-types/method-unsatisfied-assoc-type-predicate.stderr +++ b/tests/ui/generic-associated-types/method-unsatisfied-assoc-type-predicate.stderr @@ -14,6 +14,12 @@ LL | impl = i32>> M for T {} | ^^^^^^^^^^^^ - - | | | unsatisfied trait bound introduced here + = help: items from traits can only be used if the trait is implemented and in scope +note: `M` defines an item `f`, perhaps you need to implement it + --> $DIR/method-unsatisfied-assoc-type-predicate.rs:8:1 + | +LL | trait M { + | ^^^^^^^ error: aborting due to 1 previous error diff --git a/tests/ui/higher-ranked/trait-bounds/issue-30786.stderr b/tests/ui/higher-ranked/trait-bounds/issue-30786.stderr index 73870703cfbac..699a4ecc42bb9 100644 --- a/tests/ui/higher-ranked/trait-bounds/issue-30786.stderr +++ b/tests/ui/higher-ranked/trait-bounds/issue-30786.stderr @@ -15,6 +15,12 @@ note: the following trait bounds were not satisfied: | LL | impl StreamExt for T where for<'a> &'a mut T: Stream {} | --------- - ^^^^^^ unsatisfied trait bound introduced here + = help: items from traits can only be used if the trait is implemented and in scope +note: `StreamExt` defines an item `filterx`, perhaps you need to implement it + --> $DIR/issue-30786.rs:66:1 + | +LL | pub trait StreamExt + | ^^^^^^^^^^^^^^^^^^^ error[E0599]: the method `countx` exists for struct `Filter &u64 {identity::}>, {closure@issue-30786.rs:131:30}>`, but its trait bounds were not satisfied --> $DIR/issue-30786.rs:132:24 @@ -33,6 +39,12 @@ note: the following trait bounds were not satisfied: | LL | impl StreamExt for T where for<'a> &'a mut T: Stream {} | --------- - ^^^^^^ unsatisfied trait bound introduced here + = help: items from traits can only be used if the trait is implemented and in scope +note: `StreamExt` defines an item `countx`, perhaps you need to implement it + --> $DIR/issue-30786.rs:66:1 + | +LL | pub trait StreamExt + | ^^^^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors diff --git a/tests/ui/methods/method-call-err-msg.stderr b/tests/ui/methods/method-call-err-msg.stderr index f431085745405..6df49e432a13a 100644 --- a/tests/ui/methods/method-call-err-msg.stderr +++ b/tests/ui/methods/method-call-err-msg.stderr @@ -63,8 +63,9 @@ LL | | .take() note: the trait `Iterator` must be implemented --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL = help: items from traits can only be used if the trait is implemented and in scope - = note: the following trait defines an item `take`, perhaps you need to implement it: - candidate #1: `Iterator` + = note: the following traits define an item `take`, perhaps you need to implement one of them: + candidate #1: `std::io::Read` + candidate #2: `Iterator` error[E0061]: this method takes 3 arguments but 0 arguments were supplied --> $DIR/method-call-err-msg.rs:21:7 diff --git a/tests/ui/traits/method-on-unbounded-type-param.stderr b/tests/ui/traits/method-on-unbounded-type-param.stderr index 245c25bbc6f67..0d8bd8ee964e7 100644 --- a/tests/ui/traits/method-on-unbounded-type-param.stderr +++ b/tests/ui/traits/method-on-unbounded-type-param.stderr @@ -27,10 +27,13 @@ LL | (&a).cmp(&b) which is required by `&mut &T: Iterator` `T: Iterator` which is required by `&mut T: Iterator` -help: consider restricting the type parameters to satisfy the trait bounds + = help: items from traits can only be used if the type parameter is bounded by the trait +help: the following traits define an item `cmp`, perhaps you need to restrict type parameter `T` with one of them: | -LL | fn g(a: T, b: T) -> std::cmp::Ordering where T: Iterator, T: Ord { - | +++++++++++++++++++++++++ +LL | fn g(a: T, b: T) -> std::cmp::Ordering { + | +++++ +LL | fn g(a: T, b: T) -> std::cmp::Ordering { + | ++++++++++ error[E0599]: the method `cmp` exists for reference `&T`, but its trait bounds were not satisfied --> $DIR/method-on-unbounded-type-param.rs:8:7 @@ -45,10 +48,13 @@ LL | a.cmp(&b) which is required by `&mut &T: Iterator` `T: Iterator` which is required by `&mut T: Iterator` -help: consider restricting the type parameters to satisfy the trait bounds + = help: items from traits can only be used if the type parameter is bounded by the trait +help: the following traits define an item `cmp`, perhaps you need to restrict type parameter `T` with one of them: | -LL | fn h(a: &T, b: T) -> std::cmp::Ordering where T: Iterator, T: Ord { - | +++++++++++++++++++++++++ +LL | fn h(a: &T, b: T) -> std::cmp::Ordering { + | +++++ +LL | fn h(a: &T, b: T) -> std::cmp::Ordering { + | ++++++++++ error[E0599]: the method `cmp` exists for struct `Box`, but its trait bounds were not satisfied --> $DIR/method-on-unbounded-type-param.rs:14:7 @@ -68,6 +74,10 @@ LL | x.cmp(&x); which is required by `&mut Box: Iterator` `dyn T: Iterator` which is required by `&mut dyn T: Iterator` + = help: items from traits can only be used if the trait is implemented and in scope + = note: the following traits define an item `cmp`, perhaps you need to implement one of them: + candidate #1: `Ord` + candidate #2: `Iterator` error: aborting due to 4 previous errors From e8c3cbf44b9f003482871e8638859c8f65b234bb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 31 Jan 2024 09:25:42 +1100 Subject: [PATCH 10/15] Make `Diagnostic::is_error` return false for `Level::FailureNote`. It doesn't affect behaviour, but makes sense with (a) `FailureNote` having `()` as its emission guarantee, and (b) in `Level` the `is_error` levels now are all listed before the non-`is_error` levels. --- compiler/rustc_errors/src/diagnostic.rs | 4 ++-- compiler/rustc_errors/src/lib.rs | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 8ad4925cff288..09570fe74b6ff 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -238,8 +238,7 @@ impl Diagnostic { Level::Bug | Level::DelayedBug(DelayedBugKind::Normal) | Level::Fatal - | Level::Error - | Level::FailureNote => true, + | Level::Error => true, Level::ForceWarning(_) | Level::Warning @@ -248,6 +247,7 @@ impl Diagnostic { | Level::OnceNote | Level::Help | Level::OnceHelp + | Level::FailureNote | Level::Allow | Level::Expect(_) => false, } diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index b2bd4d8eb956e..42b1c85e93968 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1541,6 +1541,8 @@ pub enum Level { /// /// The [`LintExpectationId`] is used for expected lint diagnostics. In all other cases this /// should be `None`. + /// + /// Its `EmissionGuarantee` is `()`. ForceWarning(Option), /// A warning about the code being compiled. Does not prevent compilation from finishing. @@ -1570,7 +1572,8 @@ pub enum Level { /// Its `EmissionGuarantee` is `()`. OnceHelp, - /// Similar to `Note`, but used in cases where compilation has failed. Rare. + /// Similar to `Note`, but used in cases where compilation has failed. When printed for human + /// consumption, it doesn't have any kind of `note:` label. Rare. /// /// Its `EmissionGuarantee` is `()`. FailureNote, From 5dd0431386867a051f726cbba6e0f46f7bba5e5d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 31 Jan 2024 13:56:22 +1100 Subject: [PATCH 11/15] Tighten the assertion in `downgrade_to_delayed_bug`. I.e. `Bug` and `Fatal` level diagnostics are never downgraded. --- compiler/rustc_errors/src/diagnostic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 09570fe74b6ff..299e4a840f726 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -306,7 +306,7 @@ impl Diagnostic { #[track_caller] pub fn downgrade_to_delayed_bug(&mut self) { assert!( - self.is_error(), + matches!(self.level, Level::Error | Level::DelayedBug(_)), "downgrade_to_delayed_bug: cannot downgrade {:?} to DelayedBug: not an error", self.level ); From c3673868325f95203d5291f2fa3a399425c14876 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 31 Jan 2024 15:10:23 +1100 Subject: [PATCH 12/15] Refactor `emit_diagnostic`. - Combine two different blocks involving `diagnostic.level.get_expectation_id()` into one. - Combine several `if`s involving `diagnostic.level` into a single `match`. This requires reordering some of the operations, but this has no functional effect. --- compiler/rustc_errors/src/lib.rs | 82 +++++++++++++++----------------- 1 file changed, 39 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 42b1c85e93968..b3461b676be27 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1247,24 +1247,41 @@ impl DiagCtxtInner { } fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { - // The `LintExpectationId` can be stable or unstable depending on when it was created. - // Diagnostics created before the definition of `HirId`s are unstable and can not yet - // be stored. Instead, they are buffered until the `LintExpectationId` is replaced by - // a stable one by the `LintLevelsBuilder`. - if let Some(LintExpectationId::Unstable { .. }) = diagnostic.level.get_expectation_id() { - self.unstable_expect_diagnostics.push(diagnostic); - return None; + if let Some(expectation_id) = diagnostic.level.get_expectation_id() { + // The `LintExpectationId` can be stable or unstable depending on when it was created. + // Diagnostics created before the definition of `HirId`s are unstable and can not yet + // be stored. Instead, they are buffered until the `LintExpectationId` is replaced by + // a stable one by the `LintLevelsBuilder`. + if let LintExpectationId::Unstable { .. } = expectation_id { + self.unstable_expect_diagnostics.push(diagnostic); + return None; + } + self.suppressed_expected_diag = true; + self.fulfilled_expectations.insert(expectation_id.normalize()); + } + + if diagnostic.has_future_breakage() { + // Future breakages aren't emitted if they're Level::Allow, + // but they still need to be constructed and stashed below, + // so they'll trigger the good-path bug check. + self.suppressed_expected_diag = true; + self.future_breakage_diagnostics.push(diagnostic.clone()); + } + + if matches!(diagnostic.level, DelayedBug(_)) && self.flags.eagerly_emit_delayed_bugs { + diagnostic.level = Error; } - // FIXME(eddyb) this should check for `has_errors` and stop pushing - // once *any* errors were emitted (and truncate `span_delayed_bugs` - // when an error is first emitted, also), but maybe there's a case - // in which that's not sound? otherwise this is really inefficient. match diagnostic.level { - DelayedBug(_) if self.flags.eagerly_emit_delayed_bugs => { - diagnostic.level = Error; + // This must come after the possible promotion of `DelayedBug` to `Error` above. + Fatal | Error if self.treat_next_err_as_bug() => { + diagnostic.level = Bug; } DelayedBug(DelayedBugKind::Normal) => { + // FIXME(eddyb) this should check for `has_errors` and stop pushing + // once *any* errors were emitted (and truncate `span_delayed_bugs` + // when an error is first emitted, also), but maybe there's a case + // in which that's not sound? otherwise this is really inefficient. let backtrace = std::backtrace::Backtrace::capture(); self.span_delayed_bugs .push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); @@ -1277,38 +1294,17 @@ impl DiagCtxtInner { .push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); return None; } - _ => {} - } - - // This must come after the possible promotion of `DelayedBug` to - // `Error` above. - if matches!(diagnostic.level, Error | Fatal) && self.treat_next_err_as_bug() { - diagnostic.level = Bug; - } - - if diagnostic.has_future_breakage() { - // Future breakages aren't emitted if they're Level::Allow, - // but they still need to be constructed and stashed below, - // so they'll trigger the good-path bug check. - self.suppressed_expected_diag = true; - self.future_breakage_diagnostics.push(diagnostic.clone()); - } - - if let Some(expectation_id) = diagnostic.level.get_expectation_id() { - self.suppressed_expected_diag = true; - self.fulfilled_expectations.insert(expectation_id.normalize()); - } - - if diagnostic.level == Warning && !self.flags.can_emit_warnings { - if diagnostic.has_future_breakage() { + Warning if !self.flags.can_emit_warnings => { + if diagnostic.has_future_breakage() { + (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {}); + } + return None; + } + Allow | Expect(_) => { (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {}); + return None; } - return None; - } - - if matches!(diagnostic.level, Expect(_) | Allow) { - (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {}); - return None; + _ => {} } let mut guaranteed = None; From 59e0bc2de7134a2d88e9c14db32884e631e90373 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 31 Jan 2024 11:23:54 +1100 Subject: [PATCH 13/15] Split `Level::DelayedBug` in two. The two kinds of delayed bug have quite different semantics so a stronger conceptual separation is nice. (`is_error` is a good example, because the two kinds have different behaviour.) The commit also moves the `DelayedBug` variant after `Error` in `Level`, to reflect the fact that it's weaker than `Error` -- it might trigger an error but also might not. (The pre-existing `downgrade_to_delayed_bug` function also reflects the notion that delayed bugs are lower/after normal errors.) Plus it condenses some of the comments on `Level` into a table, for easier reading, and introduces `can_be_top_or_sub` to indicate which levels can be used in top-level diagnostics vs. subdiagnostics. Finally, it renames `DiagCtxtInner::span_delayed_bugs` as `DiagCtxtInner::delayed_bugs`. The `span_` prefix is unnecessary because some delayed bugs don't have a span. --- compiler/rustc_error_messages/src/lib.rs | 2 +- .../src/annotate_snippet_emitter_writer.rs | 6 +- compiler/rustc_errors/src/diagnostic.rs | 18 +- compiler/rustc_errors/src/emitter.rs | 1 + compiler/rustc_errors/src/lib.rs | 159 ++++++++++-------- .../equality-in-canonical-query.clone.stderr | 2 +- ...equality_in_canonical_query.current.stderr | 2 +- 7 files changed, 101 insertions(+), 89 deletions(-) diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 8fd7c5764797e..d212e18b4cd72 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -378,7 +378,7 @@ impl From> for DiagnosticMessage { } } -/// A workaround for "good path" ICEs when formatting types in disabled lints. +/// A workaround for good_path_delayed_bug ICEs when formatting types in disabled lints. /// /// Delays formatting until `.into(): DiagnosticMessage` is used. pub struct DelayDm(pub F); diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 949f52ef6b586..06f6c58c5ff2e 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -85,7 +85,11 @@ fn source_string(file: Lrc, line: &Line) -> String { /// Maps `Diagnostic::Level` to `snippet::AnnotationType` fn annotation_type_for_level(level: Level) -> AnnotationType { match level { - Level::Bug | Level::DelayedBug(_) | Level::Fatal | Level::Error => AnnotationType::Error, + Level::Bug + | Level::Fatal + | Level::Error + | Level::DelayedBug + | Level::GoodPathDelayedBug => AnnotationType::Error, Level::ForceWarning(_) | Level::Warning => AnnotationType::Warning, Level::Note | Level::OnceNote => AnnotationType::Note, Level::Help | Level::OnceHelp => AnnotationType::Help, diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 299e4a840f726..1763c355069a9 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -1,8 +1,7 @@ use crate::snippet::Style; use crate::{ - CodeSuggestion, DelayedBugKind, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee, - ErrCode, Level, MultiSpan, SubdiagnosticMessage, Substitution, SubstitutionPart, - SuggestionStyle, + CodeSuggestion, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee, ErrCode, Level, + MultiSpan, SubdiagnosticMessage, Substitution, SubstitutionPart, SuggestionStyle, }; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_error_messages::fluent_value_from_str_list_sep_by_and; @@ -235,14 +234,11 @@ impl Diagnostic { pub fn is_error(&self) -> bool { match self.level { - Level::Bug - | Level::DelayedBug(DelayedBugKind::Normal) - | Level::Fatal - | Level::Error => true, + Level::Bug | Level::Fatal | Level::Error | Level::DelayedBug => true, - Level::ForceWarning(_) + Level::GoodPathDelayedBug + | Level::ForceWarning(_) | Level::Warning - | Level::DelayedBug(DelayedBugKind::GoodPath) | Level::Note | Level::OnceNote | Level::Help @@ -306,11 +302,11 @@ impl Diagnostic { #[track_caller] pub fn downgrade_to_delayed_bug(&mut self) { assert!( - matches!(self.level, Level::Error | Level::DelayedBug(_)), + matches!(self.level, Level::Error | Level::DelayedBug), "downgrade_to_delayed_bug: cannot downgrade {:?} to DelayedBug: not an error", self.level ); - self.level = Level::DelayedBug(DelayedBugKind::Normal); + self.level = Level::DelayedBug; } /// Appends a labeled span to the diagnostic. diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 4be5ed923e5e0..6370e1d387c37 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -2116,6 +2116,7 @@ impl HumanEmitter { } if !self.short_message { for child in children { + assert!(child.level.can_be_top_or_sub().1); let span = &child.span; if let Err(err) = self.emit_messages_default_inner( span, diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index b3461b676be27..cfb2dfbeb984a 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -439,7 +439,7 @@ struct DiagCtxtInner { has_printed: bool, emitter: Box, - span_delayed_bugs: Vec, + delayed_bugs: Vec, good_path_delayed_bugs: Vec, /// This flag indicates that an expected diagnostic was emitted and suppressed. /// This is used for the `good_path_delayed_bugs` check. @@ -523,8 +523,7 @@ fn default_track_diagnostic(diag: Diagnostic, f: &mut dyn FnMut(Diagnostic)) { pub static TRACK_DIAGNOSTIC: AtomicRef = AtomicRef::new(&(default_track_diagnostic as _)); -#[derive(Copy, PartialEq, Eq, Clone, Hash, Debug, Encodable, Decodable)] -pub enum DelayedBugKind { +enum DelayedBugKind { Normal, GoodPath, } @@ -557,11 +556,6 @@ impl Drop for DiagCtxtInner { self.flush_delayed(DelayedBugKind::Normal) } - // FIXME(eddyb) this explains what `good_path_delayed_bugs` are! - // They're `span_delayed_bugs` but for "require some diagnostic happened" - // instead of "require some error happened". Sadly that isn't ideal, as - // lints can be `#[allow]`'d, potentially leading to this triggering. - // Also, "good path" should be replaced with a better naming. if !self.has_printed && !self.suppressed_expected_diag && !std::thread::panicking() { self.flush_delayed(DelayedBugKind::GoodPath); } @@ -608,7 +602,7 @@ impl DiagCtxt { deduplicated_warn_count: 0, has_printed: false, emitter, - span_delayed_bugs: Vec::new(), + delayed_bugs: Vec::new(), good_path_delayed_bugs: Vec::new(), suppressed_expected_diag: false, taught_diagnostics: Default::default(), @@ -664,7 +658,7 @@ impl DiagCtxt { inner.has_printed = false; // actually free the underlying memory (which `clear` would not do) - inner.span_delayed_bugs = Default::default(); + inner.delayed_bugs = Default::default(); inner.good_path_delayed_bugs = Default::default(); inner.taught_diagnostics = Default::default(); inner.emitted_diagnostic_codes = Default::default(); @@ -865,8 +859,7 @@ impl DiagCtxt { /// directly). #[track_caller] pub fn delayed_bug(&self, msg: impl Into) -> ErrorGuaranteed { - DiagnosticBuilder::::new(self, DelayedBug(DelayedBugKind::Normal), msg) - .emit() + DiagnosticBuilder::::new(self, DelayedBug, msg).emit() } /// Like `delayed_bug`, but takes an additional span. @@ -879,15 +872,12 @@ impl DiagCtxt { sp: impl Into, msg: impl Into, ) -> ErrorGuaranteed { - DiagnosticBuilder::::new(self, DelayedBug(DelayedBugKind::Normal), msg) - .with_span(sp) - .emit() + DiagnosticBuilder::::new(self, DelayedBug, msg).with_span(sp).emit() } - // FIXME(eddyb) note the comment inside `impl Drop for DiagCtxtInner`, that's - // where the explanation of what "good path" is (also, it should be renamed). + /// Ensures that a diagnostic is printed. See `Level::GoodPathDelayedBug`. pub fn good_path_delayed_bug(&self, msg: impl Into) { - DiagnosticBuilder::<()>::new(self, DelayedBug(DelayedBugKind::GoodPath), msg).emit() + DiagnosticBuilder::<()>::new(self, GoodPathDelayedBug, msg).emit() } #[track_caller] @@ -961,7 +951,7 @@ impl DiagCtxt { pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option { let inner = self.inner.borrow(); let result = - inner.has_errors() || inner.lint_err_count > 0 || !inner.span_delayed_bugs.is_empty(); + inner.has_errors() || inner.lint_err_count > 0 || !inner.delayed_bugs.is_empty(); result.then(|| { #[allow(deprecated)] ErrorGuaranteed::unchecked_claim_error_was_emitted() @@ -1247,6 +1237,8 @@ impl DiagCtxtInner { } fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { + assert!(diagnostic.level.can_be_top_or_sub().0); + if let Some(expectation_id) = diagnostic.level.get_expectation_id() { // The `LintExpectationId` can be stable or unstable depending on when it was created. // Diagnostics created before the definition of `HirId`s are unstable and can not yet @@ -1268,27 +1260,29 @@ impl DiagCtxtInner { self.future_breakage_diagnostics.push(diagnostic.clone()); } - if matches!(diagnostic.level, DelayedBug(_)) && self.flags.eagerly_emit_delayed_bugs { + if matches!(diagnostic.level, DelayedBug | GoodPathDelayedBug) + && self.flags.eagerly_emit_delayed_bugs + { diagnostic.level = Error; } match diagnostic.level { - // This must come after the possible promotion of `DelayedBug` to `Error` above. + // This must come after the possible promotion of `DelayedBug`/`GoodPathDelayedBug` to + // `Error` above. Fatal | Error if self.treat_next_err_as_bug() => { diagnostic.level = Bug; } - DelayedBug(DelayedBugKind::Normal) => { + DelayedBug => { // FIXME(eddyb) this should check for `has_errors` and stop pushing - // once *any* errors were emitted (and truncate `span_delayed_bugs` + // once *any* errors were emitted (and truncate `delayed_bugs` // when an error is first emitted, also), but maybe there's a case // in which that's not sound? otherwise this is really inefficient. let backtrace = std::backtrace::Backtrace::capture(); - self.span_delayed_bugs - .push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); + self.delayed_bugs.push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); #[allow(deprecated)] return Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); } - DelayedBug(DelayedBugKind::GoodPath) => { + GoodPathDelayedBug => { let backtrace = std::backtrace::Backtrace::capture(); self.good_path_delayed_bugs .push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); @@ -1392,12 +1386,12 @@ impl DiagCtxtInner { fn flush_delayed(&mut self, kind: DelayedBugKind) { let (bugs, note1) = match kind { DelayedBugKind::Normal => ( - std::mem::take(&mut self.span_delayed_bugs), - "no errors encountered even though `span_delayed_bug` issued", + std::mem::take(&mut self.delayed_bugs), + "no errors encountered even though delayed bugs were created", ), DelayedBugKind::GoodPath => ( std::mem::take(&mut self.good_path_delayed_bugs), - "no warnings or errors encountered even though `good_path_delayed_bugs` issued", + "no warnings or errors encountered even though good path delayed bugs were created", ), }; let note2 = "those delayed bugs will now be shown as internal compiler errors"; @@ -1436,8 +1430,8 @@ impl DiagCtxtInner { let mut bug = if backtrace || self.ice_file.is_none() { bug.decorate() } else { bug.inner }; - // "Undelay" the `DelayedBug`s (into plain `Bug`s). - if !matches!(bug.level, DelayedBug(_)) { + // "Undelay" the delayed bugs (into plain `Bug`s). + if !matches!(bug.level, DelayedBug | GoodPathDelayedBug) { // NOTE(eddyb) not panicking here because we're already producing // an ICE, and the more information the merrier. bug.subdiagnostic(InvalidFlushedDelayedDiagnosticLevel { @@ -1503,85 +1497,89 @@ impl DelayedDiagnostic { } } +/// Level is_error EmissionGuarantee Top-level Sub Used in lints? +/// ----- -------- ----------------- --------- --- -------------- +/// Bug yes BugAbort yes - - +/// Fatal yes FatalAbort/FatalError(*) yes - - +/// Error yes ErrorGuaranteed yes - yes +/// DelayedBug yes ErrorGuaranteed yes - - +/// GoodPathDelayedBug - () yes - - +/// ForceWarning - () yes - lint-only +/// Warning - () yes yes yes +/// Note - () rare yes - +/// OnceNote - () - yes lint-only +/// Help - () rare yes - +/// OnceHelp - () - yes lint-only +/// FailureNote - () rare - - +/// Allow - () yes - lint-only +/// Expect - () yes - lint-only +/// +/// (*) `FatalAbort` normally, `FatalError` in the non-aborting "almost fatal" case that is +/// occasionally used. +/// #[derive(Copy, PartialEq, Eq, Clone, Hash, Debug, Encodable, Decodable)] pub enum Level { /// For bugs in the compiler. Manifests as an ICE (internal compiler error) panic. - /// - /// Its `EmissionGuarantee` is `BugAbort`. Bug, - /// This is a strange one: lets you register an error without emitting it. If compilation ends - /// without any other errors occurring, this will be emitted as a bug. Otherwise, it will be - /// silently dropped. I.e. "expect other errors are emitted" semantics. Useful on code paths - /// that should only be reached when compiling erroneous code. - /// - /// Its `EmissionGuarantee` is `ErrorGuaranteed` for `Normal` delayed bugs, and `()` for - /// `GoodPath` delayed bugs. - DelayedBug(DelayedBugKind), - /// An error that causes an immediate abort. Used for things like configuration errors, /// internal overflows, some file operation errors. - /// - /// Its `EmissionGuarantee` is `FatalAbort`, except in the non-aborting "almost fatal" case - /// that is occasionally used, where it is `FatalError`. Fatal, /// An error in the code being compiled, which prevents compilation from finishing. This is the /// most common case. - /// - /// Its `EmissionGuarantee` is `ErrorGuaranteed`. Error, + /// This is a strange one: lets you register an error without emitting it. If compilation ends + /// without any other errors occurring, this will be emitted as a bug. Otherwise, it will be + /// silently dropped. I.e. "expect other errors are emitted" semantics. Useful on code paths + /// that should only be reached when compiling erroneous code. + DelayedBug, + + /// Like `DelayedBug`, but weaker: lets you register an error without emitting it. If + /// compilation ends without any other diagnostics being emitted (and without an expected lint + /// being suppressed), this will be emitted as a bug. Otherwise, it will be silently dropped. + /// I.e. "expect other diagnostics are emitted (or suppressed)" semantics. Useful on code paths + /// that should only be reached when emitting diagnostics, e.g. for expensive one-time + /// diagnostic formatting operations. + /// + /// FIXME(nnethercote) good path delayed bugs are semantically strange: if printed they produce + /// an ICE, but they don't satisfy `is_error` and they don't guarantee an error is emitted. + /// Plus there's the extra complication with expected (suppressed) lints. They have limited + /// use, and are used in very few places, and "good path" isn't a good name. It would be good + /// to remove them. + GoodPathDelayedBug, + /// A `force-warn` lint warning about the code being compiled. Does not prevent compilation /// from finishing. /// /// The [`LintExpectationId`] is used for expected lint diagnostics. In all other cases this /// should be `None`. - /// - /// Its `EmissionGuarantee` is `()`. ForceWarning(Option), /// A warning about the code being compiled. Does not prevent compilation from finishing. - /// - /// Its `EmissionGuarantee` is `()`. Warning, - /// A message giving additional context. Rare, because notes are more commonly attached to other - /// diagnostics such as errors. - /// - /// Its `EmissionGuarantee` is `()`. + /// A message giving additional context. Note, - /// A note that is only emitted once. Rare, mostly used in circumstances relating to lints. - /// - /// Its `EmissionGuarantee` is `()`. + /// A note that is only emitted once. OnceNote, - /// A message suggesting how to fix something. Rare, because help messages are more commonly - /// attached to other diagnostics such as errors. - /// - /// Its `EmissionGuarantee` is `()`. + /// A message suggesting how to fix something. Help, - /// A help that is only emitted once. Rare. - /// - /// Its `EmissionGuarantee` is `()`. + /// A help that is only emitted once. OnceHelp, /// Similar to `Note`, but used in cases where compilation has failed. When printed for human - /// consumption, it doesn't have any kind of `note:` label. Rare. - /// - /// Its `EmissionGuarantee` is `()`. + /// consumption, it doesn't have any kind of `note:` label. FailureNote, /// Only used for lints. - /// - /// Its `EmissionGuarantee` is `()`. Allow, /// Only used for lints. - /// - /// Its `EmissionGuarantee` is `()`. Expect(LintExpectationId), } @@ -1595,7 +1593,7 @@ impl Level { fn color(self) -> ColorSpec { let mut spec = ColorSpec::new(); match self { - Bug | DelayedBug(_) | Fatal | Error => { + Bug | Fatal | Error | DelayedBug | GoodPathDelayedBug => { spec.set_fg(Some(Color::Red)).set_intense(true); } ForceWarning(_) | Warning => { @@ -1615,7 +1613,7 @@ impl Level { pub fn to_str(self) -> &'static str { match self { - Bug | DelayedBug(_) => "error: internal compiler error", + Bug | DelayedBug | GoodPathDelayedBug => "error: internal compiler error", Fatal | Error => "error", ForceWarning(_) | Warning => "warning", Note | OnceNote => "note", @@ -1635,6 +1633,19 @@ impl Level { _ => None, } } + + // Can this level be used in a top-level diagnostic message and/or a + // subdiagnostic message? + fn can_be_top_or_sub(&self) -> (bool, bool) { + match self { + Bug | DelayedBug | Fatal | Error | GoodPathDelayedBug | ForceWarning(_) + | FailureNote | Allow | Expect(_) => (true, false), + + Warning | Note | Help => (true, true), + + OnceNote | OnceHelp => (false, true), + } + } } // FIXME(eddyb) this doesn't belong here AFAICT, should be moved to callsite. diff --git a/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr b/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr index 1011fc4163bca..0e3cd2ff06099 100644 --- a/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr +++ b/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr @@ -1,4 +1,4 @@ -note: no errors encountered even though `span_delayed_bug` issued +note: no errors encountered even though delayed bugs were created note: those delayed bugs will now be shown as internal compiler errors diff --git a/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr b/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr index d92bafce142c2..fd76526644bdd 100644 --- a/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr +++ b/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr @@ -1,4 +1,4 @@ -note: no errors encountered even though `span_delayed_bug` issued +note: no errors encountered even though delayed bugs were created note: those delayed bugs will now be shown as internal compiler errors From 9cd6c68033929d60441ad4286d1270eeafbb8620 Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 5 Feb 2024 10:51:18 +0100 Subject: [PATCH 14/15] cleanup effect var handling --- .../src/infer/canonical/canonicalizer.rs | 2 +- .../rustc_infer/src/infer/canonical/mod.rs | 8 +++- compiler/rustc_infer/src/infer/freshen.rs | 9 +--- compiler/rustc_infer/src/infer/mod.rs | 15 ++++--- .../rustc_infer/src/infer/relate/combine.rs | 38 +++-------------- compiler/rustc_infer/src/infer/resolve.rs | 9 ++-- compiler/rustc_middle/src/infer/unify_key.rs | 42 ++++++++++--------- 7 files changed, 52 insertions(+), 71 deletions(-) diff --git a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs index e4b37f05b778f..30ca70326fa87 100644 --- a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs +++ b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs @@ -480,7 +480,7 @@ impl<'cx, 'tcx> TypeFolder> for Canonicalizer<'cx, 'tcx> { } ty::ConstKind::Infer(InferConst::EffectVar(vid)) => { match self.infcx.unwrap().probe_effect_var(vid) { - Some(value) => return self.fold_const(value.as_const(self.tcx)), + Some(value) => return self.fold_const(value), None => { return self.canonicalize_const_var( CanonicalVarInfo { kind: CanonicalVarKind::Effect }, diff --git a/compiler/rustc_infer/src/infer/canonical/mod.rs b/compiler/rustc_infer/src/infer/canonical/mod.rs index 386fdb09ba5d5..1f68a5a9c6179 100644 --- a/compiler/rustc_infer/src/infer/canonical/mod.rs +++ b/compiler/rustc_infer/src/infer/canonical/mod.rs @@ -24,6 +24,7 @@ use crate::infer::{ConstVariableOrigin, ConstVariableOriginKind}; use crate::infer::{InferCtxt, RegionVariableOrigin, TypeVariableOrigin, TypeVariableOriginKind}; use rustc_index::IndexVec; +use rustc_middle::infer::unify_key::EffectVarValue; use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::GenericArg; use rustc_middle::ty::{self, List, Ty, TyCtxt}; @@ -152,7 +153,12 @@ impl<'tcx> InferCtxt<'tcx> { ) .into(), CanonicalVarKind::Effect => { - let vid = self.inner.borrow_mut().effect_unification_table().new_key(None).vid; + let vid = self + .inner + .borrow_mut() + .effect_unification_table() + .new_key(EffectVarValue::Unknown) + .vid; ty::Const::new_infer(self.tcx, ty::InferConst::EffectVar(vid), self.tcx.types.bool) .into() } diff --git a/compiler/rustc_infer/src/infer/freshen.rs b/compiler/rustc_infer/src/infer/freshen.rs index d256994d8d1fd..2d5fa1b5c7001 100644 --- a/compiler/rustc_infer/src/infer/freshen.rs +++ b/compiler/rustc_infer/src/infer/freshen.rs @@ -151,13 +151,8 @@ impl<'a, 'tcx> TypeFolder> for TypeFreshener<'a, 'tcx> { self.freshen_const(opt_ct, ty::InferConst::Var(v), ty::InferConst::Fresh, ct.ty()) } ty::ConstKind::Infer(ty::InferConst::EffectVar(v)) => { - let opt_ct = self - .infcx - .inner - .borrow_mut() - .effect_unification_table() - .probe_value(v) - .map(|effect| effect.as_const(self.infcx.tcx)); + let opt_ct = + self.infcx.inner.borrow_mut().effect_unification_table().probe_value(v).known(); self.freshen_const( opt_ct, ty::InferConst::EffectVar(v), diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 0a39fe007fd22..13baecf6002c0 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -8,6 +8,7 @@ pub use self::ValuePairs::*; pub use relate::combine::ObligationEmittingRelation; use rustc_data_structures::captures::Captures; use rustc_data_structures::undo_log::UndoLogs; +use rustc_middle::infer::unify_key::EffectVarValue; use rustc_middle::infer::unify_key::{ConstVidKey, EffectVidKey}; use self::opaque_types::OpaqueTypeStorage; @@ -25,8 +26,8 @@ use rustc_data_structures::unify as ut; use rustc_errors::{DiagCtxt, DiagnosticBuilder, ErrorGuaranteed}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_middle::infer::canonical::{Canonical, CanonicalVarValues}; +use rustc_middle::infer::unify_key::ConstVariableValue; use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind, ToType}; -use rustc_middle::infer::unify_key::{ConstVariableValue, EffectVarValue}; use rustc_middle::mir::interpret::{ErrorHandled, EvalToValTreeResult}; use rustc_middle::mir::ConstraintCategory; use rustc_middle::traits::{select, DefiningAnchor}; @@ -818,7 +819,7 @@ impl<'tcx> InferCtxt<'tcx> { (0..table.len()) .map(|i| ty::EffectVid::from_usize(i)) - .filter(|&vid| table.probe_value(vid).is_none()) + .filter(|&vid| table.probe_value(vid).is_unknown()) .map(|v| { ty::Const::new_infer(self.tcx, ty::InferConst::EffectVar(v), self.tcx.types.bool) }) @@ -1236,7 +1237,8 @@ impl<'tcx> InferCtxt<'tcx> { } pub fn var_for_effect(&self, param: &ty::GenericParamDef) -> GenericArg<'tcx> { - let effect_vid = self.inner.borrow_mut().effect_unification_table().new_key(None).vid; + let effect_vid = + self.inner.borrow_mut().effect_unification_table().new_key(EffectVarValue::Unknown).vid; let ty = self .tcx .type_of(param.def_id) @@ -1416,8 +1418,8 @@ impl<'tcx> InferCtxt<'tcx> { } } - pub fn probe_effect_var(&self, vid: EffectVid) -> Option> { - self.inner.borrow_mut().effect_unification_table().probe_value(vid) + pub fn probe_effect_var(&self, vid: EffectVid) -> Option> { + self.inner.borrow_mut().effect_unification_table().probe_value(vid).known() } /// Attempts to resolve all type/region/const variables in @@ -1893,7 +1895,8 @@ impl<'a, 'tcx> TypeFolder> for ShallowResolver<'a, 'tcx> { .borrow_mut() .effect_unification_table() .probe_value(vid) - .map_or(ct, |val| val.as_const(self.infcx.tcx)), + .known() + .unwrap_or(ct), _ => ct, } } diff --git a/compiler/rustc_infer/src/infer/relate/combine.rs b/compiler/rustc_infer/src/infer/relate/combine.rs index 1c120646f1f90..13c17ee1cd2b8 100644 --- a/compiler/rustc_infer/src/infer/relate/combine.rs +++ b/compiler/rustc_infer/src/infer/relate/combine.rs @@ -202,11 +202,7 @@ impl<'tcx> InferCtxt<'tcx> { ty::ConstKind::Infer(InferConst::EffectVar(a_vid)), ty::ConstKind::Infer(InferConst::EffectVar(b_vid)), ) => { - self.inner - .borrow_mut() - .effect_unification_table() - .unify_var_var(a_vid, b_vid) - .map_err(|a| effect_unification_error(self.tcx, relation.a_is_expected(), a))?; + self.inner.borrow_mut().effect_unification_table().union(a_vid, b_vid); return Ok(a); } @@ -233,19 +229,11 @@ impl<'tcx> InferCtxt<'tcx> { } (ty::ConstKind::Infer(InferConst::EffectVar(vid)), _) => { - return self.unify_effect_variable( - relation.a_is_expected(), - vid, - EffectVarValue::Const(b), - ); + return Ok(self.unify_effect_variable(vid, b)); } (_, ty::ConstKind::Infer(InferConst::EffectVar(vid))) => { - return self.unify_effect_variable( - !relation.a_is_expected(), - vid, - EffectVarValue::Const(a), - ); + return Ok(self.unify_effect_variable(vid, a)); } (ty::ConstKind::Unevaluated(..), _) | (_, ty::ConstKind::Unevaluated(..)) @@ -366,18 +354,12 @@ impl<'tcx> InferCtxt<'tcx> { Ok(Ty::new_float(self.tcx, val)) } - fn unify_effect_variable( - &self, - vid_is_expected: bool, - vid: ty::EffectVid, - val: EffectVarValue<'tcx>, - ) -> RelateResult<'tcx, ty::Const<'tcx>> { + fn unify_effect_variable(&self, vid: ty::EffectVid, val: ty::Const<'tcx>) -> ty::Const<'tcx> { self.inner .borrow_mut() .effect_unification_table() - .unify_var_value(vid, Some(val)) - .map_err(|e| effect_unification_error(self.tcx, vid_is_expected, e))?; - Ok(val.as_const(self.tcx)) + .union_value(vid, EffectVarValue::Known(val)); + val } } @@ -579,11 +561,3 @@ fn float_unification_error<'tcx>( let (ty::FloatVarValue(a), ty::FloatVarValue(b)) = v; TypeError::FloatMismatch(ExpectedFound::new(a_is_expected, a, b)) } - -fn effect_unification_error<'tcx>( - _tcx: TyCtxt<'tcx>, - _a_is_expected: bool, - (_a, _b): (EffectVarValue<'tcx>, EffectVarValue<'tcx>), -) -> TypeError<'tcx> { - bug!("unexpected effect unification error") -} diff --git a/compiler/rustc_infer/src/infer/resolve.rs b/compiler/rustc_infer/src/infer/resolve.rs index 959b09031277c..d5999331dfab6 100644 --- a/compiler/rustc_infer/src/infer/resolve.rs +++ b/compiler/rustc_infer/src/infer/resolve.rs @@ -237,14 +237,13 @@ impl<'tcx> TypeFolder> for EagerResolver<'_, 'tcx> { } ty::ConstKind::Infer(ty::InferConst::EffectVar(vid)) => { debug_assert_eq!(c.ty(), self.infcx.tcx.types.bool); - match self.infcx.probe_effect_var(vid) { - Some(c) => c.as_const(self.infcx.tcx), - None => ty::Const::new_infer( + self.infcx.probe_effect_var(vid).unwrap_or_else(|| { + ty::Const::new_infer( self.infcx.tcx, ty::InferConst::EffectVar(self.infcx.root_effect_var(vid)), self.infcx.tcx.types.bool, - ), - } + ) + }) } _ => { if c.has_infer() { diff --git a/compiler/rustc_middle/src/infer/unify_key.rs b/compiler/rustc_middle/src/infer/unify_key.rs index c35799ef47f2b..63c0ebd5f6b79 100644 --- a/compiler/rustc_middle/src/infer/unify_key.rs +++ b/compiler/rustc_middle/src/infer/unify_key.rs @@ -194,33 +194,37 @@ impl<'tcx> UnifyValue for ConstVariableValue<'tcx> { /// values for the effect inference variable #[derive(Clone, Copy, Debug)] pub enum EffectVarValue<'tcx> { - /// The host effect is on, enabling access to syscalls, filesystem access, etc. - Host, - /// The host effect is off. Execution is restricted to const operations only. - NoHost, - Const(ty::Const<'tcx>), + Unknown, + Known(ty::Const<'tcx>), } impl<'tcx> EffectVarValue<'tcx> { - pub fn as_const(self, tcx: TyCtxt<'tcx>) -> ty::Const<'tcx> { + pub fn known(self) -> Option> { match self { - EffectVarValue::Host => tcx.consts.true_, - EffectVarValue::NoHost => tcx.consts.false_, - EffectVarValue::Const(c) => c, + EffectVarValue::Unknown => None, + EffectVarValue::Known(value) => Some(value), + } + } + + pub fn is_unknown(self) -> bool { + match self { + EffectVarValue::Unknown => true, + EffectVarValue::Known(_) => false, } } } impl<'tcx> UnifyValue for EffectVarValue<'tcx> { - type Error = (EffectVarValue<'tcx>, EffectVarValue<'tcx>); + type Error = NoError; fn unify_values(value1: &Self, value2: &Self) -> Result { - match (value1, value2) { - (EffectVarValue::Host, EffectVarValue::Host) => Ok(EffectVarValue::Host), - (EffectVarValue::NoHost, EffectVarValue::NoHost) => Ok(EffectVarValue::NoHost), - (EffectVarValue::NoHost | EffectVarValue::Host, _) - | (_, EffectVarValue::NoHost | EffectVarValue::Host) => Err((*value1, *value2)), - (EffectVarValue::Const(_), EffectVarValue::Const(_)) => { - bug!("equating two const variables, both of which have known values") + match (*value1, *value2) { + (EffectVarValue::Unknown, EffectVarValue::Unknown) => Ok(EffectVarValue::Unknown), + (EffectVarValue::Unknown, EffectVarValue::Known(val)) + | (EffectVarValue::Known(val), EffectVarValue::Unknown) => { + Ok(EffectVarValue::Known(val)) + } + (EffectVarValue::Known(_), EffectVarValue::Known(_)) => { + bug!("equating known inference variables: {value1:?} {value2:?}") } } } @@ -229,7 +233,7 @@ impl<'tcx> UnifyValue for EffectVarValue<'tcx> { #[derive(PartialEq, Copy, Clone, Debug)] pub struct EffectVidKey<'tcx> { pub vid: ty::EffectVid, - pub phantom: PhantomData>, + pub phantom: PhantomData>, } impl<'tcx> From for EffectVidKey<'tcx> { @@ -239,7 +243,7 @@ impl<'tcx> From for EffectVidKey<'tcx> { } impl<'tcx> UnifyKey for EffectVidKey<'tcx> { - type Value = Option>; + type Value = EffectVarValue<'tcx>; #[inline] fn index(&self) -> u32 { self.vid.as_u32() From d9508a1fd2bdbc2f7c4e2ee28503f15487fdc8ce Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 2 Feb 2024 15:44:22 +1100 Subject: [PATCH 15/15] Make `Emitter::emit_diagnostic` consuming. All the other `emit`/`emit_diagnostic` methods were recently made consuming (e.g. #119606), but this one wasn't. But it makes sense to. Much of this is straightforward, and lots of `clone` calls are avoided. There are a couple of tricky bits. - `Emitter::primary_span_formatted` no longer takes a `Diagnostic` and returns a pair. Instead it takes the two fields from `Diagnostic` that it used (`span` and `suggestions`) as `&mut`, and modifies them. This is necessary to avoid the cloning of `diag.children` in two emitters. - `from_errors_diagnostic` is rearranged so various uses of `diag` occur before the consuming `emit_diagnostic` call. --- compiler/rustc_codegen_ssa/src/back/write.rs | 2 +- .../src/annotate_snippet_emitter_writer.rs | 16 +++--- compiler/rustc_errors/src/emitter.rs | 41 +++++++-------- compiler/rustc_errors/src/json.rs | 52 ++++++++++--------- compiler/rustc_errors/src/lib.rs | 26 ++++++---- .../passes/lint/check_code_block_syntax.rs | 2 +- src/tools/rustfmt/src/parse/session.rs | 20 +++---- 7 files changed, 84 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 4211f875dd01a..9b24339d2551f 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -1810,7 +1810,7 @@ impl Translate for SharedEmitter { } impl Emitter for SharedEmitter { - fn emit_diagnostic(&mut self, diag: &rustc_errors::Diagnostic) { + fn emit_diagnostic(&mut self, diag: rustc_errors::Diagnostic) { let args: FxHashMap = diag.args().map(|(name, arg)| (name.clone(), arg.clone())).collect(); drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic { diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 949f52ef6b586..1ce75a0381237 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -44,15 +44,15 @@ impl Translate for AnnotateSnippetEmitter { impl Emitter for AnnotateSnippetEmitter { /// The entry point for the diagnostics generation - fn emit_diagnostic(&mut self, diag: &Diagnostic) { + fn emit_diagnostic(&mut self, mut diag: Diagnostic) { let fluent_args = to_fluent_args(diag.args()); - let mut children = diag.children.clone(); - let (mut primary_span, suggestions) = self.primary_span_formatted(diag, &fluent_args); + let mut suggestions = diag.suggestions.unwrap_or(vec![]); + self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args); self.fix_multispans_in_extern_macros_and_render_macro_backtrace( - &mut primary_span, - &mut children, + &mut diag.span, + &mut diag.children, &diag.level, self.macro_backtrace, ); @@ -62,9 +62,9 @@ impl Emitter for AnnotateSnippetEmitter { &diag.messages, &fluent_args, &diag.code, - &primary_span, - &children, - suggestions, + &diag.span, + &diag.children, + &suggestions, ); } diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 4be5ed923e5e0..12d947f2098b1 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -193,7 +193,7 @@ pub type DynEmitter = dyn Emitter + DynSend; /// Emitter trait for emitting errors. pub trait Emitter: Translate { /// Emit a structured diagnostic. - fn emit_diagnostic(&mut self, diag: &Diagnostic); + fn emit_diagnostic(&mut self, diag: Diagnostic); /// Emit a notification that an artifact has been output. /// Currently only supported for the JSON format. @@ -230,17 +230,17 @@ pub trait Emitter: Translate { /// /// * If the current `Diagnostic` has only one visible `CodeSuggestion`, /// we format the `help` suggestion depending on the content of the - /// substitutions. In that case, we return the modified span only. + /// substitutions. In that case, we modify the span and clear the + /// suggestions. /// /// * If the current `Diagnostic` has multiple suggestions, - /// we return the original `primary_span` and the original suggestions. - fn primary_span_formatted<'a>( + /// we leave `primary_span` and the suggestions untouched. + fn primary_span_formatted( &mut self, - diag: &'a Diagnostic, + primary_span: &mut MultiSpan, + suggestions: &mut Vec, fluent_args: &FluentArgs<'_>, - ) -> (MultiSpan, &'a [CodeSuggestion]) { - let mut primary_span = diag.span.clone(); - let suggestions = diag.suggestions.as_deref().unwrap_or(&[]); + ) { if let Some((sugg, rest)) = suggestions.split_first() { let msg = self.translate_message(&sugg.msg, fluent_args).map_err(Report::new).unwrap(); if rest.is_empty() && @@ -287,16 +287,15 @@ pub trait Emitter: Translate { primary_span.push_span_label(sugg.substitutions[0].parts[0].span, msg); // We return only the modified primary_span - (primary_span, &[]) + suggestions.clear(); } else { // if there are multiple suggestions, print them all in full // to be consistent. We could try to figure out if we can // make one (or the first one) inline, but that would give // undue importance to a semi-random suggestion - (primary_span, suggestions) } } else { - (primary_span, suggestions) + // do nothing } } @@ -518,16 +517,15 @@ impl Emitter for HumanEmitter { self.sm.as_ref() } - fn emit_diagnostic(&mut self, diag: &Diagnostic) { + fn emit_diagnostic(&mut self, mut diag: Diagnostic) { let fluent_args = to_fluent_args(diag.args()); - let mut children = diag.children.clone(); - let (mut primary_span, suggestions) = self.primary_span_formatted(diag, &fluent_args); - debug!("emit_diagnostic: suggestions={:?}", suggestions); + let mut suggestions = diag.suggestions.unwrap_or(vec![]); + self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args); self.fix_multispans_in_extern_macros_and_render_macro_backtrace( - &mut primary_span, - &mut children, + &mut diag.span, + &mut diag.children, &diag.level, self.macro_backtrace, ); @@ -537,9 +535,9 @@ impl Emitter for HumanEmitter { &diag.messages, &fluent_args, &diag.code, - &primary_span, - &children, - suggestions, + &diag.span, + &diag.children, + &suggestions, self.track_diagnostics.then_some(&diag.emitted_at), ); } @@ -576,9 +574,8 @@ impl Emitter for SilentEmitter { None } - fn emit_diagnostic(&mut self, diag: &Diagnostic) { + fn emit_diagnostic(&mut self, mut diag: Diagnostic) { if diag.level == Level::Fatal { - let mut diag = diag.clone(); diag.note(self.fatal_note.clone()); self.fatal_dcx.emit_diagnostic(diag); } diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 6f92299827950..470e3d52452cf 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -176,7 +176,7 @@ impl Translate for JsonEmitter { } impl Emitter for JsonEmitter { - fn emit_diagnostic(&mut self, diag: &crate::Diagnostic) { + fn emit_diagnostic(&mut self, diag: crate::Diagnostic) { let data = Diagnostic::from_errors_diagnostic(diag, self); let result = self.emit(EmitTyped::Diagnostic(data)); if let Err(e) = result { @@ -201,7 +201,7 @@ impl Emitter for JsonEmitter { } FutureBreakageItem { diagnostic: EmitTyped::Diagnostic(Diagnostic::from_errors_diagnostic( - &diag, self, + diag, self, )), } }) @@ -340,7 +340,7 @@ struct UnusedExterns<'a, 'b, 'c> { } impl Diagnostic { - fn from_errors_diagnostic(diag: &crate::Diagnostic, je: &JsonEmitter) -> Diagnostic { + fn from_errors_diagnostic(diag: crate::Diagnostic, je: &JsonEmitter) -> Diagnostic { let args = to_fluent_args(diag.args()); let sugg = diag.suggestions.iter().flatten().map(|sugg| { let translated_message = @@ -382,6 +382,28 @@ impl Diagnostic { Ok(()) } } + + let translated_message = je.translate_messages(&diag.messages, &args); + + let code = if let Some(code) = diag.code { + Some(DiagnosticCode { + code: code.to_string(), + explanation: je.registry.as_ref().unwrap().try_find_description(code).ok(), + }) + } else if let Some(IsLint { name, .. }) = &diag.is_lint { + Some(DiagnosticCode { code: name.to_string(), explanation: None }) + } else { + None + }; + let level = diag.level.to_str(); + let spans = DiagnosticSpan::from_multispan(&diag.span, &args, je); + let children = diag + .children + .iter() + .map(|c| Diagnostic::from_sub_diagnostic(c, &args, je)) + .chain(sugg) + .collect(); + let buf = BufWriter::default(); let output = buf.clone(); je.json_rendered @@ -398,30 +420,12 @@ impl Diagnostic { let output = Arc::try_unwrap(output.0).unwrap().into_inner().unwrap(); let output = String::from_utf8(output).unwrap(); - let translated_message = je.translate_messages(&diag.messages, &args); - - let code = if let Some(code) = diag.code { - Some(DiagnosticCode { - code: code.to_string(), - explanation: je.registry.as_ref().unwrap().try_find_description(code).ok(), - }) - } else if let Some(IsLint { name, .. }) = &diag.is_lint { - Some(DiagnosticCode { code: name.to_string(), explanation: None }) - } else { - None - }; - Diagnostic { message: translated_message.to_string(), code, - level: diag.level.to_str(), - spans: DiagnosticSpan::from_multispan(&diag.span, &args, je), - children: diag - .children - .iter() - .map(|c| Diagnostic::from_sub_diagnostic(c, &args, je)) - .chain(sugg) - .collect(), + level, + spans, + children, rendered: Some(output), } } diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index b2bd4d8eb956e..a48fcf3af9b02 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -990,9 +990,13 @@ impl DiagCtxt { match (errors.len(), warnings.len()) { (0, 0) => return, - (0, _) => inner - .emitter - .emit_diagnostic(&Diagnostic::new(Warning, DiagnosticMessage::Str(warnings))), + (0, _) => { + // Use `inner.emitter` directly, otherwise the warning might not be emitted, e.g. + // with a configuration like `--cap-lints allow --force-warn bare_trait_objects`. + inner + .emitter + .emit_diagnostic(Diagnostic::new(Warning, DiagnosticMessage::Str(warnings))); + } (_, 0) => { inner.emit_diagnostic(Diagnostic::new(Fatal, errors)); } @@ -1056,7 +1060,7 @@ impl DiagCtxt { } pub fn force_print_diagnostic(&self, db: Diagnostic) { - self.inner.borrow_mut().emitter.emit_diagnostic(&db); + self.inner.borrow_mut().emitter.emit_diagnostic(db); } pub fn emit_diagnostic(&self, diagnostic: Diagnostic) -> Option { @@ -1324,11 +1328,15 @@ impl DiagCtxtInner { !self.emitted_diagnostics.insert(diagnostic_hash) }; + let is_error = diagnostic.is_error(); + let is_lint = diagnostic.is_lint.is_some(); + // Only emit the diagnostic if we've been asked to deduplicate or // haven't already emitted an equivalent diagnostic. if !(self.flags.deduplicate_diagnostics && already_emitted) { debug!(?diagnostic); debug!(?self.emitted_diagnostics); + let already_emitted_sub = |sub: &mut SubDiagnostic| { debug!(?sub); if sub.level != OnceNote && sub.level != OnceHelp { @@ -1340,7 +1348,6 @@ impl DiagCtxtInner { debug!(?diagnostic_hash); !self.emitted_diagnostics.insert(diagnostic_hash) }; - diagnostic.children.extract_if(already_emitted_sub).for_each(|_| {}); if already_emitted { diagnostic.note( @@ -1348,16 +1355,17 @@ impl DiagCtxtInner { ); } - self.emitter.emit_diagnostic(&diagnostic); - if diagnostic.is_error() { + if is_error { self.deduplicated_err_count += 1; } else if matches!(diagnostic.level, ForceWarning(_) | Warning) { self.deduplicated_warn_count += 1; } self.has_printed = true; + + self.emitter.emit_diagnostic(diagnostic); } - if diagnostic.is_error() { - if diagnostic.is_lint.is_some() { + if is_error { + if is_lint { self.lint_err_count += 1; } else { self.err_count += 1; diff --git a/src/librustdoc/passes/lint/check_code_block_syntax.rs b/src/librustdoc/passes/lint/check_code_block_syntax.rs index e73649c722493..6649894f9c25d 100644 --- a/src/librustdoc/passes/lint/check_code_block_syntax.rs +++ b/src/librustdoc/passes/lint/check_code_block_syntax.rs @@ -156,7 +156,7 @@ impl Translate for BufferEmitter { } impl Emitter for BufferEmitter { - fn emit_diagnostic(&mut self, diag: &Diagnostic) { + fn emit_diagnostic(&mut self, diag: Diagnostic) { let mut buffer = self.buffer.borrow_mut(); let fluent_args = to_fluent_args(diag.args()); diff --git a/src/tools/rustfmt/src/parse/session.rs b/src/tools/rustfmt/src/parse/session.rs index 6dc3eac44d43d..f0af401d3da4b 100644 --- a/src/tools/rustfmt/src/parse/session.rs +++ b/src/tools/rustfmt/src/parse/session.rs @@ -47,7 +47,7 @@ impl Emitter for SilentEmitter { None } - fn emit_diagnostic(&mut self, _db: &Diagnostic) {} + fn emit_diagnostic(&mut self, _db: Diagnostic) {} } fn silent_emitter() -> Box { @@ -64,7 +64,7 @@ struct SilentOnIgnoredFilesEmitter { } impl SilentOnIgnoredFilesEmitter { - fn handle_non_ignoreable_error(&mut self, db: &Diagnostic) { + fn handle_non_ignoreable_error(&mut self, db: Diagnostic) { self.has_non_ignorable_parser_errors = true; self.can_reset.store(false, Ordering::Release); self.emitter.emit_diagnostic(db); @@ -86,7 +86,7 @@ impl Emitter for SilentOnIgnoredFilesEmitter { None } - fn emit_diagnostic(&mut self, db: &Diagnostic) { + fn emit_diagnostic(&mut self, db: Diagnostic) { if db.level() == DiagnosticLevel::Fatal { return self.handle_non_ignoreable_error(db); } @@ -365,7 +365,7 @@ mod tests { None } - fn emit_diagnostic(&mut self, _db: &Diagnostic) { + fn emit_diagnostic(&mut self, _db: Diagnostic) { self.num_emitted_errors.fetch_add(1, Ordering::Release); } } @@ -424,7 +424,7 @@ mod tests { ); let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1))); let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, Some(span)); - emitter.emit_diagnostic(&fatal_diagnostic); + emitter.emit_diagnostic(fatal_diagnostic); assert_eq!(num_emitted_errors.load(Ordering::Acquire), 1); assert_eq!(can_reset_errors.load(Ordering::Acquire), false); } @@ -449,7 +449,7 @@ mod tests { ); let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1))); let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span)); - emitter.emit_diagnostic(&non_fatal_diagnostic); + emitter.emit_diagnostic(non_fatal_diagnostic); assert_eq!(num_emitted_errors.load(Ordering::Acquire), 0); assert_eq!(can_reset_errors.load(Ordering::Acquire), true); } @@ -473,7 +473,7 @@ mod tests { ); let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1))); let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span)); - emitter.emit_diagnostic(&non_fatal_diagnostic); + emitter.emit_diagnostic(non_fatal_diagnostic); assert_eq!(num_emitted_errors.load(Ordering::Acquire), 1); assert_eq!(can_reset_errors.load(Ordering::Acquire), false); } @@ -512,9 +512,9 @@ mod tests { let bar_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(bar_span)); let foo_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(foo_span)); let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, None); - emitter.emit_diagnostic(&bar_diagnostic); - emitter.emit_diagnostic(&foo_diagnostic); - emitter.emit_diagnostic(&fatal_diagnostic); + emitter.emit_diagnostic(bar_diagnostic); + emitter.emit_diagnostic(foo_diagnostic); + emitter.emit_diagnostic(fatal_diagnostic); assert_eq!(num_emitted_errors.load(Ordering::Acquire), 2); assert_eq!(can_reset_errors.load(Ordering::Acquire), false); }