Skip to content

Commit

Permalink
Unrolled build for rust-lang#126088
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#126088 - onur-ozkan:brooming, r=albertlarsan68

[1/2] clean-up / general improvements

This PR applies various clippy suggestions on the tools. I have only applied the ones that make sense and left out trivial changes (e.g., suggestions like 'remove &' are ignored to keep the original commit history for the lines).

I am planning to do the same for the library and compiler, but those will add too many changes to this PR, so I will handle them in a separate PR later.
  • Loading branch information
rust-timer authored Jun 14, 2024
2 parents 0ef0dd2 + c8d2b93 commit 0d82ff0
Show file tree
Hide file tree
Showing 26 changed files with 139 additions and 160 deletions.
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://github.com/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

0 comments on commit 0d82ff0

Please sign in to comment.