Rollup merge of #150014 - bjorn3:metadata_loader_cleanups, r=jieyouxu

Metadata loader cleanups

Couple of cleanups I found while working on #149273

This renames some fields and enum variants to clarify what they are used for, moves a check to another method and slightly simplifies the way profiler_builtins is linked.

Author: matthiaskrgr

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -2792,11 +2792,9 @@ fn add_upstream_rust_crates(
         // We must always link crates `compiler_builtins` and `profiler_builtins` statically.
         // Even if they were already included into a dylib
         // (e.g. `libstd` when `-C prefer-dynamic` is used).
-        // FIXME: `dependency_formats` can report `profiler_builtins` as `NotLinked` for some
-        // reason, it shouldn't do that because `profiler_builtins` should indeed be linked.
         let linkage = data[cnum];
         let link_static_crate = linkage == Linkage::Static
-            || (linkage == Linkage::IncludedFromDylib || linkage == Linkage::NotLinked)
+            || linkage == Linkage::IncludedFromDylib
                 && (codegen_results.crate_info.compiler_builtins == Some(cnum)
                     || codegen_results.crate_info.profiler_runtime == Some(cnum));
 

compiler/rustc_metadata/src/creader.rs

@@ -156,8 +156,8 @@ impl<'a> std::fmt::Debug for CrateDump<'a> {
 enum CrateOrigin<'a> {
     /// This crate was a dependency of another crate.
     IndirectDependency {
-        /// Where this dependency was included from.
-        dep_root: &'a CratePaths,
+        /// Where this dependency was included from. Should only be used in error messages.
+        dep_root_for_errors: &'a CratePaths,
         /// True if the parent is private, meaning the dependent should also be private.
         parent_private: bool,
         /// Dependency info about this crate.
@@ -171,9 +171,11 @@ enum CrateOrigin<'a> {
 
 impl<'a> CrateOrigin<'a> {
     /// Return the dependency root, if any.
-    fn dep_root(&self) -> Option<&'a CratePaths> {
+    fn dep_root_for_errors(&self) -> Option<&'a CratePaths> {
         match self {
-            CrateOrigin::IndirectDependency { dep_root, .. } => Some(dep_root),
+            CrateOrigin::IndirectDependency { dep_root_for_errors, .. } => {
+                Some(dep_root_for_errors)
+            }
             _ => None,
         }
     }
@@ -193,6 +195,7 @@ impl<'a> CrateOrigin<'a> {
             CrateOrigin::IndirectDependency { parent_private, dep, .. } => {
                 Some(dep.is_private || *parent_private)
             }
+            CrateOrigin::Injected => Some(true),
             _ => None,
         }
     }
@@ -544,17 +547,7 @@ impl CStore {
     /// Sometimes the directly dependent crate is not specified by `--extern`, in this case,
     /// `private-dep` is none during loading. This is equivalent to the scenario where the
     /// command parameter is set to `public-dependency`
-    fn is_private_dep(
-        &self,
-        externs: &Externs,
-        name: Symbol,
-        private_dep: Option<bool>,
-        origin: CrateOrigin<'_>,
-    ) -> bool {
-        if matches!(origin, CrateOrigin::Injected) {
-            return true;
-        }
-
+    fn is_private_dep(&self, externs: &Externs, name: Symbol, private_dep: Option<bool>) -> bool {
         let extern_private = externs.get(name.as_str()).map(|e| e.is_private_dep);
         match (extern_private, private_dep) {
             // Explicit non-private via `--extern`, explicit non-private from metadata, or
@@ -581,7 +574,7 @@ impl CStore {
         let Library { source, metadata } = lib;
         let crate_root = metadata.get_root();
         let host_hash = host_lib.as_ref().map(|lib| lib.metadata.get_root().hash());
-        let private_dep = self.is_private_dep(&tcx.sess.opts.externs, name, private_dep, origin);
+        let private_dep = self.is_private_dep(&tcx.sess.opts.externs, name, private_dep);
 
         // Claim this crate number and cache it
         let feed = self.intern_stable_crate_id(tcx, &crate_root)?;
@@ -597,16 +590,16 @@ impl CStore {
         // Maintain a reference to the top most crate.
         // Stash paths for top-most crate locally if necessary.
         let crate_paths;
-        let dep_root = if let Some(dep_root) = origin.dep_root() {
-            dep_root
+        let dep_root_for_errors = if let Some(dep_root_for_errors) = origin.dep_root_for_errors() {
+            dep_root_for_errors
         } else {
             crate_paths = CratePaths::new(crate_root.name(), source.clone());
             &crate_paths
         };
 
         let cnum_map = self.resolve_crate_deps(
             tcx,
-            dep_root,
+            dep_root_for_errors,
             &crate_root,
             &metadata,
             cnum,
@@ -726,7 +719,7 @@ impl CStore {
         self.used_extern_options.insert(name);
         match self.maybe_resolve_crate(tcx, name, dep_kind, origin) {
             Ok(cnum) => {
-                self.set_used_recursively(tcx, cnum);
+                self.set_used_recursively(cnum);
                 Some(cnum)
             }
             Err(err) => {
@@ -735,7 +728,7 @@ impl CStore {
                     .maybe_resolve_crate(
                         tcx,
                         sym::core,
-                        CrateDepKind::Explicit,
+                        CrateDepKind::Unconditional,
                         CrateOrigin::Extern,
                     )
                     .is_err();
@@ -757,7 +750,7 @@ impl CStore {
             return Err(CrateError::NonAsciiName(name));
         }
 
-        let dep_root = origin.dep_root();
+        let dep_root_for_errors = origin.dep_root_for_errors();
         let dep = origin.dep();
         let hash = dep.map(|d| d.hash);
         let host_hash = dep.map(|d| d.host_hash).flatten();
@@ -795,7 +788,11 @@ impl CStore {
                         host_hash,
                     )? {
                         Some(res) => res,
-                        None => return Err(locator.into_error(crate_rejections, dep_root.cloned())),
+                        None => {
+                            return Err(
+                                locator.into_error(crate_rejections, dep_root_for_errors.cloned())
+                            );
+                        }
                     }
                 }
             }
@@ -808,8 +805,7 @@ impl CStore {
                 // not specified by `--extern` on command line parameters, it may be
                 // `private-dependency` when `register_crate` is called for the first time. Then it must be updated to
                 // `public-dependency` here.
-                let private_dep =
-                    self.is_private_dep(&tcx.sess.opts.externs, name, private_dep, origin);
+                let private_dep = self.is_private_dep(&tcx.sess.opts.externs, name, private_dep);
                 let data = self.get_crate_data_mut(cnum);
                 if data.is_proc_macro_crate() {
                     dep_kind = CrateDepKind::MacrosOnly;
@@ -856,7 +852,7 @@ impl CStore {
     fn resolve_crate_deps(
         &mut self,
         tcx: TyCtxt<'_>,
-        dep_root: &CratePaths,
+        dep_root_for_errors: &CratePaths,
         crate_root: &CrateRoot,
         metadata: &MetadataBlob,
         krate: CrateNum,
@@ -866,7 +862,7 @@ impl CStore {
         debug!(
             "resolving deps of external crate `{}` with dep root `{}`",
             crate_root.name(),
-            dep_root.name
+            dep_root_for_errors.name
         );
         if crate_root.is_proc_macro_crate() {
             return Ok(CrateNumMap::new());
@@ -896,7 +892,7 @@ impl CStore {
                 dep.name,
                 dep_kind,
                 CrateOrigin::IndirectDependency {
-                    dep_root,
+                    dep_root_for_errors,
                     parent_private: parent_is_private,
                     dep: &dep,
                 },
@@ -979,9 +975,15 @@ impl CStore {
         };
         info!("panic runtime not found -- loading {}", name);
 
-        let Some(cnum) =
-            self.resolve_crate(tcx, name, DUMMY_SP, CrateDepKind::Implicit, CrateOrigin::Injected)
-        else {
+        // This has to be conditional as both panic_unwind and panic_abort may be present in the
+        // crate graph at the same time. One of them will later be activated in dependency_formats.
+        let Some(cnum) = self.resolve_crate(
+            tcx,
+            name,
+            DUMMY_SP,
+            CrateDepKind::Conditional,
+            CrateOrigin::Injected,
+        ) else {
             return;
         };
         let data = self.get_crate_data(cnum);
@@ -1009,9 +1011,13 @@ impl CStore {
         info!("loading profiler");
 
         let name = Symbol::intern(&tcx.sess.opts.unstable_opts.profiler_runtime);
-        let Some(cnum) =
-            self.resolve_crate(tcx, name, DUMMY_SP, CrateDepKind::Implicit, CrateOrigin::Injected)
-        else {
+        let Some(cnum) = self.resolve_crate(
+            tcx,
+            name,
+            DUMMY_SP,
+            CrateDepKind::Unconditional,
+            CrateOrigin::Injected,
+        ) else {
             return;
         };
         let data = self.get_crate_data(cnum);
@@ -1131,7 +1137,7 @@ impl CStore {
                         tcx,
                         name_interned,
                         DUMMY_SP,
-                        CrateDepKind::Explicit,
+                        CrateDepKind::Unconditional,
                         CrateOrigin::Extern,
                     );
                 }
@@ -1163,7 +1169,7 @@ impl CStore {
             tcx,
             sym::compiler_builtins,
             krate.spans.inner_span.shrink_to_lo(),
-            CrateDepKind::Explicit,
+            CrateDepKind::Unconditional,
             CrateOrigin::Injected,
         ) else {
             info!("`compiler_builtins` not resolved");
@@ -1280,7 +1286,7 @@ impl CStore {
                 let dep_kind = if attr::contains_name(&item.attrs, sym::no_link) {
                     CrateDepKind::MacrosOnly
                 } else {
-                    CrateDepKind::Explicit
+                    CrateDepKind::Unconditional
                 };
 
                 let cnum =
@@ -1310,7 +1316,7 @@ impl CStore {
         span: Span,
     ) -> Option<CrateNum> {
         let cnum =
-            self.resolve_crate(tcx, name, span, CrateDepKind::Explicit, CrateOrigin::Extern)?;
+            self.resolve_crate(tcx, name, span, CrateDepKind::Unconditional, CrateOrigin::Extern)?;
 
         self.update_extern_crate(
             cnum,
@@ -1328,7 +1334,7 @@ impl CStore {
     }
 
     pub fn maybe_process_path_extern(&mut self, tcx: TyCtxt<'_>, name: Symbol) -> Option<CrateNum> {
-        self.maybe_resolve_crate(tcx, name, CrateDepKind::Explicit, CrateOrigin::Extern).ok()
+        self.maybe_resolve_crate(tcx, name, CrateDepKind::Unconditional, CrateOrigin::Extern).ok()
     }
 }
 

compiler/rustc_metadata/src/dependency_format.rs

@@ -242,7 +242,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList {
         let src = tcx.used_crate_source(cnum);
         if src.dylib.is_none()
             && !formats.contains_key(&cnum)
-            && tcx.dep_kind(cnum) == CrateDepKind::Explicit
+            && tcx.dep_kind(cnum) == CrateDepKind::Unconditional
         {
             assert!(src.rlib.is_some() || src.rmeta.is_some());
             info!("adding staticlib: {}", tcx.crate_name(cnum));
@@ -355,8 +355,8 @@ fn attempt_static(tcx: TyCtxt<'_>, unavailable: &mut Vec<CrateNum>) -> Option<De
     for &cnum in tcx.crates(()) {
         assert_eq!(
             ret.push(match tcx.dep_kind(cnum) {
-                CrateDepKind::Explicit => Linkage::Static,
-                CrateDepKind::MacrosOnly | CrateDepKind::Implicit => Linkage::NotLinked,
+                CrateDepKind::Unconditional => Linkage::Static,
+                CrateDepKind::MacrosOnly | CrateDepKind::Conditional => Linkage::NotLinked,
             }),
             cnum
         );

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -116,8 +116,6 @@ pub(crate) struct CrateMetadata {
     /// Maps crate IDs as they are were seen from this crate's compilation sessions into
     /// IDs as they are seen from the current compilation session.
     cnum_map: CrateNumMap,
-    /// Same ID set as `cnum_map` plus maybe some injected crates like panic runtime.
-    dependencies: Vec<CrateNum>,
     /// How to link (or not link) this crate to the currently compiled crate.
     dep_kind: CrateDepKind,
     /// Filesystem location of this crate.
@@ -1897,7 +1895,6 @@ impl CrateMetadata {
             .collect();
         let alloc_decoding_state =
             AllocDecodingState::new(root.interpret_alloc_index.decode(&blob).collect());
-        let dependencies = cnum_map.iter().copied().collect();
 
         // Pre-decode the DefPathHash->DefIndex table. This is a cheap operation
         // that does not copy any data. It just does some data verification.
@@ -1915,7 +1912,6 @@ impl CrateMetadata {
             alloc_decoding_state,
             cnum,
             cnum_map,
-            dependencies,
             dep_kind,
             source: Arc::new(source),
             private_dep,
@@ -1941,7 +1937,7 @@ impl CrateMetadata {
     }
 
     pub(crate) fn dependencies(&self) -> impl Iterator<Item = CrateNum> {
-        self.dependencies.iter().copied()
+        self.cnum_map.iter().copied()
     }
 
     pub(crate) fn target_modifiers(&self) -> TargetModifiers {

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -623,15 +623,15 @@ impl CStore {
         self.get_crate_data(cnum).get_proc_macro_quoted_span(tcx, id)
     }
 
-    pub fn set_used_recursively(&mut self, tcx: TyCtxt<'_>, cnum: CrateNum) {
+    pub fn set_used_recursively(&mut self, cnum: CrateNum) {
         let cmeta = self.get_crate_data_mut(cnum);
         if !cmeta.used {
             cmeta.used = true;
-            let dependencies = mem::take(&mut cmeta.dependencies);
-            for &dep_cnum in &dependencies {
-                self.set_used_recursively(tcx, dep_cnum);
+            let cnum_map = mem::take(&mut cmeta.cnum_map);
+            for &dep_cnum in cnum_map.iter() {
+                self.set_used_recursively(dep_cnum);
             }
-            self.get_crate_data_mut(cnum).dependencies = dependencies;
+            self.get_crate_data_mut(cnum).cnum_map = cnum_map;
         }
     }
 
@@ -663,11 +663,11 @@ impl CStore {
         if cmeta.update_extern_crate_diagnostics(extern_crate) {
             // Propagate the extern crate info to dependencies if it was updated.
             let extern_crate = ExternCrate { dependency_of: cnum, ..extern_crate };
-            let dependencies = mem::take(&mut cmeta.dependencies);
-            for &dep_cnum in &dependencies {
+            let cnum_map = mem::take(&mut cmeta.cnum_map);
+            for &dep_cnum in cnum_map.iter() {
                 self.update_transitive_extern_crate_diagnostics(dep_cnum, extern_crate);
             }
-            self.get_crate_data_mut(cnum).dependencies = dependencies;
+            self.get_crate_data_mut(cnum).cnum_map = cnum_map;
         }
     }
 }

compiler/rustc_session/src/cstore.rs

@@ -39,20 +39,21 @@ impl CrateSource {
 pub enum CrateDepKind {
     /// A dependency that is only used for its macros.
     MacrosOnly,
-    /// A dependency that is always injected into the dependency list and so
-    /// doesn't need to be linked to an rlib, e.g., the injected panic runtime.
-    Implicit,
+    /// A dependency that is injected into the crate graph but which only
+    /// sometimes needs to actually be linked in, e.g., the injected panic runtime.
+    Conditional,
     /// A dependency that is required by an rlib version of this crate.
-    /// Ordinary `extern crate`s result in `Explicit` dependencies.
-    Explicit,
+    /// Ordinary `extern crate`s as well as most injected dependencies result
+    /// in `Unconditional` dependencies.
+    Unconditional,
 }
 
 impl CrateDepKind {
     #[inline]
     pub fn macros_only(self) -> bool {
         match self {
             CrateDepKind::MacrosOnly => true,
-            CrateDepKind::Implicit | CrateDepKind::Explicit => false,
+            CrateDepKind::Conditional | CrateDepKind::Unconditional => false,
         }
     }
 }