From c3fb7d1d1c3d05be8ed3abdfd01196782f6c7821 Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Mon, 30 Jun 2025 16:59:28 +0800 Subject: [PATCH] Make some compiletest errors/warnings/help more visually obvious --- src/tools/compiletest/src/common.rs | 9 ++-- src/tools/compiletest/src/diagnostics.rs | 46 +++++++++++++++++++ src/tools/compiletest/src/header.rs | 24 +++++----- src/tools/compiletest/src/lib.rs | 21 +++------ src/tools/compiletest/src/runtest.rs | 18 +++++--- .../compiletest/src/runtest/codegen_units.rs | 4 +- 6 files changed, 84 insertions(+), 38 deletions(-) create mode 100644 src/tools/compiletest/src/diagnostics.rs diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 9b9d94bbead09..cdce5d941d015 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -11,6 +11,7 @@ use serde::de::{Deserialize, Deserializer, Error as _}; pub use self::Mode::*; use crate::executor::{ColorConfig, OutputFormat}; +use crate::fatal; use crate::util::{Utf8PathBufExt, add_dylib_path}; macro_rules! string_enum { @@ -783,11 +784,13 @@ fn rustc_output(config: &Config, args: &[&str], envs: HashMap) - let output = match command.output() { Ok(output) => output, - Err(e) => panic!("error: failed to run {command:?}: {e}"), + Err(e) => { + fatal!("failed to run {command:?}: {e}"); + } }; if !output.status.success() { - panic!( - "error: failed to run {command:?}\n--- stdout\n{}\n--- stderr\n{}", + fatal!( + "failed to run {command:?}\n--- stdout\n{}\n--- stderr\n{}", String::from_utf8(output.stdout).unwrap(), String::from_utf8(output.stderr).unwrap(), ); diff --git a/src/tools/compiletest/src/diagnostics.rs b/src/tools/compiletest/src/diagnostics.rs new file mode 100644 index 0000000000000..207a6cb91d7ca --- /dev/null +++ b/src/tools/compiletest/src/diagnostics.rs @@ -0,0 +1,46 @@ +//! Collection of diagnostics helpers for `compiletest` *itself*. + +#[macro_export] +macro_rules! fatal { + ($($arg:tt)*) => { + let status = ::colored::Colorize::bright_red("FATAL: "); + let status = ::colored::Colorize::bold(status); + eprint!("{status}"); + eprintln!($($arg)*); + // This intentionally uses a seemingly-redundant panic to include backtrace location. + // + // FIXME: in the long term, we should handle "logic bug in compiletest itself" vs "fatal + // user error" separately. + panic!("fatal error"); + }; +} + +#[macro_export] +macro_rules! error { + ($($arg:tt)*) => { + let status = ::colored::Colorize::red("ERROR: "); + let status = ::colored::Colorize::bold(status); + eprint!("{status}"); + eprintln!($($arg)*); + }; +} + +#[macro_export] +macro_rules! warning { + ($($arg:tt)*) => { + let status = ::colored::Colorize::yellow("WARNING: "); + let status = ::colored::Colorize::bold(status); + eprint!("{status}"); + eprintln!($($arg)*); + }; +} + +#[macro_export] +macro_rules! help { + ($($arg:tt)*) => { + let status = ::colored::Colorize::cyan("HELP: "); + let status = ::colored::Colorize::bold(status); + eprint!("{status}"); + eprintln!($($arg)*); + }; +} diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 2b203bb309c63..5636a146b0f4d 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -15,6 +15,7 @@ use crate::errors::ErrorKind; use crate::executor::{CollectedTestDesc, ShouldPanic}; use crate::header::auxiliary::{AuxProps, parse_and_update_aux}; use crate::header::needs::CachedNeedsConditions; +use crate::help; use crate::util::static_regex; pub(crate) mod auxiliary; @@ -920,9 +921,9 @@ fn iter_header( if !is_known_directive { *poisoned = true; - eprintln!( - "error: detected unknown compiletest test directive `{}` in {}:{}", - directive_line.raw_directive, testfile, line_number, + error!( + "{testfile}:{line_number}: detected unknown compiletest test directive `{}`", + directive_line.raw_directive, ); return; @@ -931,11 +932,11 @@ fn iter_header( if let Some(trailing_directive) = &trailing_directive { *poisoned = true; - eprintln!( - "error: detected trailing compiletest test directive `{}` in {}:{}\n \ - help: put the trailing directive in it's own line: `//@ {}`", - trailing_directive, testfile, line_number, trailing_directive, + error!( + "{testfile}:{line_number}: detected trailing compiletest test directive `{}`", + trailing_directive, ); + help!("put the trailing directive in it's own line: `//@ {}`", trailing_directive); return; } @@ -1031,10 +1032,9 @@ impl Config { }; let Some((regex, replacement)) = parse_normalize_rule(raw_value) else { - panic!( - "couldn't parse custom normalization rule: `{raw_directive}`\n\ - help: expected syntax is: `{directive_name}: \"REGEX\" -> \"REPLACEMENT\"`" - ); + error!("couldn't parse custom normalization rule: `{raw_directive}`"); + help!("expected syntax is: `{directive_name}: \"REGEX\" -> \"REPLACEMENT\"`"); + panic!("invalid normalization rule detected"); }; Some(NormalizeRule { kind, regex, replacement }) } @@ -1406,7 +1406,7 @@ pub(crate) fn make_test_description( ignore_message = Some(reason.into()); } IgnoreDecision::Error { message } => { - eprintln!("error: {}:{line_number}: {message}", path); + error!("{path}:{line_number}: {message}"); *poisoned = true; return; } diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 23a4dd73796e2..09de3eb4c7025 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -11,6 +11,7 @@ mod tests; pub mod common; pub mod compute_diff; mod debuggers; +pub mod diagnostics; pub mod errors; mod executor; pub mod header; @@ -33,7 +34,7 @@ use build_helper::git::{get_git_modified_files, get_git_untracked_files}; use camino::{Utf8Path, Utf8PathBuf}; use getopts::Options; use rayon::iter::{ParallelBridge, ParallelIterator}; -use tracing::*; +use tracing::debug; use walkdir::WalkDir; use self::header::{EarlyProps, make_test_description}; @@ -651,10 +652,7 @@ pub(crate) fn collect_and_make_tests(config: Arc) -> Vec let common_inputs_stamp = common_inputs_stamp(&config); let modified_tests = modified_tests(&config, &config.src_test_suite_root).unwrap_or_else(|err| { - panic!( - "modified_tests got error from dir: {}, error: {}", - config.src_test_suite_root, err - ) + fatal!("modified_tests: {}: {err}", config.src_test_suite_root); }); let cache = HeadersCache::load(&config); @@ -1108,22 +1106,17 @@ fn check_for_overlapping_test_paths(found_path_stems: &HashSet) { pub fn early_config_check(config: &Config) { if !config.has_html_tidy && config.mode == Mode::Rustdoc { - eprintln!("warning: `tidy` (html-tidy.org) is not installed; diffs will not be generated"); + warning!("`tidy` (html-tidy.org) is not installed; diffs will not be generated"); } if !config.profiler_runtime && config.mode == Mode::CoverageRun { let actioned = if config.bless { "blessed" } else { "checked" }; - eprintln!( - r#" -WARNING: profiler runtime is not available, so `.coverage` files won't be {actioned} -help: try setting `profiler = true` in the `[build]` section of `bootstrap.toml`"# - ); + warning!("profiler runtime is not available, so `.coverage` files won't be {actioned}"); + help!("try setting `profiler = true` in the `[build]` section of `bootstrap.toml`"); } // `RUST_TEST_NOCAPTURE` is a libtest env var, but we don't callout to libtest. if env::var("RUST_TEST_NOCAPTURE").is_ok() { - eprintln!( - "WARNING: RUST_TEST_NOCAPTURE is not supported. Use the `--no-capture` flag instead." - ); + warning!("`RUST_TEST_NOCAPTURE` is not supported; use the `--no-capture` flag instead"); } } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 980e89889abba..53b5990d3cde1 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -27,7 +27,7 @@ use crate::errors::{Error, ErrorKind, load_errors}; use crate::header::TestProps; use crate::read2::{Truncated, read2_abbreviated}; use crate::util::{Utf8PathBufExt, add_dylib_path, logv, static_regex}; -use crate::{ColorConfig, json, stamp_file_path}; +use crate::{ColorConfig, help, json, stamp_file_path, warning}; mod debugger; @@ -485,12 +485,15 @@ impl<'test> TestCx<'test> { .windows(2) .any(|args| args == cfg_arg || args[0] == arg || args[1] == arg) { - panic!( - "error: redundant cfg argument `{normalized_revision}` is already created by the revision" + error!( + "redundant cfg argument `{normalized_revision}` is already created by the \ + revision" ); + panic!("redundant cfg argument"); } if self.config.builtin_cfg_names().contains(&normalized_revision) { - panic!("error: revision `{normalized_revision}` collides with a builtin cfg"); + error!("revision `{normalized_revision}` collides with a built-in cfg"); + panic!("revision collides with built-in cfg"); } cmd.args(cfg_arg); } @@ -2167,9 +2170,10 @@ impl<'test> TestCx<'test> { println!("{}", String::from_utf8_lossy(&output.stdout)); eprintln!("{}", String::from_utf8_lossy(&output.stderr)); } else { - eprintln!("warning: no pager configured, falling back to unified diff"); - eprintln!( - "help: try configuring a git pager (e.g. `delta`) with `git config --global core.pager delta`" + warning!("no pager configured, falling back to unified diff"); + help!( + "try configuring a git pager (e.g. `delta`) with \ + `git config --global core.pager delta`" ); let mut out = io::stdout(); let mut diff = BufReader::new(File::open(&diff_filename).unwrap()); diff --git a/src/tools/compiletest/src/runtest/codegen_units.rs b/src/tools/compiletest/src/runtest/codegen_units.rs index 8dfa8d18d1a0b..44ddcb1d28882 100644 --- a/src/tools/compiletest/src/runtest/codegen_units.rs +++ b/src/tools/compiletest/src/runtest/codegen_units.rs @@ -1,8 +1,8 @@ use std::collections::HashSet; use super::{Emit, TestCx, WillExecute}; -use crate::errors; use crate::util::static_regex; +use crate::{errors, fatal}; impl TestCx<'_> { pub(super) fn run_codegen_units_test(&self) { @@ -100,7 +100,7 @@ impl TestCx<'_> { } if !(missing.is_empty() && unexpected.is_empty() && wrong_cgus.is_empty()) { - panic!(); + fatal!("!(missing.is_empty() && unexpected.is_empty() && wrong_cgus.is_empty())"); } #[derive(Clone, Eq, PartialEq)]