Skip to content
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

[1/2] clean-up / general improvements #126088

Merged
merged 10 commits into from
Jun 14, 2024
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ fn get_modified_rs_files(build: &Builder<'_>) -> Result<Option<Vec<String>>, Str
return Ok(None);
}

get_git_modified_files(&build.config.git_config(), Some(&build.config.src), &vec!["rs"])
get_git_modified_files(&build.config.git_config(), Some(&build.config.src), &["rs"])
}

#[derive(serde_derive::Deserialize)]
Expand Down
2 changes: 1 addition & 1 deletion src/tools/build-manifest/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ impl Builder {
Some(p) => p,
None => return false,
};
pkg.target.get(&c.target).is_some()
pkg.target.contains_key(&c.target)
};
extensions.retain(&has_component);
components.retain(&has_component);
Expand Down
26 changes: 11 additions & 15 deletions src/tools/build_helper/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ fn output_result(cmd: &mut Command) -> Result<String, String> {
String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))?
));
}
Ok(String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?)
String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))
}

/// Finds the remote for rust-lang/rust.
Expand Down Expand Up @@ -64,18 +64,14 @@ pub fn rev_exists(rev: &str, git_dir: Option<&Path>) -> Result<bool, String> {
match output.status.code() {
Some(0) => Ok(true),
Some(128) => Ok(false),
None => {
return Err(format!(
"git didn't exit properly: {}",
String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))?
));
}
Some(code) => {
return Err(format!(
"git command exited with status code: {code}: {}",
String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))?
));
}
None => Err(format!(
"git didn't exit properly: {}",
String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))?
)),
Some(code) => Err(format!(
"git command exited with status code: {code}: {}",
String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))?
)),
}
}

Expand All @@ -96,7 +92,7 @@ pub fn updated_master_branch(
}
}

Err(format!("Cannot find any suitable upstream master branch"))
Err("Cannot find any suitable upstream master branch".to_owned())
}

