From 6ec3cf9abc15368df705b65e20e7e753d565fe71 Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 15 Feb 2025 11:53:08 +0100 Subject: [PATCH 01/14] Load all builtin targets at once instead of one by one This should give us some performance improvements as we won't need to do the lookup for the _currently_ 287 targets we have. --- compiler/rustc_session/src/config/cfg.rs | 8 ++------ compiler/rustc_target/src/spec/mod.rs | 13 +++++++++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_session/src/config/cfg.rs b/compiler/rustc_session/src/config/cfg.rs index d586f913335e0..1078757d09e9d 100644 --- a/compiler/rustc_session/src/config/cfg.rs +++ b/compiler/rustc_session/src/config/cfg.rs @@ -29,7 +29,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet}; use rustc_lint_defs::BuiltinLintDiag; use rustc_lint_defs::builtin::EXPLICIT_BUILTIN_CFGS_IN_FLAGS; use rustc_span::{Symbol, sym}; -use rustc_target::spec::{PanicStrategy, RelocModel, SanitizerSet, TARGETS, Target, TargetTuple}; +use rustc_target::spec::{PanicStrategy, RelocModel, SanitizerSet, Target}; use crate::Session; use crate::config::{CrateType, FmtDebug}; @@ -426,11 +426,7 @@ impl CheckCfg { panic!("unable to get all the check-cfg values buckets"); }; - for target in TARGETS - .iter() - .map(|target| Target::expect_builtin(&TargetTuple::from_tuple(target))) - .chain(iter::once(current_target.clone())) - { + for target in Target::builtins().chain(iter::once(current_target.clone())) { values_target_abi.insert(Symbol::intern(&target.options.abi)); values_target_arch.insert(Symbol::intern(&target.arch)); values_target_endian.insert(Symbol::intern(target.options.endian.as_str())); diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 72600225e7a38..2acc4ded5d803 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -1662,6 +1662,14 @@ macro_rules! supported_targets { Some(t) } + fn load_all_builtins() -> impl Iterator { + [ + $( targets::$module::target, )+ + ] + .into_iter() + .map(|f| f()) + } + #[cfg(test)] mod tests { // Cannot put this into a separate file without duplication, make an exception. @@ -3360,6 +3368,11 @@ impl Target { } } + /// Load all built-in targets + pub fn builtins() -> impl Iterator { + load_all_builtins() + } + /// Search for a JSON file specifying the given target tuple. /// /// If none is found in `$RUST_TARGET_PATH`, look for a file called `target.json` inside the From 17071ff8a5bef6e0f801e7d6b73529d0e1053b81 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 15 Feb 2025 21:49:50 +0000 Subject: [PATCH 02/14] Rework name_regions to not rely on reverse scc graph for non-member-constrain usages --- .../src/diagnostics/region_errors.rs | 30 +++++++++++++++---- .../src/region_infer/opaque_types.rs | 8 ++++- .../ui/borrowck/alias-liveness/name-region.rs | 13 ++++++++ .../alias-liveness/name-region.stderr | 14 +++++++++ 4 files changed, 58 insertions(+), 7 deletions(-) create mode 100644 tests/ui/borrowck/alias-liveness/name-region.rs create mode 100644 tests/ui/borrowck/alias-liveness/name-region.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs index b7e2510e035ac..df79da76bcea6 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs @@ -14,7 +14,10 @@ use rustc_infer::infer::{NllRegionVariableOrigin, RelateParamBound}; use rustc_middle::bug; use rustc_middle::hir::place::PlaceBase; use rustc_middle::mir::{AnnotationSource, ConstraintCategory, ReturnConstraint}; -use rustc_middle::ty::{self, GenericArgs, Region, RegionVid, Ty, TyCtxt, TypeVisitor}; +use rustc_middle::ty::fold::fold_regions; +use rustc_middle::ty::{ + self, GenericArgs, Region, RegionVid, Ty, TyCtxt, TypeFoldable, TypeVisitor, +}; use rustc_span::{Ident, Span, kw}; use rustc_trait_selection::error_reporting::InferCtxtErrorExt; use rustc_trait_selection::error_reporting::infer::nice_region_error::{ @@ -183,6 +186,17 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { } } + /// Map the regions in the type to named regions, where possible. + fn name_regions(&self, tcx: TyCtxt<'tcx>, ty: T) -> T + where + T: TypeFoldable>, + { + fold_regions(tcx, ty, |region, _| match *region { + ty::ReVar(vid) => self.to_error_region(vid).unwrap_or(region), + _ => region, + }) + } + /// Returns `true` if a closure is inferred to be an `FnMut` closure. fn is_closure_fn_mut(&self, fr: RegionVid) -> bool { if let Some(ty::ReLateParam(late_param)) = self.to_error_region(fr).as_deref() @@ -314,7 +328,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { let type_test_span = type_test.span; if let Some(lower_bound_region) = lower_bound_region { - let generic_ty = self.regioncx.name_regions( + let generic_ty = self.name_regions( self.infcx.tcx, type_test.generic_kind.to_ty(self.infcx.tcx), ); @@ -323,7 +337,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { self.body.source.def_id().expect_local(), type_test_span, Some(origin), - self.regioncx.name_regions(self.infcx.tcx, type_test.generic_kind), + self.name_regions(self.infcx.tcx, type_test.generic_kind), lower_bound_region, )); } else { @@ -354,9 +368,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { } RegionErrorKind::UnexpectedHiddenRegion { span, hidden_ty, key, member_region } => { - let named_ty = self.regioncx.name_regions(self.infcx.tcx, hidden_ty); - let named_key = self.regioncx.name_regions(self.infcx.tcx, key); - let named_region = self.regioncx.name_regions(self.infcx.tcx, member_region); + let named_ty = + self.regioncx.name_regions_for_member_constraint(self.infcx.tcx, hidden_ty); + let named_key = + self.regioncx.name_regions_for_member_constraint(self.infcx.tcx, key); + let named_region = self + .regioncx + .name_regions_for_member_constraint(self.infcx.tcx, member_region); let diag = unexpected_hidden_region_diagnostic( self.infcx, self.mir_def_id(), diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index 9c19f8b3ad825..54f9e82dbb82d 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -204,7 +204,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// that the regions produced are in fact equal to the named region they are /// replaced with. This is fine because this function is only to improve the /// region names in error messages. - pub(crate) fn name_regions(&self, tcx: TyCtxt<'tcx>, ty: T) -> T + /// + /// This differs from `MirBorrowckCtxt::name_regions` since it is particularly + /// lax with mapping region vids that are *shorter* than a universal region to + /// that universal region. This is useful for member region constraints since + /// we want to suggest a universal region name to capture even if it's technically + /// not equal to the error region. + pub(crate) fn name_regions_for_member_constraint(&self, tcx: TyCtxt<'tcx>, ty: T) -> T where T: TypeFoldable>, { diff --git a/tests/ui/borrowck/alias-liveness/name-region.rs b/tests/ui/borrowck/alias-liveness/name-region.rs new file mode 100644 index 0000000000000..9545a9eed2f13 --- /dev/null +++ b/tests/ui/borrowck/alias-liveness/name-region.rs @@ -0,0 +1,13 @@ +// Make sure we don't ICE when trying to name the regions that appear in the alias +// of the type test error. + +trait AnotherTrait { + type Ty2<'a>; +} + +fn test_alias(_: &'static T::Ty2<'_>) { + let _: &'static T::Ty2<'_>; + //~^ ERROR the associated type `::Ty2<'_>` may not live long enough +} + +fn main() {} diff --git a/tests/ui/borrowck/alias-liveness/name-region.stderr b/tests/ui/borrowck/alias-liveness/name-region.stderr new file mode 100644 index 0000000000000..9a5dd711c68e0 --- /dev/null +++ b/tests/ui/borrowck/alias-liveness/name-region.stderr @@ -0,0 +1,14 @@ +error[E0310]: the associated type `::Ty2<'_>` may not live long enough + --> $DIR/name-region.rs:9:12 + | +LL | let _: &'static T::Ty2<'_>; + | ^^^^^^^^^^^^^^^^^^^ + | | + | the associated type `::Ty2<'_>` must be valid for the static lifetime... + | ...so that the type `::Ty2<'_>` will meet its required lifetime bounds + | + = help: consider adding an explicit lifetime bound `::Ty2<'_>: 'static`... + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0310`. From 7e35729bfc2bbc0942ce8929d3c4d28cee7ca040 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sat, 15 Feb 2025 22:39:56 -0800 Subject: [PATCH 03/14] Don't project into `NonNull` when dropping a `Box` --- compiler/rustc_middle/src/mir/tcx.rs | 14 ++- .../rustc_mir_transform/src/elaborate_drop.rs | 30 +++++- .../src/elaborate_drops.rs | 4 + compiler/rustc_mir_transform/src/patch.rs | 8 ++ compiler/rustc_mir_transform/src/shim.rs | 3 + .../mir-opt/box_expr.main.ElaborateDrops.diff | 8 +- ...artial_move.maybe_move.ElaborateDrops.diff | 94 +++++++++++++++++++ tests/mir-opt/box_partial_move.rs | 17 ++++ 8 files changed, 168 insertions(+), 10 deletions(-) create mode 100644 tests/mir-opt/box_partial_move.maybe_move.ElaborateDrops.diff create mode 100644 tests/mir-opt/box_partial_move.rs diff --git a/compiler/rustc_middle/src/mir/tcx.rs b/compiler/rustc_middle/src/mir/tcx.rs index af23c8b2ea76d..862f78d725982 100644 --- a/compiler/rustc_middle/src/mir/tcx.rs +++ b/compiler/rustc_middle/src/mir/tcx.rs @@ -86,6 +86,14 @@ impl<'tcx> PlaceTy<'tcx> { } } + pub fn multi_projection_ty( + self, + tcx: TyCtxt<'tcx>, + elems: &[PlaceElem<'tcx>], + ) -> PlaceTy<'tcx> { + elems.iter().fold(self, |place_ty, &elem| place_ty.projection_ty(tcx, elem)) + } + /// Convenience wrapper around `projection_ty_core` for /// `PlaceElem`, where we can just use the `Ty` that is already /// stored inline on field projection elems. @@ -167,11 +175,7 @@ impl<'tcx> Place<'tcx> { where D: HasLocalDecls<'tcx>, { - projection - .iter() - .fold(PlaceTy::from_ty(local_decls.local_decls()[local].ty), |place_ty, &elem| { - place_ty.projection_ty(tcx, elem) - }) + PlaceTy::from_ty(local_decls.local_decls()[local].ty).multi_projection_ty(tcx, projection) } pub fn ty(&self, local_decls: &D, tcx: TyCtxt<'tcx>) -> PlaceTy<'tcx> diff --git a/compiler/rustc_mir_transform/src/elaborate_drop.rs b/compiler/rustc_mir_transform/src/elaborate_drop.rs index ed4903017f353..2de55e38052e3 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drop.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drop.rs @@ -89,6 +89,7 @@ pub(crate) trait DropElaborator<'a, 'tcx>: fmt::Debug { // Accessors + fn patch_ref(&self) -> &MirPatch<'tcx>; fn patch(&mut self) -> &mut MirPatch<'tcx>; fn body(&self) -> &'a Body<'tcx>; fn tcx(&self) -> TyCtxt<'tcx>; @@ -180,7 +181,14 @@ where { #[instrument(level = "trace", skip(self), ret)] fn place_ty(&self, place: Place<'tcx>) -> Ty<'tcx> { - place.ty(self.elaborator.body(), self.tcx()).ty + if place.local < self.elaborator.body().local_decls.next_index() { + place.ty(self.elaborator.body(), self.tcx()).ty + } else { + // We don't have a slice with all the locals, since some are in the patch. + tcx::PlaceTy::from_ty(self.elaborator.patch_ref().local_ty(place.local)) + .multi_projection_ty(self.elaborator.tcx(), place.projection) + .ty + } } fn tcx(&self) -> TyCtxt<'tcx> { @@ -410,12 +418,26 @@ where let unique_place = self.tcx().mk_place_field(self.place, FieldIdx::ZERO, unique_ty); let nonnull_place = self.tcx().mk_place_field(unique_place, FieldIdx::ZERO, nonnull_ty); - let ptr_place = self.tcx().mk_place_field(nonnull_place, FieldIdx::ZERO, ptr_ty); - let interior = self.tcx().mk_place_deref(ptr_place); + let ptr_local = self.new_temp(ptr_ty); + + let interior = self.tcx().mk_place_deref(Place::from(ptr_local)); let interior_path = self.elaborator.deref_subpath(self.path); - self.drop_subpath(interior, interior_path, succ, unwind) + let do_drop_bb = self.drop_subpath(interior, interior_path, succ, unwind); + + let setup_bbd = BasicBlockData { + statements: vec![self.assign( + Place::from(ptr_local), + Rvalue::Cast(CastKind::Transmute, Operand::Copy(nonnull_place), ptr_ty), + )], + terminator: Some(Terminator { + kind: TerminatorKind::Goto { target: do_drop_bb }, + source_info: self.source_info, + }), + is_cleanup: unwind.is_cleanup(), + }; + self.elaborator.patch().new_block(setup_bbd) } #[instrument(level = "debug", ret)] diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 2d74fcff415ed..ab6aafab446bb 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -138,6 +138,10 @@ impl InitializationData<'_, '_> { impl<'a, 'tcx> DropElaborator<'a, 'tcx> for ElaborateDropsCtxt<'a, 'tcx> { type Path = MovePathIndex; + fn patch_ref(&self) -> &MirPatch<'tcx> { + &self.patch + } + fn patch(&mut self) -> &mut MirPatch<'tcx> { &mut self.patch } diff --git a/compiler/rustc_mir_transform/src/patch.rs b/compiler/rustc_mir_transform/src/patch.rs index 72cd9c224f648..b4f6fa514a487 100644 --- a/compiler/rustc_mir_transform/src/patch.rs +++ b/compiler/rustc_mir_transform/src/patch.rs @@ -166,6 +166,14 @@ impl<'tcx> MirPatch<'tcx> { Local::new(index) } + /// Returns the type of a local that's newly-added in the patch. + pub(crate) fn local_ty(&self, local: Local) -> Ty<'tcx> { + let local = local.as_usize(); + assert!(local < self.next_local); + let new_local_idx = self.new_locals.len() - (self.next_local - local); + self.new_locals[new_local_idx].ty + } + pub(crate) fn new_block(&mut self, data: BasicBlockData<'tcx>) -> BasicBlock { let block = BasicBlock::new(self.patch_map.len()); debug!("MirPatch: new_block: {:?}: {:?}", block, data); diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index e8d86bad98735..34074a84e28b6 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -350,6 +350,9 @@ impl fmt::Debug for DropShimElaborator<'_, '_> { impl<'a, 'tcx> DropElaborator<'a, 'tcx> for DropShimElaborator<'a, 'tcx> { type Path = (); + fn patch_ref(&self) -> &MirPatch<'tcx> { + &self.patch + } fn patch(&mut self) -> &mut MirPatch<'tcx> { &mut self.patch } diff --git a/tests/mir-opt/box_expr.main.ElaborateDrops.diff b/tests/mir-opt/box_expr.main.ElaborateDrops.diff index ec40fac2894eb..827dc6ac7aefe 100644 --- a/tests/mir-opt/box_expr.main.ElaborateDrops.diff +++ b/tests/mir-opt/box_expr.main.ElaborateDrops.diff @@ -12,6 +12,7 @@ let mut _7: std::boxed::Box; + let mut _8: &mut std::boxed::Box; + let mut _9: (); ++ let mut _10: *const S; scope 1 { debug x => _1; } @@ -68,7 +69,7 @@ bb8 (cleanup): { - drop(_5) -> [return: bb9, unwind terminate(cleanup)]; -+ goto -> bb11; ++ goto -> bb12; } bb9 (cleanup): { @@ -82,6 +83,11 @@ + + bb11 (cleanup): { + goto -> bb10; ++ } ++ ++ bb12 (cleanup): { ++ _10 = copy ((_5.0: std::ptr::Unique).0: std::ptr::NonNull) as *const S (Transmute); ++ goto -> bb11; } } diff --git a/tests/mir-opt/box_partial_move.maybe_move.ElaborateDrops.diff b/tests/mir-opt/box_partial_move.maybe_move.ElaborateDrops.diff new file mode 100644 index 0000000000000..f090795e88656 --- /dev/null +++ b/tests/mir-opt/box_partial_move.maybe_move.ElaborateDrops.diff @@ -0,0 +1,94 @@ +- // MIR for `maybe_move` before ElaborateDrops ++ // MIR for `maybe_move` after ElaborateDrops + + fn maybe_move(_1: bool, _2: Box) -> Option { + debug cond => _1; + debug thing => _2; + let mut _0: std::option::Option; + let mut _3: bool; + let mut _4: std::string::String; ++ let mut _5: bool; ++ let mut _6: &mut std::boxed::Box; ++ let mut _7: (); ++ let mut _8: &mut std::boxed::Box; ++ let mut _9: (); ++ let mut _10: *const std::string::String; + + bb0: { ++ _5 = const false; ++ _5 = const true; + StorageLive(_3); + _3 = copy _1; + switchInt(move _3) -> [0: bb3, otherwise: bb1]; + } + + bb1: { + StorageLive(_4); ++ _5 = const false; + _4 = move (*_2); + _0 = Option::::Some(move _4); +- drop(_4) -> [return: bb2, unwind: bb6]; ++ goto -> bb2; + } + + bb2: { + StorageDead(_4); + goto -> bb4; + } + + bb3: { + _0 = Option::::None; + goto -> bb4; + } + + bb4: { + StorageDead(_3); +- drop(_2) -> [return: bb5, unwind continue]; ++ goto -> bb14; + } + + bb5: { + return; + } + + bb6 (cleanup): { +- drop(_2) -> [return: bb7, unwind terminate(cleanup)]; ++ goto -> bb7; + } + + bb7 (cleanup): { + resume; ++ } ++ ++ bb8: { ++ goto -> bb5; ++ } ++ ++ bb9: { ++ _6 = &mut _2; ++ _7 = as Drop>::drop(move _6) -> [return: bb8, unwind: bb7]; ++ } ++ ++ bb10 (cleanup): { ++ _8 = &mut _2; ++ _9 = as Drop>::drop(move _8) -> [return: bb7, unwind terminate(cleanup)]; ++ } ++ ++ bb11: { ++ goto -> bb13; ++ } ++ ++ bb12: { ++ drop((*_10)) -> [return: bb9, unwind: bb10]; ++ } ++ ++ bb13: { ++ switchInt(copy _5) -> [0: bb9, otherwise: bb12]; ++ } ++ ++ bb14: { ++ _10 = copy ((_2.0: std::ptr::Unique).0: std::ptr::NonNull) as *const std::string::String (Transmute); ++ goto -> bb11; + } + } + diff --git a/tests/mir-opt/box_partial_move.rs b/tests/mir-opt/box_partial_move.rs new file mode 100644 index 0000000000000..5cbd242986f52 --- /dev/null +++ b/tests/mir-opt/box_partial_move.rs @@ -0,0 +1,17 @@ +//@ test-mir-pass: ElaborateDrops +//@ needs-unwind + +#![feature(rustc_attrs, liballoc_internals)] + +// EMIT_MIR box_partial_move.maybe_move.ElaborateDrops.diff +fn maybe_move(cond: bool, thing: Box) -> Option { + // CHECK-LABEL: fn maybe_move( + // CHECK: let mut [[PTR:_[0-9]+]]: *const std::string::String; + // CHECK: [[PTR]] = copy ((_2.0: std::ptr::Unique).0: std::ptr::NonNull) as *const std::string::String (Transmute); + // CHECK: drop((*[[PTR]])) + if cond { Some(*thing) } else { None } +} + +fn main() { + maybe_move(false, Box::new("hello".to_string())); +} From 6bdf3407b4b5356e02afd0bfaca43839789d4a67 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 14 Feb 2025 11:43:18 +0530 Subject: [PATCH 04/14] add docs to cc-detect --- src/bootstrap/src/utils/cc_detect.rs | 31 +++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 45797c1276c58..411ab07bbe14a 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -29,6 +29,7 @@ use crate::core::config::TargetSelection; use crate::utils::exec::{BootstrapCommand, command}; use crate::{Build, CLang, GitRepo}; +/// Finds archiver tool for the given target if possible. /// FIXME(onur-ozkan): This logic should be replaced by calling into the `cc` crate. fn cc2ar(cc: &Path, target: TargetSelection, default_ar: PathBuf) -> Option { if let Some(ar) = env::var_os(format!("AR_{}", target.triple.replace('-', "_"))) { @@ -58,6 +59,7 @@ fn cc2ar(cc: &Path, target: TargetSelection, default_ar: PathBuf) -> Option cc::Build { let mut cfg = cc::Build::new(); cfg.cargo_metadata(false) @@ -84,6 +86,12 @@ fn new_cc_build(build: &Build, target: TargetSelection) -> cc::Build { cfg } +/// Probes for C and C++ compilers and configures the corresponding entries in the [`Build`] +/// structure. +/// +/// This function determines which targets need a C compiler (and, if needed, a C++ compiler) +/// by combining the primary build target, host targets, and any additional targets. For +/// each target, it calls [`find_target`] to configure the necessary compiler tools. pub fn find(build: &Build) { let targets: HashSet<_> = match build.config.cmd { // We don't need to check cross targets for these commands. @@ -112,6 +120,11 @@ pub fn find(build: &Build) { } } +/// Probes and configures the C and C++ compilers for a single target. +/// +/// This function uses both user-specified configuration (from `config.toml`) and auto-detection +/// logic to determine the correct C/C++ compilers for the target. It also determines the appropriate +/// archiver (`ar`) and sets up additional compilation flags (both handled and unhandled). pub fn find_target(build: &Build, target: TargetSelection) { let mut cfg = new_cc_build(build, target); let config = build.config.target_config.get(&target); @@ -172,6 +185,8 @@ pub fn find_target(build: &Build, target: TargetSelection) { } } +/// Determines the default compiler for a given target and language when not explicitly +/// configured in `config.toml`. fn default_compiler( cfg: &mut cc::Build, compiler: Language, @@ -248,6 +263,12 @@ fn default_compiler( } } +/// Constructs the path to the Android NDK compiler for the given target triple and language. +/// +/// This helper function transform the target triple by converting certain architecture names +/// (for example, translating "arm" to "arm7a"), appends the minimum API level (hardcoded as "21" +/// for NDK r26d), and then constructs the full path based on the provided NDK directory and host +/// platform. pub(crate) fn ndk_compiler(compiler: Language, triple: &str, ndk: &Path) -> PathBuf { let mut triple_iter = triple.split('-'); let triple_translated = if let Some(arch) = triple_iter.next() { @@ -277,7 +298,11 @@ pub(crate) fn ndk_compiler(compiler: Language, triple: &str, ndk: &Path) -> Path ndk.join("toolchains").join("llvm").join("prebuilt").join(host_tag).join("bin").join(compiler) } -/// The target programming language for a native compiler. +/// Representing the target programming language for a native compiler. +/// +/// This enum is used to indicate whether a particular compiler is intended for C or C++. +/// It also provides helper methods for obtaining the standard executable names for GCC and +/// clang-based compilers. #[derive(PartialEq)] pub(crate) enum Language { /// The compiler is targeting C. @@ -287,7 +312,7 @@ pub(crate) enum Language { } impl Language { - /// Obtains the name of a compiler in the GCC collection. + /// Returns the executable name for a GCC compiler corresponding to this language. fn gcc(self) -> &'static str { match self { Language::C => "gcc", @@ -295,7 +320,7 @@ impl Language { } } - /// Obtains the name of a compiler in the clang suite. + /// Returns the executable name for a clang-based compiler corresponding to this language. fn clang(self) -> &'static str { match self { Language::C => "clang", From f6c911ab43226fc6c90ee2bf6c300c4e53566899 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 14 Feb 2025 11:43:34 +0530 Subject: [PATCH 05/14] add unit test for cc-detect --- src/bootstrap/src/utils/cc_detect.rs | 3 + src/bootstrap/src/utils/cc_detect/tests.rs | 254 +++++++++++++++++++++ 2 files changed, 257 insertions(+) create mode 100644 src/bootstrap/src/utils/cc_detect/tests.rs diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 411ab07bbe14a..1e84a7deff12e 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -328,3 +328,6 @@ impl Language { } } } + +#[cfg(test)] +mod tests; diff --git a/src/bootstrap/src/utils/cc_detect/tests.rs b/src/bootstrap/src/utils/cc_detect/tests.rs new file mode 100644 index 0000000000000..006dfe7e5d7b3 --- /dev/null +++ b/src/bootstrap/src/utils/cc_detect/tests.rs @@ -0,0 +1,254 @@ +use std::path::{Path, PathBuf}; +use std::{env, iter}; + +use super::*; +use crate::core::config::{Target, TargetSelection}; +use crate::{Build, Config, Flags}; + +#[test] +fn test_cc2ar_env_specific() { + let triple = "x86_64-unknown-linux-gnu"; + let key = "AR_x86_64_unknown_linux_gnu"; + env::set_var(key, "custom-ar"); + let target = TargetSelection::from_user(triple); + let cc = Path::new("/usr/bin/clang"); + let default_ar = PathBuf::from("default-ar"); + let result = cc2ar(cc, target, default_ar); + env::remove_var(key); + assert_eq!(result, Some(PathBuf::from("custom-ar"))); +} + +#[test] +fn test_cc2ar_musl() { + let triple = "x86_64-unknown-linux-musl"; + env::remove_var("AR_x86_64_unknown_linux_musl"); + env::remove_var("AR"); + let target = TargetSelection::from_user(triple); + let cc = Path::new("/usr/bin/clang"); + let default_ar = PathBuf::from("default-ar"); + let result = cc2ar(cc, target, default_ar); + assert_eq!(result, Some(PathBuf::from("ar"))); +} + +#[test] +fn test_cc2ar_openbsd() { + let triple = "x86_64-unknown-openbsd"; + env::remove_var("AR_x86_64_unknown_openbsd"); + env::remove_var("AR"); + let target = TargetSelection::from_user(triple); + let cc = Path::new("/usr/bin/cc"); + let default_ar = PathBuf::from("default-ar"); + let result = cc2ar(cc, target, default_ar); + assert_eq!(result, Some(PathBuf::from("ar"))); +} + +#[test] +fn test_cc2ar_vxworks() { + let triple = "armv7-wrs-vxworks"; + env::remove_var("AR_armv7_wrs_vxworks"); + env::remove_var("AR"); + let target = TargetSelection::from_user(triple); + let cc = Path::new("/usr/bin/clang"); + let default_ar = PathBuf::from("default-ar"); + let result = cc2ar(cc, target, default_ar); + assert_eq!(result, Some(PathBuf::from("wr-ar"))); +} + +#[test] +fn test_cc2ar_nto_i586() { + let triple = "i586-unknown-nto-something"; + env::remove_var("AR_i586_unknown_nto_something"); + env::remove_var("AR"); + let target = TargetSelection::from_user(triple); + let cc = Path::new("/usr/bin/clang"); + let default_ar = PathBuf::from("default-ar"); + let result = cc2ar(cc, target, default_ar); + assert_eq!(result, Some(PathBuf::from("ntox86-ar"))); +} + +#[test] +fn test_cc2ar_nto_aarch64() { + let triple = "aarch64-unknown-nto-something"; + env::remove_var("AR_aarch64_unknown_nto_something"); + env::remove_var("AR"); + let target = TargetSelection::from_user(triple); + let cc = Path::new("/usr/bin/clang"); + let default_ar = PathBuf::from("default-ar"); + let result = cc2ar(cc, target, default_ar); + assert_eq!(result, Some(PathBuf::from("ntoaarch64-ar"))); +} + +#[test] +fn test_cc2ar_nto_x86_64() { + let triple = "x86_64-unknown-nto-something"; + env::remove_var("AR_x86_64_unknown_nto_something"); + env::remove_var("AR"); + let target = TargetSelection::from_user(triple); + let cc = Path::new("/usr/bin/clang"); + let default_ar = PathBuf::from("default-ar"); + let result = cc2ar(cc, target, default_ar); + assert_eq!(result, Some(PathBuf::from("ntox86_64-ar"))); +} + +#[test] +#[should_panic(expected = "Unknown architecture, cannot determine archiver for Neutrino QNX")] +fn test_cc2ar_nto_unknown() { + let triple = "powerpc-unknown-nto-something"; + env::remove_var("AR_powerpc_unknown_nto_something"); + env::remove_var("AR"); + let target = TargetSelection::from_user(triple); + let cc = Path::new("/usr/bin/clang"); + let default_ar = PathBuf::from("default-ar"); + let _ = cc2ar(cc, target, default_ar); +} + +#[test] +fn test_ndk_compiler_c() { + let ndk_path = PathBuf::from("/ndk"); + let target_triple = "arm-unknown-linux-android"; + let expected_triple_translated = "armv7a-unknown-linux-android"; + let expected_compiler = format!("{}21-{}", expected_triple_translated, Language::C.clang()); + let host_tag = if cfg!(target_os = "macos") { + "darwin-x86_64" + } else if cfg!(target_os = "windows") { + "windows-x86_64" + } else { + "linux-x86_64" + }; + let expected_path = ndk_path + .join("toolchains") + .join("llvm") + .join("prebuilt") + .join(host_tag) + .join("bin") + .join(&expected_compiler); + let result = ndk_compiler(Language::C, target_triple, &ndk_path); + assert_eq!(result, expected_path); +} + +#[test] +fn test_ndk_compiler_cpp() { + let ndk_path = PathBuf::from("/ndk"); + let target_triple = "arm-unknown-linux-android"; + let expected_triple_translated = "armv7a-unknown-linux-android"; + let expected_compiler = + format!("{}21-{}", expected_triple_translated, Language::CPlusPlus.clang()); + let host_tag = if cfg!(target_os = "macos") { + "darwin-x86_64" + } else if cfg!(target_os = "windows") { + "windows-x86_64" + } else { + "linux-x86_64" + }; + let expected_path = ndk_path + .join("toolchains") + .join("llvm") + .join("prebuilt") + .join(host_tag) + .join("bin") + .join(&expected_compiler); + let result = ndk_compiler(Language::CPlusPlus, target_triple, &ndk_path); + assert_eq!(result, expected_path); +} + +#[test] +fn test_language_gcc() { + assert_eq!(Language::C.gcc(), "gcc"); + assert_eq!(Language::CPlusPlus.gcc(), "g++"); +} + +#[test] +fn test_language_clang() { + assert_eq!(Language::C.clang(), "clang"); + assert_eq!(Language::CPlusPlus.clang(), "clang++"); +} + +#[test] +fn test_new_cc_build() { + let build = Build::new(Config { ..Config::parse(Flags::parse(&["check".to_owned()])) }); + let target = TargetSelection::from_user("x86_64-unknown-linux-gnu"); + let cfg = new_cc_build(&build, target.clone()); + let compiler = cfg.get_compiler(); + assert!(!compiler.path().to_str().unwrap().is_empty(), "Compiler path should not be empty"); +} + +#[test] +fn test_default_compiler_wasi() { + let build = Build::new(Config { ..Config::parse(Flags::parse(&["check".to_owned()])) }); + let target = TargetSelection::from_user("wasm32-wasi"); + let wasi_sdk = PathBuf::from("/wasi-sdk"); + env::set_var("WASI_SDK_PATH", &wasi_sdk); + let mut cfg = cc::Build::new(); + if let Some(result) = default_compiler(&mut cfg, Language::C, target.clone(), &build) { + let expected = { + let compiler = format!("{}-clang", target.triple); + wasi_sdk.join("bin").join(compiler) + }; + assert_eq!(result, expected); + } else { + panic!( + "default_compiler should return a compiler path for wasi target when WASI_SDK_PATH is set" + ); + } + env::remove_var("WASI_SDK_PATH"); +} + +#[test] +fn test_default_compiler_fallback() { + let build = Build::new(Config { ..Config::parse(Flags::parse(&["check".to_owned()])) }); + let target = TargetSelection::from_user("x86_64-unknown-linux-gnu"); + let mut cfg = cc::Build::new(); + let result = default_compiler(&mut cfg, Language::C, target, &build); + assert!(result.is_none(), "default_compiler should return None for generic targets"); +} + +#[test] +fn test_find_target_with_config() { + let mut build = Build::new(Config { ..Config::parse(Flags::parse(&["check".to_owned()])) }); + let target = TargetSelection::from_user("x86_64-unknown-linux-gnu"); + let mut target_config = Target::default(); + target_config.cc = Some(PathBuf::from("dummy-cc")); + target_config.cxx = Some(PathBuf::from("dummy-cxx")); + target_config.ar = Some(PathBuf::from("dummy-ar")); + target_config.ranlib = Some(PathBuf::from("dummy-ranlib")); + build.config.target_config.insert(target.clone(), target_config); + find_target(&build, target.clone()); + let binding = build.cc.borrow(); + let cc_tool = binding.get(&target).unwrap(); + assert_eq!(cc_tool.path(), &PathBuf::from("dummy-cc")); + let binding = build.cxx.borrow(); + let cxx_tool = binding.get(&target).unwrap(); + assert_eq!(cxx_tool.path(), &PathBuf::from("dummy-cxx")); + let binding = build.ar.borrow(); + let ar = binding.get(&target).unwrap(); + assert_eq!(ar, &PathBuf::from("dummy-ar")); + let binding = build.ranlib.borrow(); + let ranlib = binding.get(&target).unwrap(); + assert_eq!(ranlib, &PathBuf::from("dummy-ranlib")); +} + +#[test] +fn test_find_target_without_config() { + let mut build = Build::new(Config { ..Config::parse(Flags::parse(&["check".to_owned()])) }); + let target = TargetSelection::from_user("x86_64-unknown-linux-gnu"); + build.config.target_config.clear(); + find_target(&build, target.clone()); + assert!(build.cc.borrow().contains_key(&target)); + if !target.triple.contains("vxworks") { + assert!(build.cxx.borrow().contains_key(&target)); + } + assert!(build.ar.borrow().contains_key(&target)); +} + +#[test] +fn test_find() { + let mut build = Build::new(Config { ..Config::parse(Flags::parse(&["check".to_owned()])) }); + let target1 = TargetSelection::from_user("x86_64-unknown-linux-gnu"); + let target2 = TargetSelection::from_user("arm-linux-androideabi"); + build.targets.push(target1.clone()); + build.hosts.push(target2.clone()); + find(&build); + for t in build.hosts.iter().chain(build.targets.iter()).chain(iter::once(&build.build)) { + assert!(build.cc.borrow().contains_key(t), "CC not set for target {}", t.triple); + } +} From f396a3107507bb1925eba5a429da6adc40a06d61 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sun, 16 Feb 2025 07:49:22 +0000 Subject: [PATCH 06/14] Add an example for std::error::Error --- library/core/src/error.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/library/core/src/error.rs b/library/core/src/error.rs index 33cf2af30b954..69ad7239954ac 100644 --- a/library/core/src/error.rs +++ b/library/core/src/error.rs @@ -22,6 +22,30 @@ use crate::fmt::{self, Debug, Display, Formatter}; /// accessing that error via [`Error::source()`]. This makes it possible for the /// high-level module to provide its own errors while also revealing some of the /// implementation for debugging. +/// +/// # Example +/// +/// Implementing the `Error` trait only requires that `Debug` and `Display` are implemented too. +/// +/// ``` +/// use std::error::Error; +/// use std::fmt; +/// use std::path::PathBuf; +/// +/// #[derive(Debug)] +/// struct ReadConfigError { +/// path: PathBuf +/// } +/// +/// impl fmt::Display for ReadConfigError { +/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +/// let path = self.path.display(); +/// write!(f, "unable to read configuration at {path}") +/// } +/// } +/// +/// impl Error for ReadConfigError {} +/// ``` #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "Error")] #[rustc_has_incoherent_inherent_impls] From 8ae3ca98e5c225da22e7ab1a86069098a74cdfe2 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sun, 16 Feb 2025 09:08:24 +0000 Subject: [PATCH 07/14] Fix test that relies on error language --- tests/ui/unpretty/staged-api-invalid-path-108697.rs | 4 ++-- tests/ui/unpretty/staged-api-invalid-path-108697.stderr | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/ui/unpretty/staged-api-invalid-path-108697.rs b/tests/ui/unpretty/staged-api-invalid-path-108697.rs index 71bad213576c7..8a806b10d9da1 100644 --- a/tests/ui/unpretty/staged-api-invalid-path-108697.rs +++ b/tests/ui/unpretty/staged-api-invalid-path-108697.rs @@ -2,8 +2,8 @@ // ICE: tcx.resolutions(()) is not supported for local crate -Zunpretty=mir // on invalid module path with staged_api //@ compile-flags: -Zunpretty=mir -//@ normalize-stderr: "The system cannot find the file specified." -> "No such file or directory" +//@ normalize-stderr: "lol`: .*\(" -> "lol`: $$FILE_NOT_FOUND_MSG (" #![feature(staged_api)] #[path = "lol"] mod foo; -//~^ ERROR couldn't read +//~^ ERROR couldn't read `$DIR/lol` diff --git a/tests/ui/unpretty/staged-api-invalid-path-108697.stderr b/tests/ui/unpretty/staged-api-invalid-path-108697.stderr index e68e19c4dc99e..188f4985ded56 100644 --- a/tests/ui/unpretty/staged-api-invalid-path-108697.stderr +++ b/tests/ui/unpretty/staged-api-invalid-path-108697.stderr @@ -1,4 +1,4 @@ -error: couldn't read `$DIR/lol`: No such file or directory (os error 2) +error: couldn't read `$DIR/lol`: $FILE_NOT_FOUND_MSG (os error 2) --> $DIR/staged-api-invalid-path-108697.rs:8:1 | LL | mod foo; From 56f8f48e05a0635f28fa293557aa10c0162b8526 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sun, 16 Feb 2025 12:11:25 +0300 Subject: [PATCH 08/14] fix broken `x {doc, build} core` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/compile.rs | 2 +- src/bootstrap/src/core/build_steps/doc.rs | 5 +---- src/bootstrap/src/core/builder/mod.rs | 12 ++++++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 479327d63695c..84cf99b55402a 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -95,7 +95,7 @@ impl Step for Std { const DEFAULT: bool = true; fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.crate_or_deps("sysroot").path("library").alias("core") + run.crate_or_deps("sysroot").path("library") } fn make_run(run: RunConfig<'_>) { diff --git a/src/bootstrap/src/core/build_steps/doc.rs b/src/bootstrap/src/core/build_steps/doc.rs index dedcc139ae198..23bb47dcc5863 100644 --- a/src/bootstrap/src/core/build_steps/doc.rs +++ b/src/bootstrap/src/core/build_steps/doc.rs @@ -572,10 +572,7 @@ impl Step for Std { fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { let builder = run.builder; - run.crate_or_deps("sysroot") - .path("library") - .alias("core") - .default_condition(builder.config.docs) + run.crate_or_deps("sysroot").path("library").default_condition(builder.config.docs) } fn make_run(run: RunConfig<'_>) { diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs index ecec589fc32eb..52876c3fb3fd7 100644 --- a/src/bootstrap/src/core/builder/mod.rs +++ b/src/bootstrap/src/core/builder/mod.rs @@ -127,10 +127,14 @@ impl RunConfig<'_> { pub fn cargo_crates_in_set(&self) -> Vec { let mut crates = Vec::new(); for krate in &self.paths { - let path = krate.assert_single_path(); - let Some(crate_name) = self.builder.crate_paths.get(&path.path) else { - panic!("missing crate for path {}", path.path.display()) - }; + let path = &krate.assert_single_path().path; + + let crate_name = self + .builder + .crate_paths + .get(path) + .unwrap_or_else(|| panic!("missing crate for path {}", path.display())); + crates.push(crate_name.to_string()); } crates From 75195b57db1e44d6c96daac67b3c778135374679 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Mon, 13 May 2024 20:32:33 +0200 Subject: [PATCH 09/14] Optimize `Seek::stream_len` impl for `File` It uses the file metadata on Unix with a fallback for files incorrectly reported as zero-sized. It uses `GetFileSizeEx` on Windows. This reduces the number of syscalls needed for determining the file size of an open file from 3 to 1. --- library/std/src/fs.rs | 35 +++++++++++++++++++ library/std/src/io/mod.rs | 24 +++++++------ library/std/src/sys/pal/hermit/fs.rs | 4 +++ library/std/src/sys/pal/solid/fs.rs | 4 +++ library/std/src/sys/pal/unix/fs.rs | 9 +++++ library/std/src/sys/pal/unsupported/fs.rs | 4 +++ library/std/src/sys/pal/wasi/fs.rs | 4 +++ .../std/src/sys/pal/windows/c/bindings.txt | 1 + .../std/src/sys/pal/windows/c/windows_sys.rs | 1 + library/std/src/sys/pal/windows/fs.rs | 8 +++++ 10 files changed, 84 insertions(+), 10 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 83b009c86dc08..0ae4018ca5b40 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1226,9 +1226,38 @@ impl Write for &File { } #[stable(feature = "rust1", since = "1.0.0")] impl Seek for &File { + /// Seek to an offset, in bytes in a file. + /// + /// See [`Seek::seek`] docs for more info. + /// + /// # Platform-specific behavior + /// + /// This function currently corresponds to the `lseek64` function on Unix + /// and the `SetFilePointerEx` function on Windows. Note that this [may + /// change in the future][changes]. + /// + /// [changes]: io#platform-specific-behavior fn seek(&mut self, pos: SeekFrom) -> io::Result { self.inner.seek(pos) } + + /// Returns the length of this file (in bytes). + /// + /// See [`Seek::stream_len`] docs for more info. + /// + /// # Platform-specific behavior + /// + /// This function currently corresponds to the `statx` function on Linux + /// (with fallbacks) and the `GetFileSizeEx` function on Windows. Note that + /// this [may change in the future][changes]. + /// + /// [changes]: io#platform-specific-behavior + fn stream_len(&mut self) -> io::Result { + if let Some(result) = self.inner.size() { + return result; + } + io::stream_len_default(self) + } } #[stable(feature = "rust1", since = "1.0.0")] @@ -1275,6 +1304,9 @@ impl Seek for File { fn seek(&mut self, pos: SeekFrom) -> io::Result { (&*self).seek(pos) } + fn stream_len(&mut self) -> io::Result { + (&*self).stream_len() + } } #[stable(feature = "io_traits_arc", since = "1.73.0")] @@ -1321,6 +1353,9 @@ impl Seek for Arc { fn seek(&mut self, pos: SeekFrom) -> io::Result { (&**self).seek(pos) } + fn stream_len(&mut self) -> io::Result { + (&**self).stream_len() + } } impl OpenOptions { diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 980ea1478e084..32a98f2c47aae 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2051,16 +2051,7 @@ pub trait Seek { /// ``` #[unstable(feature = "seek_stream_len", issue = "59359")] fn stream_len(&mut self) -> Result { - let old_pos = self.stream_position()?; - let len = self.seek(SeekFrom::End(0))?; - - // Avoid seeking a third time when we were already at the end of the - // stream. The branch is usually way cheaper than a seek operation. - if old_pos != len { - self.seek(SeekFrom::Start(old_pos))?; - } - - Ok(len) + stream_len_default(self) } /// Returns the current seek position from the start of the stream. @@ -2121,6 +2112,19 @@ pub trait Seek { } } +pub(crate) fn stream_len_default(self_: &mut T) -> Result { + let old_pos = self_.stream_position()?; + let len = self_.seek(SeekFrom::End(0))?; + + // Avoid seeking a third time when we were already at the end of the + // stream. The branch is usually way cheaper than a seek operation. + if old_pos != len { + self_.seek(SeekFrom::Start(old_pos))?; + } + + Ok(len) +} + /// Enumeration of possible methods to seek within an I/O object. /// /// It is used by the [`Seek`] trait. diff --git a/library/std/src/sys/pal/hermit/fs.rs b/library/std/src/sys/pal/hermit/fs.rs index 783623552bb17..2eb659941ab7a 100644 --- a/library/std/src/sys/pal/hermit/fs.rs +++ b/library/std/src/sys/pal/hermit/fs.rs @@ -425,6 +425,10 @@ impl File { Err(Error::from_raw_os_error(22)) } + pub fn size(&self) -> Option> { + None + } + pub fn duplicate(&self) -> io::Result { Err(Error::from_raw_os_error(22)) } diff --git a/library/std/src/sys/pal/solid/fs.rs b/library/std/src/sys/pal/solid/fs.rs index cc424141ea80c..393231c18f931 100644 --- a/library/std/src/sys/pal/solid/fs.rs +++ b/library/std/src/sys/pal/solid/fs.rs @@ -465,6 +465,10 @@ impl File { } } + pub fn size(&self) -> Option> { + None + } + pub fn duplicate(&self) -> io::Result { unsupported() } diff --git a/library/std/src/sys/pal/unix/fs.rs b/library/std/src/sys/pal/unix/fs.rs index 00cfa7a7fcfda..53d394b86e385 100644 --- a/library/std/src/sys/pal/unix/fs.rs +++ b/library/std/src/sys/pal/unix/fs.rs @@ -1438,6 +1438,15 @@ impl File { Ok(n as u64) } + pub fn size(&self) -> Option> { + match self.file_attr().map(|attr| attr.size()) { + // Fall back to default implementation if the returned size is 0, + // we might be in a proc mount. + Ok(0) => None, + result => Some(result), + } + } + pub fn duplicate(&self) -> io::Result { self.0.duplicate().map(File) } diff --git a/library/std/src/sys/pal/unsupported/fs.rs b/library/std/src/sys/pal/unsupported/fs.rs index 9585ec24f687d..0d661a87cdc65 100644 --- a/library/std/src/sys/pal/unsupported/fs.rs +++ b/library/std/src/sys/pal/unsupported/fs.rs @@ -258,6 +258,10 @@ impl File { self.0 } + pub fn size(&self) -> io::Result { + self.0 + } + pub fn duplicate(&self) -> io::Result { self.0 } diff --git a/library/std/src/sys/pal/wasi/fs.rs b/library/std/src/sys/pal/wasi/fs.rs index faf3edd5da6ce..dcefeb2b75955 100644 --- a/library/std/src/sys/pal/wasi/fs.rs +++ b/library/std/src/sys/pal/wasi/fs.rs @@ -517,6 +517,10 @@ impl File { self.fd.seek(pos) } + pub fn size(&self) -> Option> { + None + } + pub fn duplicate(&self) -> io::Result { // https://github.com/CraneStation/wasmtime/blob/master/docs/WASI-rationale.md#why-no-dup unsupported() diff --git a/library/std/src/sys/pal/windows/c/bindings.txt b/library/std/src/sys/pal/windows/c/bindings.txt index e2c2163327968..d8ef709f8cf56 100644 --- a/library/std/src/sys/pal/windows/c/bindings.txt +++ b/library/std/src/sys/pal/windows/c/bindings.txt @@ -2156,6 +2156,7 @@ GetExitCodeProcess GetFileAttributesW GetFileInformationByHandle GetFileInformationByHandleEx +GetFileSizeEx GetFileType GETFINALPATHNAMEBYHANDLE_FLAGS GetFinalPathNameByHandleW diff --git a/library/std/src/sys/pal/windows/c/windows_sys.rs b/library/std/src/sys/pal/windows/c/windows_sys.rs index 1d0e89f5d0f0e..489210cc503c5 100644 --- a/library/std/src/sys/pal/windows/c/windows_sys.rs +++ b/library/std/src/sys/pal/windows/c/windows_sys.rs @@ -44,6 +44,7 @@ windows_targets::link!("kernel32.dll" "system" fn GetExitCodeProcess(hprocess : windows_targets::link!("kernel32.dll" "system" fn GetFileAttributesW(lpfilename : PCWSTR) -> u32); windows_targets::link!("kernel32.dll" "system" fn GetFileInformationByHandle(hfile : HANDLE, lpfileinformation : *mut BY_HANDLE_FILE_INFORMATION) -> BOOL); windows_targets::link!("kernel32.dll" "system" fn GetFileInformationByHandleEx(hfile : HANDLE, fileinformationclass : FILE_INFO_BY_HANDLE_CLASS, lpfileinformation : *mut core::ffi::c_void, dwbuffersize : u32) -> BOOL); +windows_targets::link!("kernel32.dll" "system" fn GetFileSizeEx(hfile : HANDLE, lpfilesize : *mut i64) -> BOOL); windows_targets::link!("kernel32.dll" "system" fn GetFileType(hfile : HANDLE) -> FILE_TYPE); windows_targets::link!("kernel32.dll" "system" fn GetFinalPathNameByHandleW(hfile : HANDLE, lpszfilepath : PWSTR, cchfilepath : u32, dwflags : GETFINALPATHNAMEBYHANDLE_FLAGS) -> u32); windows_targets::link!("kernel32.dll" "system" fn GetFullPathNameW(lpfilename : PCWSTR, nbufferlength : u32, lpbuffer : PWSTR, lpfilepart : *mut PWSTR) -> u32); diff --git a/library/std/src/sys/pal/windows/fs.rs b/library/std/src/sys/pal/windows/fs.rs index 62d4d727432c3..22622cf27766f 100644 --- a/library/std/src/sys/pal/windows/fs.rs +++ b/library/std/src/sys/pal/windows/fs.rs @@ -631,6 +631,14 @@ impl File { Ok(newpos as u64) } + pub fn size(&self) -> Option> { + let mut result = 0; + Some( + cvt(unsafe { c::GetFileSizeEx(self.handle.as_raw_handle(), &mut result) }) + .map(|_| result as u64), + ) + } + pub fn duplicate(&self) -> io::Result { Ok(Self { handle: self.handle.try_clone()? }) } From a5299856060237676f744c9304f6167148103669 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Thu, 9 Jan 2025 10:31:03 +0100 Subject: [PATCH 10/14] Clarify description of `Seek::stream_len` It can only describe the inner workings of the default implementation, other implementations might not be implemented using seeks at all. --- library/std/src/io/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 32a98f2c47aae..7d40094c0c1e9 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2017,7 +2017,7 @@ pub trait Seek { /// Returns the length of this stream (in bytes). /// - /// This method is implemented using up to three seek operations. If this + /// The default implementation uses up to three seek operations. If this /// method returns successfully, the seek position is unchanged (i.e. the /// position before calling this method is the same as afterwards). /// However, if this method returns an error, the seek position is From 95a5ecc995f031478708c6c654a196197e71ba20 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sun, 16 Feb 2025 10:28:08 +0000 Subject: [PATCH 11/14] Enable relative-path-include-bytes on Windows --- .../relative-path-include-bytes-132203.edition2015.stdout | 2 +- .../rustdoc-ui/doctest/relative-path-include-bytes-132203.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/rustdoc-ui/doctest/relative-path-include-bytes-132203.edition2015.stdout b/tests/rustdoc-ui/doctest/relative-path-include-bytes-132203.edition2015.stdout index ca6e77502640e..13e142df8370d 100644 --- a/tests/rustdoc-ui/doctest/relative-path-include-bytes-132203.edition2015.stdout +++ b/tests/rustdoc-ui/doctest/relative-path-include-bytes-132203.edition2015.stdout @@ -5,7 +5,7 @@ test $DIR/relative-path-include-bytes-132203.rs - (line 18) ... FAILED failures: ---- $DIR/relative-path-include-bytes-132203.rs - (line 18) stdout ---- -error: couldn't read `$DIR/relative-dir-empty-file`: No such file or directory (os error 2) +error: couldn't read `$DIR/relative-dir-empty-file`: $FILE_NOT_FOUND_MSG (os error 2) --> $DIR/relative-path-include-bytes-132203.rs:19:9 | LL | let x = include_bytes!("relative-dir-empty-file"); diff --git a/tests/rustdoc-ui/doctest/relative-path-include-bytes-132203.rs b/tests/rustdoc-ui/doctest/relative-path-include-bytes-132203.rs index 6fddaa49faced..ceacd69a5fd52 100644 --- a/tests/rustdoc-ui/doctest/relative-path-include-bytes-132203.rs +++ b/tests/rustdoc-ui/doctest/relative-path-include-bytes-132203.rs @@ -1,4 +1,3 @@ -//@ ignore-windows different error message //@ revisions: edition2015 edition2024 //@[edition2015]edition:2015 //@[edition2015]check-fail @@ -7,8 +6,9 @@ //@[edition2024]edition:2024 //@[edition2024]check-pass //@[edition2024]compile-flags:--test --test-args=--test-threads=1 -//@ normalize-stdout: "tests/rustdoc-ui/doctest" -> "$$DIR" +//@ normalize-stdout: "tests.rustdoc-ui.doctest." -> "$$DIR/" //@ normalize-stdout: "finished in \d+\.\d+s" -> "finished in $$TIME" +//@ normalize-stdout: "`: .* \(os error 2\)" -> "`: $$FILE_NOT_FOUND_MSG (os error 2)" // https://github.com/rust-lang/rust/issues/132203 // This version, because it's edition2024, passes thanks to the new From b621a485faf006c830996049ed6d601e0f562065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Sat, 15 Feb 2025 20:25:30 +0800 Subject: [PATCH 12/14] bootstrap: add more tracing to compiler/std/llvm flows --- src/bootstrap/src/core/build_steps/compile.rs | 186 +++++++++++++++++- src/bootstrap/src/core/build_steps/dist.rs | 41 +++- src/bootstrap/src/core/build_steps/llvm.rs | 17 +- src/bootstrap/src/core/build_steps/tool.rs | 24 +++ src/bootstrap/src/core/builder/mod.rs | 41 +++- src/bootstrap/src/core/config/config.rs | 9 + src/bootstrap/src/lib.rs | 9 + 7 files changed, 314 insertions(+), 13 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 479327d63695c..bae18a96d9048 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -16,6 +16,8 @@ use std::process::Stdio; use std::{env, fs, str}; use serde_derive::Deserialize; +#[cfg(feature = "tracing")] +use tracing::{instrument, span}; use crate::core::build_steps::tool::SourceType; use crate::core::build_steps::{dist, llvm}; @@ -30,7 +32,7 @@ use crate::utils::exec::command; use crate::utils::helpers::{ exe, get_clang_cl_resource_dir, is_debug_info, is_dylib, symlink_dir, t, up_to_date, }; -use crate::{CLang, Compiler, DependencyType, GitRepo, LLVM_TOOLS, Mode}; +use crate::{CLang, Compiler, DependencyType, GitRepo, LLVM_TOOLS, Mode, debug, trace}; #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Std { @@ -98,6 +100,7 @@ impl Step for Std { run.crate_or_deps("sysroot").path("library").alias("core") } + #[cfg_attr(feature = "tracing", instrument(level = "trace", name = "Std::make_run", skip_all))] fn make_run(run: RunConfig<'_>) { let crates = std_crates_for_run_make(&run); let builder = run.builder; @@ -109,6 +112,14 @@ impl Step for Std { && builder.download_rustc() && builder.config.last_modified_commit(&["library"], "download-rustc", true).is_none(); + trace!("is managed git repo: {}", builder.rust_info().is_managed_git_subrepository()); + trace!("download_rustc: {}", builder.download_rustc()); + trace!( + "last modified commit: {:?}", + builder.config.last_modified_commit(&["library"], "download-rustc", true) + ); + trace!(force_recompile); + run.builder.ensure(Std { compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()), target: run.target, @@ -124,6 +135,19 @@ impl Step for Std { /// This will build the standard library for a particular stage of the build /// using the `compiler` targeting the `target` architecture. The artifacts /// created will also be linked into the sysroot directory. + #[cfg_attr( + feature = "tracing", + instrument( + level = "debug", + name = "Std::run", + skip_all, + fields( + target = ?self.target, + compiler = ?self.compiler, + force_recompile = self.force_recompile + ), + ), + )] fn run(self, builder: &Builder<'_>) { let target = self.target; let compiler = self.compiler; @@ -148,6 +172,9 @@ impl Step for Std { if builder.config.keep_stage.contains(&compiler.stage) || builder.config.keep_stage_std.contains(&compiler.stage) { + trace!(keep_stage = ?builder.config.keep_stage); + trace!(keep_stage_std = ?builder.config.keep_stage_std); + builder.info("WARNING: Using a potentially old libstd. This may not behave well."); builder.ensure(StartupObjects { compiler, target }); @@ -163,7 +190,15 @@ impl Step for Std { let mut target_deps = builder.ensure(StartupObjects { compiler, target }); let compiler_to_use = builder.compiler_for(compiler.stage, compiler.host, target); + trace!(?compiler_to_use); + if compiler_to_use != compiler { + trace!( + ?compiler_to_use, + ?compiler, + "compiler != compiler_to_use, handling cross-compile scenario" + ); + builder.ensure(Std::new(compiler_to_use, target)); let msg = if compiler_to_use.host == target { format!( @@ -186,12 +221,21 @@ impl Step for Std { return; } + trace!( + ?compiler_to_use, + ?compiler, + "compiler == compiler_to_use, handling not-cross-compile scenario" + ); + target_deps.extend(self.copy_extra_objects(builder, &compiler, target)); // The LLD wrappers and `rust-lld` are self-contained linking components that can be // necessary to link the stdlib on some targets. We'll also need to copy these binaries to // the `stage0-sysroot` to ensure the linker is found when bootstrapping on such a target. if compiler.stage == 0 && builder.is_builder_target(&compiler.host) { + trace!( + "(build == host) copying linking components to `stage0-sysroot` for bootstrapping" + ); // We want to copy the host `bin` folder within the `rustlib` folder in the sysroot. let src_sysroot_bin = builder .rustc_snapshot_sysroot() @@ -210,6 +254,7 @@ impl Step for Std { // with -Zalways-encode-mir. This frees us from the need to have a target linker, and the // fact that this is a check build integrates nicely with run_cargo. let mut cargo = if self.is_for_mir_opt_tests { + trace!("building special sysroot for mir-opt tests"); let mut cargo = builder::Cargo::new_for_mir_opt_tests( builder, compiler, @@ -222,6 +267,7 @@ impl Step for Std { cargo.arg("--manifest-path").arg(builder.src.join("library/sysroot/Cargo.toml")); cargo } else { + trace!("building regular sysroot"); let mut cargo = builder::Cargo::new( builder, compiler, @@ -665,6 +711,19 @@ impl Step for StdLink { /// Note that this assumes that `compiler` has already generated the libstd /// libraries for `target`, and this method will find them in the relevant /// output directory. + #[cfg_attr( + feature = "tracing", + instrument( + level = "trace", + name = "StdLink::run", + skip_all, + fields( + compiler = ?self.compiler, + target_compiler = ?self.target_compiler, + target = ?self.target + ), + ), + )] fn run(self, builder: &Builder<'_>) { let compiler = self.compiler; let target_compiler = self.target_compiler; @@ -822,6 +881,15 @@ impl Step for StartupObjects { /// They don't require any library support as they're just plain old object /// files, so we just use the nightly snapshot compiler to always build them (as /// no other compilers are guaranteed to be available). + #[cfg_attr( + feature = "tracing", + instrument( + level = "trace", + name = "StartupObjects::run", + skip_all, + fields(compiler = ?self.compiler, target = ?self.target), + ), + )] fn run(self, builder: &Builder<'_>) -> Vec<(PathBuf, DependencyType)> { let for_compiler = self.compiler; let target = self.target; @@ -936,6 +1004,15 @@ impl Step for Rustc { /// This will build the compiler for a particular stage of the build using /// the `compiler` targeting the `target` architecture. The artifacts /// created will also be linked into the sysroot directory. + #[cfg_attr( + feature = "tracing", + instrument( + level = "debug", + name = "Rustc::run", + skip_all, + fields(previous_compiler = ?self.compiler, target = ?self.target), + ), + )] fn run(self, builder: &Builder<'_>) -> u32 { let compiler = self.compiler; let target = self.target; @@ -943,6 +1020,8 @@ impl Step for Rustc { // NOTE: the ABI of the beta compiler is different from the ABI of the downloaded compiler, // so its artifacts can't be reused. if builder.download_rustc() && compiler.stage != 0 { + trace!(stage = compiler.stage, "`download_rustc` requested"); + let sysroot = builder.ensure(Sysroot { compiler, force_recompile: false }); cp_rustc_component_to_ci_sysroot( builder, @@ -955,6 +1034,8 @@ impl Step for Rustc { builder.ensure(Std::new(compiler, target)); if builder.config.keep_stage.contains(&compiler.stage) { + trace!(stage = compiler.stage, "`keep-stage` requested"); + builder.info("WARNING: Using a potentially old librustc. This may not behave well."); builder.info("WARNING: Use `--keep-stage-std` if you want to rebuild the compiler when it changes"); builder.ensure(RustcLink::from_rustc(self, compiler)); @@ -1374,6 +1455,19 @@ impl Step for RustcLink { } /// Same as `std_link`, only for librustc + #[cfg_attr( + feature = "tracing", + instrument( + level = "trace", + name = "RustcLink::run", + skip_all, + fields( + compiler = ?self.compiler, + previous_stage_compiler = ?self.previous_stage_compiler, + target = ?self.target, + ), + ), + )] fn run(self, builder: &Builder<'_>) { let compiler = self.compiler; let previous_stage_compiler = self.previous_stage_compiler; @@ -1462,6 +1556,19 @@ impl Step for CodegenBackend { } } + #[cfg_attr( + feature = "tracing", + instrument( + level = "debug", + name = "CodegenBackend::run", + skip_all, + fields( + compiler = ?self.compiler, + target = ?self.target, + backend = ?self.target, + ), + ), + )] fn run(self, builder: &Builder<'_>) { let compiler = self.compiler; let target = self.target; @@ -1470,6 +1577,7 @@ impl Step for CodegenBackend { builder.ensure(Rustc::new(compiler, target)); if builder.config.keep_stage.contains(&compiler.stage) { + trace!("`keep-stage` requested"); builder.info( "WARNING: Using a potentially old codegen backend. \ This may not behave well.", @@ -1617,6 +1725,15 @@ impl Step for Sysroot { /// Returns the sysroot that `compiler` is supposed to use. /// For the stage0 compiler, this is stage0-sysroot (because of the initial std build). /// For all other stages, it's the same stage directory that the compiler lives in. + #[cfg_attr( + feature = "tracing", + instrument( + level = "debug", + name = "Sysroot::run", + skip_all, + fields(compiler = ?self.compiler), + ), + )] fn run(self, builder: &Builder<'_>) -> PathBuf { let compiler = self.compiler; let host_dir = builder.out.join(compiler.host); @@ -1633,6 +1750,7 @@ impl Step for Sysroot { } }; let sysroot = sysroot_dir(compiler.stage); + trace!(stage = ?compiler.stage, ?sysroot); builder .verbose(|| println!("Removing sysroot {} to avoid caching bugs", sysroot.display())); @@ -1787,10 +1905,20 @@ impl Step for Assemble { /// This will assemble a compiler in `build/$host/stage$stage`. The compiler /// must have been previously produced by the `stage - 1` builder.build /// compiler. + #[cfg_attr( + feature = "tracing", + instrument( + level = "debug", + name = "Assemble::run", + skip_all, + fields(target_compiler = ?self.target_compiler), + ), + )] fn run(self, builder: &Builder<'_>) -> Compiler { let target_compiler = self.target_compiler; if target_compiler.stage == 0 { + trace!("stage 0 build compiler is always available, simply returning"); assert_eq!( builder.config.build, target_compiler.host, "Cannot obtain compiler for non-native build triple at stage 0" @@ -1806,9 +1934,13 @@ impl Step for Assemble { t!(fs::create_dir_all(&libdir_bin)); if builder.config.llvm_enabled(target_compiler.host) { + trace!("target_compiler.host" = ?target_compiler.host, "LLVM enabled"); + let llvm::LlvmResult { llvm_config, .. } = builder.ensure(llvm::Llvm { target: target_compiler.host }); if !builder.config.dry_run() && builder.config.llvm_tools_enabled { + trace!("LLVM tools enabled"); + let llvm_bin_dir = command(llvm_config).arg("--bindir").run_capture_stdout(builder).stdout(); let llvm_bin_dir = Path::new(llvm_bin_dir.trim()); @@ -1818,7 +1950,13 @@ impl Step for Assemble { // rustup, and lets developers use a locally built toolchain to // build projects that expect llvm tools to be present in the sysroot // (e.g. the `bootimage` crate). + + #[cfg(feature = "tracing")] + let _llvm_tools_span = + span!(tracing::Level::TRACE, "installing llvm tools to sysroot", ?libdir_bin) + .entered(); for tool in LLVM_TOOLS { + trace!("installing `{tool}`"); let tool_exe = exe(tool, target_compiler.host); let src_path = llvm_bin_dir.join(&tool_exe); // When using `download-ci-llvm`, some of the tools @@ -1838,6 +1976,7 @@ impl Step for Assemble { let maybe_install_llvm_bitcode_linker = |compiler| { if builder.config.llvm_bitcode_linker_enabled { + trace!("llvm-bitcode-linker enabled, installing"); let src_path = builder.ensure(crate::core::build_steps::tool::LlvmBitcodeLinker { compiler, target: target_compiler.host, @@ -1850,6 +1989,8 @@ impl Step for Assemble { // If we're downloading a compiler from CI, we can use the same compiler for all stages other than 0. if builder.download_rustc() { + trace!("`download-rustc` requested, reusing CI compiler for stage > 0"); + builder.ensure(Std::new(target_compiler, target_compiler.host)); let sysroot = builder.ensure(Sysroot { compiler: target_compiler, force_recompile: false }); @@ -1879,16 +2020,17 @@ impl Step for Assemble { // // FIXME: It may be faster if we build just a stage 1 compiler and then // use that to bootstrap this compiler forward. + debug!( + "ensuring build compiler is available: compiler(stage = {}, host = {:?})", + target_compiler.stage - 1, + builder.config.build, + ); let mut build_compiler = builder.compiler(target_compiler.stage - 1, builder.config.build); // Build enzyme - let enzyme_install = if builder.config.llvm_enzyme { - Some(builder.ensure(llvm::Enzyme { target: build_compiler.host })) - } else { - None - }; - - if let Some(enzyme_install) = enzyme_install { + if builder.config.llvm_enzyme { + debug!("`llvm_enzyme` requested"); + let enzyme_install = builder.ensure(llvm::Enzyme { target: build_compiler.host }); let lib_ext = std::env::consts::DLL_EXTENSION; let src_lib = enzyme_install.join("build/Enzyme/libEnzyme-19").with_extension(lib_ext); let libdir = builder.sysroot_target_libdir(build_compiler, build_compiler.host); @@ -1905,13 +2047,27 @@ impl Step for Assemble { // link to these. (FIXME: Is that correct? It seems to be correct most // of the time but I think we do link to these for stage2/bin compilers // when not performing a full bootstrap). + debug!( + ?build_compiler, + "target_compiler.host" = ?target_compiler.host, + "building compiler libraries to link to" + ); let actual_stage = builder.ensure(Rustc::new(build_compiler, target_compiler.host)); // Current build_compiler.stage might be uplifted instead of being built; so update it // to not fail while linking the artifacts. + debug!( + "(old) build_compiler.stage" = build_compiler.stage, + "(adjusted) build_compiler.stage" = actual_stage, + "temporarily adjusting `build_compiler.stage` to account for uplifted libraries" + ); build_compiler.stage = actual_stage; + #[cfg(feature = "tracing")] + let _codegen_backend_span = + span!(tracing::Level::DEBUG, "building requested codegen backends").entered(); for backend in builder.config.codegen_backends(target_compiler.host) { if backend == "llvm" { + debug!("llvm codegen backend is already built as part of rustc"); continue; // Already built as part of rustc } @@ -1921,6 +2077,8 @@ impl Step for Assemble { backend: backend.clone(), }); } + #[cfg(feature = "tracing")] + drop(_codegen_backend_span); let stage = target_compiler.stage; let host = target_compiler.host; @@ -1980,6 +2138,7 @@ impl Step for Assemble { } } + debug!("copying codegen backends to sysroot"); copy_codegen_backends_to_sysroot(builder, build_compiler, target_compiler); if builder.config.lld_enabled { @@ -1990,6 +2149,11 @@ impl Step for Assemble { } if builder.config.llvm_enabled(target_compiler.host) && builder.config.llvm_tools_enabled { + debug!( + "llvm and llvm tools enabled; copying `llvm-objcopy` as `rust-objcopy` to \ + workaround faulty homebrew `strip`s" + ); + // `llvm-strip` is used by rustc, which is actually just a symlink to `llvm-objcopy`, so // copy and rename `llvm-objcopy`. // @@ -2022,6 +2186,11 @@ impl Step for Assemble { // Ensure that `libLLVM.so` ends up in the newly build compiler directory, // so that it can be found when the newly built `rustc` is run. + debug!( + "target_compiler.host" = ?target_compiler.host, + ?sysroot, + "ensuring availability of `libLLVM.so` in compiler directory" + ); dist::maybe_install_llvm_runtime(builder, target_compiler.host, &sysroot); dist::maybe_install_llvm_target(builder, target_compiler.host, &sysroot); @@ -2031,6 +2200,7 @@ impl Step for Assemble { let bindir = sysroot.join("bin"); t!(fs::create_dir_all(bindir)); let compiler = builder.rustc(target_compiler); + debug!(src = ?rustc, dst = ?compiler, "linking compiler binary itself"); builder.copy_link(&rustc, &compiler); target_compiler diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index ae3761a97e50f..96d0cf48fbfe9 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -16,6 +16,8 @@ use std::{env, fs}; use object::BinaryFormat; use object::read::archive::ArchiveFile; +#[cfg(feature = "tracing")] +use tracing::instrument; use crate::core::build_steps::doc::DocumentationFormat; use crate::core::build_steps::tool::{self, Tool}; @@ -30,7 +32,7 @@ use crate::utils::helpers::{ exe, is_dylib, move_file, t, target_supports_cranelift_backend, timeit, }; use crate::utils::tarball::{GeneratedTarball, OverlayKind, Tarball}; -use crate::{Compiler, DependencyType, LLVM_TOOLS, Mode}; +use crate::{Compiler, DependencyType, LLVM_TOOLS, Mode, trace}; pub fn pkgname(builder: &Builder<'_>, component: &str) -> String { format!("{}-{}", component, builder.rust_package_vers()) @@ -2029,6 +2031,15 @@ fn install_llvm_file( /// Maybe add LLVM object files to the given destination lib-dir. Allows either static or dynamic linking. /// /// Returns whether the files were actually copied. +#[cfg_attr( + feature = "tracing", + instrument( + level = "trace", + name = "maybe_install_llvm", + skip_all, + fields(target = ?target, dst_libdir = ?dst_libdir, install_symlink = install_symlink), + ), +)] fn maybe_install_llvm( builder: &Builder<'_>, target: TargetSelection, @@ -2052,6 +2063,7 @@ fn maybe_install_llvm( // If the LLVM is coming from ourselves (just from CI) though, we // still want to install it, as it otherwise won't be available. if builder.is_system_llvm(target) { + trace!("system LLVM requested, no install"); return false; } @@ -2070,6 +2082,7 @@ fn maybe_install_llvm( } else if let llvm::LlvmBuildStatus::AlreadyBuilt(llvm::LlvmResult { llvm_config, .. }) = llvm::prebuilt_llvm_config(builder, target, true) { + trace!("LLVM already built, installing LLVM files"); let mut cmd = command(llvm_config); cmd.arg("--libfiles"); builder.verbose(|| println!("running {cmd:?}")); @@ -2092,6 +2105,19 @@ fn maybe_install_llvm( } /// Maybe add libLLVM.so to the target lib-dir for linking. +#[cfg_attr( + feature = "tracing", + instrument( + level = "trace", + name = "maybe_install_llvm_target", + skip_all, + fields( + llvm_link_shared = ?builder.llvm_link_shared(), + target = ?target, + sysroot = ?sysroot, + ), + ), +)] pub fn maybe_install_llvm_target(builder: &Builder<'_>, target: TargetSelection, sysroot: &Path) { let dst_libdir = sysroot.join("lib/rustlib").join(target).join("lib"); // We do not need to copy LLVM files into the sysroot if it is not @@ -2103,6 +2129,19 @@ pub fn maybe_install_llvm_target(builder: &Builder<'_>, target: TargetSelection, } /// Maybe add libLLVM.so to the runtime lib-dir for rustc itself. +#[cfg_attr( + feature = "tracing", + instrument( + level = "trace", + name = "maybe_install_llvm_runtime", + skip_all, + fields( + llvm_link_shared = ?builder.llvm_link_shared(), + target = ?target, + sysroot = ?sysroot, + ), + ), +)] pub fn maybe_install_llvm_runtime(builder: &Builder<'_>, target: TargetSelection, sysroot: &Path) { let dst_libdir = sysroot.join(builder.sysroot_libdir_relative(Compiler { stage: 1, host: target })); diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 18da0e8252b90..9f62879c36d9b 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -16,6 +16,8 @@ use std::{env, fs}; use build_helper::ci::CiEnv; use build_helper::git::get_closest_merge_commit; +#[cfg(feature = "tracing")] +use tracing::instrument; use crate::core::builder::{Builder, RunConfig, ShouldRun, Step}; use crate::core::config::{Config, TargetSelection}; @@ -24,7 +26,7 @@ use crate::utils::exec::command; use crate::utils::helpers::{ self, exe, get_clang_cl_resource_dir, t, unhashed_basename, up_to_date, }; -use crate::{CLang, GitRepo, Kind}; +use crate::{CLang, GitRepo, Kind, trace}; #[derive(Clone)] pub struct LlvmResult { @@ -934,6 +936,15 @@ impl Step for Enzyme { } /// Compile Enzyme for `target`. + #[cfg_attr( + feature = "tracing", + instrument( + level = "debug", + name = "Enzyme::run", + skip_all, + fields(target = ?self.target), + ), + )] fn run(self, builder: &Builder<'_>) -> PathBuf { builder.require_submodule( "src/tools/enzyme", @@ -959,7 +970,9 @@ impl Step for Enzyme { let out_dir = builder.enzyme_out(target); let stamp = BuildStamp::new(&out_dir).with_prefix("enzyme").add_stamp(smart_stamp_hash); + trace!("checking build stamp to see if we need to rebuild enzyme artifacts"); if stamp.is_up_to_date() { + trace!(?out_dir, "enzyme build artifacts are up to date"); if stamp.stamp().is_empty() { builder.info( "Could not determine the Enzyme submodule commit hash. \ @@ -973,6 +986,7 @@ impl Step for Enzyme { return out_dir; } + trace!(?target, "(re)building enzyme artifacts"); builder.info(&format!("Building Enzyme for {}", target)); t!(stamp.remove()); let _time = helpers::timeit(builder); @@ -994,6 +1008,7 @@ impl Step for Enzyme { (true, false) => "Release", (true, true) => "RelWithDebInfo", }; + trace!(?profile); cfg.out_dir(&out_dir) .profile(profile) diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 1291a634a6f6f..a54db9d781573 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -1,6 +1,9 @@ use std::path::PathBuf; use std::{env, fs}; +#[cfg(feature = "tracing")] +use tracing::instrument; + use crate::core::build_steps::compile::is_lto_stage; use crate::core::build_steps::toolstate::ToolState; use crate::core::build_steps::{compile, llvm}; @@ -304,6 +307,14 @@ macro_rules! bootstrap_tool { }); } + #[cfg_attr( + feature = "tracing", + instrument( + level = "debug", + name = $tool_name, + skip_all, + ), + )] fn run(self, builder: &Builder<'_>) -> PathBuf { $( for submodule in $submodules { @@ -758,6 +769,15 @@ impl Step for LldWrapper { run.never() } + #[cfg_attr( + feature = "tracing", + instrument( + level = "debug", + name = "LldWrapper::run", + skip_all, + fields(build_compiler = ?self.build_compiler, target_compiler = ?self.target_compiler), + ), + )] fn run(self, builder: &Builder<'_>) { if builder.config.dry_run() { return; @@ -914,6 +934,10 @@ impl Step for LlvmBitcodeLinker { }); } + #[cfg_attr( + feature = "tracing", + instrument(level = "debug", name = "LlvmBitcodeLinker::run", skip_all) + )] fn run(self, builder: &Builder<'_>) -> PathBuf { let bin_name = "llvm-bitcode-linker"; diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs index ecec589fc32eb..6d77e38b1291c 100644 --- a/src/bootstrap/src/core/builder/mod.rs +++ b/src/bootstrap/src/core/builder/mod.rs @@ -10,6 +10,8 @@ use std::time::{Duration, Instant}; use std::{env, fs}; use clap::ValueEnum; +#[cfg(feature = "tracing")] +use tracing::instrument; pub use self::cargo::{Cargo, cargo_profile_var}; pub use crate::Compiler; @@ -21,7 +23,7 @@ use crate::core::config::{DryRun, TargetSelection}; use crate::utils::cache::Cache; use crate::utils::exec::{BootstrapCommand, command}; use crate::utils::helpers::{self, LldThreads, add_dylib_path, exe, libdir, linker_args, t}; -use crate::{Build, Crate}; +use crate::{Build, Crate, trace}; mod cargo; @@ -1214,6 +1216,19 @@ impl<'a> Builder<'a> { /// compiler will run on, *not* the target it will build code for). Explicitly does not take /// `Compiler` since all `Compiler` instances are meant to be obtained through this function, /// since it ensures that they are valid (i.e., built and assembled). + #[cfg_attr( + feature = "tracing", + instrument( + level = "trace", + name = "Builder::compiler", + target = "COMPILER", + skip_all, + fields( + stage = stage, + host = ?host, + ), + ), + )] pub fn compiler(&self, stage: u32, host: TargetSelection) -> Compiler { self.ensure(compile::Assemble { target_compiler: Compiler { stage, host } }) } @@ -1229,19 +1244,39 @@ impl<'a> Builder<'a> { /// sysroot. /// /// See `force_use_stage1` and `force_use_stage2` for documentation on what each argument is. + #[cfg_attr( + feature = "tracing", + instrument( + level = "trace", + name = "Builder::compiler_for", + target = "COMPILER_FOR", + skip_all, + fields( + stage = stage, + host = ?host, + target = ?target, + ), + ), + )] pub fn compiler_for( &self, stage: u32, host: TargetSelection, target: TargetSelection, ) -> Compiler { - if self.build.force_use_stage2(stage) { + #![allow(clippy::let_and_return)] + let resolved_compiler = if self.build.force_use_stage2(stage) { + trace!(target: "COMPILER_FOR", ?stage, "force_use_stage2"); self.compiler(2, self.config.build) } else if self.build.force_use_stage1(stage, target) { + trace!(target: "COMPILER_FOR", ?stage, "force_use_stage1"); self.compiler(1, self.config.build) } else { + trace!(target: "COMPILER_FOR", ?stage, ?host, "no force, fallback to `compiler()`"); self.compiler(stage, host) - } + }; + trace!(target: "COMPILER_FOR", ?resolved_compiler); + resolved_compiler } pub fn sysroot(&self, compiler: Compiler) -> PathBuf { diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 62625fc3660f9..d27a8b155df73 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -2747,6 +2747,15 @@ impl Config { /// tarball). Typically [`crate::Build::require_submodule`] should be /// used instead to provide a nice error to the user if the submodule is /// missing. + #[cfg_attr( + feature = "tracing", + instrument( + level = "trace", + name = "Config::update_submodule", + skip_all, + fields(relative_path = ?relative_path), + ), + )] pub(crate) fn update_submodule(&self, relative_path: &str) { if !self.submodules() { return; diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 7cd8aacf0d6c8..34df3ca232017 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -469,6 +469,15 @@ impl Build { /// /// The given `err_hint` will be shown to the user if the submodule is not /// checked out and submodule management is disabled. + #[cfg_attr( + feature = "tracing", + instrument( + level = "trace", + name = "Build::require_submodule", + skip_all, + fields(submodule = submodule), + ), + )] pub fn require_submodule(&self, submodule: &str, err_hint: Option<&str>) { // When testing bootstrap itself, it is much faster to ignore // submodules. Almost all Steps work fine without their submodules. From 7b118168c7902ae8ec266996a724cfec1c88ea1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Sat, 15 Feb 2025 22:58:46 +0800 Subject: [PATCH 13/14] bootstrap: take `target` by value in `is_builder_target` --- src/bootstrap/src/core/build_steps/compile.rs | 8 ++++---- src/bootstrap/src/core/build_steps/dist.rs | 6 +++--- src/bootstrap/src/core/build_steps/llvm.rs | 6 +++--- src/bootstrap/src/core/build_steps/test.rs | 2 +- src/bootstrap/src/core/builder/tests.rs | 4 ++-- src/bootstrap/src/core/sanity.rs | 2 +- src/bootstrap/src/lib.rs | 10 +++++----- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index bae18a96d9048..755ff4cd0f072 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -154,7 +154,7 @@ impl Step for Std { // When using `download-rustc`, we already have artifacts for the host available. Don't // recompile them. - if builder.download_rustc() && target == builder.build.build + if builder.download_rustc() && builder.is_builder_target(target) // NOTE: the beta compiler may generate different artifacts than the downloaded compiler, so // its artifacts can't be reused. && compiler.stage != 0 @@ -232,7 +232,7 @@ impl Step for Std { // The LLD wrappers and `rust-lld` are self-contained linking components that can be // necessary to link the stdlib on some targets. We'll also need to copy these binaries to // the `stage0-sysroot` to ensure the linker is found when bootstrapping on such a target. - if compiler.stage == 0 && builder.is_builder_target(&compiler.host) { + if compiler.stage == 0 && builder.is_builder_target(compiler.host) { trace!( "(build == host) copying linking components to `stage0-sysroot` for bootstrapping" ); @@ -1559,7 +1559,7 @@ impl Step for CodegenBackend { #[cfg_attr( feature = "tracing", instrument( - level = "debug", + level = "debug", name = "CodegenBackend::run", skip_all, fields( @@ -2485,7 +2485,7 @@ pub fn strip_debug(builder: &Builder<'_>, target: TargetSelection, path: &Path) // FIXME: to make things simpler for now, limit this to the host and target where we know // `strip -g` is both available and will fix the issue, i.e. on a x64 linux host that is not // cross-compiling. Expand this to other appropriate targets in the future. - if target != "x86_64-unknown-linux-gnu" || !builder.is_builder_target(&target) || !path.exists() + if target != "x86_64-unknown-linux-gnu" || !builder.is_builder_target(target) || !path.exists() { return; } diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index 96d0cf48fbfe9..509ee9e1acf07 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -584,7 +584,7 @@ impl Step for DebuggerScripts { fn skip_host_target_lib(builder: &Builder<'_>, compiler: Compiler) -> bool { // The only true set of target libraries came from the build triple, so // let's reduce redundant work by only producing archives from that host. - if !builder.is_builder_target(&compiler.host) { + if !builder.is_builder_target(compiler.host) { builder.info("\tskipping, not a build host"); true } else { @@ -639,7 +639,7 @@ fn copy_target_libs( for (path, dependency_type) in builder.read_stamp_file(stamp) { if dependency_type == DependencyType::TargetSelfContained { builder.copy_link(&path, &self_contained_dst.join(path.file_name().unwrap())); - } else if dependency_type == DependencyType::Target || builder.is_builder_target(&target) { + } else if dependency_type == DependencyType::Target || builder.is_builder_target(target) { builder.copy_link(&path, &dst.join(path.file_name().unwrap())); } } @@ -788,7 +788,7 @@ impl Step for Analysis { fn run(self, builder: &Builder<'_>) -> Option { let compiler = self.compiler; let target = self.target; - if !builder.is_builder_target(&compiler.host) { + if !builder.is_builder_target(compiler.host) { return None; } diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 9f62879c36d9b..3025f95566070 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -518,7 +518,7 @@ impl Step for Llvm { } // https://llvm.org/docs/HowToCrossCompileLLVM.html - if !builder.is_builder_target(&target) { + if !builder.is_builder_target(target) { let LlvmResult { llvm_config, .. } = builder.ensure(Llvm { target: builder.config.build }); if !builder.config.dry_run() { @@ -670,7 +670,7 @@ fn configure_cmake( } cfg.target(&target.triple).host(&builder.config.build.triple); - if !builder.is_builder_target(&target) { + if !builder.is_builder_target(target) { cfg.define("CMAKE_CROSSCOMPILING", "True"); if target.contains("netbsd") { @@ -1133,7 +1133,7 @@ impl Step for Lld { .define("LLVM_CMAKE_DIR", llvm_cmake_dir) .define("LLVM_INCLUDE_TESTS", "OFF"); - if !builder.is_builder_target(&target) { + if !builder.is_builder_target(target) { // Use the host llvm-tblgen binary. cfg.define( "LLVM_TABLEGEN_EXE", diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 26ed0e5deaa05..b3f4a7bad99c2 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2743,7 +2743,7 @@ impl Step for Crate { cargo } else { // Also prepare a sysroot for the target. - if !builder.is_builder_target(&target) { + if !builder.is_builder_target(target) { builder.ensure(compile::Std::new(compiler, target).force_recompile(true)); builder.ensure(RemoteCopyLibs { compiler, target }); } diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index 5e3e0ef654fdf..445b5dfbeab22 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -1080,7 +1080,7 @@ fn test_is_builder_target() { let build = Build::new(config); let builder = Builder::new(&build); - assert!(builder.is_builder_target(&target1)); - assert!(!builder.is_builder_target(&target2)); + assert!(builder.is_builder_target(target1)); + assert!(!builder.is_builder_target(target2)); } } diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index 9e4a0816e0d48..241b7386d18c5 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -329,7 +329,7 @@ than building it. if target.contains("musl") && !target.contains("unikraft") { // If this is a native target (host is also musl) and no musl-root is given, // fall back to the system toolchain in /usr before giving up - if build.musl_root(*target).is_none() && build.is_builder_target(target) { + if build.musl_root(*target).is_none() && build.is_builder_target(*target) { let target = build.config.target_config.entry(*target).or_default(); target.musl_root = Some("/usr".into()); } diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 34df3ca232017..62ddc7d682ee8 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -748,7 +748,7 @@ impl Build { /// Note that if LLVM is configured externally then the directory returned /// will likely be empty. fn llvm_out(&self, target: TargetSelection) -> PathBuf { - if self.config.llvm_from_ci && self.is_builder_target(&target) { + if self.config.llvm_from_ci && self.is_builder_target(target) { self.config.ci_llvm_root() } else { self.out.join(target).join("llvm") @@ -798,7 +798,7 @@ impl Build { fn is_system_llvm(&self, target: TargetSelection) -> bool { match self.config.target_config.get(&target) { Some(Target { llvm_config: Some(_), .. }) => { - let ci_llvm = self.config.llvm_from_ci && self.is_builder_target(&target); + let ci_llvm = self.config.llvm_from_ci && self.is_builder_target(target); !ci_llvm } // We're building from the in-tree src/llvm-project sources. @@ -1286,7 +1286,7 @@ Executed at: {executed_at}"#, // need to use CXX compiler as linker to resolve the exception functions // that are only existed in CXX libraries Some(self.cxx.borrow()[&target].path().into()) - } else if !self.is_builder_target(&target) + } else if !self.is_builder_target(target) && helpers::use_host_linker(target) && !target.is_msvc() { @@ -1939,8 +1939,8 @@ to download LLVM rather than building it. } /// Checks if the given target is the same as the builder target. - fn is_builder_target(&self, target: &TargetSelection) -> bool { - &self.config.build == target + fn is_builder_target(&self, target: TargetSelection) -> bool { + self.config.build == target } } From 05ba1a450adb266cdaa7001f73a2e5899405b3b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Sat, 15 Feb 2025 22:59:01 +0800 Subject: [PATCH 14/14] rustc-dev-guide: document `COMPILER` and `COMPILER_FOR` tracing targets --- .../src/building/bootstrapping/debugging-bootstrap.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/doc/rustc-dev-guide/src/building/bootstrapping/debugging-bootstrap.md b/src/doc/rustc-dev-guide/src/building/bootstrapping/debugging-bootstrap.md index 04fa5b204dd4e..04d8e91dcb4f1 100644 --- a/src/doc/rustc-dev-guide/src/building/bootstrapping/debugging-bootstrap.md +++ b/src/doc/rustc-dev-guide/src/building/bootstrapping/debugging-bootstrap.md @@ -76,6 +76,14 @@ $ BOOTSTRAP_TRACING=CONFIG_HANDLING=TRACE ./x build library --stage 1 [tracing-env-filter]: https://docs.rs/tracing-subscriber/0.3.19/tracing_subscriber/filter/struct.EnvFilter.html +##### FIXME(#96176): specific tracing for `compiler()` vs `compiler_for()` + +The additional targets `COMPILER` and `COMPILER_FOR` are used to help trace what +`builder.compiler()` and `builder.compiler_for()` does. They should be removed +if [#96176][cleanup-compiler-for] is resolved. + +[cleanup-compiler-for]: https://github.com/rust-lang/rust/issues/96176 + ### Using `tracing` in bootstrap Both `tracing::*` macros and the `tracing::instrument` proc-macro attribute need to be gated behind `tracing` feature. Examples: