Skip to content

[STACKED] [COMPILETEST-UNTANGLE 3/N] Use "directives" consistently within compiletest #143232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -783,11 +784,13 @@ fn rustc_output(config: &Config, args: &[&str], envs: HashMap<String, String>) -

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(),
);
Expand Down
46 changes: 46 additions & 0 deletions src/tools/compiletest/src/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -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)*);
};
}
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/directive-list.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// This was originally generated by collecting directives from ui tests and then extracting their
/// directive names. This is **not** an exhaustive list of all possible directives. Instead, this is
/// a best-effort approximation for diagnostics. Add new headers to this list when needed.
/// a best-effort approximation for diagnostics. Add new directives to this list when needed.
const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
// tidy-alphabetical-start
"add-core-stubs",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ use tracing::*;

use crate::common::{Config, Debugger, FailMode, Mode, PassMode};
use crate::debuggers::{extract_cdb_version, extract_gdb_version};
use crate::directives::auxiliary::{AuxProps, parse_and_update_aux};
use crate::directives::needs::CachedNeedsConditions;
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;
Expand All @@ -23,11 +24,11 @@ mod needs;
#[cfg(test)]
mod tests;

pub struct HeadersCache {
pub struct DirectivesCache {
needs: CachedNeedsConditions,
}

impl HeadersCache {
impl DirectivesCache {
pub fn load(config: &Config) -> Self {
Self { needs: CachedNeedsConditions::load(config) }
}
Expand All @@ -53,7 +54,7 @@ impl EarlyProps {
pub fn from_reader<R: Read>(config: &Config, testfile: &Utf8Path, rdr: R) -> Self {
let mut props = EarlyProps::default();
let mut poisoned = false;
iter_header(
iter_directives(
config.mode,
&config.suite,
&mut poisoned,
Expand Down Expand Up @@ -137,12 +138,12 @@ pub struct TestProps {
pub incremental_dir: Option<Utf8PathBuf>,
// If `true`, this test will use incremental compilation.
//
// This can be set manually with the `incremental` header, or implicitly
// This can be set manually with the `incremental` directive, or implicitly
// by being a part of an incremental mode test. Using the `incremental`
// header should be avoided if possible; using an incremental mode test is
// directive should be avoided if possible; using an incremental mode test is
// preferred. Incremental mode tests support multiple passes, which can
// verify that the incremental cache can be loaded properly after being
// created. Just setting the header will only verify the behavior with
// created. Just setting the directive will only verify the behavior with
// creating an incremental cache, but doesn't check that it is created
// correctly.
//
Expand Down Expand Up @@ -346,7 +347,7 @@ impl TestProps {

let mut poisoned = false;

iter_header(
iter_directives(
config.mode,
&config.suite,
&mut poisoned,
Expand Down Expand Up @@ -641,11 +642,11 @@ impl TestProps {
let check_ui = |mode: &str| {
// Mode::Crashes may need build-fail in order to trigger llvm errors or stack overflows
if config.mode != Mode::Ui && config.mode != Mode::Crashes {
panic!("`{}-fail` header is only supported in UI tests", mode);
panic!("`{}-fail` directive is only supported in UI tests", mode);
}
};
if config.mode == Mode::Ui && config.parse_name_directive(ln, "compile-fail") {
panic!("`compile-fail` header is useless in UI tests");
panic!("`compile-fail` directive is useless in UI tests");
}
let fail_mode = if config.parse_name_directive(ln, "check-fail") {
check_ui("check");
Expand All @@ -661,7 +662,7 @@ impl TestProps {
};
match (self.fail_mode, fail_mode) {
(None, Some(_)) => self.fail_mode = fail_mode,
(Some(_), Some(_)) => panic!("multiple `*-fail` headers in a single test"),
(Some(_), Some(_)) => panic!("multiple `*-fail` directives in a single test"),
(_, None) => {}
}
}
Expand All @@ -673,10 +674,10 @@ impl TestProps {
(Mode::Codegen, "build-pass") => (),
(Mode::Incremental, _) => {
if revision.is_some() && !self.revisions.iter().all(|r| r.starts_with("cfail")) {
panic!("`{s}` header is only supported in `cfail` incremental tests")
panic!("`{s}` directive is only supported in `cfail` incremental tests")
}
}
(mode, _) => panic!("`{s}` header is not supported in `{mode}` tests"),
(mode, _) => panic!("`{s}` directive is not supported in `{mode}` tests"),
};
let pass_mode = if config.parse_name_directive(ln, "check-pass") {
check_no_run("check-pass");
Expand All @@ -692,7 +693,7 @@ impl TestProps {
};
match (self.pass_mode, pass_mode) {
(None, Some(_)) => self.pass_mode = pass_mode,
(Some(_), Some(_)) => panic!("multiple `*-pass` headers in a single test"),
(Some(_), Some(_)) => panic!("multiple `*-pass` directives in a single test"),
(_, None) => {}
}
}
Expand Down Expand Up @@ -793,7 +794,7 @@ const KNOWN_JSONDOCCK_DIRECTIVE_NAMES: &[&str] =
&["count", "!count", "has", "!has", "is", "!is", "ismany", "!ismany", "set", "!set"];

/// The (partly) broken-down contents of a line containing a test directive,
/// which [`iter_header`] passes to its callback function.
/// which [`iter_directives`] passes to its callback function.
///
/// For example:
///
Expand Down Expand Up @@ -866,7 +867,7 @@ pub(crate) fn check_directive<'a>(

const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@";

fn iter_header(
fn iter_directives(
mode: Mode,
_suite: &str,
poisoned: &mut bool,
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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 })
}
Expand Down Expand Up @@ -1163,8 +1163,7 @@ enum NormalizeKind {
Stderr64bit,
}

/// Parses the regex and replacement values of a `//@ normalize-*` header,
/// in the format:
/// Parses the regex and replacement values of a `//@ normalize-*` directive, in the format:
/// ```text
/// "REGEX" -> "REPLACEMENT"
/// ```
Expand Down Expand Up @@ -1373,7 +1372,7 @@ where

pub(crate) fn make_test_description<R: Read>(
config: &Config,
cache: &HeadersCache,
cache: &DirectivesCache,
name: String,
path: &Utf8Path,
src: R,
Expand All @@ -1387,7 +1386,7 @@ pub(crate) fn make_test_description<R: Read>(
let mut local_poisoned = false;

// Scan through the test file to handle `ignore-*`, `only-*`, and `needs-*` directives.
iter_header(
iter_directives(
config.mode,
&config.suite,
&mut local_poisoned,
Expand All @@ -1406,7 +1405,7 @@ pub(crate) fn make_test_description<R: Read>(
ignore_message = Some(reason.into());
}
IgnoreDecision::Error { message } => {
eprintln!("error: {}:{line_number}: {message}", path);
error!("{path}:{line_number}: {message}");
*poisoned = true;
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

use std::iter;

use super::directives::{AUX_BIN, AUX_BUILD, AUX_CODEGEN_BACKEND, AUX_CRATE, PROC_MACRO};
use crate::common::Config;
use crate::header::directives::{AUX_BIN, AUX_BUILD, AUX_CODEGEN_BACKEND, AUX_CRATE, PROC_MACRO};

/// Properties parsed from `aux-*` test directives.
#[derive(Clone, Debug, Default)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::collections::HashSet;

use crate::common::{CompareMode, Config, Debugger};
use crate::header::IgnoreDecision;
use crate::directives::IgnoreDecision;

const EXTRA_ARCHS: &[&str] = &["spirv"];

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::common::{Config, KNOWN_CRATE_TYPES, KNOWN_TARGET_HAS_ATOMIC_WIDTHS, Sanitizer};
use crate::header::{IgnoreDecision, llvm_has_libzstd};
use crate::directives::{IgnoreDecision, llvm_has_libzstd};

pub(super) fn handle_needs(
cache: &CachedNeedsConditions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use camino::Utf8Path;
use semver::Version;

use super::{
EarlyProps, HeadersCache, extract_llvm_version, extract_version_range, iter_header,
DirectivesCache, EarlyProps, extract_llvm_version, extract_version_range, iter_directives,
parse_normalize_rule,
};
use crate::common::{Config, Debugger, Mode};
Expand All @@ -17,9 +17,9 @@ fn make_test_description<R: Read>(
src: R,
revision: Option<&str>,
) -> CollectedTestDesc {
let cache = HeadersCache::load(config);
let cache = DirectivesCache::load(config);
let mut poisoned = false;
let test = crate::header::make_test_description(
let test = crate::directives::make_test_description(
config,
&cache,
name,
Expand Down Expand Up @@ -785,7 +785,7 @@ fn threads_support() {

fn run_path(poisoned: &mut bool, path: &Utf8Path, buf: &[u8]) {
let rdr = std::io::Cursor::new(&buf);
iter_header(Mode::Ui, "ui", poisoned, path, rdr, &mut |_| {});
iter_directives(Mode::Ui, "ui", poisoned, path, rdr, &mut |_| {});
}

#[test]
Expand Down
Loading
Loading