From 44d2740a8d5788a60db3f3f06b5b7d36521bb873 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Thu, 2 Jan 2025 15:55:04 +0800 Subject: [PATCH 1/5] Ping me for rustc-dev-guide changes on r-l/r --- triagebot.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/triagebot.toml b/triagebot.toml index 5660eb999b789..436c88541a5f7 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -1208,7 +1208,7 @@ project-exploit-mitigations = [ "/src/doc/nomicon" = ["@ehuss"] "/src/doc/reference" = ["@ehuss"] "/src/doc/rust-by-example" = ["@ehuss"] -"/src/doc/rustc-dev-guide" = ["@kobzol"] +"/src/doc/rustc-dev-guide" = ["@kobzol", "@jieyouxu"] "/src/doc/rustdoc" = ["rustdoc"] "/src/doc/style-guide" = ["style-team"] "/src/etc" = ["@Mark-Simulacrum"] From 56bf673f0a1698a4ebcd2d29bfbcc2c0220148ac Mon Sep 17 00:00:00 2001 From: Flakebi Date: Thu, 2 Jan 2025 13:10:51 +0100 Subject: [PATCH 2/5] Remove range-metadata amdgpu workaround Range metadata was disabled for amdgpu due to a backend bug. I did not encounter any problems when removing the workaround to enable range metadata (tried compiling `core` and `alloc`), so I assume this has been fixed in LLVM in the last years. Remove the workaround to re-enable range metadata. --- compiler/rustc_codegen_llvm/src/builder.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index ab0e43e60a4f5..5a34b52e6ef30 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -610,14 +610,6 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { } fn range_metadata(&mut self, load: &'ll Value, range: WrappingRange) { - if self.sess().target.arch == "amdgpu" { - // amdgpu/LLVM does something weird and thinks an i64 value is - // split into a v2i32, halving the bitwidth LLVM expects, - // tripping an assertion. So, for now, just disable this - // optimization. - return; - } - if self.cx.sess().opts.optimize == OptLevel::No { // Don't emit metadata we're not going to use return; From da3b9ee36b6bc3d6569f51f6eb911bd74d7e501f Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 2 Jan 2025 10:02:30 -0500 Subject: [PATCH 3/5] Update mailmap --- .mailmap | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.mailmap b/.mailmap index 18a9e5e44ecc3..6825606724a02 100644 --- a/.mailmap +++ b/.mailmap @@ -654,6 +654,8 @@ Vitali Haravy Vitali Haravy Waffle Lapkin Waffle Lapkin +Weihang Lo +Weihang Lo Wesley Wiser whitequark William Ting From 1b06dfd3fa98552296ece87e38bb0e8d291b0d89 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Thu, 2 Jan 2025 10:47:39 -0500 Subject: [PATCH 4/5] try to dedup me in the mailmap --- .mailmap | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.mailmap b/.mailmap index 18a9e5e44ecc3..7d3a250e06dde 100644 --- a/.mailmap +++ b/.mailmap @@ -189,6 +189,9 @@ Eduardo Broto Edward Shen Jacob Finkelman Jacob Finkelman +Jacob Finkelman +Jacob Finkelman +Jacob Finkelman Elliott Slaughter Elly Fong-Jones Eric Holk From 8b73fc5e27009cf7b3c8f306a09a41b30f869d87 Mon Sep 17 00:00:00 2001 From: Noratrieb <48135649+Noratrieb@users.noreply.github.com> Date: Thu, 2 Jan 2025 17:26:46 +0100 Subject: [PATCH 5/5] Fix formatting command The formatting command previously had two issues: - if rustfmt failed, it would print the command invocation. this is unnecessarily noisy - there was a race condition that lead to orphan rustfmts that would print their output after bootstrap exited We fix this by - removing the printing, it's not really useful - threading failure through properly instead of just yoloing exit(1) --- src/bootstrap/src/core/build_steps/format.rs | 65 ++++++++++++++------ 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/format.rs b/src/bootstrap/src/core/build_steps/format.rs index 29a96f7767281..f15e0f38e6921 100644 --- a/src/bootstrap/src/core/build_steps/format.rs +++ b/src/bootstrap/src/core/build_steps/format.rs @@ -14,7 +14,19 @@ use crate::core::builder::Builder; use crate::utils::exec::command; use crate::utils::helpers::{self, program_out_of_date, t}; -fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl FnMut(bool) -> bool { +#[must_use] +enum RustfmtStatus { + InProgress, + Ok, + Failed, +} + +fn rustfmt( + src: &Path, + rustfmt: &Path, + paths: &[PathBuf], + check: bool, +) -> impl FnMut(bool) -> RustfmtStatus { let mut cmd = Command::new(rustfmt); // Avoid the submodule config paths from coming into play. We only allow a single global config // for the workspace for now. @@ -26,30 +38,20 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F cmd.arg("--check"); } cmd.args(paths); - let cmd_debug = format!("{cmd:?}"); let mut cmd = cmd.spawn().expect("running rustfmt"); // Poor man's async: return a closure that might wait for rustfmt's completion (depending on // the value of the `block` argument). - move |block: bool| -> bool { + move |block: bool| -> RustfmtStatus { let status = if !block { match cmd.try_wait() { Ok(Some(status)) => Ok(status), - Ok(None) => return false, + Ok(None) => return RustfmtStatus::InProgress, Err(err) => Err(err), } } else { cmd.wait() }; - if !status.unwrap().success() { - eprintln!( - "fmt error: Running `{}` failed.\nIf you're running `tidy`, \ - try again with `--bless`. Or, if you just want to format \ - code, run `./x.py fmt` instead.", - cmd_debug, - ); - crate::exit!(1); - } - true + if status.unwrap().success() { RustfmtStatus::Ok } else { RustfmtStatus::Failed } } } @@ -240,6 +242,8 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) { // Spawn child processes on a separate thread so we can batch entries we have received from // ignore. let thread = std::thread::spawn(move || { + let mut result = Ok(()); + let mut children = VecDeque::new(); while let Ok(path) = rx.recv() { // Try getting more paths from the channel to amortize the overhead of spawning @@ -251,22 +255,38 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) { // Poll completion before waiting. for i in (0..children.len()).rev() { - if children[i](false) { - children.swap_remove_back(i); - break; + match children[i](false) { + RustfmtStatus::InProgress => {} + RustfmtStatus::Failed => { + result = Err(()); + children.swap_remove_back(i); + break; + } + RustfmtStatus::Ok => { + children.swap_remove_back(i); + break; + } } } if children.len() >= max_processes { // Await oldest child. - children.pop_front().unwrap()(true); + match children.pop_front().unwrap()(true) { + RustfmtStatus::InProgress | RustfmtStatus::Ok => {} + RustfmtStatus::Failed => result = Err(()), + } } } // Await remaining children. for mut child in children { - child(true); + match child(true) { + RustfmtStatus::InProgress | RustfmtStatus::Ok => {} + RustfmtStatus::Failed => result = Err(()), + } } + + result }); let formatted_paths = Mutex::new(Vec::new()); @@ -299,7 +319,12 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) { drop(tx); - thread.join().unwrap(); + let result = thread.join().unwrap(); + + if result.is_err() { + crate::exit!(1); + } + if !check { update_rustfmt_version(build); }