From 488ffaa7260901a13ac3e51a6ba06f26b976e9cd Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 18 Feb 2024 00:39:12 +1100 Subject: [PATCH 1/2] Wrap `iter_header` callback arguments in a documentable struct --- src/tools/compiletest/src/header.rs | 36 +++++++++++++++++++---------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 117828645a94f..7395e5918884c 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -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() }); @@ -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; } @@ -678,7 +678,7 @@ fn iter_header( poisoned: &mut bool, testfile: &Path, rdr: R, - it: &mut dyn FnMut(Option<&str>, &str, &str, usize), + it: &mut dyn FnMut(HeaderLine<'_>), ) { iter_header_extra(mode, suite, poisoned, testfile, rdr, &[], it) } @@ -801,6 +801,18 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "unset-rustc-env", ]; +/// 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_extra( mode: Mode, suite: &str, @@ -808,7 +820,7 @@ fn iter_header_extra( 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; @@ -817,7 +829,7 @@ fn iter_header_extra( // 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); + it(HeaderLine { header_revision: None, original_line: "", directive, line_number: 0 }); } let comment = if testfile.extension().is_some_and(|e| e == "rs") { @@ -843,14 +855,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; @@ -1179,8 +1191,8 @@ pub fn make_test_description( 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; } @@ -1204,7 +1216,7 @@ pub fn make_test_description( }; } - 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 From c521d7fa2e69908c1be4585119e8a90140c87297 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 17 Feb 2024 22:57:26 +1100 Subject: [PATCH 2/2] 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`. --- src/tools/compiletest/src/header.rs | 60 ++++++++++------------------- 1 file changed, 21 insertions(+), 39 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 7395e5918884c..7f765fd86c87f 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -672,17 +672,6 @@ pub fn line_directive<'line>( } } -fn iter_header( - mode: Mode, - suite: &str, - poisoned: &mut bool, - testfile: &Path, - rdr: R, - it: &mut dyn FnMut(HeaderLine<'_>), -) { - 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. @@ -813,23 +802,37 @@ struct HeaderLine<'ln> { line_number: usize, } -fn iter_header_extra( +fn iter_header( mode: Mode, suite: &str, poisoned: &mut bool, testfile: &Path, rdr: impl Read, - extra_directives: &[&str], 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(HeaderLine { header_revision: None, original_line: "", directive, line_number: 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") { @@ -1162,35 +1165,14 @@ pub fn make_test_description( 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 |HeaderLine { header_revision, original_line, directive: ln, line_number }| { if header_revision.is_some() && header_revision != cfg { return;