From c66494474cc64aaf4f1e51b428d53e7dcbd14c25 Mon Sep 17 00:00:00 2001 From: joboet Date: Mon, 14 Nov 2022 14:25:44 +0100 Subject: [PATCH 01/12] std: move `ReentrantMutex` to `sync` --- library/std/src/io/stdio.rs | 3 +-- library/std/src/sync/mod.rs | 3 +++ library/std/src/{sys_common => sync}/remutex.rs | 0 library/std/src/{sys_common => sync}/remutex/tests.rs | 2 +- library/std/src/sys_common/mod.rs | 1 - 5 files changed, 5 insertions(+), 4 deletions(-) rename library/std/src/{sys_common => sync}/remutex.rs (100%) rename library/std/src/{sys_common => sync}/remutex/tests.rs (94%) diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index 1141a957d8712..14bfef4c7aad9 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -10,9 +10,8 @@ use crate::fmt; use crate::fs::File; use crate::io::{self, BufReader, IoSlice, IoSliceMut, LineWriter, Lines}; use crate::sync::atomic::{AtomicBool, Ordering}; -use crate::sync::{Arc, Mutex, MutexGuard, OnceLock}; +use crate::sync::{Arc, Mutex, MutexGuard, OnceLock, ReentrantMutex, ReentrantMutexGuard}; use crate::sys::stdio; -use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard}; type LocalStream = Arc>>; diff --git a/library/std/src/sync/mod.rs b/library/std/src/sync/mod.rs index 4fee8d3e92fc8..ba20bab87a40d 100644 --- a/library/std/src/sync/mod.rs +++ b/library/std/src/sync/mod.rs @@ -177,6 +177,8 @@ pub use self::lazy_lock::LazyLock; #[unstable(feature = "once_cell", issue = "74465")] pub use self::once_lock::OnceLock; +pub(crate) use self::remutex::{ReentrantMutex, ReentrantMutexGuard}; + pub mod mpsc; mod barrier; @@ -187,4 +189,5 @@ mod mutex; mod once; mod once_lock; mod poison; +mod remutex; mod rwlock; diff --git a/library/std/src/sys_common/remutex.rs b/library/std/src/sync/remutex.rs similarity index 100% rename from library/std/src/sys_common/remutex.rs rename to library/std/src/sync/remutex.rs diff --git a/library/std/src/sys_common/remutex/tests.rs b/library/std/src/sync/remutex/tests.rs similarity index 94% rename from library/std/src/sys_common/remutex/tests.rs rename to library/std/src/sync/remutex/tests.rs index 8e97ce11c34f5..fc553081d4227 100644 --- a/library/std/src/sys_common/remutex/tests.rs +++ b/library/std/src/sync/remutex/tests.rs @@ -1,6 +1,6 @@ +use super::{ReentrantMutex, ReentrantMutexGuard}; use crate::cell::RefCell; use crate::sync::Arc; -use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard}; use crate::thread; #[test] diff --git a/library/std/src/sys_common/mod.rs b/library/std/src/sys_common/mod.rs index 069b13e9d85ea..9bf2739deae44 100644 --- a/library/std/src/sys_common/mod.rs +++ b/library/std/src/sys_common/mod.rs @@ -27,7 +27,6 @@ pub mod lazy_box; pub mod memchr; pub mod once; pub mod process; -pub mod remutex; pub mod thread; pub mod thread_info; pub mod thread_local_dtor; From 04f1ead55243c895da7c3703e2daca319f381124 Mon Sep 17 00:00:00 2001 From: Adam Casey Date: Wed, 16 Nov 2022 09:26:56 +0000 Subject: [PATCH 02/12] available_parallelism: Handle 0 cfs_period_us There seem to be some scenarios where `cpu.cfs_period_us` can contain `0` This causes a panic when calling `std::thread::available_parallelism()` as is done so from binaries built by `cargo test`, which was how the issue was discovered. I don't feel like `0` is a good value for `cpu.cfs_period_us`, but I also don't think applications should panic if this value is seen. This case is handled by other projects which read this information: - num_cpus: https://github.com/seanmonstar/num_cpus/blob/e437b9d9083d717692e35d917de8674a7987dd06/src/linux.rs#L207-L210 - ninja: https://github.com/ninja-build/ninja/pull/2174/files - dotnet: https://github.com/dotnet/runtime/blob/c4341d45acca3ea662cd8d71e7d71094450dd045/src/coreclr/pal/src/misc/cgroup.cpp#L481-L483 Before this change, this panic could be seen in environments setup as described above: ``` $ RUST_BACKTRACE=1 cargo test Finished test [unoptimized + debuginfo] target(s) in 3.55s Running unittests src/main.rs (target/debug/deps/x-9a42e145aca2934d) thread 'main' panicked at 'attempt to divide by zero', library/std/src/sys/unix/thread.rs:546:70 stack backtrace: 0: rust_begin_unwind 1: core::panicking::panic_fmt 2: core::panicking::panic 3: std::sys::unix::thread::cgroups::quota 4: std::sys::unix::thread::available_parallelism 5: std::thread::available_parallelism 6: test::helpers::concurrency::get_concurrency 7: test::console::run_tests_console 8: test::test_main 9: test::test_main_static 10: x::main at ./src/main.rs:1:1 11: core::ops::function::FnOnce::call_once at /tmp/rust-1.64-1.64.0-1/library/core/src/ops/function.rs:248:5 note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. error: test failed, to rerun pass '--bin local-rabmq-amqpprox' ``` I've tested this change in an environment which has the bad setup and rebuilding the test executable against a fixed std library fixes the panic. --- library/std/src/sys/unix/thread.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index c1d30dd9d521b..9125174efab78 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/library/std/src/sys/unix/thread.rs @@ -510,7 +510,7 @@ mod cgroups { let limit = raw_quota.next()?; let period = raw_quota.next()?; match (limit.parse::(), period.parse::()) { - (Ok(limit), Ok(period)) => { + (Ok(limit), Ok(period)) if period > 0 => { quota = quota.min(limit / period); } _ => {} @@ -570,7 +570,7 @@ mod cgroups { let period = parse_file("cpu.cfs_period_us"); match (limit, period) { - (Some(limit), Some(period)) => quota = quota.min(limit / period), + (Some(limit), Some(period)) if period > 0 => quota = quota.min(limit / period), _ => {} } From 980065ab23650452c33bb47ef66d75fbfbcb6e04 Mon Sep 17 00:00:00 2001 From: Florian Bartels Date: Tue, 6 Dec 2022 09:32:11 +0100 Subject: [PATCH 03/12] Make sentinel value configurable There are OSs that always return the lowest free value. The algorithm in `lazy_init` always avoids keys with the sentinel value. In affected OSs, this means that each call to `lazy_init` will always request two keys from the OS and returns/frees the first one (with sentinel value) immediately afterwards. By making the sentinel value configurable, affected OSs can use a different value than zero to prevent this performance issue. --- .../std/src/sys_common/thread_local_key.rs | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/library/std/src/sys_common/thread_local_key.rs b/library/std/src/sys_common/thread_local_key.rs index 747579f178127..2672a2a75b017 100644 --- a/library/std/src/sys_common/thread_local_key.rs +++ b/library/std/src/sys_common/thread_local_key.rs @@ -117,10 +117,14 @@ pub struct Key { /// This value specifies no destructor by default. pub const INIT: StaticKey = StaticKey::new(None); +// Define a sentinel value that is unlikely to be returned +// as a TLS key (but it may be returned). +const KEY_SENTVAL: usize = 0; + impl StaticKey { #[rustc_const_unstable(feature = "thread_local_internals", issue = "none")] pub const fn new(dtor: Option) -> StaticKey { - StaticKey { key: atomic::AtomicUsize::new(0), dtor } + StaticKey { key: atomic::AtomicUsize::new(KEY_SENTVAL), dtor } } /// Gets the value associated with this TLS key @@ -144,31 +148,36 @@ impl StaticKey { #[inline] unsafe fn key(&self) -> imp::Key { match self.key.load(Ordering::Relaxed) { - 0 => self.lazy_init() as imp::Key, + KEY_SENTVAL => self.lazy_init() as imp::Key, n => n as imp::Key, } } unsafe fn lazy_init(&self) -> usize { - // POSIX allows the key created here to be 0, but the compare_exchange - // below relies on using 0 as a sentinel value to check who won the + // POSIX allows the key created here to be KEY_SENTVAL, but the compare_exchange + // below relies on using KEY_SENTVAL as a sentinel value to check who won the // race to set the shared TLS key. As far as I know, there is no // guaranteed value that cannot be returned as a posix_key_create key, // so there is no value we can initialize the inner key with to // prove that it has not yet been set. As such, we'll continue using a - // value of 0, but with some gyrations to make sure we have a non-0 + // value of KEY_SENTVAL, but with some gyrations to make sure we have a non-KEY_SENTVAL // value returned from the creation routine. // FIXME: this is clearly a hack, and should be cleaned up. let key1 = imp::create(self.dtor); - let key = if key1 != 0 { + let key = if key1 as usize != KEY_SENTVAL { key1 } else { let key2 = imp::create(self.dtor); imp::destroy(key1); key2 }; - rtassert!(key != 0); - match self.key.compare_exchange(0, key as usize, Ordering::SeqCst, Ordering::SeqCst) { + rtassert!(key as usize != KEY_SENTVAL); + match self.key.compare_exchange( + KEY_SENTVAL, + key as usize, + Ordering::SeqCst, + Ordering::SeqCst, + ) { // The CAS succeeded, so we've created the actual key Ok(_) => key as usize, // If someone beat us to the punch, use their key instead From d72a0c437bd2db922b954af7b0278e1f4bf31edf Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sun, 11 Dec 2022 21:46:30 +0100 Subject: [PATCH 04/12] Properly calculate best failure in macro matching Previously, we used spans. This was not good. Sometimes, the span of the token that failed to match may come from a position later in the file which has been transcribed into a token stream way earlier in the file. If precisely this token fails to match, we think that it was the best match because its span is so high, even though other arms might have gotten further in the token stream. We now try to properly use the location in the token stream. --- compiler/rustc_expand/src/mbe/diagnostics.rs | 40 ++++++++++++++----- compiler/rustc_expand/src/mbe/macro_parser.rs | 13 +++++- compiler/rustc_expand/src/mbe/macro_rules.rs | 6 +-- compiler/rustc_parse/src/parser/mod.rs | 4 ++ src/test/ui/macros/best-failure.rs | 11 +++++ src/test/ui/macros/best-failure.stderr | 21 ++++++++++ 6 files changed, 80 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/macros/best-failure.rs create mode 100644 src/test/ui/macros/best-failure.stderr diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs index 197f056917f5d..40aa64d9d4040 100644 --- a/compiler/rustc_expand/src/mbe/diagnostics.rs +++ b/compiler/rustc_expand/src/mbe/diagnostics.rs @@ -43,7 +43,7 @@ pub(super) fn failed_to_match_macro<'cx>( return result; } - let Some((token, label, remaining_matcher)) = tracker.best_failure else { + let Some(BestFailure { token, msg: label, remaining_matcher, .. }) = tracker.best_failure else { return DummyResult::any(sp); }; @@ -95,11 +95,24 @@ struct CollectTrackerAndEmitter<'a, 'cx, 'matcher> { cx: &'a mut ExtCtxt<'cx>, remaining_matcher: Option<&'matcher MatcherLoc>, /// Which arm's failure should we report? (the one furthest along) - best_failure: Option<(Token, &'static str, MatcherLoc)>, + best_failure: Option, root_span: Span, result: Option>, } +struct BestFailure { + token: Token, + position_in_tokenstream: usize, + msg: &'static str, + remaining_matcher: MatcherLoc, +} + +impl BestFailure { + fn is_better_position(&self, position: usize) -> bool { + position > self.position_in_tokenstream + } +} + impl<'a, 'cx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'a, 'cx, 'matcher> { fn before_match_loc(&mut self, parser: &TtParser, matcher: &'matcher MatcherLoc) { if self.remaining_matcher.is_none() @@ -119,18 +132,25 @@ impl<'a, 'cx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'a, 'cx, "should not collect detailed info for successful macro match", ); } - Failure(token, msg) => match self.best_failure { - Some((ref best_token, _, _)) if best_token.span.lo() >= token.span.lo() => {} - _ => { - self.best_failure = Some(( - token.clone(), + Failure(token, approx_position, msg) => { + debug!(?token, ?msg, "a new failure of an arm"); + + if self + .best_failure + .as_ref() + .map_or(true, |failure| failure.is_better_position(*approx_position)) + { + self.best_failure = Some(BestFailure { + token: token.clone(), + position_in_tokenstream: *approx_position, msg, - self.remaining_matcher + remaining_matcher: self + .remaining_matcher .expect("must have collected matcher already") .clone(), - )) + }) } - }, + } Error(err_sp, msg) => { let span = err_sp.substitute_dummy(self.root_span); self.cx.struct_span_err(span, msg).emit(); diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index d161868edce77..df1c1834c1dc0 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -310,7 +310,8 @@ pub(crate) enum ParseResult { Success(T), /// Arm failed to match. If the second parameter is `token::Eof`, it indicates an unexpected /// end of macro invocation. Otherwise, it indicates that no rules expected the given token. - Failure(Token, &'static str), + /// The usize is the approximate position of the token in the input token stream. + Failure(Token, usize, &'static str), /// Fatal error (malformed macro?). Abort compilation. Error(rustc_span::Span, String), ErrorReported(ErrorGuaranteed), @@ -455,6 +456,7 @@ impl TtParser { &mut self, matcher: &'matcher [MatcherLoc], token: &Token, + approx_position: usize, track: &mut T, ) -> Option { // Matcher positions that would be valid if the macro invocation was over now. Only @@ -598,6 +600,7 @@ impl TtParser { token::Eof, if token.span.is_dummy() { token.span } else { token.span.shrink_to_hi() }, ), + approx_position, "missing tokens in macro arguments", ), }) @@ -627,7 +630,12 @@ impl TtParser { // Process `cur_mps` until either we have finished the input or we need to get some // parsing from the black-box parser done. - let res = self.parse_tt_inner(matcher, &parser.token, track); + let res = self.parse_tt_inner( + matcher, + &parser.token, + parser.approx_token_stream_pos(), + track, + ); if let Some(res) = res { return res; } @@ -642,6 +650,7 @@ impl TtParser { // parser: syntax error. return Failure( parser.token.clone(), + parser.approx_token_stream_pos(), "no rules expected this token in macro call", ); } diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 2dbb90e2190f0..b4001a497afa8 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -326,8 +326,8 @@ pub(super) fn try_match_macro<'matcher, T: Tracker<'matcher>>( return Ok((i, named_matches)); } - Failure(_, _) => { - trace!("Failed to match arm, trying the next one"); + Failure(_, reached_position, _) => { + trace!(%reached_position, "Failed to match arm, trying the next one"); // Try the next arm. } Error(_, _) => { @@ -432,7 +432,7 @@ pub fn compile_declarative_macro( let argument_map = match tt_parser.parse_tt(&mut Cow::Owned(parser), &argument_gram, &mut NoopTracker) { Success(m) => m, - Failure(token, msg) => { + Failure(token, _, msg) => { let s = parse_failure_msg(&token); let sp = token.span.substitute_dummy(def.span); let mut err = sess.parse_sess.span_diagnostic.struct_span_err(sp, &s); diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index bebb012660a16..008388e3da340 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -1503,6 +1503,10 @@ impl<'a> Parser<'a> { pub fn clear_expected_tokens(&mut self) { self.expected_tokens.clear(); } + + pub fn approx_token_stream_pos(&self) -> usize { + self.token_cursor.num_next_calls + } } pub(crate) fn make_unclosed_delims_error( diff --git a/src/test/ui/macros/best-failure.rs b/src/test/ui/macros/best-failure.rs new file mode 100644 index 0000000000000..bbdd465d5ec96 --- /dev/null +++ b/src/test/ui/macros/best-failure.rs @@ -0,0 +1,11 @@ +macro_rules! number { + (neg false, $self:ident) => { $self }; + ($signed:tt => $ty:ty;) => { + number!(neg $signed, $self); + //~^ ERROR no rules expected the token `$` + }; +} + +number! { false => u8; } + +fn main() {} diff --git a/src/test/ui/macros/best-failure.stderr b/src/test/ui/macros/best-failure.stderr new file mode 100644 index 0000000000000..a52fc5e3da6a5 --- /dev/null +++ b/src/test/ui/macros/best-failure.stderr @@ -0,0 +1,21 @@ +error: no rules expected the token `$` + --> $DIR/best-failure.rs:4:30 + | +LL | macro_rules! number { + | ------------------- when calling this macro +... +LL | number!(neg $signed, $self); + | ^^^^^ no rules expected this token in macro call +... +LL | number! { false => u8; } + | ------------------------ in this macro invocation + | +note: while trying to match meta-variable `$self:ident` + --> $DIR/best-failure.rs:2:17 + | +LL | (neg false, $self:ident) => { $self }; + | ^^^^^^^^^^^ + = note: this error originates in the macro `number` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to previous error + From 37b88c842ab418df06ee7351f769ef01f5491260 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maja=20K=C4=85dzio=C5=82ka?= Date: Mon, 26 Dec 2022 22:49:22 +0100 Subject: [PATCH 05/12] Iterator::find: link to Iterator::position in docs for discoverability --- library/core/src/iter/traits/iterator.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index bac836292f8fa..ae91509d391df 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -2653,7 +2653,10 @@ pub trait Iterator { /// argument is a double reference. You can see this effect in the /// examples below, with `&&x`. /// + /// If you need the index of the element, see [`position()`]. + /// /// [`Some(element)`]: Some + /// [`position()`]: Iterator::position /// /// # Examples /// From 633a6c8b66a8a2e54d9545071312d4406fa195e5 Mon Sep 17 00:00:00 2001 From: Albert Larsan <74931857+albertlarsan68@users.noreply.github.com> Date: Wed, 14 Dec 2022 15:42:22 +0100 Subject: [PATCH 06/12] Format only modified files As discussed on #105688, this makes x fmt only format modified files --- src/bootstrap/format.rs | 46 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/bootstrap/format.rs b/src/bootstrap/format.rs index b2f6afead7980..b99bf33f4829c 100644 --- a/src/bootstrap/format.rs +++ b/src/bootstrap/format.rs @@ -44,6 +44,35 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F } } +/// Finds the remote for rust-lang/rust. +/// For example for these remotes it will return `upstream`. +/// ```text +/// origin https://github.com/Nilstrieb/rust.git (fetch) +/// origin https://github.com/Nilstrieb/rust.git (push) +/// upstream https://github.com/rust-lang/rust (fetch) +/// upstream https://github.com/rust-lang/rust (push) +/// ``` +fn get_rust_lang_rust_remote() -> Result { + let mut git = Command::new("git"); + git.args(["config", "--local", "--get-regex", "remote\\..*\\.url"]); + + let output = git.output().map_err(|err| format!("{err:?}"))?; + if !output.status.success() { + return Err("failed to execute git config command".to_owned()); + } + + let stdout = String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?; + + let rust_lang_remote = stdout + .lines() + .find(|remote| remote.contains("rust-lang")) + .ok_or_else(|| "rust-lang/rust remote not found".to_owned())?; + + let remote_name = + rust_lang_remote.split('.').nth(1).ok_or_else(|| "remote name not found".to_owned())?; + Ok(remote_name.into()) +} + #[derive(serde::Deserialize)] struct RustfmtConfig { ignore: Vec, @@ -110,6 +139,23 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) { // preventing the latter from being formatted. ignore_fmt.add(&format!("!/{}", untracked_path)).expect(&untracked_path); } + if !check && paths.is_empty() { + let remote = t!(get_rust_lang_rust_remote()); + let base = output( + build + .config + .git() + .arg("merge-base") + .arg("HEAD") + .arg(format!("{remote}/master")), + ); + let files = + output(build.config.git().arg("diff").arg("--name-only").arg(base.trim())); + for file in files.lines() { + println!("formatting modified file {file}"); + ignore_fmt.add(&format!("/{file}")).expect(file); + } + } } else { println!("Not in git tree. Skipping git-aware format checks"); } From 00b23e8d01cc5cc89b893c1d9072e0ebf1673ef4 Mon Sep 17 00:00:00 2001 From: Albert Larsan <74931857+albertlarsan68@users.noreply.github.com> Date: Wed, 14 Dec 2022 18:02:58 +0100 Subject: [PATCH 07/12] Add rustfmt version check --- src/bootstrap/format.rs | 85 +++++++++++++++++++++++++++++++++-------- 1 file changed, 70 insertions(+), 15 deletions(-) diff --git a/src/bootstrap/format.rs b/src/bootstrap/format.rs index b99bf33f4829c..985f34ff92f2c 100644 --- a/src/bootstrap/format.rs +++ b/src/bootstrap/format.rs @@ -1,7 +1,7 @@ //! Runs rustfmt on the repository. use crate::builder::Builder; -use crate::util::{output, t}; +use crate::util::{output, program_out_of_date, t}; use ignore::WalkBuilder; use std::collections::VecDeque; use std::path::{Path, PathBuf}; @@ -44,6 +44,68 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F } } +fn verify_timestamp(build: &Builder<'_>) -> bool { + let stamp_file = { + let mut s = build.out.clone(); + s.push("rustfmt.stamp"); + s + }; + + let mut cmd = Command::new(match build.initial_rustfmt() { + Some(p) => p, + None => return false, + }); + cmd.arg("--version"); + let output = match cmd.output() { + Ok(status) => status, + Err(_) => return false, + }; + if !output.status.success() { + return false; + } + let version = String::from_utf8(output.stdout).unwrap(); + !program_out_of_date(&stamp_file, &version) +} + +fn update_timestamp(build: &Builder<'_>) { + let stamp_file = { + let mut s = build.out.clone(); + s.push("rustfmt.stamp"); + s + }; + + let mut cmd = Command::new(match build.initial_rustfmt() { + Some(p) => p, + None => return, + }); + cmd.arg("--version"); + let output = match cmd.output() { + Ok(status) => status, + Err(_) => return, + }; + if !output.status.success() { + return; + } + let version = String::from_utf8(output.stdout).unwrap(); + + t!(std::fs::write(stamp_file, version)) +} + +fn get_modified_files(build: &Builder<'_>) -> Option> { + let Ok(remote) = get_rust_lang_rust_remote() else {return None;}; + if !verify_timestamp(build) { + return None; + } + let base = + output(build.config.git().arg("merge-base").arg("HEAD").arg(format!("{remote}/master"))); + Some( + output(build.config.git().arg("diff").arg("--name-only").arg(base.trim())) + .lines() + .map(|s| s.trim().to_owned()) + .collect(), + ) +} + /// Finds the remote for rust-lang/rust. /// For example for these remotes it will return `upstream`. /// ```text @@ -140,20 +202,11 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) { ignore_fmt.add(&format!("!/{}", untracked_path)).expect(&untracked_path); } if !check && paths.is_empty() { - let remote = t!(get_rust_lang_rust_remote()); - let base = output( - build - .config - .git() - .arg("merge-base") - .arg("HEAD") - .arg(format!("{remote}/master")), - ); - let files = - output(build.config.git().arg("diff").arg("--name-only").arg(base.trim())); - for file in files.lines() { - println!("formatting modified file {file}"); - ignore_fmt.add(&format!("/{file}")).expect(file); + if let Some(files) = get_modified_files(build) { + for file in files { + println!("formatting modified file {file}"); + ignore_fmt.add(&format!("/{file}")).expect(&file); + } } } } else { @@ -233,4 +286,6 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) { drop(tx); thread.join().unwrap(); + + update_timestamp(build); } From b07a1e3f5a78f0601b60e1763cd7055ceb9ab50f Mon Sep 17 00:00:00 2001 From: Albert Larsan <74931857+albertlarsan68@users.noreply.github.com> Date: Mon, 19 Dec 2022 14:48:01 +0100 Subject: [PATCH 08/12] Put final touches --- src/bootstrap/format.rs | 74 +++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 40 deletions(-) diff --git a/src/bootstrap/format.rs b/src/bootstrap/format.rs index 985f34ff92f2c..b49322e3c028f 100644 --- a/src/bootstrap/format.rs +++ b/src/bootstrap/format.rs @@ -44,65 +44,58 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F } } -fn verify_timestamp(build: &Builder<'_>) -> bool { - let stamp_file = { - let mut s = build.out.clone(); - s.push("rustfmt.stamp"); - s - }; +fn get_rustfmt_version(build: &Builder<'_>) -> Option<(String, PathBuf)> { + let stamp_file = build.out.join("rustfmt.stamp"); let mut cmd = Command::new(match build.initial_rustfmt() { Some(p) => p, - None => return false, + None => return None, }); cmd.arg("--version"); let output = match cmd.output() { Ok(status) => status, - Err(_) => return false, + Err(_) => return None, }; if !output.status.success() { - return false; + return None; } - let version = String::from_utf8(output.stdout).unwrap(); - !program_out_of_date(&stamp_file, &version) + Some((String::from_utf8(output.stdout).unwrap(), stamp_file)) } -fn update_timestamp(build: &Builder<'_>) { - let stamp_file = { - let mut s = build.out.clone(); - s.push("rustfmt.stamp"); - s - }; - - let mut cmd = Command::new(match build.initial_rustfmt() { - Some(p) => p, - None => return, - }); - cmd.arg("--version"); - let output = match cmd.output() { - Ok(status) => status, - Err(_) => return, - }; - if !output.status.success() { - return; - } - let version = String::from_utf8(output.stdout).unwrap(); +/// Return whether the format cache can be reused. +fn verify_rustfmt_version(build: &Builder<'_>) -> bool { + let Some((version, stamp_file)) = get_rustfmt_version(build) else {return false;}; + !program_out_of_date(&stamp_file, &version) +} +/// Updates the last rustfmt version used +fn update_rustfmt_version(build: &Builder<'_>) { + let Some((version, stamp_file)) = get_rustfmt_version(build) else {return;}; t!(std::fs::write(stamp_file, version)) } +/// Returns the files modified between the `merge-base` of HEAD and +/// rust-lang/master and what is now on the disk. +/// +/// Returns `None` if all files should be formatted. fn get_modified_files(build: &Builder<'_>) -> Option> { let Ok(remote) = get_rust_lang_rust_remote() else {return None;}; - if !verify_timestamp(build) { + if !verify_rustfmt_version(build) { return None; } - let base = - output(build.config.git().arg("merge-base").arg("HEAD").arg(format!("{remote}/master"))); Some( - output(build.config.git().arg("diff").arg("--name-only").arg(base.trim())) - .lines() - .map(|s| s.trim().to_owned()) - .collect(), + output( + build + .config + .git() + .arg("diff-index") + .arg("--name-only") + .arg("--merge-base") + .arg(&format!("{remote}/master")), + ) + .lines() + .map(|s| s.trim().to_owned()) + .collect(), ) } @@ -286,6 +279,7 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) { drop(tx); thread.join().unwrap(); - - update_timestamp(build); + if !check { + update_rustfmt_version(build); + } } From c5bc87713f688a36ecbcaf718a1274b0674eaee5 Mon Sep 17 00:00:00 2001 From: Albert Larsan <74931857+albertlarsan68@users.noreply.github.com> Date: Tue, 27 Dec 2022 18:28:12 +0100 Subject: [PATCH 09/12] Make `x clean` also clean the stamp file --- src/bootstrap/clean.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bootstrap/clean.rs b/src/bootstrap/clean.rs index 069f3d6acf158..7aa81893542cb 100644 --- a/src/bootstrap/clean.rs +++ b/src/bootstrap/clean.rs @@ -21,6 +21,7 @@ pub fn clean(build: &Build, all: bool) { rm_rf(&build.out.join("tmp")); rm_rf(&build.out.join("dist")); rm_rf(&build.out.join("bootstrap")); + rm_rf(&build.out.join("rustfmt.stamp")); for host in &build.hosts { let entries = match build.out.join(host.triple).read_dir() { From 0b942a879d438216e90304436e74fa1ebb3ac16c Mon Sep 17 00:00:00 2001 From: Albert Larsan <74931857+albertlarsan68@users.noreply.github.com> Date: Tue, 27 Dec 2022 22:19:56 +0100 Subject: [PATCH 10/12] Add changelog entry --- src/bootstrap/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bootstrap/CHANGELOG.md b/src/bootstrap/CHANGELOG.md index 64b74ecc9defd..4105fa5ec9600 100644 --- a/src/bootstrap/CHANGELOG.md +++ b/src/bootstrap/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Several unsupported `./configure` options have been removed: `optimize`, `parallel-compiler`. These can still be enabled with `--set`, although it isn't recommended. - `remote-test-server`'s `verbose` argument has been removed in favor of the `--verbose` flag - `remote-test-server`'s `remote` argument has been removed in favor of the `--bind` flag. Use `--bind 0.0.0.0:12345` to replicate the behavior of the `remote` argument. +- `x.py fmt` now formats only files modified between the merge-base of HEAD and the last commit in the master branch of the rust-lang repository and the current working directory. To restore old behaviour, use `x.py fmt .`. The check mode is not affected by this change. [#105702](https://github.com/rust-lang/rust/pull/105702) ### Non-breaking changes From b804c0d5a54dee10a59fc2b3d54037cb96fbb598 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Dec 2022 13:55:54 +0100 Subject: [PATCH 11/12] adjust message on non-unwinding panic --- library/std/src/panicking.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index 1039835bbbdfe..49c2f81403a9b 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -699,7 +699,11 @@ fn rust_panic_with_hook( // have limited options. Currently our preference is to // just abort. In the future we may consider resuming // unwinding or otherwise exiting the thread cleanly. - rtprintpanic!("thread panicked while panicking. aborting.\n"); + if !can_unwind { + rtprintpanic!("thread caused non-unwinding panic. aborting.\n"); + } else { + rtprintpanic!("thread panicked while panicking. aborting.\n"); + } crate::sys::abort_internal(); } From cb7c8993b957d279f9ae0a07983c4560ca725ac2 Mon Sep 17 00:00:00 2001 From: Albert Larsan <74931857+albertlarsan68@users.noreply.github.com> Date: Fri, 9 Dec 2022 14:57:57 +0100 Subject: [PATCH 12/12] Clarify catch_unwind docs about panic hooks Makes it clear from catch_unwind docs that the panic hook will be called before the panic is caught. --- library/std/src/panic.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/std/src/panic.rs b/library/std/src/panic.rs index c4f022de021ef..9fa8f5702a843 100644 --- a/library/std/src/panic.rs +++ b/library/std/src/panic.rs @@ -114,6 +114,9 @@ where /// aborting the process as well. This function *only* catches unwinding panics, /// not those that abort the process. /// +/// Note that if a custom panic hook has been set, it will be invoked before +/// the panic is caught, before unwinding. +/// /// Also note that unwinding into Rust code with a foreign exception (e.g. /// an exception thrown from C++ code) is undefined behavior. ///