pub fn get_git_merge_base(
Expand All @@ -118,7 +114,7 @@ pub fn get_git_merge_base(
pub fn get_git_modified_files(
config: &GitConfig<'_>,
git_dir: Option<&Path>,
extensions: &Vec<&str>,
extensions: &[&str],
) -> Result<Option<Vec<String>>, String> {
let merge_base = get_git_merge_base(config, git_dir)?;

Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ impl TargetCfgs {
name,
Some(
value
.strip_suffix("\"")
.strip_suffix('\"')
.expect("key-value pair should be properly quoted"),
),
)
Expand Down
24 changes: 12 additions & 12 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl EarlyProps {
panic!("errors encountered during EarlyProps parsing");
}

return props;
props
}
}

Expand Down Expand Up @@ -382,7 +382,7 @@ impl TestProps {
// Individual flags can be single-quoted to preserve spaces; see
// <https:/rust-lang/rust/pull/115948/commits/957c5db6>.
flags
.split("'")
.split('\'')
.enumerate()
.flat_map(|(i, f)| {
if i % 2 == 1 { vec![f] } else { f.split_whitespace().collect() }
Expand Down Expand Up @@ -613,7 +613,7 @@ impl TestProps {

for key in &["RUST_TEST_NOCAPTURE", "RUST_TEST_THREADS"] {
if let Ok(val) = env::var(key) {
if self.exec_env.iter().find(|&&(ref x, _)| x == key).is_none() {
if !self.exec_env.iter().any(|&(ref x, _)| x == key) {
self.exec_env.push(((*key).to_owned(), val))
}
}
Expand Down Expand Up @@ -991,7 +991,7 @@ pub(crate) fn check_directive(directive_ln: &str) -> CheckDirectiveResult<'_> {
let trailing = post.trim().split_once(' ').map(|(pre, _)| pre).unwrap_or(post);
let trailing_directive = {
// 1. is the directive name followed by a space? (to exclude `:`)
matches!(directive_ln.get(directive_name.len()..), Some(s) if s.starts_with(" "))
matches!(directive_ln.get(directive_name.len()..), Some(s) if s.starts_with(' '))
// 2. is what is after that directive also a directive (ex: "only-x86 only-arm")
&& KNOWN_DIRECTIVE_NAMES.contains(&trailing)
}
Expand Down Expand Up @@ -1363,7 +1363,7 @@ pub fn extract_llvm_version_from_binary(binary_path: &str) -> Option<u32> {
}
let version = String::from_utf8(output.stdout).ok()?;
for line in version.lines() {
if let Some(version) = line.split("LLVM version ").skip(1).next() {
if let Some(version) = line.split("LLVM version ").nth(1) {
return extract_llvm_version(version);
}
}
Expand Down Expand Up @@ -1394,7 +1394,7 @@ where

let min = parse(min)?;
let max = match max {
Some(max) if max.is_empty() => return None,
Some("") => return None,
Some(max) => parse(max)?,
_ => min,
};
Expand Down Expand Up @@ -1466,12 +1466,12 @@ pub fn make_test_description<R: Read>(
decision!(ignore_gdb(config, ln));
decision!(ignore_lldb(config, ln));

if config.target == "wasm32-unknown-unknown" {
if config.parse_name_directive(ln, directives::CHECK_RUN_RESULTS) {
decision!(IgnoreDecision::Ignore {
reason: "ignored on WASM as the run results cannot be checked there".into(),
});
}
if config.target == "wasm32-unknown-unknown"
&& config.parse_name_directive(ln, directives::CHECK_RUN_RESULTS)
{
decision!(IgnoreDecision::Ignore {
reason: "ignored on WASM as the run results cannot be checked there".into(),
});
}

should_fail |= config.parse_name_directive(ln, "should-fail");
Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/header/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub(super) fn parse_cfg_name_directive<'a>(

// Some of the matchers might be "" depending on what the target information is. To avoid
// problems we outright reject empty directives.
if name == "" {
if name.is_empty() {
return ParsedNameDirective::not_a_directive();
}

Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1147,7 +1147,7 @@ fn extract_lldb_version(full_version_line: &str) -> Option<(u32, bool)> {
}

fn not_a_digit(c: char) -> bool {
!c.is_digit(10)
!c.is_ascii_digit()
}

fn check_overlapping_tests(found_paths: &HashSet<PathBuf>) {
Expand Down
5 changes: 2 additions & 3 deletions src/tools/compiletest/src/read2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ mod tests;

pub use self::imp::read2;
use std::io::{self, Write};
use std::mem::replace;
use std::process::{Child, Output};

#[derive(Copy, Clone, Debug)]
Expand Down Expand Up @@ -101,10 +100,10 @@ impl ProcOutput {
return;
}

let mut head = replace(bytes, Vec::new());
let mut head = std::mem::take(bytes);
// Don't truncate if this as a whole line.
// That should make it less likely that we cut a JSON line in half.
if head.last() != Some(&('\n' as u8)) {
if head.last() != Some(&b'\n') {
head.truncate(MAX_OUT_LEN);
}
let skipped = new_len - head.len();
Expand Down
6 changes: 3 additions & 3 deletions src/tools/compiletest/src/read2/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ fn test_abbreviate_filterss_are_detected() {
#[test]
fn test_abbreviate_filters_avoid_abbreviations() {
let mut out = ProcOutput::new();
let filters = &[std::iter::repeat('a').take(64).collect::<String>()];
let filters = &["a".repeat(64)];

let mut expected = vec![b'.'; MAX_OUT_LEN - FILTERED_PATHS_PLACEHOLDER_LEN as usize];
let mut expected = vec![b'.'; MAX_OUT_LEN - FILTERED_PATHS_PLACEHOLDER_LEN];
expected.extend_from_slice(filters[0].as_bytes());

out.extend(&expected, filters);
Expand All @@ -81,7 +81,7 @@ fn test_abbreviate_filters_avoid_abbreviations() {
#[test]
fn test_abbreviate_filters_can_still_cause_abbreviations() {
let mut out = ProcOutput::new();
let filters = &[std::iter::repeat('a').take(64).collect::<String>()];
let filters = &["a".repeat(64)];

let mut input = vec![b'.'; MAX_OUT_LEN];
input.extend_from_slice(filters[0].as_bytes());
Expand Down
54 changes: 25 additions & 29 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,11 @@ impl<'test> TestCx<'test> {

// if a test does not crash, consider it an error
if proc_res.status.success() || matches!(proc_res.status.code(), Some(1 | 0)) {
self.fatal(&format!(
self.fatal(
"test no longer crashes/triggers ICE! Please give it a mearningful name, \
add a doc-comment to the start of the test explaining why it exists and \
move it to tests/ui or wherever you see fit."
));
move it to tests/ui or wherever you see fit.",
);
}
}

Expand Down Expand Up @@ -697,10 +697,10 @@ impl<'test> TestCx<'test> {
// since it is extensively used in the testsuite.
check_cfg.push_str("cfg(FALSE");
for revision in &self.props.revisions {
check_cfg.push_str(",");
check_cfg.push_str(&normalize_revision(&revision));
check_cfg.push(',');
check_cfg.push_str(&normalize_revision(revision));
}
check_cfg.push_str(")");
check_cfg.push(')');

cmd.args(&["--check-cfg", &check_cfg]);
}
Expand Down Expand Up @@ -818,7 +818,7 @@ impl<'test> TestCx<'test> {
// Append the other `cdb-command:`s
for line in &dbg_cmds.commands {
script_str.push_str(line);
script_str.push_str("\n");
script_str.push('\n');
}

script_str.push_str("qq\n"); // Quit the debugger (including remote debugger, if any)
Expand Down Expand Up @@ -1200,7 +1200,7 @@ impl<'test> TestCx<'test> {
// Append the other commands
for line in &dbg_cmds.commands {
script_str.push_str(line);
script_str.push_str("\n");
script_str.push('\n');
}

// Finally, quit the debugger
Expand Down Expand Up @@ -1250,7 +1250,7 @@ impl<'test> TestCx<'test> {
// Remove options that are either unwanted (-O) or may lead to duplicates due to RUSTFLAGS.
let options_to_remove = ["-O".to_owned(), "-g".to_owned(), "--debuginfo".to_owned()];

options.iter().filter(|x| !options_to_remove.contains(x)).map(|x| x.clone()).collect()
options.iter().filter(|x| !options_to_remove.contains(x)).cloned().collect()
}

fn maybe_add_external_args(&self, cmd: &mut Command, args: &Vec<String>) {
Expand Down Expand Up @@ -2504,8 +2504,8 @@ impl<'test> TestCx<'test> {
// This works with both `--emit asm` (as default output name for the assembly)
// and `ptx-linker` because the latter can write output at requested location.
let output_path = self.output_base_name().with_extension(extension);
let output_file = TargetLocation::ThisFile(output_path.clone());
output_file

TargetLocation::ThisFile(output_path.clone())
}
}

Expand Down Expand Up @@ -2752,7 +2752,7 @@ impl<'test> TestCx<'test> {
for entry in walkdir::WalkDir::new(dir) {
let entry = entry.expect("failed to read file");
if entry.file_type().is_file()
&& entry.path().extension().and_then(|p| p.to_str()) == Some("html".into())
&& entry.path().extension().and_then(|p| p.to_str()) == Some("html")
{
let status =
Command::new("tidy").args(&tidy_args).arg(entry.path()).status().unwrap();
Expand Down Expand Up @@ -2783,8 +2783,7 @@ impl<'test> TestCx<'test> {
&compare_dir,
self.config.verbose,
|file_type, extension| {
file_type.is_file()
&& (extension == Some("html".into()) || extension == Some("js".into()))
file_type.is_file() && (extension == Some("html") || extension == Some("js"))
},
) {
return;
Expand Down Expand Up @@ -2830,11 +2829,11 @@ impl<'test> TestCx<'test> {
}
match String::from_utf8(line.clone()) {
Ok(line) => {
if line.starts_with("+") {
if line.starts_with('+') {
write!(&mut out, "{}", line.green()).unwrap();
} else if line.starts_with("-") {
} else if line.starts_with('-') {
write!(&mut out, "{}", line.red()).unwrap();
} else if line.starts_with("@") {
} else if line.starts_with('@') {
write!(&mut out, "{}", line.blue()).unwrap();
} else {
out.write_all(line.as_bytes()).unwrap();
Expand Down Expand Up @@ -2907,7 +2906,7 @@ impl<'test> TestCx<'test> {
&& line.ends_with(';')
{
if let Some(ref mut other_files) = other_files {
other_files.push(line.rsplit("mod ").next().unwrap().replace(";", ""));
other_files.push(line.rsplit("mod ").next().unwrap().replace(';', ""));
}
None
} else {
Expand Down Expand Up @@ -3139,7 +3138,7 @@ impl<'test> TestCx<'test> {
let mut string = String::new();
for cgu in cgus {
string.push_str(&cgu[..]);
string.push_str(" ");
string.push(' ');
}

string
Expand Down Expand Up @@ -3172,10 +3171,7 @@ impl<'test> TestCx<'test> {
// CGUs joined with "--". This function splits such composite CGU names
// and handles each component individually.
fn remove_crate_disambiguators_from_set_of_cgu_names(cgus: &str) -> String {
cgus.split("--")
.map(|cgu| remove_crate_disambiguator_from_cgu(cgu))
.collect::<Vec<_>>()
.join("--")
cgus.split("--").map(remove_crate_disambiguator_from_cgu).collect::<Vec<_>>().join("--")
}
}

Expand Down Expand Up @@ -3357,7 +3353,7 @@ impl<'test> TestCx<'test> {
// endif
}

if self.config.target.contains("msvc") && self.config.cc != "" {
if self.config.target.contains("msvc") && !self.config.cc.is_empty() {
// We need to pass a path to `lib.exe`, so assume that `cc` is `cl.exe`
// and that `lib.exe` lives next to it.
let lib = Path::new(&self.config.cc).parent().unwrap().join("lib.exe");
Expand Down Expand Up @@ -3639,7 +3635,7 @@ impl<'test> TestCx<'test> {
// endif
}

if self.config.target.contains("msvc") && self.config.cc != "" {
if self.config.target.contains("msvc") && !self.config.cc.is_empty() {
// We need to pass a path to `lib.exe`, so assume that `cc` is `cl.exe`
// and that `lib.exe` lives next to it.
let lib = Path::new(&self.config.cc).parent().unwrap().join("lib.exe");
Expand Down Expand Up @@ -3830,7 +3826,7 @@ impl<'test> TestCx<'test> {
&& !self.props.dont_check_compiler_stderr
{
self.fatal_proc_rec(
&format!("compiler output got truncated, cannot compare with reference file"),
"compiler output got truncated, cannot compare with reference file",
&proc_res,
);
}
Expand Down Expand Up @@ -4011,8 +4007,8 @@ impl<'test> TestCx<'test> {
crate_name.to_str().expect("crate name implies file name must be valid UTF-8");
// replace `a.foo` -> `a__foo` for crate name purposes.
// replace `revision-name-with-dashes` -> `revision_name_with_underscore`
let crate_name = crate_name.replace(".", "__");
let crate_name = crate_name.replace("-", "_");
let crate_name = crate_name.replace('.', "__");
let crate_name = crate_name.replace('-', "_");
rustc.arg("--crate-name");
rustc.arg(crate_name);
}
Expand Down Expand Up @@ -4060,7 +4056,7 @@ impl<'test> TestCx<'test> {
fn check_mir_dump(&self, test_info: MiroptTest) {
let test_dir = self.testpaths.file.parent().unwrap();
let test_crate =
self.testpaths.file.file_stem().unwrap().to_str().unwrap().replace("-", "_");
self.testpaths.file.file_stem().unwrap().to_str().unwrap().replace('-', "_");

let MiroptTest { run_filecheck, suffix, files, passes: _ } = test_info;

Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/runtest/debugger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,5 +148,5 @@ fn check_single_line(line: &str, check_line: &str) -> bool {
rest = &rest[pos + current_fragment.len()..];
}

if !can_end_anywhere && !rest.is_empty() { false } else { true }
can_end_anywhere || rest.is_empty()
}
Loading
Loading