Skip to content

Commit 610bfad

Browse files
Rollup merge of rust-lang#155630 - Zalathar:skip-filecheck, r=jieyouxu
Make `//@ skip-filecheck` a normal compiletest directive The `skip-filecheck` directive is currently used by mir-opt tests, to suppress the default behaviour of running LLVM's `FileCheck` tool to check MIR output against FileCheck rules in the test file. The `skip-filecheck` directive was not included in the big migration to `//@` directive syntax (rust-lang#121370), perhaps because it was parsed and processed in the *miropt-test-tools* helper crate, not in compiletest itself. Recently I noticed that a small number of *codegen-llvm* tests were using the `//@ build-pass` directive, which has the non-obvious effect of skipping FileCheck in codegen tests. That's quite confusing, so I decided to have the mir-opt tests migrate over to a proper `//@ skip-filecheck` directive, which could then be used by codegen tests as well. (I also added skip-filecheck support to assembly tests, which are very similar to codegen tests, though there are currently no assembly tests that actually use `//@ skip-filecheck`.) --- Support for using `//@ build-pass` in codegen tests to skip FileCheck was introduced in rust-lang#113603. With hindsight, I think doing things that way was pretty clearly a mistake, and we'll be better off with `//@ skip-filecheck`. r? jieyouxu
2 parents d289cc7 + 1e8cd1f commit 610bfad

148 files changed

Lines changed: 180 additions & 162 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/doc/rustc-dev-guide/src/tests/directives.md

Lines changed: 2 additions & 0 deletions

