Rollup merge of #143369 - bjorn3:cleanup_metadata_loading, r=lcnr

Various refactorings to the metadata loader

2 files changed, 134 insertions, 95 deletions

diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs
index 2c882b84c58..e65c7a68426 100644
--- a/compiler/rustc_metadata/src/creader.rs
+++ b/compiler/rustc_metadata/src/creader.rs
@@ -24,8 +24,7 @@ use rustc_middle::ty::data_structures::IndexSet;
 use rustc_middle::ty::{TyCtxt, TyCtxtFeed};
 use rustc_proc_macro::bridge::client::ProcMacro;
 use rustc_session::config::{
-    self, CrateType, ExtendedTargetModifierInfo, ExternLocation, OptionsTargetModifiers,
-    TargetModifier,
+    CrateType, ExtendedTargetModifierInfo, ExternLocation, OptionsTargetModifiers, TargetModifier,
 };
 use rustc_session::cstore::{CrateDepKind, CrateSource, ExternCrate, ExternCrateSource};
 use rustc_session::lint::{self, BuiltinLintDiag};
@@ -33,11 +32,11 @@ use rustc_session::output::validate_crate_name;
 use rustc_session::search_paths::PathKind;
 use rustc_span::edition::Edition;
 use rustc_span::{DUMMY_SP, Ident, Span, Symbol, sym};
-use rustc_target::spec::{PanicStrategy, Target, TargetTuple};
+use rustc_target::spec::{PanicStrategy, Target};
 use tracing::{debug, info, trace};
 
 use crate::errors;
-use crate::locator::{CrateError, CrateLocator, CratePaths};
+use crate::locator::{CrateError, CrateLocator, CratePaths, CrateRejections};
 use crate::rmeta::{
     CrateDep, CrateMetadata, CrateNumMap, CrateRoot, MetadataBlob, TargetModifiers,
 };
@@ -684,61 +683,67 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
     fn load_proc_macro<'b>(
         &self,
         locator: &mut CrateLocator<'b>,
+        crate_rejections: &mut CrateRejections,
         path_kind: PathKind,
         host_hash: Option<Svh>,
     ) -> Result<Option<(LoadResult, Option<Library>)>, CrateError>
     where
         'a: 'b,
     {
-        // Use a new crate locator so trying to load a proc macro doesn't affect the error
-        // message we emit
-        let mut proc_macro_locator = locator.clone();
-
-        // Try to load a proc macro
-        proc_macro_locator.is_proc_macro = true;
-
-        // Load the proc macro crate for the target
-        let (locator, target_result) = if self.sess.opts.unstable_opts.dual_proc_macros {
-            proc_macro_locator.reset();
-            let result = match self.load(&mut proc_macro_locator)? {
-                Some(LoadResult::Previous(cnum)) => {
-                    return Ok(Some((LoadResult::Previous(cnum), None)));
-                }
-                Some(LoadResult::Loaded(library)) => Some(LoadResult::Loaded(library)),
-                None => return Ok(None),
-            };
-            locator.hash = host_hash;
-            // Use the locator when looking for the host proc macro crate, as that is required
-            // so we want it to affect the error message
-            (locator, result)
-        } else {
-            (&mut proc_macro_locator, None)
-        };
+        if self.sess.opts.unstable_opts.dual_proc_macros {
+            // Use a new crate locator and crate rejections so trying to load a proc macro doesn't
+            // affect the error message we emit
+            let mut proc_macro_locator = locator.clone();
+
+            // Try to load a proc macro
+            proc_macro_locator.for_target_proc_macro(self.sess, path_kind);
+
+            // Load the proc macro crate for the target
+            let target_result =
+                match self.load(&mut proc_macro_locator, &mut CrateRejections::default())? {
+                    Some(LoadResult::Previous(cnum)) => {
+                        return Ok(Some((LoadResult::Previous(cnum), None)));
+                    }
+                    Some(LoadResult::Loaded(library)) => Some(LoadResult::Loaded(library)),
+                    None => return Ok(None),
+                };
 
-        // Load the proc macro crate for the host
+            // Use the existing crate_rejections as we want the error message to be affected by
+            // loading the host proc macro.
+            *crate_rejections = CrateRejections::default();
 
-        locator.reset();
-        locator.is_proc_macro = true;
-        locator.target = &self.sess.host;
-        locator.tuple = TargetTuple::from_tuple(config::host_tuple());
-        locator.filesearch = self.sess.host_filesearch();
-        locator.path_kind = path_kind;
+            // Load the proc macro crate for the host
+            locator.for_proc_macro(self.sess, path_kind);
 
-        let Some(host_result) = self.load(locator)? else {
-            return Ok(None);
-        };
+            locator.hash = host_hash;
+
+            let Some(host_result) = self.load(locator, crate_rejections)? else {
+                return Ok(None);
+            };
 
-        Ok(Some(if self.sess.opts.unstable_opts.dual_proc_macros {
             let host_result = match host_result {
                 LoadResult::Previous(..) => {
                     panic!("host and target proc macros must be loaded in lock-step")
                 }
                 LoadResult::Loaded(library) => library,
             };
-            (target_result.unwrap(), Some(host_result))
+            Ok(Some((target_result.unwrap(), Some(host_result))))
         } else {
-            (host_result, None)
-        }))
+            // Use a new crate locator and crate rejections so trying to load a proc macro doesn't
+            // affect the error message we emit
+            let mut proc_macro_locator = locator.clone();
+
+            // Load the proc macro crate for the host
+            proc_macro_locator.for_proc_macro(self.sess, path_kind);
+
+            let Some(host_result) =
+                self.load(&mut proc_macro_locator, &mut CrateRejections::default())?
+            else {
+                return Ok(None);
+            };
+
+            Ok(Some((host_result, None)))
+        }
     }
 
     fn resolve_crate(
@@ -799,15 +804,21 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
                 extra_filename,
                 path_kind,
             );
