Skip to content

Detect unused files in src/test/mir-opt and error on them in tidy. #103781

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

Merged
merged 1 commit into from
Nov 2, 2022
Merged
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: 9 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
@@ -816,6 +816,7 @@ dependencies = [
"lazycell",
"libc",
"miow",
"miropt-test-tools",
"regex",
"rustfix",
"serde",
@@ -2268,6 +2269,13 @@ dependencies = [
"ui_test",
]

[[package]]
name = "miropt-test-tools"
version = "0.1.0"
dependencies = [
"regex",
]

[[package]]
name = "new_debug_unreachable"
version = "1.0.4"
@@ -4920,6 +4928,7 @@ version = "0.1.0"
dependencies = [
"cargo_metadata 0.14.0",
"lazy_static",
"miropt-test-tools",
"regex",
"walkdir",
]
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ members = [
"src/tools/error_index_generator",
"src/tools/linkchecker",
"src/tools/lint-docs",
"src/tools/miropt-test-tools",
"src/tools/rustbook",
"src/tools/unstable-book-gen",
"src/tools/tidy",
1 change: 1 addition & 0 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
@@ -622,6 +622,7 @@ impl<'a> Builder<'a> {
check::Clippy,
check::Miri,
check::CargoMiri,
check::MiroptTestTools,
check::Rls,
check::RustAnalyzer,
check::Rustfmt,
1 change: 1 addition & 0 deletions src/bootstrap/check.rs
Original file line number Diff line number Diff line change
@@ -460,6 +460,7 @@ tool_check_step!(Miri, "src/tools/miri", SourceType::InTree);
tool_check_step!(CargoMiri, "src/tools/miri/cargo-miri", SourceType::InTree);
tool_check_step!(Rls, "src/tools/rls", SourceType::InTree);
tool_check_step!(Rustfmt, "src/tools/rustfmt", SourceType::InTree);
tool_check_step!(MiroptTestTools, "src/tools/miropt-test-tools", SourceType::InTree);

tool_check_step!(Bootstrap, "src/bootstrap", SourceType::InTree, false);

72 changes: 0 additions & 72 deletions src/test/mir-opt/rustc.try_identity.DestinationPropagation.diff

This file was deleted.

106 changes: 0 additions & 106 deletions src/test/mir-opt/simplify_try.try_identity.DestinationPropagation.diff

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

1 change: 1 addition & 0 deletions src/tools/compiletest/Cargo.toml
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ colored = "2"
diff = "0.1.10"
unified-diff = "0.2.1"
getopts = "0.2"
miropt-test-tools = { path = "../miropt-test-tools" }
tracing = "0.1"
tracing-subscriber = { version = "0.3.3", default-features = false, features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] }
regex = "1.0"
130 changes: 38 additions & 92 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
@@ -3399,103 +3399,49 @@ impl<'test> TestCx<'test> {
}
}

for l in test_file_contents.lines() {
if l.starts_with("// EMIT_MIR ") {
let test_name = l.trim_start_matches("// EMIT_MIR ").trim();
let mut test_names = test_name.split(' ');
// sometimes we specify two files so that we get a diff between the two files
let test_name = test_names.next().unwrap();
let mut expected_file;
let from_file;
let to_file;

if test_name.ends_with(".diff") {
let trimmed = test_name.trim_end_matches(".diff");
let test_against = format!("{}.after.mir", trimmed);
from_file = format!("{}.before.mir", trimmed);
expected_file = format!("{}{}.diff", trimmed, bit_width);
assert!(
test_names.next().is_none(),
"two mir pass names specified for MIR diff"
);
to_file = Some(test_against);
} else if let Some(first_pass) = test_names.next() {
let second_pass = test_names.next().unwrap();
assert!(
test_names.next().is_none(),
"three mir pass names specified for MIR diff"
);
expected_file =
format!("{}{}.{}-{}.diff", test_name, bit_width, first_pass, second_pass);
let second_file = format!("{}.{}.mir", test_name, second_pass);
from_file = format!("{}.{}.mir", test_name, first_pass);
to_file = Some(second_file);
} else {
let ext_re = Regex::new(r#"(\.(mir|dot|html))$"#).unwrap();
let cap = ext_re
.captures_iter(test_name)
.next()
.expect("test_name has an invalid extension");
let extension = cap.get(1).unwrap().as_str();
expected_file = format!(
"{}{}{}",
test_name.trim_end_matches(extension),
bit_width,
extension,
);
from_file = test_name.to_string();
assert!(
test_names.next().is_none(),
"two mir pass names specified for MIR dump"
let files = miropt_test_tools::files_for_miropt_test(
&self.testpaths.file,
self.config.get_pointer_width(),
);

for miropt_test_tools::MiroptTestFiles { from_file, to_file, expected_file } in files {
let dumped_string = if let Some(after) = to_file {
self.diff_mir_files(from_file.into(), after.into())
} else {
let mut output_file = PathBuf::new();
output_file.push(self.get_mir_dump_dir());
output_file.push(&from_file);
debug!(
"comparing the contents of: {} with {}",
output_file.display(),
expected_file.display()
);
if !output_file.exists() {
panic!(
"Output file `{}` from test does not exist, available files are in `{}`",
output_file.display(),
output_file.parent().unwrap().display()
);
to_file = None;
};
if !expected_file.starts_with(&test_crate) {
expected_file = format!("{}.{}", test_crate, expected_file);
}
let expected_file = test_dir.join(expected_file);
self.check_mir_test_timestamp(&from_file, &output_file);
let dumped_string = fs::read_to_string(&output_file).unwrap();
self.normalize_output(&dumped_string, &[])
};

let dumped_string = if let Some(after) = to_file {
self.diff_mir_files(from_file.into(), after.into())
} else {
let mut output_file = PathBuf::new();
output_file.push(self.get_mir_dump_dir());
output_file.push(&from_file);
debug!(
"comparing the contents of: {} with {}",
output_file.display(),
if self.config.bless {
let _ = std::fs::remove_file(&expected_file);
std::fs::write(expected_file, dumped_string.as_bytes()).unwrap();
} else {
if !expected_file.exists() {
panic!("Output file `{}` from test does not exist", expected_file.display());
}
let expected_string = fs::read_to_string(&expected_file).unwrap();
if dumped_string != expected_string {
print!("{}", write_diff(&expected_string, &dumped_string, 3));
panic!(
"Actual MIR output differs from expected MIR output {}",
expected_file.display()
);
if !output_file.exists() {
panic!(
"Output file `{}` from test does not exist, available files are in `{}`",
output_file.display(),
output_file.parent().unwrap().display()
);
}
self.check_mir_test_timestamp(&from_file, &output_file);
let dumped_string = fs::read_to_string(&output_file).unwrap();
self.normalize_output(&dumped_string, &[])
};

if self.config.bless {
let _ = std::fs::remove_file(&expected_file);
std::fs::write(expected_file, dumped_string.as_bytes()).unwrap();
} else {
if !expected_file.exists() {
panic!(
"Output file `{}` from test does not exist",
expected_file.display()
);
}
let expected_string = fs::read_to_string(&expected_file).unwrap();
if dumped_string != expected_string {
print!("{}", write_diff(&expected_string, &dumped_string, 3));
panic!(
"Actual MIR output differs from expected MIR output {}",
expected_file.display()
);
}
}
}
}
7 changes: 7 additions & 0 deletions src/tools/miropt-test-tools/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "miropt-test-tools"
version = "0.1.0"
edition = "2021"

[dependencies]
regex = "1.0"
70 changes: 70 additions & 0 deletions src/tools/miropt-test-tools/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
use std::fs;

pub struct MiroptTestFiles {
pub expected_file: std::path::PathBuf,
pub from_file: String,
pub to_file: Option<String>,
}

pub fn files_for_miropt_test(testfile: &std::path::Path, bit_width: u32) -> Vec<MiroptTestFiles> {
let mut out = Vec::new();
let test_file_contents = fs::read_to_string(&testfile).unwrap();

let test_dir = testfile.parent().unwrap();
let test_crate = testfile.file_stem().unwrap().to_str().unwrap().replace("-", "_");

let bit_width = if test_file_contents.lines().any(|l| l == "// EMIT_MIR_FOR_EACH_BIT_WIDTH") {
format!(".{}bit", bit_width)
} else {
String::new()
};

for l in test_file_contents.lines() {
if l.starts_with("// EMIT_MIR ") {
let test_name = l.trim_start_matches("// EMIT_MIR ").trim();
let mut test_names = test_name.split(' ');
// sometimes we specify two files so that we get a diff between the two files
let test_name = test_names.next().unwrap();
let mut expected_file;
let from_file;
let to_file;

if test_name.ends_with(".diff") {
let trimmed = test_name.trim_end_matches(".diff");
let test_against = format!("{}.after.mir", trimmed);
from_file = format!("{}.before.mir", trimmed);
expected_file = format!("{}{}.diff", trimmed, bit_width);
assert!(test_names.next().is_none(), "two mir pass names specified for MIR diff");
to_file = Some(test_against);
} else if let Some(first_pass) = test_names.next() {
let second_pass = test_names.next().unwrap();
assert!(test_names.next().is_none(), "three mir pass names specified for MIR diff");
expected_file =
format!("{}{}.{}-{}.diff", test_name, bit_width, first_pass, second_pass);
let second_file = format!("{}.{}.mir", test_name, second_pass);
from_file = format!("{}.{}.mir", test_name, first_pass);
to_file = Some(second_file);
} else {
let ext_re = regex::Regex::new(r#"(\.(mir|dot|html))$"#).unwrap();
let cap = ext_re
.captures_iter(test_name)
.next()
.expect("test_name has an invalid extension");
let extension = cap.get(1).unwrap().as_str();
expected_file =
format!("{}{}{}", test_name.trim_end_matches(extension), bit_width, extension,);
from_file = test_name.to_string();
assert!(test_names.next().is_none(), "two mir pass names specified for MIR dump");
to_file = None;
};
if !expected_file.starts_with(&test_crate) {
expected_file = format!("{}.{}", test_crate, expected_file);
}
let expected_file = test_dir.join(expected_file);

out.push(MiroptTestFiles { expected_file, from_file, to_file });
}
}

out
}
1 change: 1 addition & 0 deletions src/tools/tidy/Cargo.toml
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ autobins = false
[dependencies]
cargo_metadata = "0.14"
regex = "1"
miropt-test-tools = { path = "../miropt-test-tools" }
lazy_static = "1"
walkdir = "2"

1 change: 1 addition & 0 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
@@ -47,6 +47,7 @@ pub mod error_codes_check;
pub mod errors;
pub mod extdeps;
pub mod features;
pub mod mir_opt_tests;
pub mod pal;
pub mod primitive_docs;
pub mod style;
1 change: 1 addition & 0 deletions src/tools/tidy/src/main.rs
Original file line number Diff line number Diff line change
@@ -64,6 +64,7 @@ fn main() {
// Checks over tests.
check!(debug_artifacts, &src_path);
check!(ui_tests, &src_path);
check!(mir_opt_tests, &src_path);

// Checks that only make sense for the compiler.
check!(errors, &compiler_path);
37 changes: 37 additions & 0 deletions src/tools/tidy/src/mir_opt_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//! Tidy check to ensure that mir opt directories do not have stale files.
use std::collections::HashSet;
use std::path::{Path, PathBuf};

pub fn check(path: &Path, bad: &mut bool) {
let mut rs_files = Vec::<PathBuf>::new();
let mut output_files = HashSet::<PathBuf>::new();
let files = walkdir::WalkDir::new(&path.join("test/mir-opt")).into_iter();

for file in files.filter_map(Result::ok).filter(|e| e.file_type().is_file()) {
let filepath = file.path();
if filepath.extension() == Some("rs".as_ref()) {
rs_files.push(filepath.to_owned());
} else {
output_files.insert(filepath.to_owned());
}
}

for file in rs_files {
for bw in [32, 64] {
for output_file in miropt_test_tools::files_for_miropt_test(&file, bw) {
output_files.remove(&output_file.expected_file);
}
}
}

for extra in output_files {
if extra.file_name() != Some("README.md".as_ref()) {
tidy_error!(
bad,
"the following output file is not associated with any mir-opt test, you can remove it: {}",
extra.display()
);
}
}
}