diff --git a/Cargo.lock b/Cargo.lock index c211024e1acdd..1c218bf2cde40 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -766,7 +766,7 @@ dependencies = [ "miropt-test-tools", "once_cell", "regex", - "rustfix", + "rustfix 0.8.1", "serde", "serde_json", "tracing", @@ -4855,6 +4855,18 @@ dependencies = [ "serde_json", ] +[[package]] +name = "rustfix" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "81864b097046da5df3758fdc6e4822bbb70afa06317e8ca45ea1b51cb8c5e5a4" +dependencies = [ + "serde", + "serde_json", + "thiserror", + "tracing", +] + [[package]] name = "rustfmt-config_proc_macro" version = "0.3.0" @@ -5896,7 +5908,7 @@ dependencies = [ "prettydiff", "regex", "rustc_version", - "rustfix", + "rustfix 0.6.1", "serde", "serde_json", "tempfile", @@ -5923,7 +5935,7 @@ dependencies = [ "prettydiff", "regex", "rustc_version", - "rustfix", + "rustfix 0.6.1", "serde", "serde_json", "spanned", diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 50f88eb970ab4..a668a1045757e 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -1723,6 +1723,7 @@ pub(super) fn compare_impl_const_raw( compare_number_of_generics(tcx, impl_const_item, trait_const_item, false)?; compare_generic_param_kinds(tcx, impl_const_item, trait_const_item, false)?; + check_region_bounds_on_impl_item(tcx, impl_const_item, trait_const_item, false)?; compare_const_predicate_entailment(tcx, impl_const_item, trait_const_item, impl_trait_ref) } @@ -1763,8 +1764,6 @@ fn compare_const_predicate_entailment<'tcx>( let impl_ct_predicates = tcx.predicates_of(impl_ct.def_id); let trait_ct_predicates = tcx.predicates_of(trait_ct.def_id); - check_region_bounds_on_impl_item(tcx, impl_ct, trait_ct, false)?; - // The predicates declared by the impl definition, the trait and the // associated const in the trait are assumed. let impl_predicates = tcx.predicates_of(impl_ct_predicates.parent.unwrap()); @@ -1866,6 +1865,7 @@ pub(super) fn compare_impl_ty<'tcx>( let _: Result<(), ErrorGuaranteed> = try { compare_number_of_generics(tcx, impl_ty, trait_ty, false)?; compare_generic_param_kinds(tcx, impl_ty, trait_ty, false)?; + check_region_bounds_on_impl_item(tcx, impl_ty, trait_ty, false)?; compare_type_predicate_entailment(tcx, impl_ty, trait_ty, impl_trait_ref)?; check_type_bounds(tcx, trait_ty, impl_ty, impl_trait_ref)?; }; @@ -1886,8 +1886,6 @@ fn compare_type_predicate_entailment<'tcx>( let impl_ty_predicates = tcx.predicates_of(impl_ty.def_id); let trait_ty_predicates = tcx.predicates_of(trait_ty.def_id); - check_region_bounds_on_impl_item(tcx, impl_ty, trait_ty, false)?; - let impl_ty_own_bounds = impl_ty_predicates.instantiate_own(tcx, impl_args); if impl_ty_own_bounds.len() == 0 { // Nothing to check. diff --git a/library/alloc/src/collections/vec_deque/mod.rs b/library/alloc/src/collections/vec_deque/mod.rs index 693ecb0b6b48d..b8a1760bdb775 100644 --- a/library/alloc/src/collections/vec_deque/mod.rs +++ b/library/alloc/src/collections/vec_deque/mod.rs @@ -978,6 +978,8 @@ impl VecDeque { // `head` and `len` are at most `isize::MAX` and `target_cap < self.capacity()`, so nothing can // overflow. let tail_outside = (target_cap + 1..=self.capacity()).contains(&(self.head + self.len)); + // Used in the drop guard below. + let old_head = self.head; if self.len == 0 { self.head = 0; @@ -1030,12 +1032,74 @@ impl VecDeque { } self.head = new_head; } - self.buf.shrink_to_fit(target_cap); + + struct Guard<'a, T, A: Allocator> { + deque: &'a mut VecDeque, + old_head: usize, + target_cap: usize, + } + + impl Drop for Guard<'_, T, A> { + #[cold] + fn drop(&mut self) { + unsafe { + // SAFETY: This is only called if `buf.shrink_to_fit` unwinds, + // which is the only time it's safe to call `abort_shrink`. + self.deque.abort_shrink(self.old_head, self.target_cap) + } + } + } + + let guard = Guard { deque: self, old_head, target_cap }; + + guard.deque.buf.shrink_to_fit(target_cap); + + // Don't drop the guard if we didn't unwind. + mem::forget(guard); debug_assert!(self.head < self.capacity() || self.capacity() == 0); debug_assert!(self.len <= self.capacity()); } + /// Reverts the deque back into a consistent state in case `shrink_to` failed. + /// This is necessary to prevent UB if the backing allocator returns an error + /// from `shrink` and `handle_alloc_error` subsequently unwinds (see #123369). + /// + /// `old_head` refers to the head index before `shrink_to` was called. `target_cap` + /// is the capacity that it was trying to shrink to. + unsafe fn abort_shrink(&mut self, old_head: usize, target_cap: usize) { + // Moral equivalent of self.head + self.len <= target_cap. Won't overflow + // because `self.len <= target_cap`. + if self.head <= target_cap - self.len { + // The deque's buffer is contiguous, so no need to copy anything around. + return; + } + + // `shrink_to` already copied the head to fit into the new capacity, so this won't overflow. + let head_len = target_cap - self.head; + // `self.head > target_cap - self.len` => `self.len > target_cap - self.head =: head_len` so this must be positive. + let tail_len = self.len - head_len; + + if tail_len <= cmp::min(head_len, self.capacity() - target_cap) { + // There's enough spare capacity to copy the tail to the back (because `tail_len < self.capacity() - target_cap`), + // and copying the tail should be cheaper than copying the head (because `tail_len <= head_len`). + + unsafe { + // The old tail and the new tail can't overlap because the head slice lies between them. The + // head slice ends at `target_cap`, so that's where we copy to. + self.copy_nonoverlapping(0, target_cap, tail_len); + } + } else { + // Either there's not enough spare capacity to make the deque contiguous, or the head is shorter than the tail + // (and therefore hopefully cheaper to copy). + unsafe { + // The old and the new head slice can overlap, so we can't use `copy_nonoverlapping` here. + self.copy(self.head, old_head, head_len); + self.head = old_head; + } + } + } + /// Shortens the deque, keeping the first `len` elements and dropping /// the rest. /// diff --git a/library/alloc/src/collections/vec_deque/tests.rs b/library/alloc/src/collections/vec_deque/tests.rs index f8ce4ca97884e..5329ad1aed5c3 100644 --- a/library/alloc/src/collections/vec_deque/tests.rs +++ b/library/alloc/src/collections/vec_deque/tests.rs @@ -1,4 +1,11 @@ -use core::iter::TrustedLen; +#![feature(alloc_error_hook)] + +use crate::alloc::{AllocError, Layout}; +use core::{iter::TrustedLen, ptr::NonNull}; +use std::{ + alloc::{set_alloc_error_hook, take_alloc_error_hook, System}, + panic::{catch_unwind, AssertUnwindSafe}, +}; use super::*; @@ -790,6 +797,52 @@ fn test_shrink_to() { } } +#[test] +fn test_shrink_to_unwind() { + // This tests that `shrink_to` leaves the deque in a consistent state when + // the call to `RawVec::shrink_to_fit` unwinds. The code is adapted from #123369 + // but changed to hopefully not have any UB even if the test fails. + + struct BadAlloc; + + unsafe impl Allocator for BadAlloc { + fn allocate(&self, l: Layout) -> Result, AllocError> { + // We allocate zeroed here so that the whole buffer of the deque + // is always initialized. That way, even if the deque is left in + // an inconsistent state, no uninitialized memory should be accessed. + System.allocate_zeroed(l) + } + + unsafe fn deallocate(&self, ptr: NonNull, layout: Layout) { + unsafe { System.deallocate(ptr, layout) } + } + + unsafe fn shrink( + &self, + _ptr: NonNull, + _old_layout: Layout, + _new_layout: Layout, + ) -> Result, AllocError> { + Err(AllocError) + } + } + + // preserve the old error hook just in case. + let old_error_hook = take_alloc_error_hook(); + set_alloc_error_hook(|_| panic!("alloc error")); + + let mut v = VecDeque::with_capacity_in(15, BadAlloc); + v.push_back(1); + v.push_front(2); + // This should unwind because it calls `BadAlloc::shrink` and then `handle_alloc_error` which unwinds. + assert!(catch_unwind(AssertUnwindSafe(|| v.shrink_to_fit())).is_err()); + // This should only pass if the deque is left in a consistent state. + assert_eq!(v, [2, 1]); + + // restore the old error hook. + set_alloc_error_hook(old_error_hook); +} + #[test] fn test_shrink_to_fit() { // This test checks that every single combination of head and tail position, diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index dec04d7e421e3..7f254e191fdcb 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -92,6 +92,7 @@ // tidy-alphabetical-start #![cfg_attr(not(no_global_oom_handling), feature(const_alloc_error))] #![cfg_attr(not(no_global_oom_handling), feature(const_btree_len))] +#![cfg_attr(test, feature(alloc_error_hook))] #![cfg_attr(test, feature(is_sorted))] #![cfg_attr(test, feature(new_uninit))] #![feature(alloc_layout_extra)] diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index a0de748b5e114..ce0643a3f5ef5 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -860,10 +860,10 @@ pub trait Binary { /// Basic usage with `i32`: /// /// ``` -/// let x = 42; // 42 is '2a' in hex +/// let y = 42; // 42 is '2a' in hex /// -/// assert_eq!(format!("{x:x}"), "2a"); -/// assert_eq!(format!("{x:#x}"), "0x2a"); +/// assert_eq!(format!("{y:x}"), "2a"); +/// assert_eq!(format!("{y:#x}"), "0x2a"); /// /// assert_eq!(format!("{:x}", -16), "fffffff0"); /// ``` @@ -915,10 +915,10 @@ pub trait LowerHex { /// Basic usage with `i32`: /// /// ``` -/// let x = 42; // 42 is '2A' in hex +/// let y = 42; // 42 is '2A' in hex /// -/// assert_eq!(format!("{x:X}"), "2A"); -/// assert_eq!(format!("{x:#X}"), "0x2A"); +/// assert_eq!(format!("{y:X}"), "2A"); +/// assert_eq!(format!("{y:#X}"), "0x2A"); /// /// assert_eq!(format!("{:X}", -16), "FFFFFFF0"); /// ``` diff --git a/library/std/src/os/unix/net/addr.rs b/library/std/src/os/unix/net/addr.rs index 9757653e02c06..1787eba0ef84a 100644 --- a/library/std/src/os/unix/net/addr.rs +++ b/library/std/src/os/unix/net/addr.rs @@ -107,6 +107,16 @@ impl SocketAddr { addr: libc::sockaddr_un, mut len: libc::socklen_t, ) -> io::Result { + if cfg!(target_os = "openbsd") { + // on OpenBSD, getsockname(2) returns the actual size of the socket address, + // and not the len of the content. Figure out the length for ourselves. + // https://marc.info/?l=openbsd-bugs&m=170105481926736&w=2 + let sun_path: &[u8] = + unsafe { mem::transmute::<&[libc::c_char], &[u8]>(&addr.sun_path) }; + len = core::slice::memchr::memchr(0, sun_path) + .map_or(len, |new_len| (new_len + sun_path_offset(&addr)) as libc::socklen_t); + } + if len == 0 { // When there is a datagram from unnamed unix socket // linux returns zero bytes of address diff --git a/library/std/src/sys/pal/unix/thread.rs b/library/std/src/sys/pal/unix/thread.rs index 5eb71f1f5ce52..6a6bfc77a8541 100644 --- a/library/std/src/sys/pal/unix/thread.rs +++ b/library/std/src/sys/pal/unix/thread.rs @@ -709,7 +709,7 @@ mod cgroups { // is created in an application with big thread-local storage requirements. // See #6233 for rationale and details. #[cfg(all(target_os = "linux", target_env = "gnu"))] -fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize { +unsafe fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize { // We use dlsym to avoid an ELF version dependency on GLIBC_PRIVATE. (#23628) // We shouldn't really be using such an internal symbol, but there's currently // no other way to account for the TLS size. @@ -723,11 +723,11 @@ fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize { // No point in looking up __pthread_get_minstack() on non-glibc platforms. #[cfg(all(not(all(target_os = "linux", target_env = "gnu")), not(target_os = "netbsd")))] -fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { +unsafe fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { libc::PTHREAD_STACK_MIN } #[cfg(target_os = "netbsd")] -fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { +unsafe fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { 2048 // just a guess } diff --git a/library/std/src/sys/sync/rwlock/queue.rs b/library/std/src/sys/sync/rwlock/queue.rs index d1918855797ad..337cc6c2ca094 100644 --- a/library/std/src/sys/sync/rwlock/queue.rs +++ b/library/std/src/sys/sync/rwlock/queue.rs @@ -202,7 +202,7 @@ impl Node { fn prepare(&mut self) { // Fall back to creating an unnamed `Thread` handle to allow locking in // TLS destructors. - self.thread.get_or_init(|| thread::try_current().unwrap_or_else(|| Thread::new(None))); + self.thread.get_or_init(|| thread::try_current().unwrap_or_else(Thread::new_unnamed)); self.completed = AtomicBool::new(false); } diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index fbb882e640b64..c1b4440e56088 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -53,25 +53,25 @@ use crate::fmt; /// # Examples /// /// ``` -/// use std::cell::RefCell; +/// use std::cell::Cell; /// use std::thread; /// -/// thread_local!(static FOO: RefCell = RefCell::new(1)); +/// thread_local!(static FOO: Cell = Cell::new(1)); /// -/// FOO.with_borrow(|v| assert_eq!(*v, 1)); -/// FOO.with_borrow_mut(|v| *v = 2); +/// assert_eq!(FOO.get(), 1); +/// FOO.set(2); /// /// // each thread starts out with the initial value of 1 /// let t = thread::spawn(move|| { -/// FOO.with_borrow(|v| assert_eq!(*v, 1)); -/// FOO.with_borrow_mut(|v| *v = 3); +/// assert_eq!(FOO.get(), 1); +/// FOO.set(3); /// }); /// /// // wait for the thread to complete and bail out on panic /// t.join().unwrap(); /// /// // we retain our original value of 2 despite the child thread -/// FOO.with_borrow(|v| assert_eq!(*v, 2)); +/// assert_eq!(FOO.get(), 2); /// ``` /// /// # Platform-specific behavior @@ -141,15 +141,16 @@ impl fmt::Debug for LocalKey { /// Publicity and attributes for each static are allowed. Example: /// /// ``` -/// use std::cell::RefCell; +/// use std::cell::{Cell, RefCell}; +/// /// thread_local! { -/// pub static FOO: RefCell = RefCell::new(1); +/// pub static FOO: Cell = Cell::new(1); /// -/// static BAR: RefCell = RefCell::new(1.0); +/// static BAR: RefCell> = RefCell::new(vec![1.0, 2.0]); /// } /// -/// FOO.with_borrow(|v| assert_eq!(*v, 1)); -/// BAR.with_borrow(|v| assert_eq!(*v, 1.0)); +/// assert_eq!(FOO.get(), 1); +/// BAR.with_borrow(|v| assert_eq!(v[1], 2.0)); /// ``` /// /// Note that only shared references (`&T`) to the inner data may be obtained, so a @@ -164,12 +165,13 @@ impl fmt::Debug for LocalKey { /// track any additional state. /// /// ``` -/// use std::cell::Cell; +/// use std::cell::RefCell; +/// /// thread_local! { -/// pub static FOO: Cell = const { Cell::new(1) }; +/// pub static FOO: RefCell> = const { RefCell::new(Vec::new()) }; /// } /// -/// assert_eq!(FOO.get(), 1); +/// FOO.with_borrow(|v| assert_eq!(v.len(), 0)); /// ``` /// /// See [`LocalKey` documentation][`std::thread::LocalKey`] for more @@ -279,10 +281,9 @@ impl LocalKey { where F: FnOnce(&T) -> R, { - unsafe { - let thread_local = (self.inner)(None).ok_or(AccessError)?; - Ok(f(thread_local)) - } + // SAFETY: `inner` is safe to call within the lifetime of the thread + let thread_local = unsafe { (self.inner)(None).ok_or(AccessError)? }; + Ok(f(thread_local)) } /// Acquires a reference to the value in this TLS key, initializing it with @@ -301,14 +302,17 @@ impl LocalKey { where F: FnOnce(Option, &T) -> R, { - unsafe { - let mut init = Some(init); - let reference = (self.inner)(Some(&mut init)).expect( + let mut init = Some(init); + + // SAFETY: `inner` is safe to call within the lifetime of the thread + let reference = unsafe { + (self.inner)(Some(&mut init)).expect( "cannot access a Thread Local Storage value \ during or after destruction", - ); - f(init, reference) - } + ) + }; + + f(init, reference) } } @@ -377,7 +381,7 @@ impl LocalKey> { where T: Copy, { - self.with(|cell| cell.get()) + self.with(Cell::get) } /// Takes the contained value, leaving `Default::default()` in its place. @@ -407,7 +411,7 @@ impl LocalKey> { where T: Default, { - self.with(|cell| cell.take()) + self.with(Cell::take) } /// Replaces the contained value, returning the old value. @@ -578,7 +582,7 @@ impl LocalKey> { where T: Default, { - self.with(|cell| cell.take()) + self.with(RefCell::take) } /// Replaces the contained value, returning the old value. diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index fd89edd4acebe..604eb05040b2d 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -487,9 +487,11 @@ impl Builder { amt }); - let my_thread = Thread::new(name.map(|name| { - CString::new(name).expect("thread name may not contain interior null bytes") - })); + let my_thread = name.map_or_else(Thread::new_unnamed, |name| unsafe { + Thread::new( + CString::new(name).expect("thread name may not contain interior null bytes"), + ) + }); let their_thread = my_thread.clone(); let my_packet: Arc> = Arc::new(Packet { @@ -711,7 +713,7 @@ pub(crate) fn set_current(thread: Thread) { /// In contrast to the public `current` function, this will not panic if called /// from inside a TLS destructor. pub(crate) fn try_current() -> Option { - CURRENT.try_with(|current| current.get_or_init(|| Thread::new(None)).clone()).ok() + CURRENT.try_with(|current| current.get_or_init(|| Thread::new_unnamed()).clone()).ok() } /// Gets a handle to the thread that invokes it. @@ -1307,21 +1309,26 @@ pub struct Thread { } impl Thread { - // Used only internally to construct a thread object without spawning - pub(crate) fn new(name: Option) -> Thread { - if let Some(name) = name { - Self::new_inner(ThreadName::Other(name)) - } else { - Self::new_inner(ThreadName::Unnamed) - } + /// Used only internally to construct a thread object without spawning. + /// + /// # Safety + /// `name` must be valid UTF-8. + pub(crate) unsafe fn new(name: CString) -> Thread { + unsafe { Self::new_inner(ThreadName::Other(name)) } + } + + pub(crate) fn new_unnamed() -> Thread { + unsafe { Self::new_inner(ThreadName::Unnamed) } } // Used in runtime to construct main thread pub(crate) fn new_main() -> Thread { - Self::new_inner(ThreadName::Main) + unsafe { Self::new_inner(ThreadName::Main) } } - fn new_inner(name: ThreadName) -> Thread { + /// # Safety + /// If `name` is `ThreadName::Other(_)`, the contained string must be valid UTF-8. + unsafe fn new_inner(name: ThreadName) -> Thread { // We have to use `unsafe` here to construct the `Parker` in-place, // which is required for the UNIX implementation. // diff --git a/src/tools/compiletest/Cargo.toml b/src/tools/compiletest/Cargo.toml index 4539c9b3285ff..38c29b91928c7 100644 --- a/src/tools/compiletest/Cargo.toml +++ b/src/tools/compiletest/Cargo.toml @@ -20,7 +20,7 @@ tracing-subscriber = { version = "0.3.3", default-features = false, features = [ regex = "1.0" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" -rustfix = "0.6.0" +rustfix = "0.8.1" once_cell = "1.16.0" walkdir = "2" glob = "0.3.0" diff --git a/src/tools/run-make-support/src/cc.rs b/src/tools/run-make-support/src/cc.rs index 2c9ad4f17006c..a2d51902652bc 100644 --- a/src/tools/run-make-support/src/cc.rs +++ b/src/tools/run-make-support/src/cc.rs @@ -1,6 +1,6 @@ use std::env; use std::path::Path; -use std::process::{Command, Output}; +use std::process::Command; use crate::{bin_name, cygpath_windows, handle_failed_output, is_msvc, is_windows, tmp_dir, uname}; @@ -19,6 +19,8 @@ pub struct Cc { cmd: Command, } +crate::impl_common_helpers!(Cc); + impl Cc { /// Construct a new platform-specific C compiler invocation. /// @@ -43,22 +45,6 @@ impl Cc { self } - /// Add a *platform-and-compiler-specific* argument. Please consult the docs for the various - /// possible C compilers on the various platforms to check which arguments are legal for - /// which compiler. - pub fn arg(&mut self, flag: &str) -> &mut Self { - self.cmd.arg(flag); - self - } - - /// Add multiple *platform-and-compiler-specific* arguments. Please consult the docs for the - /// various possible C compilers on the various platforms to check which arguments are legal - /// for which compiler. - pub fn args(&mut self, args: &[&str]) -> &mut Self { - self.cmd.args(args); - self - } - /// Specify `-o` or `-Fe`/`-Fo` depending on platform/compiler. This assumes that the executable /// is under `$TMPDIR`. pub fn out_exe(&mut self, name: &str) -> &mut Self { @@ -85,25 +71,6 @@ impl Cc { self } - - /// Run the constructed C invocation command and assert that it is successfully run. - #[track_caller] - pub fn run(&mut self) -> Output { - let caller_location = std::panic::Location::caller(); - let caller_line_number = caller_location.line(); - - let output = self.cmd.output().unwrap(); - if !output.status.success() { - handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number); - } - output - } - - /// Inspect what the underlying [`Command`] is up to the current construction. - pub fn inspect(&mut self, f: impl FnOnce(&Command)) -> &mut Self { - f(&self.cmd); - self - } } /// `EXTRACFLAGS` diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index e0a278d634c25..47b46a0a699c1 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -1,3 +1,8 @@ +//! `run-make-support` is a support library for run-make tests. It provides command wrappers and +//! convenience utility functions to help test writers reduce duplication. The support library +//! notably is built via cargo: this means that if your test wants some non-trivial utility, such +//! as `object` or `wasmparser`, they can be re-exported and be made available through this library. + pub mod cc; pub mod run; pub mod rustc; @@ -82,7 +87,7 @@ pub fn cygpath_windows>(path: P) -> String { cygpath.arg(path.as_ref()); let output = cygpath.output().unwrap(); if !output.status.success() { - handle_failed_output(&format!("{:#?}", cygpath), output, caller_line_number); + handle_failed_output(&cygpath, output, caller_line_number); } let s = String::from_utf8(output.stdout).unwrap(); // cygpath -w can attach a newline @@ -98,18 +103,18 @@ pub fn uname() -> String { let mut uname = Command::new("uname"); let output = uname.output().unwrap(); if !output.status.success() { - handle_failed_output(&format!("{:#?}", uname), output, caller_line_number); + handle_failed_output(&uname, output, caller_line_number); } String::from_utf8(output.stdout).unwrap() } -fn handle_failed_output(cmd: &str, output: Output, caller_line_number: u32) -> ! { +fn handle_failed_output(cmd: &Command, output: Output, caller_line_number: u32) -> ! { if output.status.success() { - eprintln!("command incorrectly succeeded at line {caller_line_number}"); + eprintln!("command unexpectedly succeeded at line {caller_line_number}"); } else { eprintln!("command failed at line {caller_line_number}"); } - eprintln!("{cmd}"); + eprintln!("{cmd:?}"); eprintln!("output status: `{}`", output.status); eprintln!("=== STDOUT ===\n{}\n\n", String::from_utf8(output.stdout).unwrap()); eprintln!("=== STDERR ===\n{}\n\n", String::from_utf8(output.stderr).unwrap()); @@ -129,3 +134,127 @@ pub fn set_host_rpath(cmd: &mut Command) { env::join_paths(paths.iter()).unwrap() }); } + +/// Implement common helpers for command wrappers. This assumes that the command wrapper is a struct +/// containing a `cmd: Command` field. The provided helpers are: +/// +/// 1. Generic argument acceptors: `arg` and `args` (delegated to [`Command`]). These are intended +/// to be *fallback* argument acceptors, when specific helpers don't make sense. Prefer to add +/// new specific helper methods over relying on these generic argument providers. +/// 2. Environment manipulation methods: `env`, `env_remove` and `env_clear`: these delegate to +/// methods of the same name on [`Command`]. +/// 3. Output and execution: `output`, `run` and `run_fail` are provided. `output` waits for the +/// command to finish running and returns the process's [`Output`]. `run` and `run_fail` are +/// higher-level convenience methods which waits for the command to finish running and assert +/// that the command successfully ran or failed as expected. Prefer `run` and `run_fail` when +/// possible. +/// +/// Example usage: +/// +/// ```ignore (illustrative) +/// struct CommandWrapper { cmd: Command } +/// +/// crate::impl_common_helpers!(CommandWrapper); +/// +/// impl CommandWrapper { +/// // ... additional specific helper methods +/// } +/// ``` +/// +/// [`Command`]: ::std::process::Command +/// [`Output`]: ::std::process::Output +macro_rules! impl_common_helpers { + ($wrapper: ident) => { + impl $wrapper { + /// Specify an environment variable. + pub fn env(&mut self, key: K, value: V) -> &mut Self + where + K: AsRef<::std::ffi::OsStr>, + V: AsRef<::std::ffi::OsStr>, + { + self.cmd.env(key, value); + self + } + + /// Remove an environmental variable. + pub fn env_remove(&mut self, key: K) -> &mut Self + where + K: AsRef<::std::ffi::OsStr>, + { + self.cmd.env_remove(key); + self + } + + /// Clear all environmental variables. + pub fn env_var(&mut self) -> &mut Self { + self.cmd.env_clear(); + self + } + + /// Generic command argument provider. Prefer specific helper methods if possible. + /// Note that for some executables, arguments might be platform specific. For C/C++ + /// compilers, arguments might be platform *and* compiler specific. + pub fn arg(&mut self, arg: S) -> &mut Self + where + S: AsRef<::std::ffi::OsStr>, + { + self.cmd.arg(arg); + self + } + + /// Generic command arguments provider. Prefer specific helper methods if possible. + /// Note that for some executables, arguments might be platform specific. For C/C++ + /// compilers, arguments might be platform *and* compiler specific. + pub fn args(&mut self, args: &[S]) -> &mut Self + where + S: AsRef<::std::ffi::OsStr>, + { + self.cmd.args(args); + self + } + + /// Inspect what the underlying [`Command`][::std::process::Command] is up to the + /// current construction. + pub fn inspect(&mut self, inspector: I) -> &mut Self + where + I: FnOnce(&::std::process::Command), + { + inspector(&self.cmd); + self + } + + /// Get the [`Output`][::std::process::Output] of the finished process. + pub fn output(&mut self) -> ::std::process::Output { + self.cmd.output().expect("failed to get output of finished process") + } + + /// Run the constructed command and assert that it is successfully run. + #[track_caller] + pub fn run(&mut self) -> ::std::process::Output { + let caller_location = ::std::panic::Location::caller(); + let caller_line_number = caller_location.line(); + + let output = self.cmd.output().unwrap(); + if !output.status.success() { + handle_failed_output(&self.cmd, output, caller_line_number); + } + output + } + + /// Run the constructed command and assert that it does not successfully run. + #[track_caller] + pub fn run_fail(&mut self) -> ::std::process::Output { + let caller_location = ::std::panic::Location::caller(); + let caller_line_number = caller_location.line(); + + let output = self.cmd.output().unwrap(); + if output.status.success() { + handle_failed_output(&self.cmd, output, caller_line_number); + } + output + } + } + }; +} + +pub(crate) use impl_common_helpers; diff --git a/src/tools/run-make-support/src/run.rs b/src/tools/run-make-support/src/run.rs index e33ea9d6e40a3..9aad91f1b4630 100644 --- a/src/tools/run-make-support/src/run.rs +++ b/src/tools/run-make-support/src/run.rs @@ -45,7 +45,7 @@ pub fn run(name: &str) -> Output { let (cmd, output) = run_common(name); if !output.status.success() { - handle_failed_output(&format!("{:#?}", cmd), output, caller_line_number); + handle_failed_output(&cmd, output, caller_line_number); } output } @@ -58,7 +58,7 @@ pub fn run_fail(name: &str) -> Output { let (cmd, output) = run_common(name); if output.status.success() { - handle_failed_output(&format!("{:#?}", cmd), output, caller_line_number); + handle_failed_output(&cmd, output, caller_line_number); } output } diff --git a/src/tools/run-make-support/src/rustc.rs b/src/tools/run-make-support/src/rustc.rs index b76711b4e979a..ebda151b908b0 100644 --- a/src/tools/run-make-support/src/rustc.rs +++ b/src/tools/run-make-support/src/rustc.rs @@ -1,5 +1,5 @@ use std::env; -use std::ffi::{OsStr, OsString}; +use std::ffi::OsString; use std::path::Path; use std::process::{Command, Output}; @@ -21,6 +21,8 @@ pub struct Rustc { cmd: Command, } +crate::impl_common_helpers!(Rustc); + fn setup_common() -> Command { let rustc = env::var("RUSTC").unwrap(); let mut cmd = Command::new(rustc); @@ -133,12 +135,6 @@ impl Rustc { self } - /// Generic command argument provider. Use `.arg("-Zname")` over `.arg("-Z").arg("arg")`. - pub fn arg>(&mut self, arg: S) -> &mut Self { - self.cmd.arg(arg); - self - } - /// Specify the crate type. pub fn crate_type(&mut self, crate_type: &str) -> &mut Self { self.cmd.arg("--crate-type"); @@ -153,49 +149,6 @@ impl Rustc { self } - /// Generic command arguments provider. Use `.arg("-Zname")` over `.arg("-Z").arg("arg")`. - pub fn args>(&mut self, args: &[S]) -> &mut Self { - self.cmd.args(args); - self - } - - pub fn env(&mut self, name: impl AsRef, value: impl AsRef) -> &mut Self { - self.cmd.env(name, value); - self - } - - // Command inspection, output and running helper methods - - /// Get the [`Output`][std::process::Output] of the finished `rustc` process. - pub fn output(&mut self) -> Output { - self.cmd.output().unwrap() - } - - /// Run the constructed `rustc` command and assert that it is successfully run. - #[track_caller] - pub fn run(&mut self) -> Output { - let caller_location = std::panic::Location::caller(); - let caller_line_number = caller_location.line(); - - let output = self.cmd.output().unwrap(); - if !output.status.success() { - handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number); - } - output - } - - #[track_caller] - pub fn run_fail(&mut self) -> Output { - let caller_location = std::panic::Location::caller(); - let caller_line_number = caller_location.line(); - - let output = self.cmd.output().unwrap(); - if output.status.success() { - handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number); - } - output - } - #[track_caller] pub fn run_fail_assert_exit_code(&mut self, code: i32) -> Output { let caller_location = std::panic::Location::caller(); @@ -203,14 +156,8 @@ impl Rustc { let output = self.cmd.output().unwrap(); if output.status.code().unwrap() != code { - handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number); + handle_failed_output(&self.cmd, output, caller_line_number); } output } - - /// Inspect what the underlying [`Command`] is up to the current construction. - pub fn inspect(&mut self, f: impl FnOnce(&Command)) -> &mut Self { - f(&self.cmd); - self - } } diff --git a/src/tools/run-make-support/src/rustdoc.rs b/src/tools/run-make-support/src/rustdoc.rs index 02af216d80506..1054ac83c103c 100644 --- a/src/tools/run-make-support/src/rustdoc.rs +++ b/src/tools/run-make-support/src/rustdoc.rs @@ -1,5 +1,4 @@ use std::env; -use std::ffi::OsStr; use std::path::Path; use std::process::{Command, Output}; @@ -20,6 +19,8 @@ pub struct Rustdoc { cmd: Command, } +crate::impl_common_helpers!(Rustdoc); + fn setup_common() -> Command { let rustdoc = env::var("RUSTDOC").unwrap(); let mut cmd = Command::new(rustdoc); @@ -61,25 +62,6 @@ impl Rustdoc { self } - /// Generic command argument provider. Use `.arg("-Zname")` over `.arg("-Z").arg("arg")`. - pub fn arg>(&mut self, arg: S) -> &mut Self { - self.cmd.arg(arg); - self - } - - /// Run the build `rustdoc` command and assert that the run is successful. - #[track_caller] - pub fn run(&mut self) -> Output { - let caller_location = std::panic::Location::caller(); - let caller_line_number = caller_location.line(); - - let output = self.cmd.output().unwrap(); - if !output.status.success() { - handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number); - } - output - } - #[track_caller] pub fn run_fail_assert_exit_code(&mut self, code: i32) -> Output { let caller_location = std::panic::Location::caller(); @@ -87,7 +69,7 @@ impl Rustdoc { let output = self.cmd.output().unwrap(); if output.status.code().unwrap() != code { - handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number); + handle_failed_output(&self.cmd, output, caller_line_number); } output } diff --git a/src/tools/tidy/src/run_make_tests.rs b/src/tools/tidy/src/run_make_tests.rs index 98021cec13011..2d9f8448a35bf 100644 --- a/src/tools/tidy/src/run_make_tests.rs +++ b/src/tools/tidy/src/run_make_tests.rs @@ -6,10 +6,26 @@ use std::io::Write; use std::path::{Path, PathBuf}; pub fn check(tests_path: &Path, src_path: &Path, bless: bool, bad: &mut bool) { + let mut is_sorted = true; + let allowed_makefiles = { - let allowed_makefiles = include_str!("allowed_run_make_makefiles.txt"); - let allowed_makefiles = allowed_makefiles.lines().collect::>(); - let is_sorted = allowed_makefiles.windows(2).all(|w| w[0] < w[1]); + let mut total_lines = 0; + let mut prev_line = ""; + let allowed_makefiles: BTreeSet<&str> = include_str!("allowed_run_make_makefiles.txt") + .lines() + .map(|line| { + total_lines += 1; + + if prev_line > line { + is_sorted = false; + } + + prev_line = line; + + line + }) + .collect(); + if !is_sorted && !bless { tidy_error!( bad, @@ -18,9 +34,7 @@ pub fn check(tests_path: &Path, src_path: &Path, bless: bool, bad: &mut bool) { `x test tidy --bless`" ); } - let allowed_makefiles_unique = - allowed_makefiles.iter().map(ToString::to_string).collect::>(); - if allowed_makefiles_unique.len() != allowed_makefiles.len() { + if allowed_makefiles.len() != total_lines { tidy_error!( bad, "`src/tools/tidy/src/allowed_run_make_makefiles.txt` contains duplicate entries, \ @@ -28,7 +42,8 @@ pub fn check(tests_path: &Path, src_path: &Path, bless: bool, bad: &mut bool) { `x test tidy --bless`" ); } - allowed_makefiles_unique + + allowed_makefiles }; let mut remaining_makefiles = allowed_makefiles.clone(); @@ -48,7 +63,7 @@ pub fn check(tests_path: &Path, src_path: &Path, bless: bool, bad: &mut bool) { let makefile_path = entry.path().strip_prefix(&tests_path).unwrap(); let makefile_path = makefile_path.to_str().unwrap().replace('\\', "/"); - if !remaining_makefiles.remove(&makefile_path) { + if !remaining_makefiles.remove(makefile_path.as_str()) { tidy_error!( bad, "found run-make Makefile not permitted in \ @@ -64,7 +79,7 @@ pub fn check(tests_path: &Path, src_path: &Path, bless: bool, bad: &mut bool) { // Our data must remain up to date, so they must be removed from // `src/tools/tidy/src/allowed_run_make_makefiles.txt`. // This can be done automatically on --bless, or else a tidy error will be issued. - if bless && !remaining_makefiles.is_empty() { + if bless && (!remaining_makefiles.is_empty() || !is_sorted) { let tidy_src = src_path.join("tools").join("tidy").join("src"); let org_file_path = tidy_src.join("allowed_run_make_makefiles.txt"); let temp_file_path = tidy_src.join("blessed_allowed_run_make_makefiles.txt"); diff --git a/tests/ui/generic-const-items/compare-impl-item.rs b/tests/ui/generic-const-items/compare-impl-item.rs index 01e4477c698da..21c958a0abec2 100644 --- a/tests/ui/generic-const-items/compare-impl-item.rs +++ b/tests/ui/generic-const-items/compare-impl-item.rs @@ -6,9 +6,10 @@ trait Trait

{ const B: u64; const C: T; const D: usize; + const E<'a>: &'a (); - const E: usize; - const F: (); + const F: usize; + const G: (); } impl

Trait

for () { @@ -20,11 +21,13 @@ impl

Trait

for () { //~^ ERROR const `C` has 0 type parameters but its trait declaration has 1 type parameter const D: u16 = N; //~^ ERROR const `D` has an incompatible generic parameter for trait `Trait` + const E: &'static () = &(); + //~^ ERROR lifetime parameters or bounds on const `E` do not match the trait declaration - const E: usize = 1024 + const F: usize = 1024 where P: Copy; //~ ERROR impl has stricter requirements than trait - const F: () = (); //~ ERROR impl has stricter requirements than trait + const G: () = (); //~ ERROR impl has stricter requirements than trait } fn main() {} diff --git a/tests/ui/generic-const-items/compare-impl-item.stderr b/tests/ui/generic-const-items/compare-impl-item.stderr index 8610d8cba0004..3bf28e9da60f6 100644 --- a/tests/ui/generic-const-items/compare-impl-item.stderr +++ b/tests/ui/generic-const-items/compare-impl-item.stderr @@ -1,5 +1,5 @@ error[E0049]: const `A` has 1 type parameter but its trait declaration has 0 type parameters - --> $DIR/compare-impl-item.rs:15:13 + --> $DIR/compare-impl-item.rs:16:13 | LL | const A: (); | - expected 0 type parameters @@ -8,7 +8,7 @@ LL | const A: () = (); | ^ found 1 type parameter error[E0049]: const `B` has 1 const parameter but its trait declaration has 2 const parameters - --> $DIR/compare-impl-item.rs:17:13 + --> $DIR/compare-impl-item.rs:18:13 | LL | const B: u64; | ------------ ------------ @@ -19,7 +19,7 @@ LL | const B: u64 = 0; | ^^^^^^^^^^^^ found 1 const parameter error[E0049]: const `C` has 0 type parameters but its trait declaration has 1 type parameter - --> $DIR/compare-impl-item.rs:19:13 + --> $DIR/compare-impl-item.rs:20:13 | LL | const C: T; | - expected 1 type parameter @@ -28,7 +28,7 @@ LL | const C<'a>: &'a str = ""; | ^^ found 0 type parameters error[E0053]: const `D` has an incompatible generic parameter for trait `Trait` - --> $DIR/compare-impl-item.rs:21:13 + --> $DIR/compare-impl-item.rs:22:13 | LL | trait Trait

{ | ----- @@ -42,25 +42,34 @@ LL | impl

Trait

for () { LL | const D: u16 = N; | ^^^^^^^^^^^^ found const parameter of type `u16` +error[E0195]: lifetime parameters or bounds on const `E` do not match the trait declaration + --> $DIR/compare-impl-item.rs:24:12 + | +LL | const E<'a>: &'a (); + | ---- lifetimes in impl do not match this const in trait +... +LL | const E: &'static () = &(); + | ^ lifetimes do not match const in trait + error[E0276]: impl has stricter requirements than trait - --> $DIR/compare-impl-item.rs:26:12 + --> $DIR/compare-impl-item.rs:29:12 | -LL | const E: usize; - | -------------- definition of `E` from trait +LL | const F: usize; + | -------------- definition of `F` from trait ... LL | P: Copy; | ^^^^ impl has extra requirement `P: Copy` error[E0276]: impl has stricter requirements than trait - --> $DIR/compare-impl-item.rs:27:16 + --> $DIR/compare-impl-item.rs:30:16 | -LL | const F: (); - | ------------------------- definition of `F` from trait +LL | const G: (); + | ------------------------- definition of `G` from trait ... -LL | const F: () = (); +LL | const G: () = (); | ^^ impl has extra requirement `T: Eq` -error: aborting due to 6 previous errors +error: aborting due to 7 previous errors -Some errors have detailed explanations: E0049, E0053, E0276. +Some errors have detailed explanations: E0049, E0053, E0195, E0276. For more information about an error, try `rustc --explain E0049`.