src/tools/compiletest/src/directives.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,9 @@ pub(crate) struct TestProps {
195195
/// Extra flags to pass to `llvm-cov` when producing coverage reports.
196196
/// Only used by the "coverage-run" test mode.
197197
pub(crate) llvm_cov_flags: Vec<String>,
198+
/// Don't run LLVM's `filecheck` tool to check compiler output,
199+
/// in tests that would normally run it.
200+
pub(crate) skip_filecheck: bool,
198201
/// Extra flags to pass to LLVM's `filecheck` tool, in tests that use it.
199202
pub(crate) filecheck_flags: Vec<String>,
200203
/// Don't automatically insert any `--check-cfg` args
@@ -308,6 +311,7 @@ impl TestProps {
308311
mir_unit_test: None,
309312
remap_src_base: false,
310313
llvm_cov_flags: vec![],
314+
skip_filecheck: false,
311315
filecheck_flags: vec![],
312316
no_auto_check_cfg: false,
313317
add_minicore: false,
@@ -438,7 +442,6 @@ impl TestProps {
438442
let check_no_run = |s| match (config.mode, s) {
439443
(TestMode::Ui, _) => (),
440444
(TestMode::Crashes, _) => (),
441-
(TestMode::Codegen, "build-pass") => (),
442445
(mode, _) => panic!("`{s}` directive is not supported in `{mode}` tests"),
443446
};
444447
let pass_mode = if config.parse_name_directive(ln, "check-pass") {

src/tools/compiletest/src/directives/directive_names.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ pub(crate) const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
286286
"rustc-env",
287287
"rustfix-only-machine-applicable",
288288
"should-fail",
289+
"skip-filecheck",
289290
"stderr-per-bitwidth",
290291
"test-mir-pass",
291292
"unique-doc-out-dir",

src/tools/compiletest/src/directives/handlers.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::collections::HashMap;
22
use std::sync::{Arc, LazyLock};
33

4-
use crate::common::Config;
4+
use crate::common::{Config, TestMode};
55
use crate::directives::{
66
DirectiveLine, NormalizeKind, NormalizeRule, TestProps, parse_and_update_aux,
77
parse_edition_range, split_flags,
@@ -312,6 +312,18 @@ fn make_directive_handlers_map() -> HashMap<&'static str, Handler> {
312312
props.llvm_cov_flags.extend(split_flags(&flags));
313313
}
314314
}),
315+
handler("skip-filecheck", |config, ln, props| {
316+
let directive_name = ln.name;
317+
// FIXME(Zalathar): Someday we should add unified support for declaring
318+
// and checking which modes are supported by each directive.
319+
if !matches!(config.mode, TestMode::Assembly | TestMode::Codegen | TestMode::MirOpt) {
320+
panic!(
321+
"directive `//@ {directive_name}` is not supported by this test suite (mode: {mode:?})",
322+
mode = config.mode
323+
);
324+
}
325+
config.set_name_directive(ln, directive_name, &mut props.skip_filecheck);
326+
}),
315327
handler(FILECHECK_FLAGS, |config, ln, props| {
316328
if let Some(flags) = config.parse_name_value_directive(ln, FILECHECK_FLAGS) {
317329
props.filecheck_flags.extend(split_flags(&flags));

src/tools/compiletest/src/runtest/assembly.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ impl TestCx<'_> {
1313
self.fatal_proc_rec("compilation failed!", &proc_res);
1414
}
1515

16-
let proc_res = self.verify_with_filecheck(&output_path);
17-
if !proc_res.status.success() {
18-
self.fatal_proc_rec("verification with 'FileCheck' failed", &proc_res);
16+
if !self.props.skip_filecheck {
17+
let proc_res = self.verify_with_filecheck(&output_path);
18+
if !proc_res.status.success() {
19+
self.fatal_proc_rec("verification with 'FileCheck' failed", &proc_res);
20+
}
1921
}
2022
}
2123

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::{PassMode, TestCx};
1+
use super::TestCx;
22

33
impl TestCx<'_> {
44
pub(super) fn run_codegen_test(&self) {
@@ -11,12 +11,11 @@ impl TestCx<'_> {
1111
self.fatal_proc_rec("compilation failed!", &proc_res);
1212
}
1313

14-
if let Some(PassMode::Build) = self.pass_mode() {
15-
return;
16-
}
17-
let proc_res = self.verify_with_filecheck(&output_path);
18-
if !proc_res.status.success() {
19-
self.fatal_proc_rec("verification with 'FileCheck' failed", &proc_res);
14+
if !self.props.skip_filecheck {
15+
let proc_res = self.verify_with_filecheck(&output_path);
16+
if !proc_res.status.success() {
17+
self.fatal_proc_rec("verification with 'FileCheck' failed", &proc_res);
18+
}
2019
}
2120
}
2221
}

src/tools/compiletest/src/runtest/mir_opt.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ impl TestCx<'_> {
4040
let test_dir = self.testpaths.file.parent().unwrap();
4141
let test_crate = self.testpaths.file.file_stem().unwrap().replace('-', "_");
4242

43-
let MiroptTest { run_filecheck, suffix, files, passes: _ } = test_info;
43+
let MiroptTest { suffix, files, passes: _ } = test_info;
4444

4545
if self.config.bless {
4646
for e in glob(&format!("{}/{}.*{}.mir", test_dir, test_crate, suffix)).unwrap() {
@@ -89,7 +89,7 @@ impl TestCx<'_> {
8989
}
9090
}
9191

92-
if run_filecheck {
92+
if !self.props.skip_filecheck {
9393
let output_path = self.output_base_name().with_extension("mir");
9494
let proc_res = self.verify_with_filecheck(&output_path);
9595
if !proc_res.status.success() {

src/tools/miropt-test-tools/src/lib.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ pub struct MiroptTestFile {
88
}
99

1010
pub struct MiroptTest {
11-
pub run_filecheck: bool,
1211
pub suffix: String,
1312
pub files: Vec<MiroptTestFile>,
1413
/// Vec of passes under test to be dumped
@@ -57,13 +56,15 @@ pub fn files_for_miropt_test(
5756
let test_crate = testfile.file_stem().unwrap().to_str().unwrap().replace('-', "_");
5857

5958
let suffix = output_file_suffix(testfile, bit_width, panic_strategy);
60-
let mut run_filecheck = true;
6159
let mut passes = Vec::new();
6260

6361
for l in test_file_contents.lines() {
62+
// FIXME(Zalathar): Remove this `skip-filecheck` migration error in 2027,
63+
// or perhaps earlier if it seems no longer useful.
6464
if l.starts_with("// skip-filecheck") {
65-
run_filecheck = false;
66-
continue;
65+
panic!(
66+
"error: `// skip-filecheck` is no longer supported, use `//@ skip-filecheck` instead."
67+
);
6768
}
6869
if l.starts_with("// EMIT_MIR ") {
6970
let test_name = l.trim_start_matches("// EMIT_MIR ").trim();
@@ -128,10 +129,7 @@ pub fn files_for_miropt_test(
128129

129130
out.push(MiroptTestFile { expected_file, from_file, to_file });
130131
}
131-
if !run_filecheck && l.trim_start().starts_with("// CHECK") {
132-
panic!("error: test contains filecheck directive but is marked `skip-filecheck`");
133-
}
134132
}
135133

136-
MiroptTest { run_filecheck, suffix, files: out, passes }
134+
MiroptTest { suffix, files: out, passes }
137135
}

tests/codegen-llvm/scalable-vectors/tuple-intrinsics.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
//@ build-pass
1+
// FIXME: The FileCheck directives in this test are unchecked and probably broken.
2+
//@ skip-filecheck
23
//@ only-aarch64
34
#![crate_type = "lib"]
45
#![allow(incomplete_features, internal_features)]

tests/codegen-llvm/simd/simd-wide-sum.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//@ edition: 2021
44
//@ only-x86_64
55
//@ [mir-opt3]compile-flags: -Zmir-opt-level=3
6-
//@ [mir-opt3]build-pass
6+
//@ [mir-opt3] skip-filecheck
77

88
// mir-opt3 is a regression test for https://github.com/rust-lang/rust/issues/98016
99

0 commit comments

Comments
 (0)