From fb2c27d73f3528da5585bc7e49254164e0a2a5a2 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Wed, 28 Dec 2022 14:31:31 -0800 Subject: [PATCH 01/10] CFI: Monomorphize transparent ADTs before typeid Monomorphise `#[repr(transparent)]` parameterized ADTs before turning them into an Itanium mangled String. `#[repr(transparent)]` ADTs currently use the single field to represent them in their CFI type ID to ensure that they are compatible. However, if that type involves a type parameter instantiated at the ADT level, as in `ManuallyDrop`, this will currently ICE as the `Parameter` type cannot be mangled. Since this happens at lowering time, it should always be concrete after substitution. Fixes #106230 --- .../src/typeid/typeid_itanium_cxx_abi.rs | 5 ++++- ...i-emit-type-metadata-id-itanium-cxx-abi.rs | 22 ++++++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs index 493e31a688fcc..e9b85705086b5 100644 --- a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs +++ b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs @@ -164,6 +164,7 @@ fn encode_const<'tcx>( /// Encodes a FnSig using the Itanium C++ ABI with vendor extended type qualifiers and types for /// Rust types that are not used at the FFI boundary. +#[instrument(level = "trace", skip(tcx, dict))] fn encode_fnsig<'tcx>( tcx: TyCtxt<'tcx>, fn_sig: &FnSig<'tcx>, @@ -653,6 +654,7 @@ fn encode_ty<'tcx>( // Transforms a ty:Ty for being encoded and used in the substitution dictionary. It transforms all // c_void types into unit types unconditionally, and generalizes all pointers if // TransformTyOptions::GENERALIZE_POINTERS option is set. +#[instrument(level = "trace", skip(tcx))] fn transform_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, options: TransformTyOptions) -> Ty<'tcx> { let mut ty = ty; @@ -698,7 +700,7 @@ fn transform_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, options: TransformTyOptio !is_zst }); if let Some(field) = field { - let ty0 = tcx.type_of(field.did); + let ty0 = tcx.bound_type_of(field.did).subst(tcx, substs); // Generalize any repr(transparent) user-defined type that is either a pointer // or reference, and either references itself or any other type that contains or // references itself, to avoid a reference cycle. @@ -827,6 +829,7 @@ fn transform_substs<'tcx>( /// Returns a type metadata identifier for the specified FnAbi using the Itanium C++ ABI with vendor /// extended type qualifiers and types for Rust types that are not used at the FFI boundary. +#[instrument(level = "trace", skip(tcx))] pub fn typeid_for_fnabi<'tcx>( tcx: TyCtxt<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, diff --git a/src/test/codegen/sanitizer-cfi-emit-type-metadata-id-itanium-cxx-abi.rs b/src/test/codegen/sanitizer-cfi-emit-type-metadata-id-itanium-cxx-abi.rs index ece2adbdf4325..b9c33914360ba 100644 --- a/src/test/codegen/sanitizer-cfi-emit-type-metadata-id-itanium-cxx-abi.rs +++ b/src/test/codegen/sanitizer-cfi-emit-type-metadata-id-itanium-cxx-abi.rs @@ -131,6 +131,13 @@ pub struct Type13<'a> { member3: &'a Type13<'a>, } +// Helper type to allow `Type14` to be a unique ID +pub struct Bar; + +// repr(transparent) parameterized type +#[repr(transparent)] +pub struct Type14(T); + pub fn foo0(_: ()) { } // CHECK: define{{.*}}foo0{{.*}}!type ![[TYPE0:[0-9]+]] pub fn foo1(_: c_void, _: ()) { } @@ -425,6 +432,12 @@ pub fn foo145(_: Type13, _: Type13) { } // CHECK: define{{.*}}foo145{{.*}}!type ![[TYPE145:[0-9]+]] pub fn foo146(_: Type13, _: Type13, _: Type13) { } // CHECK: define{{.*}}foo146{{.*}}!type ![[TYPE146:[0-9]+]] +pub fn foo147(_: Type14) { } +// CHECK: define{{.*}}foo147{{.*}}!type ![[TYPE147:[0-9]+]] +pub fn foo148(_: Type14, _: Type14) { } +// CHECK: define{{.*}}foo148{{.*}}!type ![[TYPE148:[0-9]+]] +pub fn foo149(_: Type14, _: Type14, _: Type14) { } +// CHECK: define{{.*}}foo149{{.*}}!type ![[TYPE149:[0-9]+]] // CHECK: ![[TYPE0]] = !{i64 0, !"_ZTSFvvE"} // CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvvvE"} @@ -570,6 +583,9 @@ pub fn foo146(_: Type13, _: Type13, _: Type13) { } // CHECK: ![[TYPE141]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_51sanitizer_cfi_emit_type_metadata_id_itanium_cxx_abi3FooE"} // CHECK: ![[TYPE142]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_51sanitizer_cfi_emit_type_metadata_id_itanium_cxx_abi3FooS_E"} // CHECK: ![[TYPE143]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_51sanitizer_cfi_emit_type_metadata_id_itanium_cxx_abi3FooS_S_E"} -// CHECK: ![[TYPE144]] = !{i64 0, !"_ZTSFvu3refIu3refIvEEE"} -// CHECK: ![[TYPE145]] = !{i64 0, !"_ZTSFvu3refIu3refIvEES0_E"} -// CHECK: ![[TYPE146]] = !{i64 0, !"_ZTSFvu3refIu3refIvEES0_S0_E"} +// CHECK: ![[TYPE144]] = !{i64 0, !"_ZTSFvu3refIvEE"} +// CHECK: ![[TYPE145]] = !{i64 0, !"_ZTSFvu3refIvES_E"} +// CHECK: ![[TYPE146]] = !{i64 0, !"_ZTSFvu3refIvES_S_E"} +// CHECK: ![[TYPE147]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_51sanitizer_cfi_emit_type_metadata_id_itanium_cxx_abi3BarE +// CHECK: ![[TYPE148]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_51sanitizer_cfi_emit_type_metadata_id_itanium_cxx_abi3BarS_E +// CHECK: ![[TYPE149]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_51sanitizer_cfi_emit_type_metadata_id_itanium_cxx_abi3BarS_S_E From cbede85538d3ee59819c5d069ffe8d2dd7931749 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 29 Dec 2022 20:57:54 +0000 Subject: [PATCH 02/10] Support `x clean --stage 1 rustc_query_impl` Previously, clean only supported `--stage 0` for specific crates. The new `crate_description` function generates a string that looks like ``` : {rustc_query_impl} ``` --- src/bootstrap/builder.rs | 19 +++++++++++++++++++ src/bootstrap/clean.rs | 22 ++++++++++------------ src/bootstrap/compile.rs | 15 +++++++++++---- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 707e4169002d9..97b353b5462f5 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -109,6 +109,25 @@ impl RunConfig<'_> { } } +/// A description of the crates in this set, suitable for passing to `builder.info`. +/// +/// `crates` should be generated by [`RunConfig::cargo_crates_in_set`]. +pub fn crate_description(crates: Interned>) -> String { + if crates.is_empty() { + return "".into(); + } + + let mut descr = String::from(": {"); + for krate in &*crates { + write!(descr, "{}, ", krate.strip_prefix("-p=").unwrap()).unwrap(); + } + + descr.pop(); + descr.pop(); + descr.push('}'); + descr +} + struct StepDescription { default: bool, only_hosts: bool, diff --git a/src/bootstrap/clean.rs b/src/bootstrap/clean.rs index 8e363ee1290ee..d887495d633f0 100644 --- a/src/bootstrap/clean.rs +++ b/src/bootstrap/clean.rs @@ -9,11 +9,10 @@ use std::fs; use std::io::{self, ErrorKind}; use std::path::Path; -use crate::builder::{Builder, RunConfig, ShouldRun, Step}; +use crate::builder::{crate_description, Builder, RunConfig, ShouldRun, Step}; use crate::cache::Interned; -use crate::config::TargetSelection; use crate::util::t; -use crate::{Build, Mode, Subcommand}; +use crate::{Build, Compiler, Mode, Subcommand}; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct CleanAll {} @@ -40,7 +39,7 @@ macro_rules! clean_crate_tree { ( $( $name:ident, $mode:path, $root_crate:literal);+ $(;)? ) => { $( #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct $name { - target: TargetSelection, + compiler: Compiler, crates: Interned>, } @@ -54,22 +53,21 @@ macro_rules! clean_crate_tree { fn make_run(run: RunConfig<'_>) { let builder = run.builder; - if builder.top_stage != 0 { - panic!("non-stage-0 clean not supported for individual crates"); - } - builder.ensure(Self { crates: run.cargo_crates_in_set(), target: run.target }); + let compiler = builder.compiler(builder.top_stage, run.target); + builder.ensure(Self { crates: run.cargo_crates_in_set(), compiler }); } fn run(self, builder: &Builder<'_>) -> Self::Output { - let compiler = builder.compiler(0, self.target); - let mut cargo = builder.bare_cargo(compiler, $mode, self.target, "clean"); + let compiler = self.compiler; + let target = compiler.host; + let mut cargo = builder.bare_cargo(compiler, $mode, target, "clean"); for krate in &*self.crates { cargo.arg(krate); } builder.info(&format!( - "Cleaning stage{} {} artifacts ({} -> {})", - compiler.stage, stringify!($name).to_lowercase(), &compiler.host, self.target + "Cleaning stage{} {} artifacts ({} -> {}){}", + compiler.stage, stringify!($name).to_lowercase(), &compiler.host, target, crate_description(self.crates), )); // NOTE: doesn't use `run_cargo` because we don't want to save a stamp file, diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 1030247b890c3..5bf5683f85dea 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -18,6 +18,7 @@ use std::str; use serde::Deserialize; +use crate::builder::crate_description; use crate::builder::Cargo; use crate::builder::{Builder, Kind, RunConfig, ShouldRun, Step}; use crate::cache::{Interned, INTERNER}; @@ -128,8 +129,11 @@ impl Step for Std { std_cargo(builder, target, compiler.stage, &mut cargo); builder.info(&format!( - "Building stage{} std artifacts ({} -> {})", - compiler.stage, &compiler.host, target + "Building stage{} std artifacts ({} -> {}){}", + compiler.stage, + &compiler.host, + target, + crate_description(self.crates), )); run_cargo( builder, @@ -715,8 +719,11 @@ impl Step for Rustc { } builder.info(&format!( - "Building stage{} compiler artifacts ({} -> {})", - compiler.stage, &compiler.host, target + "Building stage{} compiler artifacts ({} -> {}){}", + compiler.stage, + &compiler.host, + target, + crate_description(self.crates), )); run_cargo( builder, From 284f0110d1d83f3b751be1273391dd4914a2b53d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 29 Dec 2022 22:46:17 -0800 Subject: [PATCH 03/10] Merge multiple mutable borrows of immutable binding errors Fix #53466. --- .../rustc_borrowck/src/borrowck_errors.rs | 28 +++--- .../src/diagnostics/mutability_errors.rs | 90 ++++++++++++++----- compiler/rustc_borrowck/src/lib.rs | 24 +++++ compiler/rustc_errors/src/diagnostic.rs | 4 +- compiler/rustc_expand/src/mbe/diagnostics.rs | 4 +- .../src/traits/error_reporting/suggestions.rs | 2 +- src/test/ui/asm/x86_64/type-check-5.rs | 3 +- src/test/ui/asm/x86_64/type-check-5.stderr | 22 ++--- src/test/ui/borrowck/many-mutable-borrows.rs | 18 ++++ .../ui/borrowck/many-mutable-borrows.stderr | 32 +++++++ src/test/ui/borrowck/mut-borrow-of-mut-ref.rs | 9 +- .../ui/borrowck/mut-borrow-of-mut-ref.stderr | 30 +++---- src/test/ui/borrowck/mutability-errors.rs | 6 +- src/test/ui/borrowck/mutability-errors.stderr | 20 ++--- 14 files changed, 199 insertions(+), 93 deletions(-) create mode 100644 src/test/ui/borrowck/many-mutable-borrows.rs create mode 100644 src/test/ui/borrowck/many-mutable-borrows.stderr diff --git a/compiler/rustc_borrowck/src/borrowck_errors.rs b/compiler/rustc_borrowck/src/borrowck_errors.rs index 01be379120dc7..e4942f9b666e0 100644 --- a/compiler/rustc_borrowck/src/borrowck_errors.rs +++ b/compiler/rustc_borrowck/src/borrowck_errors.rs @@ -12,7 +12,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { place: &str, borrow_place: &str, value_place: &str, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { self.infcx.tcx.sess.create_err(crate::session_diagnostics::MoveBorrow { place, span, @@ -28,7 +28,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { desc: &str, borrow_span: Span, borrow_desc: &str, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { let mut err = struct_span_err!( self, span, @@ -50,7 +50,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { old_loan_span: Span, old_opt_via: &str, old_load_end_span: Option, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { let via = |msg: &str| if msg.is_empty() { "".to_string() } else { format!(" (via {})", msg) }; let mut err = struct_span_err!( @@ -98,7 +98,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { desc: &str, old_loan_span: Span, old_load_end_span: Option, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { let mut err = struct_span_err!( self, new_loan_span, @@ -269,7 +269,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { &self, span: Span, desc: &str, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { struct_span_err!(self, span, E0594, "cannot assign to {}", desc) } @@ -348,7 +348,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { span: Span, path: &str, reason: &str, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { struct_span_err!(self, span, E0596, "cannot borrow {} as mutable{}", path, reason,) } @@ -359,7 +359,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { immutable_place: &str, immutable_section: &str, action: &str, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { let mut err = struct_span_err!( self, mutate_span, @@ -378,7 +378,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { &self, span: Span, yield_span: Span, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { let mut err = struct_span_err!( self, span, @@ -392,7 +392,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { pub(crate) fn cannot_borrow_across_destructor( &self, borrow_span: Span, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { struct_span_err!( self, borrow_span, @@ -405,7 +405,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { &self, span: Span, path: &str, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { struct_span_err!(self, span, E0597, "{} does not live long enough", path,) } @@ -415,7 +415,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { return_kind: &str, reference_desc: &str, path_desc: &str, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { let mut err = struct_span_err!( self, span, @@ -440,7 +440,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { closure_kind: &str, borrowed_path: &str, capture_span: Span, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { let mut err = struct_span_err!( self, closure_span, @@ -458,14 +458,14 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { pub(crate) fn thread_local_value_does_not_live_long_enough( &self, span: Span, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { struct_span_err!(self, span, E0712, "thread-local variable borrowed past end of function",) } pub(crate) fn temporary_value_borrowed_for_too_long( &self, span: Span, - ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { struct_span_err!(self, span, E0716, "temporary value dropped while borrowed",) } diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index f13fb842bb601..140c93b785b93 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -180,6 +180,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { // the verbs used in some diagnostic messages. let act; let acted_on; + let mut suggest = true; + let mut mut_error = None; + let mut count = 1; let span = match error_access { AccessKind::Mutate => { @@ -194,15 +197,50 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let borrow_spans = self.borrow_spans(span, location); let borrow_span = borrow_spans.args_or_use(); - err = self.cannot_borrow_path_as_mutable_because(borrow_span, &item_msg, &reason); - borrow_spans.var_span_label( - &mut err, - format!( - "mutable borrow occurs due to use of {} in closure", - self.describe_any_place(access_place.as_ref()), - ), - "mutable", - ); + match the_place_err { + PlaceRef { local, projection: [] } + if self.body.local_decls[local].can_be_made_mutable() => + { + let span = self.body.local_decls[local].source_info.span; + mut_error = Some(span); + if let Some((buffer, c)) = self.get_buffered_mut_error(span) { + // We've encountered a second (or more) attempt to mutably borrow an + // immutable binding, so the likely problem is with the binding + // declaration, not the use. We collect these in a single diagnostic + // and make the binding the primary span of the error. + err = buffer; + count = c + 1; + if count == 2 { + err.replace_span_with(span, false); + err.span_label(span, "not mutable"); + } + suggest = false; + } else { + err = self.cannot_borrow_path_as_mutable_because( + borrow_span, + &item_msg, + &reason, + ); + } + } + _ => { + err = self.cannot_borrow_path_as_mutable_because( + borrow_span, + &item_msg, + &reason, + ); + } + } + if suggest { + borrow_spans.var_span_label( + &mut err, + format!( + "mutable borrow occurs due to use of {} in closure", + self.describe_any_place(access_place.as_ref()), + ), + "mutable", + ); + } borrow_span } }; @@ -276,7 +314,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { pat_span: _, }, )))) => { - err.span_note(sp, "the binding is already a mutable borrow"); + if suggest { + err.span_note(sp, "the binding is already a mutable borrow"); + } } _ => { err.span_note( @@ -333,16 +373,20 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let local_decl = &self.body.local_decls[local]; assert_eq!(local_decl.mutability, Mutability::Not); - err.span_label(span, format!("cannot {ACT}", ACT = act)); - err.span_suggestion( - local_decl.source_info.span, - "consider changing this to be mutable", - format!("mut {}", self.local_names[local].unwrap()), - Applicability::MachineApplicable, - ); - let tcx = self.infcx.tcx; - if let ty::Closure(id, _) = *the_place_err.ty(self.body, tcx).ty.kind() { - self.show_mutating_upvar(tcx, id.expect_local(), the_place_err, &mut err); + if count < 10 { + err.span_label(span, format!("cannot {act}")); + } + if suggest { + err.span_suggestion( + local_decl.source_info.span, + "consider changing this to be mutable", + format!("mut {}", self.local_names[local].unwrap()), + Applicability::MachineApplicable, + ); + let tcx = self.infcx.tcx; + if let ty::Closure(id, _) = *the_place_err.ty(self.body, tcx).ty.kind() { + self.show_mutating_upvar(tcx, id.expect_local(), the_place_err, &mut err); + } } } @@ -624,7 +668,11 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } } - self.buffer_error(err); + if let Some(span) = mut_error { + self.buffer_mut_error(span, err, count); + } else { + self.buffer_error(err); + } } fn suggest_map_index_mut_alternatives(&self, ty: Ty<'tcx>, err: &mut Diagnostic, span: Span) { diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 168b798788b4c..ae1bea008b6ce 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -2270,6 +2270,7 @@ mod error { /// same primary span come out in a consistent order. buffered_move_errors: BTreeMap, (PlaceRef<'tcx>, DiagnosticBuilder<'tcx, ErrorGuaranteed>)>, + buffered_mut_errors: FxHashMap, usize)>, /// Diagnostics to be reported buffer. buffered: Vec, /// Set to Some if we emit an error during borrowck @@ -2281,6 +2282,7 @@ mod error { BorrowckErrors { tcx, buffered_move_errors: BTreeMap::new(), + buffered_mut_errors: Default::default(), buffered: Default::default(), tainted_by_errors: None, } @@ -2331,12 +2333,34 @@ mod error { } } + pub fn get_buffered_mut_error( + &mut self, + span: Span, + ) -> Option<(DiagnosticBuilder<'tcx, ErrorGuaranteed>, usize)> { + self.errors.buffered_mut_errors.remove(&span) + } + + pub fn buffer_mut_error( + &mut self, + span: Span, + t: DiagnosticBuilder<'tcx, ErrorGuaranteed>, + count: usize, + ) { + self.errors.buffered_mut_errors.insert(span, (t, count)); + } + pub fn emit_errors(&mut self) -> Option { // Buffer any move errors that we collected and de-duplicated. for (_, (_, diag)) in std::mem::take(&mut self.errors.buffered_move_errors) { // We have already set tainted for this error, so just buffer it. diag.buffer(&mut self.errors.buffered); } + for (_, (mut diag, count)) in std::mem::take(&mut self.errors.buffered_mut_errors) { + if count > 10 { + diag.note(&format!("...and {} other attempted mutable borrows", count - 10)); + } + diag.buffer(&mut self.errors.buffered); + } if !self.errors.buffered.is_empty() { self.errors.buffered.sort_by_key(|diag| diag.sort_span); diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 585a54308c62e..e19a6fe0ee9bf 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -365,12 +365,12 @@ impl Diagnostic { self } - pub fn replace_span_with(&mut self, after: Span) -> &mut Self { + pub fn replace_span_with(&mut self, after: Span, keep_label: bool) -> &mut Self { let before = self.span.clone(); self.set_span(after); for span_label in before.span_labels() { if let Some(label) = span_label.label { - if span_label.is_primary { + if span_label.is_primary && keep_label { self.span.push_span_label(after, label); } else { self.span.push_span_label(span_label.span, label); diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs index 40aa64d9d4040..3a38d7a966960 100644 --- a/compiler/rustc_expand/src/mbe/diagnostics.rs +++ b/compiler/rustc_expand/src/mbe/diagnostics.rs @@ -198,12 +198,12 @@ pub(super) fn emit_frag_parse_err( ); if !e.span.is_dummy() { // early end of macro arm (#52866) - e.replace_span_with(parser.token.span.shrink_to_hi()); + e.replace_span_with(parser.token.span.shrink_to_hi(), true); } } if e.span.is_dummy() { // Get around lack of span in error (#30128) - e.replace_span_with(site_span); + e.replace_span_with(site_span, true); if !parser.sess.source_map().is_imported(arm_span) { e.span_label(arm_span, "in this macro arm"); } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 06ab0e8d94470..7c21a1047bcbf 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -3237,7 +3237,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { })) = call_node { if Some(rcvr.span) == err.span.primary_span() { - err.replace_span_with(path.ident.span); + err.replace_span_with(path.ident.span, true); } } if let Some(Node::Expr(hir::Expr { diff --git a/src/test/ui/asm/x86_64/type-check-5.rs b/src/test/ui/asm/x86_64/type-check-5.rs index 8198df91095f9..1d579ccc90eec 100644 --- a/src/test/ui/asm/x86_64/type-check-5.rs +++ b/src/test/ui/asm/x86_64/type-check-5.rs @@ -22,11 +22,10 @@ fn main() { // Outputs require mutable places let v: Vec = vec![0, 1, 2]; + //~^ ERROR cannot borrow `v` as mutable, as it is not declared as mutable asm!("{}", in(reg) v[0]); asm!("{}", out(reg) v[0]); - //~^ ERROR cannot borrow `v` as mutable, as it is not declared as mutable asm!("{}", inout(reg) v[0]); - //~^ ERROR cannot borrow `v` as mutable, as it is not declared as mutable // Sym operands must point to a function or static diff --git a/src/test/ui/asm/x86_64/type-check-5.stderr b/src/test/ui/asm/x86_64/type-check-5.stderr index bd90461e52c17..af89e5e91174e 100644 --- a/src/test/ui/asm/x86_64/type-check-5.stderr +++ b/src/test/ui/asm/x86_64/type-check-5.stderr @@ -25,24 +25,20 @@ LL | let mut y: u64 = 0; | +++ error[E0596]: cannot borrow `v` as mutable, as it is not declared as mutable - --> $DIR/type-check-5.rs:26:29 + --> $DIR/type-check-5.rs:24:13 | LL | let v: Vec = vec![0, 1, 2]; - | - help: consider changing this to be mutable: `mut v` -LL | asm!("{}", in(reg) v[0]); -LL | asm!("{}", out(reg) v[0]); - | ^ cannot borrow as mutable - -error[E0596]: cannot borrow `v` as mutable, as it is not declared as mutable - --> $DIR/type-check-5.rs:28:31 - | -LL | let v: Vec = vec![0, 1, 2]; - | - help: consider changing this to be mutable: `mut v` + | ^ + | | + | not mutable + | help: consider changing this to be mutable: `mut v` ... +LL | asm!("{}", out(reg) v[0]); + | - cannot borrow as mutable LL | asm!("{}", inout(reg) v[0]); - | ^ cannot borrow as mutable + | - cannot borrow as mutable -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors Some errors have detailed explanations: E0381, E0596. For more information about an error, try `rustc --explain E0381`. diff --git a/src/test/ui/borrowck/many-mutable-borrows.rs b/src/test/ui/borrowck/many-mutable-borrows.rs new file mode 100644 index 0000000000000..3e6ea9d25d910 --- /dev/null +++ b/src/test/ui/borrowck/many-mutable-borrows.rs @@ -0,0 +1,18 @@ +fn main() { + let v = Vec::new(); //~ ERROR cannot borrow `v` as mutable, as it is not declared as mutable + v.push(0); + v.push(0); + v.push(0); + v.push(0); + v.push(0); + v.push(0); + v.push(0); + v.push(0); + v.push(0); + v.push(0); + v.push(0); + v.push(0); + v.push(0); + v.push(0); + v.push(0); +} diff --git a/src/test/ui/borrowck/many-mutable-borrows.stderr b/src/test/ui/borrowck/many-mutable-borrows.stderr new file mode 100644 index 0000000000000..25755d9432362 --- /dev/null +++ b/src/test/ui/borrowck/many-mutable-borrows.stderr @@ -0,0 +1,32 @@ +error[E0596]: cannot borrow `v` as mutable, as it is not declared as mutable + --> $DIR/many-mutable-borrows.rs:2:9 + | +LL | let v = Vec::new(); + | ^ + | | + | not mutable + | help: consider changing this to be mutable: `mut v` +LL | v.push(0); + | --------- cannot borrow as mutable +LL | v.push(0); + | --------- cannot borrow as mutable +LL | v.push(0); + | --------- cannot borrow as mutable +LL | v.push(0); + | --------- cannot borrow as mutable +LL | v.push(0); + | --------- cannot borrow as mutable +LL | v.push(0); + | --------- cannot borrow as mutable +LL | v.push(0); + | --------- cannot borrow as mutable +LL | v.push(0); + | --------- cannot borrow as mutable +LL | v.push(0); + | --------- cannot borrow as mutable + | + = note: ...and 5 other attempted mutable borrows + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs b/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs index 7cdb16b282d54..477a2aa48d5dc 100644 --- a/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs +++ b/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs @@ -2,15 +2,14 @@ #![crate_type = "rlib"] pub fn f(b: &mut i32) { - //~^ NOTE the binding is already a mutable borrow + //~^ ERROR cannot borrow + //~| NOTE not mutable //~| NOTE the binding is already a mutable borrow h(&mut b); - //~^ ERROR cannot borrow - //~| NOTE cannot borrow as mutable + //~^ NOTE cannot borrow as mutable //~| HELP try removing `&mut` here g(&mut &mut b); - //~^ ERROR cannot borrow - //~| NOTE cannot borrow as mutable + //~^ NOTE cannot borrow as mutable //~| HELP try removing `&mut` here } diff --git a/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr b/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr index 7782047574ce8..c6f75b1c0d022 100644 --- a/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr +++ b/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr @@ -1,8 +1,14 @@ error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable - --> $DIR/mut-borrow-of-mut-ref.rs:7:7 + --> $DIR/mut-borrow-of-mut-ref.rs:4:10 | +LL | pub fn f(b: &mut i32) { + | ^ not mutable +... LL | h(&mut b); - | ^^^^^^ cannot borrow as mutable + | ------ cannot borrow as mutable +... +LL | g(&mut &mut b); + | ------ cannot borrow as mutable | note: the binding is already a mutable borrow --> $DIR/mut-borrow-of-mut-ref.rs:4:13 @@ -14,18 +20,6 @@ help: try removing `&mut` here LL - h(&mut b); LL + h(b); | - -error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable - --> $DIR/mut-borrow-of-mut-ref.rs:11:12 - | -LL | g(&mut &mut b); - | ^^^^^^ cannot borrow as mutable - | -note: the binding is already a mutable borrow - --> $DIR/mut-borrow-of-mut-ref.rs:4:13 - | -LL | pub fn f(b: &mut i32) { - | ^^^^^^^^ help: try removing `&mut` here | LL - g(&mut &mut b); @@ -33,13 +27,13 @@ LL + g(&mut b); | error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable - --> $DIR/mut-borrow-of-mut-ref.rs:18:12 + --> $DIR/mut-borrow-of-mut-ref.rs:17:12 | LL | h(&mut &mut b); | ^^^^^^ cannot borrow as mutable | note: the binding is already a mutable borrow - --> $DIR/mut-borrow-of-mut-ref.rs:17:13 + --> $DIR/mut-borrow-of-mut-ref.rs:16:13 | LL | pub fn g(b: &mut i32) { | ^^^^^^^^ @@ -50,7 +44,7 @@ LL + h(&mut b); | error[E0596]: cannot borrow `f` as mutable, as it is not declared as mutable - --> $DIR/mut-borrow-of-mut-ref.rs:35:5 + --> $DIR/mut-borrow-of-mut-ref.rs:34:5 | LL | f.bar(); | ^^^^^^^ cannot borrow as mutable @@ -60,6 +54,6 @@ help: consider making the binding mutable LL | pub fn baz(mut f: &mut String) { | +++ -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/borrowck/mutability-errors.rs b/src/test/ui/borrowck/mutability-errors.rs index 5be0df1376135..82116425f06ca 100644 --- a/src/test/ui/borrowck/mutability-errors.rs +++ b/src/test/ui/borrowck/mutability-errors.rs @@ -50,9 +50,9 @@ fn ref_closure(mut x: (i32,)) { }); } -fn imm_local(x: (i32,)) { - &mut x; //~ ERROR - &mut x.0; //~ ERROR +fn imm_local(x: (i32,)) { //~ ERROR + &mut x; + &mut x.0; } fn imm_capture(x: (i32,)) { diff --git a/src/test/ui/borrowck/mutability-errors.stderr b/src/test/ui/borrowck/mutability-errors.stderr index dd29ae492d604..9d0435bf9b815 100644 --- a/src/test/ui/borrowck/mutability-errors.stderr +++ b/src/test/ui/borrowck/mutability-errors.stderr @@ -227,21 +227,17 @@ LL | &mut x.0; | ^^^^^^^^ cannot borrow as mutable error[E0596]: cannot borrow `x` as mutable, as it is not declared as mutable - --> $DIR/mutability-errors.rs:54:5 + --> $DIR/mutability-errors.rs:53:14 | LL | fn imm_local(x: (i32,)) { - | - help: consider changing this to be mutable: `mut x` -LL | &mut x; - | ^^^^^^ cannot borrow as mutable - -error[E0596]: cannot borrow `x.0` as mutable, as `x` is not declared as mutable - --> $DIR/mutability-errors.rs:55:5 - | -LL | fn imm_local(x: (i32,)) { - | - help: consider changing this to be mutable: `mut x` + | ^ + | | + | not mutable + | help: consider changing this to be mutable: `mut x` LL | &mut x; + | ------ cannot borrow as mutable LL | &mut x.0; - | ^^^^^^^^ cannot borrow as mutable + | -------- cannot borrow as mutable error[E0594]: cannot assign to `x`, as it is not declared as mutable --> $DIR/mutability-errors.rs:60:9 @@ -339,7 +335,7 @@ error[E0596]: cannot borrow `X.0` as mutable, as `X` is an immutable static item LL | &mut X.0; | ^^^^^^^^ cannot borrow as mutable -error: aborting due to 38 previous errors +error: aborting due to 37 previous errors Some errors have detailed explanations: E0594, E0596. For more information about an error, try `rustc --explain E0594`. From f9cc01126954e2fde28162fd83b4ef3e447526a6 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Fri, 30 Dec 2022 12:11:05 +0100 Subject: [PATCH 04/10] Tidy up tidy error codes check --- src/tools/tidy/src/error_codes_check.rs | 37 ++++++++----------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/src/tools/tidy/src/error_codes_check.rs b/src/tools/tidy/src/error_codes_check.rs index fd870b0997ce0..40a46c630d70a 100644 --- a/src/tools/tidy/src/error_codes_check.rs +++ b/src/tools/tidy/src/error_codes_check.rs @@ -80,15 +80,6 @@ fn check_if_error_code_is_test_in_explanation(f: &str, err_code: &str) -> bool { ignore_found } -macro_rules! some_or_continue { - ($e:expr) => { - match $e { - Some(e) => e, - None => continue, - } - }; -} - fn extract_error_codes( f: &str, error_codes: &mut HashMap, @@ -122,10 +113,16 @@ fn extract_error_codes( Some((file_name, _)) => file_name, }, }; - let path = some_or_continue!(path.parent()) + + let Some(parent) = path.parent() else { + continue; + }; + + let path = parent .join(md_file_name) .canonicalize() .expect("failed to canonicalize error explanation file path"); + match read_to_string(&path) { Ok(content) => { let has_test = check_if_error_code_is_test_in_explanation(&content, &err_code); @@ -215,8 +212,6 @@ pub fn check(paths: &[&Path], bad: &mut bool) { // * #[error = "E0111"] let regex = Regex::new(r#"[(,"\s](E\d{4})[,)"]"#).unwrap(); - println!("Checking which error codes lack tests..."); - for path in paths { walk(path, &mut filter_dirs, &mut |entry, contents| { let file_name = entry.file_name(); @@ -245,20 +240,15 @@ pub fn check(paths: &[&Path], bad: &mut bool) { }); } if found_explanations == 0 { - eprintln!("No error code explanation was tested!"); - *bad = true; + tidy_error!(bad, "No error code explanation was tested!"); } if found_tests == 0 { - eprintln!("No error code was found in compilation errors!"); - *bad = true; + tidy_error!(bad, "No error code was found in compilation errors!"); } if explanations.is_empty() { - eprintln!("No error code explanation was found!"); - *bad = true; + tidy_error!(bad, "No error code explanation was found!"); } if errors.is_empty() { - println!("Found {} error codes", error_codes.len()); - for (err_code, error_status) in &error_codes { if !error_status.has_test && !EXEMPTED_FROM_TEST.contains(&err_code.as_str()) { errors.push(format!("Error code {err_code} needs to have at least one UI test!")); @@ -310,11 +300,6 @@ pub fn check(paths: &[&Path], bad: &mut bool) { } errors.sort(); for err in &errors { - eprintln!("{err}"); - } - println!("Found {} error(s) in error codes", errors.len()); - if !errors.is_empty() { - *bad = true; + tidy_error!(bad, "{err}"); } - println!("Done!"); } From 87bc29d02b65bd21a49136021d701de42b57390b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 30 Dec 2022 16:27:56 +0100 Subject: [PATCH 05/10] Extend scraped examples layout GUI test for position of buttons --- src/test/rustdoc-gui/scrape-examples-layout.goml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/test/rustdoc-gui/scrape-examples-layout.goml b/src/test/rustdoc-gui/scrape-examples-layout.goml index fde9a0ab0bc30..95102528ec11d 100644 --- a/src/test/rustdoc-gui/scrape-examples-layout.goml +++ b/src/test/rustdoc-gui/scrape-examples-layout.goml @@ -33,3 +33,17 @@ assert-property: ( ".more-scraped-examples .scraped-example:nth-child(6) .code-wrapper .src-line-numbers", {"clientWidth": |clientWidth|} ) + +// Check that for both mobile and desktop sizes, the buttons in scraped examples are displayed +// correctly. + +store-value: (offset_y, 4) + +// First with desktop +assert-position: (".scraped-example .code-wrapper", {"y": 255}) +assert-position: (".scraped-example .code-wrapper .prev", {"y": 255 + |offset_y|}) + +// Then with mobile +size: (600, 600) +assert-position: (".scraped-example .code-wrapper", {"y": 314}) +assert-position: (".scraped-example .code-wrapper .prev", {"y": 314 + |offset_y|}) From c8c849ef5c4932d5aec5ba6dbf936d3d18856f71 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 30 Dec 2022 20:03:24 +0000 Subject: [PATCH 06/10] Use more consistent progress messages in bootstrap Before: ``` Testing ["rustc_interface"] stage0 (aarch64-unknown-linux-gnu -> aarch64-unknown-linux-gnu) ``` After: ``` Testing {rustc_interface} stage0 (aarch64-unknown-linux-gnu -> aarch64-unknown-linux-gnu) ``` Note there is a slight consistency between `build` and `test`: The former doesn't print "compiler artifacts". It would be annoying to fix and doesn't hurt anything, so I left it be. ``` ; x t rustc_interface --stage 0 --dry-run Testing {rustc_interface} stage0 (aarch64-unknown-linux-gnu -> aarch64-unknown-linux-gnu) ; x b rustc_interface --stage 0 --dry-run Building {rustc_interface} stage0 compiler artifacts (aarch64-unknown-linux-gnu -> aarch64-unknown-linux-gnu) ``` --- src/bootstrap/builder.rs | 17 ++++++++--------- src/bootstrap/check.rs | 4 ++-- src/bootstrap/clean.rs | 4 ++-- src/bootstrap/compile.rs | 26 ++++++++++++++++++-------- src/bootstrap/doc.rs | 4 +++- src/bootstrap/test.rs | 9 +++++++-- 6 files changed, 40 insertions(+), 24 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 97b353b5462f5..66bc0f023b6c9 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -97,13 +97,13 @@ impl RunConfig<'_> { self.builder.build.build } - /// Return a `-p=x -p=y` string suitable for passing to a cargo invocation. + /// Return a list of crate names selected by `run.paths`. pub fn cargo_crates_in_set(&self) -> Interned> { let mut crates = Vec::new(); for krate in &self.paths { let path = krate.assert_single_path(); let crate_name = self.builder.crate_paths[&path.path]; - crates.push(format!("-p={crate_name}")); + crates.push(crate_name.to_string()); } INTERNER.intern_list(crates) } @@ -112,18 +112,17 @@ impl RunConfig<'_> { /// A description of the crates in this set, suitable for passing to `builder.info`. /// /// `crates` should be generated by [`RunConfig::cargo_crates_in_set`]. -pub fn crate_description(crates: Interned>) -> String { +pub fn crate_description(crates: &[impl AsRef]) -> String { if crates.is_empty() { return "".into(); } - let mut descr = String::from(": {"); - for krate in &*crates { - write!(descr, "{}, ", krate.strip_prefix("-p=").unwrap()).unwrap(); + let mut descr = String::from(" {"); + descr.push_str(crates[0].as_ref()); + for krate in &crates[1..] { + descr.push_str(", "); + descr.push_str(krate.as_ref()); } - - descr.pop(); - descr.pop(); descr.push('}'); descr } diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 2e1bd8d6d1f6d..188c0a43e50d7 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -101,7 +101,7 @@ impl Step for Std { std_cargo(builder, target, compiler.stage, &mut cargo); builder.info(&format!( - "Checking stage{} std artifacts ({} -> {})", + "Checking stage{} library artifacts ({} -> {})", builder.top_stage, &compiler.host, target )); run_cargo( @@ -157,7 +157,7 @@ impl Step for Std { } builder.info(&format!( - "Checking stage{} std test/bench/example targets ({} -> {})", + "Checking stage{} library test/bench/example targets ({} -> {})", builder.top_stage, &compiler.host, target )); run_cargo( diff --git a/src/bootstrap/clean.rs b/src/bootstrap/clean.rs index d887495d633f0..468efc1114c43 100644 --- a/src/bootstrap/clean.rs +++ b/src/bootstrap/clean.rs @@ -66,8 +66,8 @@ macro_rules! clean_crate_tree { } builder.info(&format!( - "Cleaning stage{} {} artifacts ({} -> {}){}", - compiler.stage, stringify!($name).to_lowercase(), &compiler.host, target, crate_description(self.crates), + "Cleaning{} stage{} {} artifacts ({} -> {})", + crate_description(&self.crates), compiler.stage, stringify!($name).to_lowercase(), &compiler.host, target, )); // NOTE: doesn't use `run_cargo` because we don't want to save a stamp file, diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 5bf5683f85dea..35bdfbfa0ed1a 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -111,7 +111,10 @@ impl Step for Std { let compiler_to_use = builder.compiler_for(compiler.stage, compiler.host, target); if compiler_to_use != compiler { builder.ensure(Std::new(compiler_to_use, target)); - builder.info(&format!("Uplifting stage1 std ({} -> {})", compiler_to_use.host, target)); + builder.info(&format!( + "Uplifting stage1 library ({} -> {})", + compiler_to_use.host, target + )); // Even if we're not building std this stage, the new sysroot must // still contain the third party objects needed by various targets. @@ -127,18 +130,21 @@ impl Step for Std { let mut cargo = builder.cargo(compiler, Mode::Std, SourceType::InTree, target, "build"); std_cargo(builder, target, compiler.stage, &mut cargo); + for krate in &*self.crates { + cargo.arg("-p").arg(krate); + } builder.info(&format!( - "Building stage{} std artifacts ({} -> {}){}", + "Building{} stage{} library artifacts ({} -> {})", + crate_description(&self.crates), compiler.stage, &compiler.host, target, - crate_description(self.crates), )); run_cargo( builder, cargo, - self.crates.to_vec(), + vec![], &libstd_stamp(builder, compiler, target), target_deps, false, @@ -429,7 +435,7 @@ impl Step for StdLink { let target_compiler = self.target_compiler; let target = self.target; builder.info(&format!( - "Copying stage{} std from stage{} ({} -> {} / {})", + "Copying stage{} library from stage{} ({} -> {} / {})", target_compiler.stage, compiler.stage, &compiler.host, target_compiler.host, target )); let libdir = builder.sysroot_libdir(target_compiler, target); @@ -718,17 +724,21 @@ impl Step for Rustc { } } + for krate in &*self.crates { + cargo.arg("-p").arg(krate); + } + builder.info(&format!( - "Building stage{} compiler artifacts ({} -> {}){}", + "Building{} stage{} compiler artifacts ({} -> {})", + crate_description(&self.crates), compiler.stage, &compiler.host, target, - crate_description(self.crates), )); run_cargo( builder, cargo, - self.crates.to_vec(), + vec![], &librustc_stamp(builder, compiler, target), vec![], false, diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index 5838049aa5c79..0562c270d54c0 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -12,6 +12,7 @@ use std::fs; use std::io; use std::path::{Path, PathBuf}; +use crate::builder::crate_description; use crate::builder::{Builder, Compiler, Kind, RunConfig, ShouldRun, Step}; use crate::cache::{Interned, INTERNER}; use crate::compile; @@ -554,7 +555,8 @@ fn doc_std( requested_crates: &[String], ) { builder.info(&format!( - "Documenting stage{} std ({}) in {} format", + "Documenting{} stage{} library ({}) in {} format", + crate_description(requested_crates), stage, target, format.as_str() diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index c8b4134391e5f..d5bec268a4567 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -11,6 +11,7 @@ use std::iter; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use crate::builder::crate_description; use crate::builder::{Builder, Compiler, Kind, RunConfig, ShouldRun, Step}; use crate::cache::Interned; use crate::compile; @@ -2154,8 +2155,12 @@ impl Step for Crate { } builder.info(&format!( - "{} {:?} stage{} ({} -> {})", - test_kind, self.crates, compiler.stage, &compiler.host, target + "{}{} stage{} ({} -> {})", + test_kind, + crate_description(&self.crates), + compiler.stage, + &compiler.host, + target )); let _time = util::timeit(&builder); try_run(builder, &mut cargo.into()); From 9dfe50440e6d48bd2fd40a4b7b3992998e55eace Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 30 Dec 2022 20:36:31 +0000 Subject: [PATCH 07/10] bootstrap: Get rid of `tail_args` in `stream_cargo` --- src/bootstrap/check.rs | 39 ++++++++++++--------------------------- src/bootstrap/compile.rs | 28 ++++------------------------ src/bootstrap/tool.rs | 2 +- 3 files changed, 17 insertions(+), 52 deletions(-) diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 188c0a43e50d7..32e5d414061ec 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -99,19 +99,13 @@ impl Step for Std { cargo_subcommand(builder.kind), ); std_cargo(builder, target, compiler.stage, &mut cargo); + cargo.args(args(builder)); builder.info(&format!( "Checking stage{} library artifacts ({} -> {})", builder.top_stage, &compiler.host, target )); - run_cargo( - builder, - cargo, - args(builder), - &libstd_stamp(builder, compiler, target), - vec![], - true, - ); + run_cargo(builder, cargo, &libstd_stamp(builder, compiler, target), vec![], true); // We skip populating the sysroot in non-zero stage because that'll lead // to rlib/rmeta conflicts if std gets built during this session. @@ -155,19 +149,13 @@ impl Step for Std { for krate in builder.in_tree_crates("test", Some(target)) { cargo.arg("-p").arg(krate.name); } + cargo.args(args(builder)); builder.info(&format!( "Checking stage{} library test/bench/example targets ({} -> {})", builder.top_stage, &compiler.host, target )); - run_cargo( - builder, - cargo, - args(builder), - &libstd_test_stamp(builder, compiler, target), - vec![], - true, - ); + run_cargo(builder, cargo, &libstd_test_stamp(builder, compiler, target), vec![], true); } } @@ -231,19 +219,13 @@ impl Step for Rustc { for krate in builder.in_tree_crates("rustc-main", Some(target)) { cargo.arg("-p").arg(krate.name); } + cargo.args(args(builder)); builder.info(&format!( "Checking stage{} compiler artifacts ({} -> {})", builder.top_stage, &compiler.host, target )); - run_cargo( - builder, - cargo, - args(builder), - &librustc_stamp(builder, compiler, target), - vec![], - true, - ); + run_cargo(builder, cargo, &librustc_stamp(builder, compiler, target), vec![], true); let libdir = builder.sysroot_libdir(compiler, target); let hostdir = builder.sysroot_libdir(compiler, compiler.host); @@ -290,6 +272,7 @@ impl Step for CodegenBackend { .arg("--manifest-path") .arg(builder.src.join(format!("compiler/rustc_codegen_{}/Cargo.toml", backend))); rustc_cargo_env(builder, &mut cargo, target); + cargo.args(args(builder)); builder.info(&format!( "Checking stage{} {} artifacts ({} -> {})", @@ -299,7 +282,6 @@ impl Step for CodegenBackend { run_cargo( builder, cargo, - args(builder), &codegen_backend_stamp(builder, compiler, target, backend), vec![], true, @@ -355,11 +337,13 @@ impl Step for RustAnalyzer { cargo.arg("--benches"); } + cargo.args(args(builder)); + builder.info(&format!( "Checking stage{} {} artifacts ({} -> {})", compiler.stage, "rust-analyzer", &compiler.host.triple, target.triple )); - run_cargo(builder, cargo, args(builder), &stamp(builder, compiler, target), vec![], true); + run_cargo(builder, cargo, &stamp(builder, compiler, target), vec![], true); /// Cargo's output path in a given stage, compiled by a particular /// compiler for the specified target. @@ -413,6 +397,8 @@ macro_rules! tool_check_step { cargo.arg("--all-targets"); } + cargo.args(args(builder)); + // Enable internal lints for clippy and rustdoc // NOTE: this doesn't enable lints for any other tools unless they explicitly add `#![warn(rustc::internal)]` // See https://github.com/rust-lang/rust/pull/80573#issuecomment-754010776 @@ -428,7 +414,6 @@ macro_rules! tool_check_step { run_cargo( builder, cargo, - args(builder), &stamp(builder, compiler, target), vec![], true, diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 35bdfbfa0ed1a..f9a04f2e91dbf 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -141,14 +141,7 @@ impl Step for Std { &compiler.host, target, )); - run_cargo( - builder, - cargo, - vec![], - &libstd_stamp(builder, compiler, target), - target_deps, - false, - ); + run_cargo(builder, cargo, &libstd_stamp(builder, compiler, target), target_deps, false); builder.ensure(StdLink::from_std( self, @@ -735,14 +728,7 @@ impl Step for Rustc { &compiler.host, target, )); - run_cargo( - builder, - cargo, - vec![], - &librustc_stamp(builder, compiler, target), - vec![], - false, - ); + run_cargo(builder, cargo, &librustc_stamp(builder, compiler, target), vec![], false); builder.ensure(RustcLink::from_rustc( self, @@ -998,7 +984,7 @@ impl Step for CodegenBackend { "Building stage{} codegen backend {} ({} -> {})", compiler.stage, backend, &compiler.host, target )); - let files = run_cargo(builder, cargo, vec![], &tmp_stamp, vec![], false); + let files = run_cargo(builder, cargo, &tmp_stamp, vec![], false); if builder.config.dry_run() { return; } @@ -1422,7 +1408,6 @@ pub fn add_to_sysroot( pub fn run_cargo( builder: &Builder<'_>, cargo: Cargo, - tail_args: Vec, stamp: &Path, additional_target_deps: Vec<(PathBuf, DependencyType)>, is_check: bool, @@ -1448,7 +1433,7 @@ pub fn run_cargo( // files we need to probe for later. let mut deps = Vec::new(); let mut toplevel = Vec::new(); - let ok = stream_cargo(builder, cargo, tail_args, &mut |msg| { + let ok = stream_cargo(builder, cargo, &mut |msg| { let (filenames, crate_types) = match msg { CargoMessage::CompilerArtifact { filenames, @@ -1563,7 +1548,6 @@ pub fn run_cargo( pub fn stream_cargo( builder: &Builder<'_>, cargo: Cargo, - tail_args: Vec, cb: &mut dyn FnMut(CargoMessage<'_>), ) -> bool { let mut cargo = Command::from(cargo); @@ -1583,10 +1567,6 @@ pub fn stream_cargo( } cargo.arg("--message-format").arg(message_format).stdout(Stdio::piped()); - for arg in tail_args { - cargo.arg(arg); - } - builder.verbose(&format!("running: {:?}", cargo)); let mut child = match cargo.spawn() { Ok(child) => child, diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index 24b033cc0dc5e..63026bd44d475 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -72,7 +72,7 @@ impl Step for ToolBuild { builder.info(&format!("Building stage{} tool {} ({})", compiler.stage, tool, target)); let mut duplicates = Vec::new(); - let is_expected = compile::stream_cargo(builder, cargo, vec![], &mut |msg| { + let is_expected = compile::stream_cargo(builder, cargo, &mut |msg| { // Only care about big things like the RLS/Cargo for now match tool { "rls" | "cargo" | "clippy-driver" | "miri" | "rustfmt" => {} From 75b3ee26cbeddcf2244e284d3c822d067cada2e2 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Fri, 30 Dec 2022 12:23:05 +0100 Subject: [PATCH 08/10] Make tidy errors red This makes it easier to see them (and makes people go owo). --- Cargo.lock | 9 +++++---- src/tools/tidy/Cargo.toml | 1 + src/tools/tidy/src/lib.rs | 31 ++++++++++++++++++++++--------- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9581099c210ea..f99e58e59b8e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2675,9 +2675,9 @@ dependencies = [ [[package]] name = "owo-colors" -version = "3.4.0" +version = "3.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "decf7381921fea4dcb2549c5667eda59b3ec297ab7e2b5fc33eac69d2e7da87b" +checksum = "c1b04fb49957986fdce4d6ee7a65027d55d4b6d2265e5848bbb507b58ccfdb6f" [[package]] name = "packed_simd_2" @@ -5203,9 +5203,9 @@ dependencies = [ [[package]] name = "termcolor" -version = "1.1.2" +version = "1.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2dfed899f0eb03f32ee8c6a0aabdb8a7949659e3466561fc0adf54e26d88c5f4" +checksum = "bab24d30b911b2376f3a13cc2cd443142f0c81dda04c118693e35b3835757755" dependencies = [ "winapi-util", ] @@ -5309,6 +5309,7 @@ dependencies = [ "lazy_static", "miropt-test-tools", "regex", + "termcolor", "walkdir", ] diff --git a/src/tools/tidy/Cargo.toml b/src/tools/tidy/Cargo.toml index 97d038da702d5..fff83a1d097b3 100644 --- a/src/tools/tidy/Cargo.toml +++ b/src/tools/tidy/Cargo.toml @@ -11,6 +11,7 @@ miropt-test-tools = { path = "../miropt-test-tools" } lazy_static = "1" walkdir = "2" ignore = "0.4.18" +termcolor = "1.1.3" [[bin]] name = "rust-tidy" diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 698e4850bea9b..ce7e7ac5cd4ca 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -3,6 +3,10 @@ //! This library contains the tidy lints and exposes it //! to be used by tools. +use std::fmt::Display; + +use termcolor::WriteColor; + /// A helper macro to `unwrap` a result except also print out details like: /// /// * The expression that failed @@ -26,18 +30,27 @@ macro_rules! t { } macro_rules! tidy_error { - ($bad:expr, $fmt:expr) => ({ - *$bad = true; - eprint!("tidy error: "); - eprintln!($fmt); - }); - ($bad:expr, $fmt:expr, $($arg:tt)*) => ({ - *$bad = true; - eprint!("tidy error: "); - eprintln!($fmt, $($arg)*); + ($bad:expr, $($fmt:tt)*) => ({ + $crate::tidy_error($bad, format_args!($($fmt)*)).expect("failed to output error"); }); } +fn tidy_error(bad: &mut bool, args: impl Display) -> std::io::Result<()> { + use std::io::Write; + use termcolor::{Color, ColorChoice, ColorSpec, StandardStream}; + + *bad = true; + + let mut stderr = StandardStream::stdout(ColorChoice::Auto); + stderr.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?; + + write!(&mut stderr, "tidy error")?; + stderr.set_color(&ColorSpec::new())?; + + writeln!(&mut stderr, ": {args}")?; + Ok(()) +} + pub mod alphabetical; pub mod bins; pub mod debug_artifacts; From e2c5999265916399b0700b513cda0221fc8f7ad3 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 30 Dec 2022 22:43:54 +0000 Subject: [PATCH 09/10] Dont use `--merge-base` during bootstrap formatting subcommand --- src/bootstrap/format.rs | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/bootstrap/format.rs b/src/bootstrap/format.rs index 1d57c6ecbbb8d..84e4611895965 100644 --- a/src/bootstrap/format.rs +++ b/src/bootstrap/format.rs @@ -79,24 +79,19 @@ fn update_rustfmt_version(build: &Builder<'_>) { /// /// Returns `None` if all files should be formatted. fn get_modified_rs_files(build: &Builder<'_>) -> Option> { - let Ok(remote) = get_rust_lang_rust_remote() else {return None;}; + let Ok(remote) = get_rust_lang_rust_remote() else { return None; }; if !verify_rustfmt_version(build) { return None; } + + let merge_base = + output(build.config.git().arg("merge-base").arg(&format!("{remote}/master")).arg("HEAD")); Some( - output( - build - .config - .git() - .arg("diff-index") - .arg("--name-only") - .arg("--merge-base") - .arg(&format!("{remote}/master")), - ) - .lines() - .map(|s| s.trim().to_owned()) - .filter(|f| Path::new(f).extension().map_or(false, |ext| ext == "rs")) - .collect(), + output(build.config.git().arg("diff-index").arg("--name-only").arg(merge_base.trim())) + .lines() + .map(|s| s.trim().to_owned()) + .filter(|f| Path::new(f).extension().map_or(false, |ext| ext == "rs")) + .collect(), ) } From 6d2fe52dc5065278c57542487f5d0ab31313d3bf Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 31 Dec 2022 00:53:46 +0000 Subject: [PATCH 10/10] Fix panic on `x build --help` --- src/bootstrap/flags.rs | 44 +++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 3be7ad4075ed8..2c6d201d18fbe 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -352,32 +352,32 @@ To learn more about a subcommand, run `./x.py -h`", // fn usage() let usage = |exit_code: i32, opts: &Options, verbose: bool, subcommand_help: &str| -> ! { - // We have an unfortunate situation here: some Steps use `builder.in_tree_crates` to determine their paths. - // To determine those crates, we need to run `cargo metadata`, which means we need all submodules to be checked out. - // That takes a while to run, so only do it when paths were explicitly requested, not on all CLI errors. - // `Build::new` won't load submodules for the `setup` command. - let cmd = if verbose { - println!("note: updating submodules before printing available paths"); - "build" - } else { - "setup" - }; - let config = Config::parse(&[cmd.to_string()]); - let build = Build::new(config); - let paths = Builder::get_help(&build, subcommand); - println!("{}", opts.usage(subcommand_help)); - if let Some(s) = paths { - if verbose { + if verbose { + // We have an unfortunate situation here: some Steps use `builder.in_tree_crates` to determine their paths. + // To determine those crates, we need to run `cargo metadata`, which means we need all submodules to be checked out. + // That takes a while to run, so only do it when paths were explicitly requested, not on all CLI errors. + // `Build::new` won't load submodules for the `setup` command. + let cmd = if verbose { + println!("note: updating submodules before printing available paths"); + "build" + } else { + "setup" + }; + let config = Config::parse(&[cmd.to_string()]); + let build = Build::new(config); + let paths = Builder::get_help(&build, subcommand); + + if let Some(s) = paths { println!("{}", s); } else { - println!( - "Run `./x.py {} -h -v` to see a list of available paths.", - subcommand.as_str() - ); + panic!("No paths available for subcommand `{}`", subcommand.as_str()); } - } else if verbose { - panic!("No paths available for subcommand `{}`", subcommand.as_str()); + } else { + println!( + "Run `./x.py {} -h -v` to see a list of available paths.", + subcommand.as_str() + ); } crate::detail_exit(exit_code); };