Skip to content

Commit f122a51

Browse files
GuillaumeGomezGuillaumeGomez
authored
Rollup merge of rust-lang#124069 - onur-ozkan:run-clippy-on-bootstrap, r=albertlarsan68
enable clippy for bootstrap on CI PRs (in `mingw-check` image) Let's keep the bootstrap codebase cleaner.
2 parents b9be3c4 + c5fdb39 commit f122a51

File tree

23 files changed

+166
-169
lines changed

23 files changed

+166
-169
lines changed

‎src/bootstrap/src/bin/main.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ fn main() {
3737

3838
build_lock = fd_lock::RwLock::new(t!(fs::OpenOptions::new()
3939
.write(true)
40+
.truncate(true)
4041
.create(true)
4142
.open(&lock_path)));
4243
_build_lock_guard = match build_lock.try_write() {
@@ -143,8 +144,8 @@ fn check_version(config: &Config) -> Option<String> {
143144
// then use the one from the config.toml. This way we never show the same warnings
144145
// more than once.
145146
if let Ok(t) = fs::read_to_string(&warned_id_path) {
146-
let last_warned_id =
147-
usize::from_str(&t).expect(&format!("{} is corrupted.", warned_id_path.display()));
147+
let last_warned_id = usize::from_str(&t)
148+
.unwrap_or_else(|_| panic!("{} is corrupted.", warned_id_path.display()));
148149

149150
// We only use the last_warned_id if it exists in `CONFIG_CHANGE_HISTORY`.
150151
// Otherwise, we may retrieve all the changes if it's not the highest value.

‎src/bootstrap/src/core/build_steps/clean.rs

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -181,39 +181,33 @@ fn rm_rf(path: &Path) {
181181
}
182182
Ok(metadata) => {
183183
if metadata.file_type().is_file() || metadata.file_type().is_symlink() {
184-
do_op(path, "remove file", |p| {
185-
fs::remove_file(p).or_else(|e| {
186-
// Work around the fact that we cannot
187-
// delete an executable while it runs on Windows.
188-
#[cfg(windows)]
184+
do_op(path, "remove file", |p| match fs::remove_file(p) {
185+
#[cfg(windows)]
186+
Err(e)
189187
if e.kind() == std::io::ErrorKind::PermissionDenied
190188
&& p.file_name().and_then(std::ffi::OsStr::to_str)
191-
== Some("bootstrap.exe")
192-
{
193-
eprintln!("WARNING: failed to delete '{}'.", p.display());
194-
return Ok(());
195-
}
196-
Err(e)
197-
})
189+
== Some("bootstrap.exe") =>
190+
{
191+
eprintln!("WARNING: failed to delete '{}'.", p.display());
192+
Ok(())
193+
}
194+
r => r,
198195
});
196+
199197
return;
200198
}
201199

202200
for file in t!(fs::read_dir(path)) {
203201
rm_rf(&t!(file).path());
204202
}
205-
do_op(path, "remove dir", |p| {
206-
fs::remove_dir(p).or_else(|e| {
207-
// Check for dir not empty on Windows
208-
// FIXME: Once `ErrorKind::DirectoryNotEmpty` is stabilized,
209-
// match on `e.kind()` instead.
210-
#[cfg(windows)]
211-
if e.raw_os_error() == Some(145) {
212-
return Ok(());
213-
}
214203

215-
Err(e)
216-
})
204+
do_op(path, "remove dir", |p| match fs::remove_dir(p) {
205+
// Check for dir not empty on Windows
206+
// FIXME: Once `ErrorKind::DirectoryNotEmpty` is stabilized,
207+
// match on `e.kind()` instead.
208+
#[cfg(windows)]
209+
Err(e) if e.raw_os_error() == Some(145) => Ok(()),
210+
r => r,
217211
});
218212
}
219213
};
@@ -228,14 +222,14 @@ where
228222
// On windows we can't remove a readonly file, and git will often clone files as readonly.
229223
// As a result, we have some special logic to remove readonly files on windows.
230224
// This is also the reason that we can't use things like fs::remove_dir_all().
231-
Err(ref e) if cfg!(windows) && e.kind() == ErrorKind::PermissionDenied => {
225+
#[cfg(windows)]
226+
Err(ref e) if e.kind() == ErrorKind::PermissionDenied => {
232227
let m = t!(path.symlink_metadata());
233228
let mut p = m.permissions();
234229
p.set_readonly(false);
235230
t!(fs::set_permissions(path, p));
236231
f(path).unwrap_or_else(|e| {
237232
// Delete symlinked directories on Windows
238-
#[cfg(windows)]
239233
if m.file_type().is_symlink() && path.is_dir() && fs::remove_dir(path).is_ok() {
240234
return;
241235
}

‎src/bootstrap/src/core/build_steps/compile.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,7 +1139,7 @@ pub fn rustc_cargo_env(
11391139
// busting caches (e.g. like #71152).
11401140
if builder.config.llvm_enabled(target) {
11411141
let building_is_expensive =
1142-
crate::core::build_steps::llvm::prebuilt_llvm_config(builder, target).is_err();
1142+
crate::core::build_steps::llvm::prebuilt_llvm_config(builder, target).should_build();
11431143
// `top_stage == stage` might be false for `check --stage 1`, if we are building the stage 1 compiler
11441144
let can_skip_build = builder.kind == Kind::Check && builder.top_stage == stage;
11451145
let should_skip_build = building_is_expensive && can_skip_build;
@@ -1282,11 +1282,7 @@ fn is_codegen_cfg_needed(path: &TaskPath, run: &RunConfig<'_>) -> bool {
12821282
if path.path.to_str().unwrap().contains(CODEGEN_BACKEND_PREFIX) {
12831283
let mut needs_codegen_backend_config = true;
12841284
for backend in run.builder.config.codegen_backends(run.target) {
1285-
if path
1286-
.path
1287-
.to_str()
1288-
.unwrap()
1289-
.ends_with(&(CODEGEN_BACKEND_PREFIX.to_owned() + &backend))
1285+
if path.path.to_str().unwrap().ends_with(&(CODEGEN_BACKEND_PREFIX.to_owned() + backend))
12901286
{
12911287
needs_codegen_backend_config = false;
12921288
}
@@ -1846,7 +1842,7 @@ impl Step for Assemble {
18461842
extra_features: vec![],
18471843
});
18481844
let tool_exe = exe("llvm-bitcode-linker", target_compiler.host);
1849-
builder.copy_link(&src_path, &libdir_bin.join(&tool_exe));
1845+
builder.copy_link(&src_path, &libdir_bin.join(tool_exe));
18501846
}
18511847

18521848
// Ensure that `libLLVM.so` ends up in the newly build compiler directory,

‎src/bootstrap/src/core/build_steps/dist.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ impl Step for JsonDocs {
107107
builder.top_stage,
108108
host,
109109
builder,
110-
DocumentationFormat::JSON,
110+
DocumentationFormat::Json,
111111
));
112112

113113
let dest = "share/doc/rust/json";
@@ -1131,7 +1131,7 @@ impl Step for Rls {
11311131
let rls = builder.ensure(tool::Rls { compiler, target, extra_features: Vec::new() });
11321132

11331133
let mut tarball = Tarball::new(builder, "rls", &target.triple);
1134-
tarball.set_overlay(OverlayKind::RLS);
1134+
tarball.set_overlay(OverlayKind::Rls);
11351135
tarball.is_preview(true);
11361136
tarball.add_file(rls, "bin", 0o755);
11371137
tarball.add_legal_and_readme_to("share/doc/rls");
@@ -2059,7 +2059,7 @@ fn install_llvm_file(
20592059
if install_symlink {
20602060
// For download-ci-llvm, also install the symlink, to match what LLVM does. Using a
20612061
// symlink is fine here, as this is not a rustup component.
2062-
builder.copy_link(&source, &full_dest);
2062+
builder.copy_link(source, &full_dest);
20632063
} else {
20642064
// Otherwise, replace the symlink with an equivalent linker script. This is used when
20652065
// projects like miri link against librustc_driver.so. We don't use a symlink, as
@@ -2076,7 +2076,7 @@ fn install_llvm_file(
20762076
}
20772077
}
20782078
} else {
2079-
builder.install(&source, destination, 0o644);
2079+
builder.install(source, destination, 0o644);
20802080
}
20812081
}
20822082

@@ -2121,7 +2121,7 @@ fn maybe_install_llvm(
21212121
builder.install(&llvm_dylib_path, dst_libdir, 0o644);
21222122
}
21232123
!builder.config.dry_run()
2124-
} else if let Ok(llvm::LlvmResult { llvm_config, .. }) =
2124+
} else if let llvm::LlvmBuildStatus::AlreadyBuilt(llvm::LlvmResult { llvm_config, .. }) =
21252125
llvm::prebuilt_llvm_config(builder, target)
21262126
{
21272127
let mut cmd = Command::new(llvm_config);
@@ -2202,7 +2202,7 @@ impl Step for LlvmTools {
22022202
builder.ensure(crate::core::build_steps::llvm::Llvm { target });
22032203

22042204
let mut tarball = Tarball::new(builder, "llvm-tools", &target.triple);
2205-
tarball.set_overlay(OverlayKind::LLVM);
2205+
tarball.set_overlay(OverlayKind::Llvm);
22062206
tarball.is_preview(true);
22072207

22082208
if builder.config.llvm_tools_enabled {
@@ -2305,7 +2305,7 @@ impl Step for RustDev {
23052305
}
23062306

23072307
let mut tarball = Tarball::new(builder, "rust-dev", &target.triple);
2308-
tarball.set_overlay(OverlayKind::LLVM);
2308+
tarball.set_overlay(OverlayKind::Llvm);
23092309
// LLVM requires a shared object symlink to exist on some platforms.
23102310
tarball.permit_symlinks(true);
23112311

‎src/bootstrap/src/core/build_steps/doc.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -567,9 +567,9 @@ impl Step for Std {
567567
stage: run.builder.top_stage,
568568
target: run.target,
569569
format: if run.builder.config.cmd.json() {
570-
DocumentationFormat::JSON
570+
DocumentationFormat::Json
571571
} else {
572-
DocumentationFormat::HTML
572+
DocumentationFormat::Html
573573
},
574574
crates: run.make_run_crates(Alias::Library),
575575
});
@@ -583,13 +583,13 @@ impl Step for Std {
583583
let stage = self.stage;
584584
let target = self.target;
585585
let out = match self.format {
586-
DocumentationFormat::HTML => builder.doc_out(target),
587-
DocumentationFormat::JSON => builder.json_doc_out(target),
586+
DocumentationFormat::Html => builder.doc_out(target),
587+
DocumentationFormat::Json => builder.json_doc_out(target),
588588
};
589589

590590
t!(fs::create_dir_all(&out));
591591

592-
if self.format == DocumentationFormat::HTML {
592+
if self.format == DocumentationFormat::Html {
593593
builder.ensure(SharedAssets { target: self.target });
594594
}
595595

@@ -600,10 +600,10 @@ impl Step for Std {
600600
.into_string()
601601
.expect("non-utf8 paths are unsupported");
602602
let mut extra_args = match self.format {
603-
DocumentationFormat::HTML => {
603+
DocumentationFormat::Html => {
604604
vec!["--markdown-css", "rust.css", "--markdown-no-toc", "--index-page", &index_page]
605605
}
606-
DocumentationFormat::JSON => vec!["--output-format", "json"],
606+
DocumentationFormat::Json => vec!["--output-format", "json"],
607607
};
608608

609609
if !builder.config.docs_minification {
@@ -613,7 +613,7 @@ impl Step for Std {
613613
doc_std(builder, self.format, stage, target, &out, &extra_args, &self.crates);
614614

615615
// Don't open if the format is json
616-
if let DocumentationFormat::JSON = self.format {
616+
if let DocumentationFormat::Json = self.format {
617617
return;
618618
}
619619

@@ -646,15 +646,15 @@ const STD_PUBLIC_CRATES: [&str; 5] = ["core", "alloc", "std", "proc_macro", "tes
646646

647647
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
648648
pub enum DocumentationFormat {
649-
HTML,
650-
JSON,
649+
Html,
650+
Json,
651651
}
652652

653653
impl DocumentationFormat {
654654
fn as_str(&self) -> &str {
655655
match self {
656-
DocumentationFormat::HTML => "HTML",
657-
DocumentationFormat::JSON => "JSON",
656+
DocumentationFormat::Html => "HTML",
657+
DocumentationFormat::Json => "JSON",
658658
}
659659
}
660660
}
@@ -678,7 +678,7 @@ fn doc_std(
678678

679679
let compiler = builder.compiler(stage, builder.config.build);
680680

681-
let target_doc_dir_name = if format == DocumentationFormat::JSON { "json-doc" } else { "doc" };
681+
let target_doc_dir_name = if format == DocumentationFormat::Json { "json-doc" } else { "doc" };
682682
let target_dir =
683683
builder.stage_out(compiler, Mode::Std).join(target.triple).join(target_doc_dir_name);
684684

‎src/bootstrap/src/core/build_steps/format.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,12 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) {
225225
Some(first) => {
226226
let find_shortcut_candidates = |p: &PathBuf| {
227227
let mut candidates = Vec::new();
228-
for candidate in WalkBuilder::new(src.clone()).max_depth(Some(3)).build() {
229-
if let Ok(entry) = candidate {
230-
if let Some(dir_name) = p.file_name() {
231-
if entry.path().is_dir() && entry.file_name() == dir_name {
232-
candidates.push(entry.into_path());
233-
}
228+
for entry in
229+
WalkBuilder::new(src.clone()).max_depth(Some(3)).build().map_while(Result::ok)
230+
{
231+
if let Some(dir_name) = p.file_name() {
232+
if entry.path().is_dir() && entry.file_name() == dir_name {
233+
candidates.push(entry.into_path());
234234
}
235235
}
236236
}

‎src/bootstrap/src/core/build_steps/install.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ fn prepare_dir(destdir_env: &Option<PathBuf>, mut path: PathBuf) -> String {
130130
// https://www.gnu.org/prep/standards/html_node/DESTDIR.html
131131
if let Some(destdir) = destdir_env {
132132
let without_destdir = path.clone();
133-
path = destdir.clone();
133+
path.clone_from(destdir);
134134
// Custom .join() which ignores disk roots.
135135
for part in without_destdir.components() {
136136
if let Component::Normal(s) = part {

‎src/bootstrap/src/core/build_steps/llvm.rs

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,20 @@ pub struct Meta {
4242
root: String,
4343
}
4444

45+
pub enum LlvmBuildStatus {
46+
AlreadyBuilt(LlvmResult),
47+
ShouldBuild(Meta),
48+
}
49+
50+
impl LlvmBuildStatus {
51+
pub fn should_build(&self) -> bool {
52+
match self {
53+
LlvmBuildStatus::AlreadyBuilt(_) => false,
54+
LlvmBuildStatus::ShouldBuild(_) => true,
55+
}
56+
}
57+
}
58+
4559
// Linker flags to pass to LLVM's CMake invocation.
4660
#[derive(Debug, Clone, Default)]
4761
struct LdFlags {
@@ -72,10 +86,7 @@ impl LdFlags {
7286
///
7387
/// This will return the llvm-config if it can get it (but it will not build it
7488
/// if not).
75-
pub fn prebuilt_llvm_config(
76-
builder: &Builder<'_>,
77-
target: TargetSelection,
78-
) -> Result<LlvmResult, Meta> {
89+
pub fn prebuilt_llvm_config(builder: &Builder<'_>, target: TargetSelection) -> LlvmBuildStatus {
7990
// If we have llvm submodule initialized already, sync it.
8091
builder.update_existing_submodule(&Path::new("src").join("llvm-project"));
8192

@@ -93,7 +104,7 @@ pub fn prebuilt_llvm_config(
93104
llvm_cmake_dir.push("lib");
94105
llvm_cmake_dir.push("cmake");
95106
llvm_cmake_dir.push("llvm");
96-
return Ok(LlvmResult { llvm_config, llvm_cmake_dir });
107+
return LlvmBuildStatus::AlreadyBuilt(LlvmResult { llvm_config, llvm_cmake_dir });
97108
}
98109
}
99110

@@ -131,10 +142,10 @@ pub fn prebuilt_llvm_config(
131142
stamp.path.display()
132143
));
133144
}
134-
return Ok(res);
145+
return LlvmBuildStatus::AlreadyBuilt(res);
135146
}
136147

137-
Err(Meta { stamp, res, out_dir, root: root.into() })
148+
LlvmBuildStatus::ShouldBuild(Meta { stamp, res, out_dir, root: root.into() })
138149
}
139150

140151
/// This retrieves the LLVM sha we *want* to use, according to git history.
@@ -282,8 +293,8 @@ impl Step for Llvm {
282293

283294
// If LLVM has already been built or been downloaded through download-ci-llvm, we avoid building it again.
284295
let Meta { stamp, res, out_dir, root } = match prebuilt_llvm_config(builder, target) {
285-
Ok(p) => return p,
286-
Err(m) => m,
296+
LlvmBuildStatus::AlreadyBuilt(p) => return p,
297+
LlvmBuildStatus::ShouldBuild(m) => m,
287298
};
288299

289300
if builder.llvm_link_shared() && target.is_windows() {

‎src/bootstrap/src/core/build_steps/setup.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,9 @@ impl FromStr for Profile {
9393
Ok(Profile::Tools)
9494
}
9595
"none" => Ok(Profile::None),
96-
"llvm" | "codegen" => Err(format!(
97-
"the \"llvm\" and \"codegen\" profiles have been removed,\
96+
"llvm" | "codegen" => Err("the \"llvm\" and \"codegen\" profiles have been removed,\
9897
use \"compiler\" instead which has the same functionality"
99-
)),
98+
.to_string()),
10099
_ => Err(format!("unknown profile: '{s}'")),
101100
}
102101
}
@@ -474,10 +473,10 @@ impl Step for Hook {
474473

475474
// install a git hook to automatically run tidy, if they want
476475
fn install_git_hook_maybe(config: &Config) -> io::Result<()> {
477-
let git = t!(config.git().args(["rev-parse", "--git-common-dir"]).output().map(|output| {
476+
let git = config.git().args(["rev-parse", "--git-common-dir"]).output().map(|output| {
478477
assert!(output.status.success(), "failed to run `git`");
479478
PathBuf::from(t!(String::from_utf8(output.stdout)).trim())
480-
}));
479+
})?;
481480
let hooks_dir = git.join("hooks");
482481
let dst = hooks_dir.join("pre-push");
483482
if dst.exists() {

0 commit comments

Comments
 (0)