diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index c165620f657b6..9e2756f07eded 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -1088,6 +1088,14 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ErrorFollowing, EncodeCrossCrate::No, "the `#[custom_mir]` attribute is just used for the Rust test suite", ), + rustc_attr!( + TEST, rustc_dump_item_bounds, Normal, template!(Word), + WarnFollowing, EncodeCrossCrate::No + ), + rustc_attr!( + TEST, rustc_dump_predicates, Normal, template!(Word), + WarnFollowing, EncodeCrossCrate::No + ), rustc_attr!( TEST, rustc_object_lifetime_default, Normal, template!(Word), WarnFollowing, EncodeCrossCrate::No diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index ca653e50aab76..7ed32fb9d9f37 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -510,7 +510,7 @@ hir_analysis_ty_param_some = type parameter `{$param}` must be used as the type .note = implementing a foreign trait is only possible if at least one of the types for which it is implemented is local .only_note = only traits defined in the current crate can be implemented for a type parameter -hir_analysis_type_of = {$type_of} +hir_analysis_type_of = {$ty} hir_analysis_typeof_reserved_keyword_used = `typeof` is a reserved keyword but unimplemented @@ -566,7 +566,7 @@ hir_analysis_value_of_associated_struct_already_specified = hir_analysis_variadic_function_compatible_convention = C-variadic function must have a compatible calling convention, like {$conventions} .label = C-variadic function must have a compatible calling convention -hir_analysis_variances_of = {$variances_of} +hir_analysis_variances_of = {$variances} hir_analysis_where_clause_on_main = `main` function is not allowed to have a `where` clause .label = `main` cannot have a `where` clause diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index c6e8759327f03..e5bd147352dea 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -45,8 +45,8 @@ use std::ops::Bound; use crate::check::intrinsic::intrinsic_operation_unsafety; use crate::errors; use crate::hir_ty_lowering::{HirTyLowerer, RegionInferReason}; -pub use type_of::test_opaque_hidden_types; +pub(crate) mod dump; mod generics_of; mod item_bounds; mod predicates_of; diff --git a/compiler/rustc_hir_analysis/src/collect/dump.rs b/compiler/rustc_hir_analysis/src/collect/dump.rs new file mode 100644 index 0000000000000..85e1c600d6da4 --- /dev/null +++ b/compiler/rustc_hir_analysis/src/collect/dump.rs @@ -0,0 +1,43 @@ +use rustc_hir::def::DefKind; +use rustc_hir::def_id::CRATE_DEF_ID; +use rustc_middle::ty::TyCtxt; +use rustc_span::sym; + +pub(crate) fn opaque_hidden_types(tcx: TyCtxt<'_>) { + if !tcx.has_attr(CRATE_DEF_ID, sym::rustc_hidden_type_of_opaques) { + return; + } + + for id in tcx.hir().items() { + let DefKind::OpaqueTy = tcx.def_kind(id.owner_id) else { continue }; + + let ty = tcx.type_of(id.owner_id).instantiate_identity(); + + tcx.dcx().emit_err(crate::errors::TypeOf { span: tcx.def_span(id.owner_id), ty }); + } +} + +pub(crate) fn predicates_and_item_bounds(tcx: TyCtxt<'_>) { + for id in tcx.hir_crate_items(()).owners() { + if tcx.has_attr(id, sym::rustc_dump_predicates) { + let preds = tcx.predicates_of(id).instantiate_identity(tcx).predicates; + let span = tcx.def_span(id); + + let mut diag = tcx.dcx().struct_span_err(span, sym::rustc_dump_predicates.as_str()); + for pred in preds { + diag.note(format!("{pred:?}")); + } + diag.emit(); + } + if tcx.has_attr(id, sym::rustc_dump_item_bounds) { + let bounds = tcx.item_bounds(id).instantiate_identity(); + let span = tcx.def_span(id); + + let mut diag = tcx.dcx().struct_span_err(span, sym::rustc_dump_item_bounds.as_str()); + for bound in bounds { + diag.note(format!("{bound:?}")); + } + diag.emit(); + } + } +} diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs index 2684467a4381e..1e2b0c4323385 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs @@ -15,7 +15,6 @@ use crate::errors::TypeofReservedKeywordUsed; use super::bad_placeholder; use super::ItemCtxt; -pub use opaque::test_opaque_hidden_types; mod opaque; diff --git a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs index 2b2f07001d2f4..d1048b742a07b 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs @@ -1,28 +1,14 @@ use rustc_errors::StashKey; use rustc_hir::def::DefKind; -use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID}; +use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{self as hir, def, Expr, ImplItem, Item, Node, TraitItem}; use rustc_middle::bug; use rustc_middle::hir::nested_filter; use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt}; -use rustc_span::{sym, ErrorGuaranteed, DUMMY_SP}; +use rustc_span::DUMMY_SP; -use crate::errors::{TaitForwardCompat, TaitForwardCompat2, TypeOf, UnconstrainedOpaqueType}; - -pub fn test_opaque_hidden_types(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> { - let mut res = Ok(()); - if tcx.has_attr(CRATE_DEF_ID, sym::rustc_hidden_type_of_opaques) { - for id in tcx.hir().items() { - if matches!(tcx.def_kind(id.owner_id), DefKind::OpaqueTy) { - let type_of = tcx.type_of(id.owner_id).instantiate_identity(); - - res = Err(tcx.dcx().emit_err(TypeOf { span: tcx.def_span(id.owner_id), type_of })); - } - } - } - res -} +use crate::errors::{TaitForwardCompat, TaitForwardCompat2, UnconstrainedOpaqueType}; /// Checks "defining uses" of opaque `impl Trait` in associated types. /// These can only be defined by associated items of the same trait. diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index cff8d5a5ea50c..44025c3cd61c1 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -682,7 +682,7 @@ pub(crate) enum CannotCaptureLateBound { pub(crate) struct VariancesOf { #[primary_span] pub span: Span, - pub variances_of: String, + pub variances: String, } #[derive(Diagnostic)] @@ -690,7 +690,7 @@ pub(crate) struct VariancesOf { pub(crate) struct TypeOf<'tcx> { #[primary_span] pub span: Span, - pub type_of: Ty<'tcx>, + pub ty: Ty<'tcx>, } #[derive(Diagnostic)] diff --git a/compiler/rustc_hir_analysis/src/lib.rs b/compiler/rustc_hir_analysis/src/lib.rs index 1927359421d00..0428abcdf24e8 100644 --- a/compiler/rustc_hir_analysis/src/lib.rs +++ b/compiler/rustc_hir_analysis/src/lib.rs @@ -151,10 +151,6 @@ pub fn provide(providers: &mut Providers) { pub fn check_crate(tcx: TyCtxt<'_>) { let _prof_timer = tcx.sess.timer("type_check_crate"); - if tcx.features().rustc_attrs { - let _ = tcx.sess.time("outlives_testing", || outlives::test::test_inferred_outlives(tcx)); - } - tcx.sess.time("coherence_checking", || { tcx.hir().par_for_each_module(|module| { let _ = tcx.ensure().check_mod_type_wf(module); @@ -169,11 +165,10 @@ pub fn check_crate(tcx: TyCtxt<'_>) { }); if tcx.features().rustc_attrs { - let _ = tcx.sess.time("variance_testing", || variance::test::test_variance(tcx)); - } - - if tcx.features().rustc_attrs { - let _ = collect::test_opaque_hidden_types(tcx); + tcx.sess.time("outlives_dumping", || outlives::dump::inferred_outlives(tcx)); + tcx.sess.time("variance_dumping", || variance::dump::variances(tcx)); + collect::dump::opaque_hidden_types(tcx); + collect::dump::predicates_and_item_bounds(tcx); } // Make sure we evaluate all static and (non-associated) const items, even if unused. diff --git a/compiler/rustc_hir_analysis/src/outlives/dump.rs b/compiler/rustc_hir_analysis/src/outlives/dump.rs new file mode 100644 index 0000000000000..ab50d9e86efb8 --- /dev/null +++ b/compiler/rustc_hir_analysis/src/outlives/dump.rs @@ -0,0 +1,29 @@ +use rustc_middle::bug; +use rustc_middle::ty::{self, TyCtxt}; +use rustc_span::sym; + +pub(crate) fn inferred_outlives(tcx: TyCtxt<'_>) { + for id in tcx.hir().items() { + if !tcx.has_attr(id.owner_id, sym::rustc_outlives) { + continue; + } + + let preds = tcx.inferred_outlives_of(id.owner_id); + let mut preds: Vec<_> = preds + .iter() + .map(|(pred, _)| match pred.kind().skip_binder() { + ty::ClauseKind::RegionOutlives(p) => p.to_string(), + ty::ClauseKind::TypeOutlives(p) => p.to_string(), + err => bug!("unexpected clause {:?}", err), + }) + .collect(); + preds.sort(); + + let span = tcx.def_span(id.owner_id); + let mut err = tcx.dcx().struct_span_err(span, sym::rustc_outlives.as_str()); + for pred in preds { + err.note(pred); + } + err.emit(); + } +} diff --git a/compiler/rustc_hir_analysis/src/outlives/mod.rs b/compiler/rustc_hir_analysis/src/outlives/mod.rs index 97fd7731b1e54..1f74ebf99f166 100644 --- a/compiler/rustc_hir_analysis/src/outlives/mod.rs +++ b/compiler/rustc_hir_analysis/src/outlives/mod.rs @@ -5,10 +5,9 @@ use rustc_middle::ty::GenericArgKind; use rustc_middle::ty::{self, CratePredicatesMap, TyCtxt, Upcast}; use rustc_span::Span; +pub(crate) mod dump; mod explicit; mod implicit_infer; -/// Code to write unit test for outlives. -pub mod test; mod utils; pub fn provide(providers: &mut Providers) { diff --git a/compiler/rustc_hir_analysis/src/outlives/test.rs b/compiler/rustc_hir_analysis/src/outlives/test.rs deleted file mode 100644 index e9b6c679bd5a4..0000000000000 --- a/compiler/rustc_hir_analysis/src/outlives/test.rs +++ /dev/null @@ -1,31 +0,0 @@ -use rustc_middle::bug; -use rustc_middle::ty::{self, TyCtxt}; -use rustc_span::{symbol::sym, ErrorGuaranteed}; - -pub fn test_inferred_outlives(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> { - let mut res = Ok(()); - for id in tcx.hir().items() { - // For unit testing: check for a special "rustc_outlives" - // attribute and report an error with various results if found. - if tcx.has_attr(id.owner_id, sym::rustc_outlives) { - let predicates = tcx.inferred_outlives_of(id.owner_id); - let mut pred: Vec = predicates - .iter() - .map(|(out_pred, _)| match out_pred.kind().skip_binder() { - ty::ClauseKind::RegionOutlives(p) => p.to_string(), - ty::ClauseKind::TypeOutlives(p) => p.to_string(), - err => bug!("unexpected clause {:?}", err), - }) - .collect(); - pred.sort(); - - let span = tcx.def_span(id.owner_id); - let mut err = tcx.dcx().struct_span_err(span, "rustc_outlives"); - for p in pred { - err.note(p); - } - res = Err(err.emit()); - } - } - res -} diff --git a/compiler/rustc_hir_analysis/src/variance/dump.rs b/compiler/rustc_hir_analysis/src/variance/dump.rs new file mode 100644 index 0000000000000..1a17dabb677ac --- /dev/null +++ b/compiler/rustc_hir_analysis/src/variance/dump.rs @@ -0,0 +1,32 @@ +use rustc_hir::def::DefKind; +use rustc_hir::def_id::CRATE_DEF_ID; +use rustc_middle::ty::TyCtxt; +use rustc_span::symbol::sym; + +pub(crate) fn variances(tcx: TyCtxt<'_>) { + if tcx.has_attr(CRATE_DEF_ID, sym::rustc_variance_of_opaques) { + for id in tcx.hir().items() { + let DefKind::OpaqueTy = tcx.def_kind(id.owner_id) else { continue }; + + let variances = tcx.variances_of(id.owner_id); + + tcx.dcx().emit_err(crate::errors::VariancesOf { + span: tcx.def_span(id.owner_id), + variances: format!("{variances:?}"), + }); + } + } + + for id in tcx.hir().items() { + if !tcx.has_attr(id.owner_id, sym::rustc_variance) { + continue; + } + + let variances = tcx.variances_of(id.owner_id); + + tcx.dcx().emit_err(crate::errors::VariancesOf { + span: tcx.def_span(id.owner_id), + variances: format!("{variances:?}"), + }); + } +} diff --git a/compiler/rustc_hir_analysis/src/variance/mod.rs b/compiler/rustc_hir_analysis/src/variance/mod.rs index 1977451f39e0a..29f96e27b6492 100644 --- a/compiler/rustc_hir_analysis/src/variance/mod.rs +++ b/compiler/rustc_hir_analysis/src/variance/mod.rs @@ -22,8 +22,7 @@ mod constraints; /// Code to solve constraints and write out the results. mod solve; -/// Code to write unit tests of variance. -pub mod test; +pub(crate) mod dump; /// Code for transforming variances. mod xform; diff --git a/compiler/rustc_hir_analysis/src/variance/test.rs b/compiler/rustc_hir_analysis/src/variance/test.rs deleted file mode 100644 index c211e1af046af..0000000000000 --- a/compiler/rustc_hir_analysis/src/variance/test.rs +++ /dev/null @@ -1,37 +0,0 @@ -use rustc_hir::def::DefKind; -use rustc_hir::def_id::CRATE_DEF_ID; -use rustc_middle::ty::TyCtxt; -use rustc_span::symbol::sym; -use rustc_span::ErrorGuaranteed; - -use crate::errors; - -pub fn test_variance(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> { - let mut res = Ok(()); - if tcx.has_attr(CRATE_DEF_ID, sym::rustc_variance_of_opaques) { - for id in tcx.hir().items() { - if matches!(tcx.def_kind(id.owner_id), DefKind::OpaqueTy) { - let variances_of = tcx.variances_of(id.owner_id); - - res = Err(tcx.dcx().emit_err(errors::VariancesOf { - span: tcx.def_span(id.owner_id), - variances_of: format!("{variances_of:?}"), - })); - } - } - } - - // For unit testing: check for a special "rustc_variance" - // attribute and report an error with various results if found. - for id in tcx.hir().items() { - if tcx.has_attr(id.owner_id, sym::rustc_variance) { - let variances_of = tcx.variances_of(id.owner_id); - - res = Err(tcx.dcx().emit_err(errors::VariancesOf { - span: tcx.def_span(id.owner_id), - variances_of: format!("{variances_of:?}"), - })); - } - } - res -} diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 4cb325c3c0cda..a8123fe994c34 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1593,6 +1593,8 @@ symbols! { rustc_do_not_const_check, rustc_doc_primitive, rustc_dummy, + rustc_dump_item_bounds, + rustc_dump_predicates, rustc_dump_user_args, rustc_dump_vtable, rustc_effective_visibility, diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index 6677534eafc6e..1833a7f477f00 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -424,29 +424,3 @@ pub mod __alloc_error_handler { } } } - -#[cfg(not(no_global_oom_handling))] -/// Specialize clones into pre-allocated, uninitialized memory. -/// Used by `Box::clone` and `Rc`/`Arc::make_mut`. -pub(crate) trait WriteCloneIntoRaw: Sized { - unsafe fn write_clone_into_raw(&self, target: *mut Self); -} - -#[cfg(not(no_global_oom_handling))] -impl WriteCloneIntoRaw for T { - #[inline] - default unsafe fn write_clone_into_raw(&self, target: *mut Self) { - // Having allocated *first* may allow the optimizer to create - // the cloned value in-place, skipping the local and move. - unsafe { target.write(self.clone()) }; - } -} - -#[cfg(not(no_global_oom_handling))] -impl WriteCloneIntoRaw for T { - #[inline] - unsafe fn write_clone_into_raw(&self, target: *mut Self) { - // We can always copy in-place, without ever involving a local value. - unsafe { target.copy_from_nonoverlapping(self, 1) }; - } -} diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 01a954ed75beb..1ec095a46f704 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -188,6 +188,8 @@ use core::any::Any; use core::async_iter::AsyncIterator; use core::borrow; +#[cfg(not(no_global_oom_handling))] +use core::clone::CloneToUninit; use core::cmp::Ordering; use core::error::Error; use core::fmt; @@ -207,7 +209,7 @@ use core::slice; use core::task::{Context, Poll}; #[cfg(not(no_global_oom_handling))] -use crate::alloc::{handle_alloc_error, WriteCloneIntoRaw}; +use crate::alloc::handle_alloc_error; use crate::alloc::{AllocError, Allocator, Global, Layout}; #[cfg(not(no_global_oom_handling))] use crate::borrow::Cow; @@ -1346,7 +1348,7 @@ impl Clone for Box { // Pre-allocate memory to allow writing the cloned value directly. let mut boxed = Self::new_uninit_in(self.1.clone()); unsafe { - (**self).write_clone_into_raw(boxed.as_mut_ptr()); + (**self).clone_to_uninit(boxed.as_mut_ptr()); boxed.assume_init() } } diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 022a14b931a3c..0f759d64f669d 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -103,6 +103,7 @@ #![feature(assert_matches)] #![feature(async_fn_traits)] #![feature(async_iterator)] +#![feature(clone_to_uninit)] #![feature(coerce_unsized)] #![feature(const_align_of_val)] #![feature(const_box)] diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 2b7ab2f6e2590..3745ecb48c18e 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -249,6 +249,8 @@ use std::boxed::Box; use core::any::Any; use core::borrow; use core::cell::Cell; +#[cfg(not(no_global_oom_handling))] +use core::clone::CloneToUninit; use core::cmp::Ordering; use core::fmt; use core::hash::{Hash, Hasher}; @@ -268,8 +270,6 @@ use core::slice::from_raw_parts_mut; #[cfg(not(no_global_oom_handling))] use crate::alloc::handle_alloc_error; -#[cfg(not(no_global_oom_handling))] -use crate::alloc::WriteCloneIntoRaw; use crate::alloc::{AllocError, Allocator, Global, Layout}; use crate::borrow::{Cow, ToOwned}; #[cfg(not(no_global_oom_handling))] @@ -1749,7 +1749,8 @@ impl Rc { } } -impl Rc { +#[cfg(not(no_global_oom_handling))] +impl Rc { /// Makes a mutable reference into the given `Rc`. /// /// If there are other `Rc` pointers to the same allocation, then `make_mut` will @@ -1800,31 +1801,52 @@ impl Rc { /// assert!(76 == *data); /// assert!(weak.upgrade().is_none()); /// ``` - #[cfg(not(no_global_oom_handling))] #[inline] #[stable(feature = "rc_unique", since = "1.4.0")] pub fn make_mut(this: &mut Self) -> &mut T { + let size_of_val = size_of_val::(&**this); + if Rc::strong_count(this) != 1 { // Gotta clone the data, there are other Rcs. - // Pre-allocate memory to allow writing the cloned value directly. - let mut rc = Self::new_uninit_in(this.alloc.clone()); - unsafe { - let data = Rc::get_mut_unchecked(&mut rc); - (**this).write_clone_into_raw(data.as_mut_ptr()); - *this = rc.assume_init(); - } + + let this_data_ref: &T = &**this; + // `in_progress` drops the allocation if we panic before finishing initializing it. + let mut in_progress: UniqueRcUninit = + UniqueRcUninit::new(this_data_ref, this.alloc.clone()); + + // Initialize with clone of this. + let initialized_clone = unsafe { + // Clone. If the clone panics, `in_progress` will be dropped and clean up. + this_data_ref.clone_to_uninit(in_progress.data_ptr()); + // Cast type of pointer, now that it is initialized. + in_progress.into_rc() + }; + + // Replace `this` with newly constructed Rc. + *this = initialized_clone; } else if Rc::weak_count(this) != 0 { // Can just steal the data, all that's left is Weaks - let mut rc = Self::new_uninit_in(this.alloc.clone()); + + // We don't need panic-protection like the above branch does, but we might as well + // use the same mechanism. + let mut in_progress: UniqueRcUninit = + UniqueRcUninit::new(&**this, this.alloc.clone()); unsafe { - let data = Rc::get_mut_unchecked(&mut rc); - data.as_mut_ptr().copy_from_nonoverlapping(&**this, 1); + // Initialize `in_progress` with move of **this. + // We have to express this in terms of bytes because `T: ?Sized`; there is no + // operation that just copies a value based on its `size_of_val()`. + ptr::copy_nonoverlapping( + ptr::from_ref(&**this).cast::(), + in_progress.data_ptr().cast::(), + size_of_val, + ); this.inner().dec_strong(); // Remove implicit strong-weak ref (no need to craft a fake // Weak here -- we know other Weaks can clean up for us) this.inner().dec_weak(); - ptr::write(this, rc.assume_init()); + // Replace `this` with newly constructed Rc that has the moved data. + ptr::write(this, in_progress.into_rc()); } } // This unsafety is ok because we're guaranteed that the pointer @@ -3686,3 +3708,67 @@ unsafe impl<#[may_dangle] T: ?Sized, A: Allocator> Drop for UniqueRc { } } } + +/// A unique owning pointer to a [`RcBox`] **that does not imply the contents are initialized,** +/// but will deallocate it (without dropping the value) when dropped. +/// +/// This is a helper for [`Rc::make_mut()`] to ensure correct cleanup on panic. +/// It is nearly a duplicate of `UniqueRc, A>` except that it allows `T: !Sized`, +/// which `MaybeUninit` does not. +#[cfg(not(no_global_oom_handling))] +struct UniqueRcUninit { + ptr: NonNull>, + layout_for_value: Layout, + alloc: Option, +} + +#[cfg(not(no_global_oom_handling))] +impl UniqueRcUninit { + /// Allocate a RcBox with layout suitable to contain `for_value` or a clone of it. + fn new(for_value: &T, alloc: A) -> UniqueRcUninit { + let layout = Layout::for_value(for_value); + let ptr = unsafe { + Rc::allocate_for_layout( + layout, + |layout_for_rcbox| alloc.allocate(layout_for_rcbox), + |mem| mem.with_metadata_of(ptr::from_ref(for_value) as *const RcBox), + ) + }; + Self { ptr: NonNull::new(ptr).unwrap(), layout_for_value: layout, alloc: Some(alloc) } + } + + /// Returns the pointer to be written into to initialize the [`Rc`]. + fn data_ptr(&mut self) -> *mut T { + let offset = data_offset_align(self.layout_for_value.align()); + unsafe { self.ptr.as_ptr().byte_add(offset) as *mut T } + } + + /// Upgrade this into a normal [`Rc`]. + /// + /// # Safety + /// + /// The data must have been initialized (by writing to [`Self::data_ptr()`]). + unsafe fn into_rc(mut self) -> Rc { + let ptr = self.ptr; + let alloc = self.alloc.take().unwrap(); + mem::forget(self); + // SAFETY: The pointer is valid as per `UniqueRcUninit::new`, and the caller is responsible + // for having initialized the data. + unsafe { Rc::from_ptr_in(ptr.as_ptr(), alloc) } + } +} + +#[cfg(not(no_global_oom_handling))] +impl Drop for UniqueRcUninit { + fn drop(&mut self) { + // SAFETY: + // * new() produced a pointer safe to deallocate. + // * We own the pointer unless into_rc() was called, which forgets us. + unsafe { + self.alloc + .take() + .unwrap() + .deallocate(self.ptr.cast(), rcbox_layout_for_value_layout(self.layout_for_value)); + } + } +} diff --git a/library/alloc/src/rc/tests.rs b/library/alloc/src/rc/tests.rs index 0f09be7721fa9..5e2e4beb94a2b 100644 --- a/library/alloc/src/rc/tests.rs +++ b/library/alloc/src/rc/tests.rs @@ -316,6 +316,24 @@ fn test_cowrc_clone_weak() { assert!(cow1_weak.upgrade().is_none()); } +/// This is similar to the doc-test for `Rc::make_mut()`, but on an unsized type (slice). +#[test] +fn test_cowrc_unsized() { + use std::rc::Rc; + + let mut data: Rc<[i32]> = Rc::new([10, 20, 30]); + + Rc::make_mut(&mut data)[0] += 1; // Won't clone anything + let mut other_data = Rc::clone(&data); // Won't clone inner data + Rc::make_mut(&mut data)[1] += 1; // Clones inner data + Rc::make_mut(&mut data)[2] += 1; // Won't clone anything + Rc::make_mut(&mut other_data)[0] *= 10; // Won't clone anything + + // Now `data` and `other_data` point to different allocations. + assert_eq!(*data, [11, 21, 31]); + assert_eq!(*other_data, [110, 20, 30]); +} + #[test] fn test_show() { let foo = Rc::new(75); diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index f9d884e0ea551..90672164cb932 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -10,6 +10,8 @@ use core::any::Any; use core::borrow; +#[cfg(not(no_global_oom_handling))] +use core::clone::CloneToUninit; use core::cmp::Ordering; use core::fmt; use core::hash::{Hash, Hasher}; @@ -30,8 +32,6 @@ use core::sync::atomic::Ordering::{Acquire, Relaxed, Release}; #[cfg(not(no_global_oom_handling))] use crate::alloc::handle_alloc_error; -#[cfg(not(no_global_oom_handling))] -use crate::alloc::WriteCloneIntoRaw; use crate::alloc::{AllocError, Allocator, Global, Layout}; use crate::borrow::{Cow, ToOwned}; use crate::boxed::Box; @@ -2150,7 +2150,8 @@ unsafe impl DerefPure for Arc {} #[unstable(feature = "receiver_trait", issue = "none")] impl Receiver for Arc {} -impl Arc { +#[cfg(not(no_global_oom_handling))] +impl Arc { /// Makes a mutable reference into the given `Arc`. /// /// If there are other `Arc` pointers to the same allocation, then `make_mut` will @@ -2201,10 +2202,11 @@ impl Arc { /// assert!(76 == *data); /// assert!(weak.upgrade().is_none()); /// ``` - #[cfg(not(no_global_oom_handling))] #[inline] #[stable(feature = "arc_unique", since = "1.4.0")] pub fn make_mut(this: &mut Self) -> &mut T { + let size_of_val = mem::size_of_val::(&**this); + // Note that we hold both a strong reference and a weak reference. // Thus, releasing our strong reference only will not, by itself, cause // the memory to be deallocated. @@ -2215,13 +2217,19 @@ impl Arc { // deallocated. if this.inner().strong.compare_exchange(1, 0, Acquire, Relaxed).is_err() { // Another strong pointer exists, so we must clone. - // Pre-allocate memory to allow writing the cloned value directly. - let mut arc = Self::new_uninit_in(this.alloc.clone()); - unsafe { - let data = Arc::get_mut_unchecked(&mut arc); - (**this).write_clone_into_raw(data.as_mut_ptr()); - *this = arc.assume_init(); - } + + let this_data_ref: &T = &**this; + // `in_progress` drops the allocation if we panic before finishing initializing it. + let mut in_progress: UniqueArcUninit = + UniqueArcUninit::new(this_data_ref, this.alloc.clone()); + + let initialized_clone = unsafe { + // Clone. If the clone panics, `in_progress` will be dropped and clean up. + this_data_ref.clone_to_uninit(in_progress.data_ptr()); + // Cast type of pointer, now that it is initialized. + in_progress.into_arc() + }; + *this = initialized_clone; } else if this.inner().weak.load(Relaxed) != 1 { // Relaxed suffices in the above because this is fundamentally an // optimization: we are always racing with weak pointers being @@ -2240,11 +2248,22 @@ impl Arc { let _weak = Weak { ptr: this.ptr, alloc: this.alloc.clone() }; // Can just steal the data, all that's left is Weaks - let mut arc = Self::new_uninit_in(this.alloc.clone()); + // + // We don't need panic-protection like the above branch does, but we might as well + // use the same mechanism. + let mut in_progress: UniqueArcUninit = + UniqueArcUninit::new(&**this, this.alloc.clone()); unsafe { - let data = Arc::get_mut_unchecked(&mut arc); - data.as_mut_ptr().copy_from_nonoverlapping(&**this, 1); - ptr::write(this, arc.assume_init()); + // Initialize `in_progress` with move of **this. + // We have to express this in terms of bytes because `T: ?Sized`; there is no + // operation that just copies a value based on its `size_of_val()`. + ptr::copy_nonoverlapping( + ptr::from_ref(&**this).cast::(), + in_progress.data_ptr().cast::(), + size_of_val, + ); + + ptr::write(this, in_progress.into_arc()); } } else { // We were the sole reference of either kind; bump back up the @@ -3809,6 +3828,68 @@ fn data_offset_align(align: usize) -> usize { layout.size() + layout.padding_needed_for(align) } +/// A unique owning pointer to a [`ArcInner`] **that does not imply the contents are initialized,** +/// but will deallocate it (without dropping the value) when dropped. +/// +/// This is a helper for [`Arc::make_mut()`] to ensure correct cleanup on panic. +#[cfg(not(no_global_oom_handling))] +struct UniqueArcUninit { + ptr: NonNull>, + layout_for_value: Layout, + alloc: Option, +} + +#[cfg(not(no_global_oom_handling))] +impl UniqueArcUninit { + /// Allocate a ArcInner with layout suitable to contain `for_value` or a clone of it. + fn new(for_value: &T, alloc: A) -> UniqueArcUninit { + let layout = Layout::for_value(for_value); + let ptr = unsafe { + Arc::allocate_for_layout( + layout, + |layout_for_arcinner| alloc.allocate(layout_for_arcinner), + |mem| mem.with_metadata_of(ptr::from_ref(for_value) as *const ArcInner), + ) + }; + Self { ptr: NonNull::new(ptr).unwrap(), layout_for_value: layout, alloc: Some(alloc) } + } + + /// Returns the pointer to be written into to initialize the [`Arc`]. + fn data_ptr(&mut self) -> *mut T { + let offset = data_offset_align(self.layout_for_value.align()); + unsafe { self.ptr.as_ptr().byte_add(offset) as *mut T } + } + + /// Upgrade this into a normal [`Arc`]. + /// + /// # Safety + /// + /// The data must have been initialized (by writing to [`Self::data_ptr()`]). + unsafe fn into_arc(mut self) -> Arc { + let ptr = self.ptr; + let alloc = self.alloc.take().unwrap(); + mem::forget(self); + // SAFETY: The pointer is valid as per `UniqueArcUninit::new`, and the caller is responsible + // for having initialized the data. + unsafe { Arc::from_ptr_in(ptr.as_ptr(), alloc) } + } +} + +#[cfg(not(no_global_oom_handling))] +impl Drop for UniqueArcUninit { + fn drop(&mut self) { + // SAFETY: + // * new() produced a pointer safe to deallocate. + // * We own the pointer unless into_arc() was called, which forgets us. + unsafe { + self.alloc.take().unwrap().deallocate( + self.ptr.cast(), + arcinner_layout_for_value_layout(self.layout_for_value), + ); + } + } +} + #[stable(feature = "arc_error", since = "1.52.0")] impl core::error::Error for Arc { #[allow(deprecated, deprecated_in_future)] diff --git a/library/alloc/tests/arc.rs b/library/alloc/tests/arc.rs index d564a30b10394..c37a80dca95c8 100644 --- a/library/alloc/tests/arc.rs +++ b/library/alloc/tests/arc.rs @@ -209,3 +209,21 @@ fn weak_may_dangle() { // `val` dropped here while still borrowed // borrow might be used here, when `val` is dropped and runs the `Drop` code for type `std::sync::Weak` } + +/// This is similar to the doc-test for `Arc::make_mut()`, but on an unsized type (slice). +#[test] +fn make_mut_unsized() { + use alloc::sync::Arc; + + let mut data: Arc<[i32]> = Arc::new([10, 20, 30]); + + Arc::make_mut(&mut data)[0] += 1; // Won't clone anything + let mut other_data = Arc::clone(&data); // Won't clone inner data + Arc::make_mut(&mut data)[1] += 1; // Clones inner data + Arc::make_mut(&mut data)[2] += 1; // Won't clone anything + Arc::make_mut(&mut other_data)[0] *= 10; // Won't clone anything + + // Now `data` and `other_data` point to different allocations. + assert_eq!(*data, [11, 21, 31]); + assert_eq!(*other_data, [110, 20, 30]); +} diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index d448c5338fc46..d7ce65f6c53a9 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -36,6 +36,9 @@ #![stable(feature = "rust1", since = "1.0.0")] +use crate::mem::{self, MaybeUninit}; +use crate::ptr; + /// A common trait for the ability to explicitly duplicate an object. /// /// Differs from [`Copy`] in that [`Copy`] is implicit and an inexpensive bit-wise copy, while @@ -204,6 +207,189 @@ pub struct AssertParamIsCopy { _field: crate::marker::PhantomData, } +/// A generalization of [`Clone`] to dynamically-sized types stored in arbitrary containers. +/// +/// This trait is implemented for all types implementing [`Clone`], and also [slices](slice) of all +/// such types. You may also implement this trait to enable cloning trait objects and custom DSTs +/// (structures containing dynamically-sized fields). +/// +/// # Safety +/// +/// Implementations must ensure that when `.clone_to_uninit(dst)` returns normally rather than +/// panicking, it always leaves `*dst` initialized as a valid value of type `Self`. +/// +/// # See also +/// +/// * [`Clone::clone_from`] is a safe function which may be used instead when `Self` is a [`Sized`] +/// and the destination is already initialized; it may be able to reuse allocations owned by +/// the destination. +/// * [`ToOwned`], which allocates a new destination container. +/// +/// [`ToOwned`]: ../../std/borrow/trait.ToOwned.html +#[unstable(feature = "clone_to_uninit", issue = "126799")] +pub unsafe trait CloneToUninit { + /// Performs copy-assignment from `self` to `dst`. + /// + /// This is analogous to to `std::ptr::write(dst, self.clone())`, + /// except that `self` may be a dynamically-sized type ([`!Sized`](Sized)). + /// + /// Before this function is called, `dst` may point to uninitialized memory. + /// After this function is called, `dst` will point to initialized memory; it will be + /// sound to create a `&Self` reference from the pointer. + /// + /// # Safety + /// + /// Behavior is undefined if any of the following conditions are violated: + /// + /// * `dst` must be [valid] for writes. + /// * `dst` must be properly aligned. + /// * `dst` must have the same [pointer metadata] (slice length or `dyn` vtable) as `self`. + /// + /// [valid]: ptr#safety + /// [pointer metadata]: crate::ptr::metadata() + /// + /// # Panics + /// + /// This function may panic. (For example, it might panic if memory allocation for a clone + /// of a value owned by `self` fails.) + /// If the call panics, then `*dst` should be treated as uninitialized memory; it must not be + /// read or dropped, because even if it was previously valid, it may have been partially + /// overwritten. + /// + /// The caller may also need to take care to deallocate the allocation pointed to by `dst`, + /// if applicable, to avoid a memory leak, and may need to take other precautions to ensure + /// soundness in the presence of unwinding. + /// + /// Implementors should avoid leaking values by, upon unwinding, dropping all component values + /// that might have already been created. (For example, if a `[Foo]` of length 3 is being + /// cloned, and the second of the three calls to `Foo::clone()` unwinds, then the first `Foo` + /// cloned should be dropped.) + unsafe fn clone_to_uninit(&self, dst: *mut Self); +} + +#[unstable(feature = "clone_to_uninit", issue = "126799")] +unsafe impl CloneToUninit for T { + default unsafe fn clone_to_uninit(&self, dst: *mut Self) { + // SAFETY: The safety conditions of clone_to_uninit() are a superset of those of + // ptr::write(). + unsafe { + // We hope the optimizer will figure out to create the cloned value in-place, + // skipping ever storing it on the stack and the copy to the destination. + ptr::write(dst, self.clone()); + } + } +} + +// Specialized implementation for types that are [`Copy`], not just [`Clone`], +// and can therefore be copied bitwise. +#[unstable(feature = "clone_to_uninit", issue = "126799")] +unsafe impl CloneToUninit for T { + unsafe fn clone_to_uninit(&self, dst: *mut Self) { + // SAFETY: The safety conditions of clone_to_uninit() are a superset of those of + // ptr::copy_nonoverlapping(). + unsafe { + ptr::copy_nonoverlapping(self, dst, 1); + } + } +} + +#[unstable(feature = "clone_to_uninit", issue = "126799")] +unsafe impl CloneToUninit for [T] { + #[cfg_attr(debug_assertions, track_caller)] + default unsafe fn clone_to_uninit(&self, dst: *mut Self) { + let len = self.len(); + // This is the most likely mistake to make, so check it as a debug assertion. + debug_assert_eq!( + len, + dst.len(), + "clone_to_uninit() source and destination must have equal lengths", + ); + + // SAFETY: The produced `&mut` is valid because: + // * The caller is obligated to provide a pointer which is valid for writes. + // * All bytes pointed to are in MaybeUninit, so we don't care about the memory's + // initialization status. + let uninit_ref = unsafe { &mut *(dst as *mut [MaybeUninit]) }; + + // Copy the elements + let mut initializing = InitializingSlice::from_fully_uninit(uninit_ref); + for element_ref in self.iter() { + // If the clone() panics, `initializing` will take care of the cleanup. + initializing.push(element_ref.clone()); + } + // If we reach here, then the entire slice is initialized, and we've satisfied our + // responsibilities to the caller. Disarm the cleanup guard by forgetting it. + mem::forget(initializing); + } +} + +#[unstable(feature = "clone_to_uninit", issue = "126799")] +unsafe impl CloneToUninit for [T] { + #[cfg_attr(debug_assertions, track_caller)] + unsafe fn clone_to_uninit(&self, dst: *mut Self) { + let len = self.len(); + // This is the most likely mistake to make, so check it as a debug assertion. + debug_assert_eq!( + len, + dst.len(), + "clone_to_uninit() source and destination must have equal lengths", + ); + + // SAFETY: The safety conditions of clone_to_uninit() are a superset of those of + // ptr::copy_nonoverlapping(). + unsafe { + ptr::copy_nonoverlapping(self.as_ptr(), dst.as_mut_ptr(), len); + } + } +} + +/// Ownership of a collection of values stored in a non-owned `[MaybeUninit]`, some of which +/// are not yet initialized. This is sort of like a `Vec` that doesn't own its allocation. +/// Its responsibility is to provide cleanup on unwind by dropping the values that *are* +/// initialized, unless disarmed by forgetting. +/// +/// This is a helper for `impl CloneToUninit for [T]`. +struct InitializingSlice<'a, T> { + data: &'a mut [MaybeUninit], + /// Number of elements of `*self.data` that are initialized. + initialized_len: usize, +} + +impl<'a, T> InitializingSlice<'a, T> { + #[inline] + fn from_fully_uninit(data: &'a mut [MaybeUninit]) -> Self { + Self { data, initialized_len: 0 } + } + + /// Push a value onto the end of the initialized part of the slice. + /// + /// # Panics + /// + /// Panics if the slice is already fully initialized. + #[inline] + fn push(&mut self, value: T) { + MaybeUninit::write(&mut self.data[self.initialized_len], value); + self.initialized_len += 1; + } +} + +impl<'a, T> Drop for InitializingSlice<'a, T> { + #[cold] // will only be invoked on unwind + fn drop(&mut self) { + let initialized_slice = ptr::slice_from_raw_parts_mut( + MaybeUninit::slice_as_mut_ptr(self.data), + self.initialized_len, + ); + // SAFETY: + // * the pointer is valid because it was made from a mutable reference + // * `initialized_len` counts the initialized elements as an invariant of this type, + // so each of the pointed-to elements is initialized and may be dropped. + unsafe { + ptr::drop_in_place::<[T]>(initialized_slice); + } + } +} + /// Implementations of `Clone` for primitive types. /// /// Implementations that cannot be described in Rust diff --git a/library/core/tests/clone.rs b/library/core/tests/clone.rs index 64193e1155890..dd43bdc9f570a 100644 --- a/library/core/tests/clone.rs +++ b/library/core/tests/clone.rs @@ -1,3 +1,7 @@ +use core::clone::CloneToUninit; +use core::mem::MaybeUninit; +use core::sync::atomic::{AtomicUsize, Ordering::Relaxed}; + #[test] #[allow(suspicious_double_ref_op)] fn test_borrowed_clone() { @@ -14,3 +18,64 @@ fn test_clone_from() { b.clone_from(&a); assert_eq!(*b, 5); } + +#[test] +fn test_clone_to_uninit_slice_success() { + // Using `String`s to exercise allocation and Drop of the individual elements; + // if something is aliased or double-freed, at least Miri will catch that. + let a: [String; 3] = ["a", "b", "c"].map(String::from); + + let mut storage: MaybeUninit<[String; 3]> = MaybeUninit::uninit(); + let b: [String; 3] = unsafe { + a[..].clone_to_uninit(storage.as_mut_ptr() as *mut [String]); + storage.assume_init() + }; + + assert_eq!(a, b); +} + +#[test] +#[cfg(panic = "unwind")] +fn test_clone_to_uninit_slice_drops_on_panic() { + /// A static counter is OK to use as long as _this one test_ isn't run several times in + /// multiple threads. + static COUNTER: AtomicUsize = AtomicUsize::new(0); + /// Counts how many instances are live, and panics if a fifth one is created + struct CountsDropsAndPanics {} + impl CountsDropsAndPanics { + fn new() -> Self { + COUNTER.fetch_add(1, Relaxed); + Self {} + } + } + impl Clone for CountsDropsAndPanics { + fn clone(&self) -> Self { + if COUNTER.load(Relaxed) == 4 { panic!("intentional panic") } else { Self::new() } + } + } + impl Drop for CountsDropsAndPanics { + fn drop(&mut self) { + COUNTER.fetch_sub(1, Relaxed); + } + } + + let a: [CountsDropsAndPanics; 3] = core::array::from_fn(|_| CountsDropsAndPanics::new()); + assert_eq!(COUNTER.load(Relaxed), 3); + + let panic_payload = std::panic::catch_unwind(|| { + let mut storage: MaybeUninit<[CountsDropsAndPanics; 3]> = MaybeUninit::uninit(); + // This should panic halfway through + unsafe { + a[..].clone_to_uninit(storage.as_mut_ptr() as *mut [CountsDropsAndPanics]); + } + }) + .unwrap_err(); + assert_eq!(panic_payload.downcast().unwrap(), Box::new("intentional panic")); + + // Check for lack of leak, which is what this test is looking for + assert_eq!(COUNTER.load(Relaxed), 3, "leaked during clone!"); + + // Might as well exercise the rest of the drops + drop(a); + assert_eq!(COUNTER.load(Relaxed), 0); +} diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 6845a630dff39..3a2c98db0d50a 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -8,6 +8,7 @@ #![feature(async_iterator)] #![feature(bigint_helper_methods)] #![feature(cell_update)] +#![feature(clone_to_uninit)] #![feature(const_align_offset)] #![feature(const_align_of_val_raw)] #![feature(const_black_box)] @@ -53,6 +54,7 @@ #![feature(slice_split_once)] #![feature(split_as_slice)] #![feature(maybe_uninit_fill)] +#![feature(maybe_uninit_slice)] #![feature(maybe_uninit_uninit_array)] #![feature(maybe_uninit_write_slice)] #![feature(maybe_uninit_uninit_array_transpose)] diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 5459b4c7b295f..8a2bc3b9d484b 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -26,7 +26,7 @@ use crate::core::builder::{Builder, Compiler, Kind, RunConfig, ShouldRun, Step}; use crate::core::config::flags::get_completion; use crate::core::config::flags::Subcommand; use crate::core::config::TargetSelection; -use crate::utils::exec::BootstrapCommand; +use crate::utils::exec::{BootstrapCommand, OutputMode}; use crate::utils::helpers::{ self, add_link_lib_path, add_rustdoc_cargo_linker_args, dylib_path, dylib_path_var, linker_args, linker_flags, output, t, target_supports_cranelift_backend, up_to_date, @@ -156,7 +156,10 @@ You can skip linkcheck with --skip src/tools/linkchecker" let _guard = builder.msg(Kind::Test, compiler.stage, "Linkcheck", bootstrap_host, bootstrap_host); let _time = helpers::timeit(builder); - builder.run_delaying_failure(linkchecker.arg(builder.out.join(host.triple).join("doc"))); + builder.run_tracked( + BootstrapCommand::from(linkchecker.arg(builder.out.join(host.triple).join("doc"))) + .delay_failure(), + ); } fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { @@ -213,8 +216,11 @@ impl Step for HtmlCheck { builder, )); - builder.run_delaying_failure( - builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)), + builder.run_tracked( + BootstrapCommand::from( + builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)), + ) + .delay_failure(), ); } } @@ -261,7 +267,7 @@ impl Step for Cargotest { .env("RUSTC", builder.rustc(compiler)) .env("RUSTDOC", builder.rustdoc(compiler)); add_rustdoc_cargo_linker_args(cmd, builder, compiler.host, LldThreads::No); - builder.run_delaying_failure(cmd); + builder.run_tracked(BootstrapCommand::from(cmd).delay_failure()); } } @@ -813,7 +819,7 @@ impl Step for RustdocTheme { .env("RUSTC_BOOTSTRAP", "1"); cmd.args(linker_args(builder, self.compiler.host, LldThreads::No)); - builder.run_delaying_failure(&mut cmd); + builder.run_tracked(BootstrapCommand::from(&mut cmd).delay_failure()); } } @@ -1093,7 +1099,7 @@ HELP: to skip test's attempt to check tidiness, pass `--skip src/tools/tidy` to } builder.info("tidy check"); - builder.run_delaying_failure(&mut cmd); + builder.run_tracked(BootstrapCommand::from(&mut cmd).delay_failure()); builder.info("x.py completions check"); let [bash, zsh, fish, powershell] = ["x.py.sh", "x.py.zsh", "x.py.fish", "x.py.ps1"] @@ -2179,7 +2185,8 @@ impl BookTest { compiler.host, ); let _time = helpers::timeit(builder); - let toolstate = if builder.run_delaying_failure(&mut rustbook_cmd) { + let cmd = BootstrapCommand::from(&mut rustbook_cmd).delay_failure(); + let toolstate = if builder.run_tracked(cmd).is_success() { ToolState::TestPass } else { ToolState::TestFail @@ -2312,7 +2319,8 @@ impl Step for ErrorIndex { let guard = builder.msg(Kind::Test, compiler.stage, "error-index", compiler.host, compiler.host); let _time = helpers::timeit(builder); - builder.run_quiet(&mut tool); + builder + .run_tracked(BootstrapCommand::from(&mut tool).output_mode(OutputMode::OnlyOnFailure)); drop(guard); // The tests themselves need to link to std, so make sure it is // available. @@ -2341,11 +2349,11 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) -> let test_args = builder.config.test_args().join(" "); cmd.arg("--test-args").arg(test_args); - if builder.config.verbose_tests { - builder.run_delaying_failure(&mut cmd) - } else { - builder.run_quiet_delaying_failure(&mut cmd) + let mut cmd = BootstrapCommand::from(&mut cmd).delay_failure(); + if !builder.config.verbose_tests { + cmd = cmd.quiet(); } + builder.run_tracked(cmd).is_success() } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -2370,7 +2378,8 @@ impl Step for RustcGuide { let src = builder.src.join(relative_path); let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook); - let toolstate = if builder.run_delaying_failure(rustbook_cmd.arg("linkcheck").arg(&src)) { + let cmd = BootstrapCommand::from(rustbook_cmd.arg("linkcheck").arg(&src)).delay_failure(); + let toolstate = if builder.run_tracked(cmd).is_success() { ToolState::TestPass } else { ToolState::TestFail @@ -2984,7 +2993,7 @@ impl Step for Bootstrap { .current_dir(builder.src.join("src/bootstrap/")); // NOTE: we intentionally don't pass test_args here because the args for unittest and cargo test are mutually incompatible. // Use `python -m unittest` manually if you want to pass arguments. - builder.run_delaying_failure(&mut check_bootstrap); + builder.run_tracked(BootstrapCommand::from(&mut check_bootstrap).delay_failure()); let mut cmd = Command::new(&builder.initial_cargo); cmd.arg("test") @@ -3061,7 +3070,7 @@ impl Step for TierCheck { self.compiler.host, self.compiler.host, ); - builder.run_delaying_failure(&mut cargo.into()); + builder.run_tracked(BootstrapCommand::from(&mut cargo.into()).delay_failure()); } } @@ -3147,7 +3156,7 @@ impl Step for RustInstaller { cmd.env("CARGO", &builder.initial_cargo); cmd.env("RUSTC", &builder.initial_rustc); cmd.env("TMP_DIR", &tmpdir); - builder.run_delaying_failure(&mut cmd); + builder.run_tracked(BootstrapCommand::from(&mut cmd).delay_failure()); } fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 449d8c128ec63..9b414b24d2f3a 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -23,7 +23,7 @@ use std::fmt::Display; use std::fs::{self, File}; use std::io; use std::path::{Path, PathBuf}; -use std::process::{Command, Output, Stdio}; +use std::process::{Command, Stdio}; use std::str; use std::sync::OnceLock; @@ -41,7 +41,7 @@ use crate::core::builder::Kind; use crate::core::config::{flags, LldMode}; use crate::core::config::{DryRun, Target}; use crate::core::config::{LlvmLibunwind, TargetSelection}; -use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand, OutputMode}; +use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand, CommandOutput, OutputMode}; use crate::utils::helpers::{self, dir_is_empty, exe, libdir, mtime, output, symlink_dir}; mod core; @@ -585,8 +585,8 @@ impl Build { BootstrapCommand::from(submodule_git().args(["diff-index", "--quiet", "HEAD"])) .allow_failure() .output_mode(match self.is_verbose() { - true => OutputMode::PrintAll, - false => OutputMode::PrintOutput, + true => OutputMode::All, + false => OutputMode::OnlyOutput, }), ); if has_local_modifications { @@ -958,73 +958,36 @@ impl Build { }) } - /// Runs a command, printing out nice contextual information if it fails. - fn run(&self, cmd: &mut Command) { - self.run_cmd(BootstrapCommand::from(cmd).fail_fast().output_mode( - match self.is_verbose() { - true => OutputMode::PrintAll, - false => OutputMode::PrintOutput, - }, - )); - } - - /// Runs a command, printing out contextual info if it fails, and delaying errors until the build finishes. - pub(crate) fn run_delaying_failure(&self, cmd: &mut Command) -> bool { - self.run_cmd(BootstrapCommand::from(cmd).delay_failure().output_mode( - match self.is_verbose() { - true => OutputMode::PrintAll, - false => OutputMode::PrintOutput, - }, - )) - } - - /// Runs a command, printing out nice contextual information if it fails. - fn run_quiet(&self, cmd: &mut Command) { - self.run_cmd( - BootstrapCommand::from(cmd).fail_fast().output_mode(OutputMode::SuppressOnSuccess), - ); - } - - /// Runs a command, printing out nice contextual information if it fails. - /// Exits if the command failed to execute at all, otherwise returns its - /// `status.success()`. - fn run_quiet_delaying_failure(&self, cmd: &mut Command) -> bool { - self.run_cmd( - BootstrapCommand::from(cmd).delay_failure().output_mode(OutputMode::SuppressOnSuccess), - ) - } - - /// A centralized function for running commands that do not return output. - pub(crate) fn run_cmd<'a, C: Into>>(&self, cmd: C) -> bool { + /// Execute a command and return its output. + fn run_tracked(&self, command: BootstrapCommand<'_>) -> CommandOutput { if self.config.dry_run() { - return true; + return CommandOutput::default(); } - let command = cmd.into(); self.verbose(|| println!("running: {command:?}")); - let (output, print_error) = match command.output_mode { - mode @ (OutputMode::PrintAll | OutputMode::PrintOutput) => ( - command.command.status().map(|status| Output { - status, - stdout: Vec::new(), - stderr: Vec::new(), - }), - matches!(mode, OutputMode::PrintAll), + let output_mode = command.output_mode.unwrap_or_else(|| match self.is_verbose() { + true => OutputMode::All, + false => OutputMode::OnlyOutput, + }); + let (output, print_error): (io::Result, bool) = match output_mode { + mode @ (OutputMode::All | OutputMode::OnlyOutput) => ( + command.command.status().map(|status| status.into()), + matches!(mode, OutputMode::All), ), - OutputMode::SuppressOnSuccess => (command.command.output(), true), + OutputMode::OnlyOnFailure => (command.command.output().map(|o| o.into()), true), }; let output = match output { Ok(output) => output, Err(e) => fail(&format!("failed to execute command: {:?}\nerror: {}", command, e)), }; - let result = if !output.status.success() { + if !output.is_success() { if print_error { println!( "\n\nCommand did not execute successfully.\ - \nExpected success, got: {}", - output.status, + \nExpected success, got: {}", + output.status(), ); if !self.is_verbose() { @@ -1034,37 +997,45 @@ impl Build { self.verbose(|| { println!( "\nSTDOUT ----\n{}\n\ - STDERR ----\n{}\n", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) + STDERR ----\n{}\n", + output.stdout(), + output.stderr(), ) }); } - Err(()) - } else { - Ok(()) - }; - match result { - Ok(_) => true, - Err(_) => { - match command.failure_behavior { - BehaviorOnFailure::DelayFail => { - if self.fail_fast { - exit!(1); - } - - let mut failures = self.delayed_failures.borrow_mut(); - failures.push(format!("{command:?}")); - } - BehaviorOnFailure::Exit => { + match command.failure_behavior { + BehaviorOnFailure::DelayFail => { + if self.fail_fast { exit!(1); } - BehaviorOnFailure::Ignore => {} + + let mut failures = self.delayed_failures.borrow_mut(); + failures.push(format!("{command:?}")); + } + BehaviorOnFailure::Exit => { + exit!(1); } - false + BehaviorOnFailure::Ignore => {} } } + output + } + + /// Runs a command, printing out nice contextual information if it fails. + fn run(&self, cmd: &mut Command) { + self.run_cmd(BootstrapCommand::from(cmd).fail_fast().output_mode( + match self.is_verbose() { + true => OutputMode::All, + false => OutputMode::OnlyOutput, + }, + )); + } + + /// A centralized function for running commands that do not return output. + pub(crate) fn run_cmd<'a, C: Into>>(&self, cmd: C) -> bool { + let command = cmd.into(); + self.run_tracked(command).is_success() } /// Check if verbosity is greater than the `level` diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 0aede2022badd..e8c588b75b381 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -1,4 +1,4 @@ -use std::process::Command; +use std::process::{Command, ExitStatus, Output}; /// What should be done when the command fails. #[derive(Debug, Copy, Clone)] @@ -16,11 +16,11 @@ pub enum BehaviorOnFailure { pub enum OutputMode { /// Print both the output (by inheriting stdout/stderr) and also the command itself, if it /// fails. - PrintAll, + All, /// Print the output (by inheriting stdout/stderr). - PrintOutput, + OnlyOutput, /// Suppress the output if the command succeeds, otherwise print the output. - SuppressOnSuccess, + OnlyOnFailure, } /// Wrapper around `std::process::Command`. @@ -28,7 +28,7 @@ pub enum OutputMode { pub struct BootstrapCommand<'a> { pub command: &'a mut Command, pub failure_behavior: BehaviorOnFailure, - pub output_mode: OutputMode, + pub output_mode: Option, } impl<'a> BootstrapCommand<'a> { @@ -44,17 +44,62 @@ impl<'a> BootstrapCommand<'a> { Self { failure_behavior: BehaviorOnFailure::Ignore, ..self } } + /// Do not print the output of the command, unless it fails. + pub fn quiet(self) -> Self { + self.output_mode(OutputMode::OnlyOnFailure) + } + pub fn output_mode(self, output_mode: OutputMode) -> Self { - Self { output_mode, ..self } + Self { output_mode: Some(output_mode), ..self } } } impl<'a> From<&'a mut Command> for BootstrapCommand<'a> { fn from(command: &'a mut Command) -> Self { - Self { - command, - failure_behavior: BehaviorOnFailure::Exit, - output_mode: OutputMode::PrintAll, - } + Self { command, failure_behavior: BehaviorOnFailure::Exit, output_mode: None } + } +} + +/// Represents the output of an executed process. +#[allow(unused)] +pub struct CommandOutput(Output); + +impl CommandOutput { + pub fn is_success(&self) -> bool { + self.0.status.success() + } + + pub fn is_failure(&self) -> bool { + !self.is_success() + } + + pub fn status(&self) -> ExitStatus { + self.0.status + } + + pub fn stdout(&self) -> String { + String::from_utf8(self.0.stdout.clone()).expect("Cannot parse process stdout as UTF-8") + } + + pub fn stderr(&self) -> String { + String::from_utf8(self.0.stderr.clone()).expect("Cannot parse process stderr as UTF-8") + } +} + +impl Default for CommandOutput { + fn default() -> Self { + Self(Output { status: Default::default(), stdout: vec![], stderr: vec![] }) + } +} + +impl From for CommandOutput { + fn from(output: Output) -> Self { + Self(output) + } +} + +impl From for CommandOutput { + fn from(status: ExitStatus) -> Self { + Self(Output { status, stdout: vec![], stderr: vec![] }) } } diff --git a/tests/ui/attributes/dump-preds.rs b/tests/ui/attributes/dump-preds.rs new file mode 100644 index 0000000000000..1e15ff2f9bdb3 --- /dev/null +++ b/tests/ui/attributes/dump-preds.rs @@ -0,0 +1,20 @@ +//@ normalize-stderr-test "DefId\(.+?\)" -> "DefId(..)" + +#![feature(rustc_attrs)] + +#[rustc_dump_predicates] +trait Trait: Iterator +//~^ ERROR rustc_dump_predicates +where + String: From +{ + #[rustc_dump_predicates] + #[rustc_dump_item_bounds] + type Assoc: std::ops::Deref + //~^ ERROR rustc_dump_predicates + //~| ERROR rustc_dump_item_bounds + where + Self::Assoc<()>: Copy; +} + +fn main() {} diff --git a/tests/ui/attributes/dump-preds.stderr b/tests/ui/attributes/dump-preds.stderr new file mode 100644 index 0000000000000..26834376e7612 --- /dev/null +++ b/tests/ui/attributes/dump-preds.stderr @@ -0,0 +1,39 @@ +error: rustc_dump_predicates + --> $DIR/dump-preds.rs:6:1 + | +LL | trait Trait: Iterator + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: Binder { value: TraitPredicate(, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(<::Item as std::marker::Copy>, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(>, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(>, polarity:Positive), bound_vars: [] } + +error: rustc_dump_predicates + --> $DIR/dump-preds.rs:13:5 + | +LL | type Assoc: std::ops::Deref + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: Binder { value: TraitPredicate(, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(<::Item as std::marker::Copy>, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(>, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(>, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(

, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(

, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(<>::Assoc<()> as std::marker::Copy>, polarity:Positive), bound_vars: [] } + +error: rustc_dump_item_bounds + --> $DIR/dump-preds.rs:13:5 + | +LL | type Assoc: std::ops::Deref + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: Binder { value: ProjectionPredicate(AliasTerm { args: [Alias(Projection, AliasTy { args: [Self/#0, T/#1, P/#2], def_id: DefId(..) })], def_id: DefId(..) }, Term::Ty(())), bound_vars: [] } + = note: Binder { value: TraitPredicate(<>::Assoc

as std::ops::Deref>, polarity:Positive), bound_vars: [] } + = note: Binder { value: TraitPredicate(<>::Assoc

as std::marker::Sized>, polarity:Positive), bound_vars: [] } + +error: aborting due to 3 previous errors +