From cffc5c21fc49e86859ea5601a4f80d4446b594e9 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 7 Apr 2025 17:28:58 +0300 Subject: [PATCH 1/2] compiletest: Add directive `dont-require-annotations` for making matching on specific diagnostic kinds non-exhaustive --- src/tools/compiletest/src/directive-list.rs | 1 + src/tools/compiletest/src/errors.rs | 11 +++++++++- src/tools/compiletest/src/header.rs | 20 ++++++++++++++++- src/tools/compiletest/src/runtest.rs | 24 ++++++++++----------- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/tools/compiletest/src/directive-list.rs b/src/tools/compiletest/src/directive-list.rs index b2ad5a3b3d0bb..44d9c0330f76e 100644 --- a/src/tools/compiletest/src/directive-list.rs +++ b/src/tools/compiletest/src/directive-list.rs @@ -22,6 +22,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "dont-check-compiler-stderr", "dont-check-compiler-stdout", "dont-check-failure-status", + "dont-require-annotations", "edition", "error-pattern", "exact-llvm-major-version", diff --git a/src/tools/compiletest/src/errors.rs b/src/tools/compiletest/src/errors.rs index 9b59e4968a3dc..64d68eb7f23e5 100644 --- a/src/tools/compiletest/src/errors.rs +++ b/src/tools/compiletest/src/errors.rs @@ -8,7 +8,7 @@ use std::sync::OnceLock; use regex::Regex; use tracing::*; -#[derive(Copy, Clone, Debug, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub enum ErrorKind { Help, Error, @@ -40,6 +40,15 @@ impl ErrorKind { _ => return None, }) } + + pub fn expect_from_user_str(s: &str) -> ErrorKind { + ErrorKind::from_user_str(s).unwrap_or_else(|| { + panic!( + "unexpected diagnostic kind `{s}`, expected \ + `ERROR`, `WARN`, `NOTE`, `HELP` or `SUGGESTION`" + ) + }) + } } impl fmt::Display for ErrorKind { diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index a0178f4bcc576..36a9e5df5839b 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::env; use std::fs::File; use std::io::BufReader; @@ -11,6 +11,7 @@ use tracing::*; use crate::common::{Config, Debugger, FailMode, Mode, PassMode}; use crate::debuggers::{extract_cdb_version, extract_gdb_version}; +use crate::errors::ErrorKind; use crate::executor::{CollectedTestDesc, ShouldPanic}; use crate::header::auxiliary::{AuxProps, parse_and_update_aux}; use crate::header::needs::CachedNeedsConditions; @@ -196,6 +197,8 @@ pub struct TestProps { /// Build and use `minicore` as `core` stub for `no_core` tests in cross-compilation scenarios /// that don't otherwise want/need `-Z build-std`. pub add_core_stubs: bool, + /// Whether line annotatins are required for the given error kind. + pub require_annotations: HashMap, } mod directives { @@ -212,6 +215,7 @@ mod directives { pub const CHECK_RUN_RESULTS: &'static str = "check-run-results"; pub const DONT_CHECK_COMPILER_STDOUT: &'static str = "dont-check-compiler-stdout"; pub const DONT_CHECK_COMPILER_STDERR: &'static str = "dont-check-compiler-stderr"; + pub const DONT_REQUIRE_ANNOTATIONS: &'static str = "dont-require-annotations"; pub const NO_PREFER_DYNAMIC: &'static str = "no-prefer-dynamic"; pub const PRETTY_MODE: &'static str = "pretty-mode"; pub const PRETTY_COMPARE_ONLY: &'static str = "pretty-compare-only"; @@ -297,6 +301,13 @@ impl TestProps { no_auto_check_cfg: false, has_enzyme: false, add_core_stubs: false, + require_annotations: HashMap::from([ + (ErrorKind::Help, true), + (ErrorKind::Note, true), + (ErrorKind::Error, true), + (ErrorKind::Warning, true), + (ErrorKind::Suggestion, false), + ]), } } @@ -570,6 +581,13 @@ impl TestProps { config.set_name_directive(ln, NO_AUTO_CHECK_CFG, &mut self.no_auto_check_cfg); self.update_add_core_stubs(ln, config); + + if let Some(err_kind) = + config.parse_name_value_directive(ln, DONT_REQUIRE_ANNOTATIONS) + { + self.require_annotations + .insert(ErrorKind::expect_from_user_str(&err_kind), false); + } }, ); diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 13f3479247a23..872f5f6ce2967 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -710,10 +710,6 @@ impl<'test> TestCx<'test> { self.testpaths.file.display().to_string() }; - // If the testcase being checked contains at least one expected "help" - // message, then we'll ensure that all "help" messages are expected. - // Otherwise, all "help" messages reported by the compiler will be ignored. - // This logic also applies to "note" messages. let expect_help = expected_errors.iter().any(|ee| ee.kind == Some(ErrorKind::Help)); let expect_note = expected_errors.iter().any(|ee| ee.kind == Some(ErrorKind::Note)); @@ -801,9 +797,7 @@ impl<'test> TestCx<'test> { } /// Returns `true` if we should report an error about `actual_error`, - /// which did not match any of the expected error. We always require - /// errors/warnings to be explicitly listed, but only require - /// helps/notes if there are explicit helps/notes given. + /// which did not match any of the expected error. fn is_unexpected_compiler_message( &self, actual_error: &Error, @@ -811,12 +805,16 @@ impl<'test> TestCx<'test> { expect_note: bool, ) -> bool { actual_error.require_annotation - && match actual_error.kind { - Some(ErrorKind::Help) => expect_help, - Some(ErrorKind::Note) => expect_note, - Some(ErrorKind::Error) | Some(ErrorKind::Warning) => true, - Some(ErrorKind::Suggestion) | None => false, - } + && actual_error.kind.map_or(false, |err_kind| { + // If the test being checked doesn't contain any "help" or "note" annotations, then + // we don't require annotating "help" or "note" (respecively) diagnostics at all. + let default_require_annotations = self.props.require_annotations[&err_kind]; + match err_kind { + ErrorKind::Help => expect_help && default_require_annotations, + ErrorKind::Note => expect_note && default_require_annotations, + _ => default_require_annotations, + } + }) } fn should_emit_metadata(&self, pm: Option) -> Emit { From 12829122bf2216e492e2befb0c1305b5efbfee7d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 9 Apr 2025 09:51:50 +0300 Subject: [PATCH 2/2] Migrate some tests to `dont-require-annotations` --- tests/ui/cfg/cfg_false_no_std-2.rs | 8 ++++---- tests/ui/panic-runtime/two-panic-runtimes.rs | 8 ++++---- tests/ui/panic-runtime/want-abort-got-unwind.rs | 8 ++++---- tests/ui/panic-runtime/want-abort-got-unwind2.rs | 8 ++++---- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/ui/cfg/cfg_false_no_std-2.rs b/tests/ui/cfg/cfg_false_no_std-2.rs index 35e545aae34b8..349c49412ffa0 100644 --- a/tests/ui/cfg/cfg_false_no_std-2.rs +++ b/tests/ui/cfg/cfg_false_no_std-2.rs @@ -1,7 +1,6 @@ // Error, the linked empty library is `no_std` and doesn't provide a panic handler. -//@ compile-flags: --error-format=human -//@ error-pattern: `#[panic_handler]` function required, but not found +//@ dont-require-annotations:ERROR //@ dont-check-compiler-stderr //@ aux-build: cfg_false_lib_no_std_before.rs @@ -11,6 +10,7 @@ extern crate cfg_false_lib_no_std_before as _; fn main() {} -// FIXME: The second error is target-dependent. -//FIXME~? ERROR `#[panic_handler]` function required, but not found +//~? ERROR `#[panic_handler]` function required, but not found +// FIXME: This error is target-dependent, could be served by some "optional error" annotation +// instead of `dont-require-annotations`. //FIXME~? ERROR unwinding panics are not supported without std diff --git a/tests/ui/panic-runtime/two-panic-runtimes.rs b/tests/ui/panic-runtime/two-panic-runtimes.rs index 15c08cbe30d36..7add07ef60045 100644 --- a/tests/ui/panic-runtime/two-panic-runtimes.rs +++ b/tests/ui/panic-runtime/two-panic-runtimes.rs @@ -1,7 +1,6 @@ // ignore-tidy-linelength //@ build-fail -//@ compile-flags: --error-format=human -//@ error-pattern: cannot link together two panic runtimes: panic_runtime_unwind and panic_runtime_unwind2 +//@ dont-require-annotations:ERROR //@ dont-check-compiler-stderr //@ aux-build:panic-runtime-unwind.rs //@ aux-build:panic-runtime-unwind2.rs @@ -16,7 +15,8 @@ extern crate panic_runtime_lang_items; fn main() {} -// FIXME: The second and third errors are target-dependent. -//FIXME~? ERROR cannot link together two panic runtimes: panic_runtime_unwind and panic_runtime_unwind2 +//~? ERROR cannot link together two panic runtimes: panic_runtime_unwind and panic_runtime_unwind2 +// FIXME: These errors are target-dependent, could be served by some "optional error" annotation +// instead of `dont-require-annotations`. //FIXME~? ERROR the linked panic runtime `panic_runtime_unwind2` is not compiled with this crate's panic strategy `abort` //FIXME~? ERROR the crate `panic_runtime_unwind` requires panic strategy `unwind` which is incompatible with this crate's strategy of `abort` diff --git a/tests/ui/panic-runtime/want-abort-got-unwind.rs b/tests/ui/panic-runtime/want-abort-got-unwind.rs index ed61c2613df80..1ae2e623f1030 100644 --- a/tests/ui/panic-runtime/want-abort-got-unwind.rs +++ b/tests/ui/panic-runtime/want-abort-got-unwind.rs @@ -1,7 +1,6 @@ // ignore-tidy-linelength //@ build-fail -//@ compile-flags: --error-format=human -//@ error-pattern: the linked panic runtime `panic_runtime_unwind` is not compiled with this crate's panic strategy `abort` +//@ dont-require-annotations:ERROR //@ dont-check-compiler-stderr //@ aux-build:panic-runtime-unwind.rs //@ compile-flags:-C panic=abort @@ -10,7 +9,8 @@ extern crate panic_runtime_unwind; fn main() {} -// FIXME: The first and third errors are target-dependent. +//~? ERROR the linked panic runtime `panic_runtime_unwind` is not compiled with this crate's panic strategy `abort` +// FIXME: These errors are target-dependent, could be served by some "optional error" annotation +// instead of `dont-require-annotations`. //FIXME~? ERROR cannot link together two panic runtimes: panic_unwind and panic_runtime_unwind -//FIXME~? ERROR the linked panic runtime `panic_runtime_unwind` is not compiled with this crate's panic strategy `abort` //FIXME~? ERROR the crate `panic_unwind` requires panic strategy `unwind` which is incompatible with this crate's strategy of `abort` diff --git a/tests/ui/panic-runtime/want-abort-got-unwind2.rs b/tests/ui/panic-runtime/want-abort-got-unwind2.rs index 504fd779e09ac..dc4d3ea86d865 100644 --- a/tests/ui/panic-runtime/want-abort-got-unwind2.rs +++ b/tests/ui/panic-runtime/want-abort-got-unwind2.rs @@ -1,7 +1,6 @@ // ignore-tidy-linelength //@ build-fail -//@ compile-flags: --error-format=human -//@ error-pattern: the linked panic runtime `panic_runtime_unwind` is not compiled with this crate's panic strategy `abort` +//@ dont-require-annotations:ERROR //@ dont-check-compiler-stderr //@ aux-build:panic-runtime-unwind.rs //@ aux-build:wants-panic-runtime-unwind.rs @@ -11,7 +10,8 @@ extern crate wants_panic_runtime_unwind; fn main() {} -// FIXME: The first and third errors are target-dependent. +//~? ERROR the linked panic runtime `panic_runtime_unwind` is not compiled with this crate's panic strategy `abort` +// FIXME: These errors are target-dependent, could be served by some "optional error" annotation +// instead of `dont-require-annotations`. //FIXME~? ERROR cannot link together two panic runtimes: panic_unwind and panic_runtime_unwind -//FIXME~? ERROR the linked panic runtime `panic_runtime_unwind` is not compiled with this crate's panic strategy `abort` //FIXME~? ERROR the crate `panic_unwind` requires panic strategy `unwind` which is incompatible with this crate's strategy of `abort`