Skip to content

Commit

Permalink
Rollup merge of #121233 - Zalathar:extra-directives, r=oli-obk
Browse files Browse the repository at this point in the history
Move the extra directives for `Mode::CoverageRun` into `iter_header`

When these extra directives were ported over as part of #112300, it made sense to introduce `iter_header_extra` and pass them in as an extra argument.

But now that #120881 has added a `mode` parameter to `iter_header` for its own purposes, it's slightly simpler to move the coverage special-case code directly into `iter_header` as well. This lets us get rid of `iter_header_extra`.
  • Loading branch information
Noratrieb authored Feb 20, 2024
2 parents 328a5b7 + c521d7f commit 4131f6e
Showing 1 changed file with 43 additions and 49 deletions.
92 changes: 43 additions & 49 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl EarlyProps {
&mut poisoned,
testfile,
rdr,
&mut |_, _, ln, _| {
&mut |HeaderLine { directive: ln, .. }| {
config.push_name_value_directive(ln, directives::AUX_BUILD, &mut props.aux, |r| {
r.trim().to_string()
});
Expand Down Expand Up @@ -330,8 +330,8 @@ impl TestProps {
&mut poisoned,
testfile,
file,
&mut |revision, _, ln, _| {
if revision.is_some() && revision != cfg {
&mut |HeaderLine { header_revision, directive: ln, .. }| {
if header_revision.is_some() && header_revision != cfg {
return;
}

Expand Down Expand Up @@ -672,17 +672,6 @@ pub fn line_directive<'line>(
}
}

fn iter_header<R: Read>(
mode: Mode,
suite: &str,
poisoned: &mut bool,
testfile: &Path,
rdr: R,
it: &mut dyn FnMut(Option<&str>, &str, &str, usize),
) {
iter_header_extra(mode, suite, poisoned, testfile, rdr, &[], it)
}

/// This is 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.
Expand Down Expand Up @@ -801,23 +790,49 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[
"unset-rustc-env",
];

fn iter_header_extra(
/// Arguments passed to the callback in [`iter_header`].
struct HeaderLine<'ln> {
/// Contents of the square brackets preceding this header, if present.
header_revision: Option<&'ln str>,
/// Raw line from the test file, including comment prefix and any revision.
original_line: &'ln str,
/// Remainder of the directive line, after the initial comment prefix
/// (`//` or `//@` or `#`) and revision (if any) have been stripped.
directive: &'ln str,
line_number: usize,
}

fn iter_header(
mode: Mode,
suite: &str,
poisoned: &mut bool,
testfile: &Path,
rdr: impl Read,
extra_directives: &[&str],
it: &mut dyn FnMut(Option<&str>, &str, &str, usize),
it: &mut dyn FnMut(HeaderLine<'_>),
) {
if testfile.is_dir() {
return;
}

// Process any extra directives supplied by the caller (e.g. because they
// are implied by the test mode), with a dummy line number of 0.
for directive in extra_directives {
it(None, directive, directive, 0);
// Coverage tests in coverage-run mode always have these extra directives,
// without needing to specify them manually in every test file.
// (Some of the comments below have been copied over from the old
// `tests/run-make/coverage-reports/Makefile`, which no longer exists.)
if mode == Mode::CoverageRun {
let extra_directives: &[&str] = &[
"needs-profiler-support",
// FIXME(mati865): MinGW GCC miscompiles compiler-rt profiling library but with Clang it works
// properly. Since we only have GCC on the CI ignore the test for now.
"ignore-windows-gnu",
// FIXME(pietroalbini): this test currently does not work on cross-compiled
// targets because remote-test is not capable of sending back the *.profraw
// files generated by the LLVM instrumentation.
"ignore-cross-compile",
];
// Process the extra implied directives, with a dummy line number of 0.
for directive in extra_directives {
it(HeaderLine { header_revision: None, original_line: "", directive, line_number: 0 });
}
}

let comment = if testfile.extension().is_some_and(|e| e == "rs") {
Expand All @@ -843,14 +858,14 @@ fn iter_header_extra(
// Assume that any directives will be found before the first
// module or function. This doesn't seem to be an optimization
// with a warm page cache. Maybe with a cold one.
let orig_ln = &ln;
let original_line = &ln;
let ln = ln.trim();
if ln.starts_with("fn") || ln.starts_with("mod") {
return;

// First try to accept `ui_test` style comments
} else if let Some((lncfg, ln)) = line_directive(comment, ln) {
it(lncfg, orig_ln, ln, line_number);
} else if let Some((header_revision, directive)) = line_directive(comment, ln) {
it(HeaderLine { header_revision, original_line, directive, line_number });
} else if mode == Mode::Ui && suite == "ui" && !REVISION_MAGIC_COMMENT_RE.is_match(ln) {
let Some((_, rest)) = line_directive("//", ln) else {
continue;
Expand Down Expand Up @@ -1150,37 +1165,16 @@ pub fn make_test_description<R: Read>(
let mut ignore_message = None;
let mut should_fail = false;

let extra_directives: &[&str] = match config.mode {
// The coverage-run tests are treated as having these extra directives,
// without needing to specify them manually in every test file.
// (Some of the comments below have been copied over from
// `tests/run-make/coverage-reports/Makefile`, which no longer exists.)
Mode::CoverageRun => {
&[
"needs-profiler-support",
// FIXME(mati865): MinGW GCC miscompiles compiler-rt profiling library but with Clang it works
// properly. Since we only have GCC on the CI ignore the test for now.
"ignore-windows-gnu",
// FIXME(pietroalbini): this test currently does not work on cross-compiled
// targets because remote-test is not capable of sending back the *.profraw
// files generated by the LLVM instrumentation.
"ignore-cross-compile",
]
}
_ => &[],
};

let mut local_poisoned = false;

iter_header_extra(
iter_header(
config.mode,
&config.suite,
&mut local_poisoned,
path,
src,
extra_directives,
&mut |revision, og_ln, ln, line_number| {
if revision.is_some() && revision != cfg {
&mut |HeaderLine { header_revision, original_line, directive: ln, line_number }| {
if header_revision.is_some() && header_revision != cfg {
return;
}

Expand All @@ -1204,7 +1198,7 @@ pub fn make_test_description<R: Read>(
};
}

if let Some((_, post)) = og_ln.trim_start().split_once("//") {
if let Some((_, post)) = original_line.trim_start().split_once("//") {
let post = post.trim_start();
if post.starts_with("ignore-tidy")
&& config.mode == Mode::Ui
Expand Down

0 comments on commit 4131f6e

Please sign in to comment.