+            let mut crate_rejections = CrateRejections::default();
 
-            match self.load(&mut locator)? {
+            match self.load(&mut locator, &mut crate_rejections)? {
                 Some(res) => (res, None),
                 None => {
                     info!("falling back to loading proc_macro");
                     dep_kind = CrateDepKind::MacrosOnly;
-                    match self.load_proc_macro(&mut locator, path_kind, host_hash)? {
+                    match self.load_proc_macro(
+                        &mut locator,
+                        &mut crate_rejections,
+                        path_kind,
+                        host_hash,
+                    )? {
                         Some(res) => res,
-                        None => return Err(locator.into_error(dep_root.cloned())),
+                        None => return Err(locator.into_error(crate_rejections, dep_root.cloned())),
                     }
                 }
             }
@@ -837,8 +848,12 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
         }
     }
 
-    fn load(&self, locator: &mut CrateLocator<'_>) -> Result<Option<LoadResult>, CrateError> {
-        let Some(library) = locator.maybe_load_library_crate()? else {
+    fn load(
+        &self,
+        locator: &CrateLocator<'_>,
+        crate_rejections: &mut CrateRejections,
+    ) -> Result<Option<LoadResult>, CrateError> {
+        let Some(library) = locator.maybe_load_library_crate(crate_rejections)? else {
             return Ok(None);
         };
 
diff --git a/compiler/rustc_metadata/src/locator.rs b/compiler/rustc_metadata/src/locator.rs
index 82254f0a381..c30cfd1fcf7 100644
--- a/compiler/rustc_metadata/src/locator.rs
+++ b/compiler/rustc_metadata/src/locator.rs
@@ -224,11 +224,11 @@ use rustc_data_structures::owned_slice::{OwnedSlice, slice_owned};
 use rustc_data_structures::svh::Svh;
 use rustc_errors::{DiagArgValue, IntoDiagArg};
 use rustc_fs_util::try_canonicalize;
-use rustc_session::Session;
 use rustc_session::cstore::CrateSource;
 use rustc_session::filesearch::FileSearch;
 use rustc_session::search_paths::PathKind;
 use rustc_session::utils::CanonicalizedPath;
+use rustc_session::{Session, config};
 use rustc_span::{Span, Symbol};
 use rustc_target::spec::{Target, TargetTuple};
 use tempfile::Builder as TempFileBuilder;
@@ -251,14 +251,11 @@ pub(crate) struct CrateLocator<'a> {
     exact_paths: Vec<CanonicalizedPath>,
     pub hash: Option<Svh>,
     extra_filename: Option<&'a str>,
-    pub target: &'a Target,
-    pub tuple: TargetTuple,
-    pub filesearch: &'a FileSearch,
-    pub is_proc_macro: bool,
-
-    pub path_kind: PathKind,
-    // Mutable in-progress state or output.
-    crate_rejections: CrateRejections,
+    target: &'a Target,
+    tuple: TargetTuple,
+    filesearch: &'a FileSearch,
+    is_proc_macro: bool,
+    path_kind: PathKind,
 }
 
 #[derive(Clone, Debug)]
@@ -346,34 +343,46 @@ impl<'a> CrateLocator<'a> {
             filesearch: sess.target_filesearch(),
             path_kind,
             is_proc_macro: false,
-            crate_rejections: CrateRejections::default(),
         }
     }
 
-    pub(crate) fn reset(&mut self) {
-        self.crate_rejections.via_hash.clear();
-        self.crate_rejections.via_triple.clear();
-        self.crate_rejections.via_kind.clear();
-        self.crate_rejections.via_version.clear();
-        self.crate_rejections.via_filename.clear();
-        self.crate_rejections.via_invalid.clear();
+    pub(crate) fn for_proc_macro(&mut self, sess: &'a Session, path_kind: PathKind) {
+        self.is_proc_macro = true;
+        self.target = &sess.host;
+        self.tuple = TargetTuple::from_tuple(config::host_tuple());
+        self.filesearch = sess.host_filesearch();
+        self.path_kind = path_kind;
     }
 
-    pub(crate) fn maybe_load_library_crate(&mut self) -> Result<Option<Library>, CrateError> {
+    pub(crate) fn for_target_proc_macro(&mut self, sess: &'a Session, path_kind: PathKind) {
+        self.is_proc_macro = true;
+        self.target = &sess.target;
+        self.tuple = sess.opts.target_triple.clone();
+        self.filesearch = sess.target_filesearch();
+        self.path_kind = path_kind;
+    }
+
+    pub(crate) fn maybe_load_library_crate(
+        &self,
+        crate_rejections: &mut CrateRejections,
+    ) -> Result<Option<Library>, CrateError> {
         if !self.exact_paths.is_empty() {
-            return self.find_commandline_library();
+            return self.find_commandline_library(crate_rejections);
         }
         let mut seen_paths = FxHashSet::default();
         if let Some(extra_filename) = self.extra_filename {
-            if let library @ Some(_) = self.find_library_crate(extra_filename, &mut seen_paths)? {
+            if let library @ Some(_) =
+                self.find_library_crate(crate_rejections, extra_filename, &mut seen_paths)?
+            {
                 return Ok(library);
             }
         }
-        self.find_library_crate("", &mut seen_paths)
+        self.find_library_crate(crate_rejections, "", &mut seen_paths)
     }
 
     fn find_library_crate(
-        &mut self,
+        &self,
+        crate_rejections: &mut CrateRejections,
         extra_prefix: &str,
         seen_paths: &mut FxHashSet<PathBuf>,
     ) -> Result<Option<Library>, CrateError> {
@@ -465,7 +474,7 @@ impl<'a> CrateLocator<'a> {
                 .flatten()
             {
                 for (_, spf) in static_matches {
-                    self.crate_rejections.via_kind.push(CrateMismatch {
+                    crate_rejections.via_kind.push(CrateMismatch {
                         path: spf.path.to_path_buf(),
                         got: "static".to_string(),
                     });
@@ -483,7 +492,9 @@ impl<'a> CrateLocator<'a> {
         // search is being performed for.
         let mut libraries = FxIndexMap::default();
         for (_hash, (rlibs, rmetas, dylibs, interfaces)) in candidates {
-            if let Some((svh, lib)) = self.extract_lib(rlibs, rmetas, dylibs, interfaces)? {
+            if let Some((svh, lib)) =
+                self.extract_lib(crate_rejections, rlibs, rmetas, dylibs, interfaces)?
+            {
                 libraries.insert(svh, lib);
             }
         }
@@ -495,13 +506,11 @@ impl<'a> CrateLocator<'a> {
             0 => Ok(None),
             1 => Ok(Some(libraries.into_iter().next().unwrap().1)),
             _ => {
-                let mut libraries: Vec<_> = libraries.into_values().collect();
-
-                libraries.sort_by_cached_key(|lib| lib.source.paths().next().unwrap().clone());
-                let candidates = libraries
-                    .iter()
+                let mut candidates: Vec<PathBuf> = libraries
+                    .into_values()
                     .map(|lib| lib.source.paths().next().unwrap().clone())
-                    .collect::<Vec<_>>();
+                    .collect();
+                candidates.sort();
 
                 Err(CrateError::MultipleCandidates(
                     self.crate_name,
@@ -514,7 +523,8 @@ impl<'a> CrateLocator<'a> {
     }
 
     fn extract_lib(
-        &mut self,
+        &self,
+        crate_rejections: &mut CrateRejections,
         rlibs: FxIndexMap<PathBuf, PathKind>,
         rmetas: FxIndexMap<PathBuf, PathKind>,
         dylibs: FxIndexMap<PathBuf, PathKind>,
@@ -526,10 +536,11 @@ impl<'a> CrateLocator<'a> {
         // Make sure there's at most one rlib and at most one dylib.
         //
         // See comment in `extract_one` below.
-        let rmeta = self.extract_one(rmetas, CrateFlavor::Rmeta, &mut slot)?;
-        let rlib = self.extract_one(rlibs, CrateFlavor::Rlib, &mut slot)?;
-        let sdylib_interface = self.extract_one(interfaces, CrateFlavor::SDylib, &mut slot)?;
-        let dylib = self.extract_one(dylibs, CrateFlavor::Dylib, &mut slot)?;
+        let rmeta = self.extract_one(crate_rejections, rmetas, CrateFlavor::Rmeta, &mut slot)?;
+        let rlib = self.extract_one(crate_rejections, rlibs, CrateFlavor::Rlib, &mut slot)?;
+        let sdylib_interface =
+            self.extract_one(crate_rejections, interfaces, CrateFlavor::SDylib, &mut slot)?;
+        let dylib = self.extract_one(crate_rejections, dylibs, CrateFlavor::Dylib, &mut slot)?;
 
         if sdylib_interface.is_some() && dylib.is_none() {
             return Err(CrateError::FullMetadataNotFound(self.crate_name, CrateFlavor::SDylib));
@@ -563,7 +574,8 @@ impl<'a> CrateLocator<'a> {
     //
     // The `PathBuf` in `slot` will only be used for diagnostic purposes.
     fn extract_one(
-        &mut self,
+        &self,
+        crate_rejections: &mut CrateRejections,
         m: FxIndexMap<PathBuf, PathKind>,
         flavor: CrateFlavor,
         slot: &mut Option<(Svh, MetadataBlob, PathBuf, CrateFlavor)>,
@@ -605,7 +617,7 @@ impl<'a> CrateLocator<'a> {
                 Some(self.crate_name),
             ) {
                 Ok(blob) => {
-                    if let Some(h) = self.crate_matches(&blob, &lib) {
+                    if let Some(h) = self.crate_matches(crate_rejections, &blob, &lib) {
                         (h, blob)
                     } else {
                         info!("metadata mismatch");
@@ -620,7 +632,7 @@ impl<'a> CrateLocator<'a> {
                         "Rejecting via version: expected {} got {}",
                         expected_version, found_version
                     );
-                    self.crate_rejections
+                    crate_rejections
                         .via_version
                         .push(CrateMismatch { path: lib, got: found_version });
                     continue;
@@ -635,7 +647,7 @@ impl<'a> CrateLocator<'a> {
                     // The file was present and created by the same compiler version, but we
                     // couldn't load it for some reason. Give a hard error instead of silently
                     // ignoring it, but only if we would have given an error anyway.
-                    self.crate_rejections.via_invalid.push(CrateMismatch { path: lib, got: err });
+                    crate_rejections.via_invalid.push(CrateMismatch { path: lib, got: err });
                     continue;
                 }
                 Err(err @ MetadataError::NotPresent(_)) => {
@@ -713,7 +725,12 @@ impl<'a> CrateLocator<'a> {
         }
     }
 
-    fn crate_matches(&mut self, metadata: &MetadataBlob, libpath: &Path) -> Option<Svh> {
+    fn crate_matches(
+        &self,
+        crate_rejections: &mut CrateRejections,
+        metadata: &MetadataBlob,
+        libpath: &Path,
+    ) -> Option<Svh> {
         let header = metadata.get_header();
         if header.is_proc_macro_crate != self.is_proc_macro {
             info!(
@@ -730,7 +747,7 @@ impl<'a> CrateLocator<'a> {
 
         if header.triple != self.tuple {
             info!("Rejecting via crate triple: expected {} got {}", self.tuple, header.triple);
-            self.crate_rejections.via_triple.push(CrateMismatch {
+            crate_rejections.via_triple.push(CrateMismatch {
                 path: libpath.to_path_buf(),
                 got: header.triple.to_string(),
             });
@@ -741,7 +758,7 @@ impl<'a> CrateLocator<'a> {
         if let Some(expected_hash) = self.hash {
             if hash != expected_hash {
                 info!("Rejecting via hash: expected {} got {}", expected_hash, hash);
-                self.crate_rejections
+                crate_rejections
                     .via_hash
                     .push(CrateMismatch { path: libpath.to_path_buf(), got: hash.to_string() });
                 return None;
@@ -751,7 +768,10 @@ impl<'a> CrateLocator<'a> {
         Some(hash)
     }
 
-    fn find_commandline_library(&mut self) -> Result<Option<Library>, CrateError> {
+    fn find_commandline_library(
+        &self,
+        crate_rejections: &mut CrateRejections,
+    ) -> Result<Option<Library>, CrateError> {
         // First, filter out all libraries that look suspicious. We only accept
         // files which actually exist that have the correct naming scheme for
         // rlibs/dylibs.
@@ -796,24 +816,28 @@ impl<'a> CrateLocator<'a> {
                 dylibs.insert(loc_canon.clone(), PathKind::ExternFlag);
                 continue;
             }
-            self.crate_rejections
+            crate_rejections
                 .via_filename
                 .push(CrateMismatch { path: loc_orig.clone(), got: String::new() });
         }
 
         // Extract the dylib/rlib/rmeta triple.
-        self.extract_lib(rlibs, rmetas, dylibs, sdylib_interfaces)
+        self.extract_lib(crate_rejections, rlibs, rmetas, dylibs, sdylib_interfaces)
             .map(|opt| opt.map(|(_, lib)| lib))
     }
 
-    pub(crate) fn into_error(self, dep_root: Option<CratePaths>) -> CrateError {
+    pub(crate) fn into_error(
+        self,
+        crate_rejections: CrateRejections,
+        dep_root: Option<CratePaths>,
+    ) -> CrateError {
         CrateError::LocatorCombined(Box::new(CombinedLocatorError {
             crate_name: self.crate_name,
             dep_root,
             triple: self.tuple,
             dll_prefix: self.target.dll_prefix.to_string(),
             dll_suffix: self.target.dll_suffix.to_string(),
-            crate_rejections: self.crate_rejections,
+            crate_rejections,
         }))
     }
 }
@@ -993,7 +1017,7 @@ struct CrateMismatch {
 }
 
 #[derive(Clone, Debug, Default)]
-struct CrateRejections {
+pub(crate) struct CrateRejections {
     via_hash: Vec<CrateMismatch>,
     via_triple: Vec<CrateMismatch>,
     via_kind: Vec<CrateMismatch>,