Auto merge of #88538 - bjorn3:no_session_in_crate_loader, r=petrochenkov

CrateLocator refactorings

This makes the `CrateLocator` a lot cleaner IMHO and much more self-contained. The last commit removes `extra_filename` from the crate metadata. This is an **insta-stable** change as it allows a crate like `libfoo-abc.rlib` to be used as dependency and then be renamed as `libfoo-bcd.rlib` while still being found as indirect dependency. This may reduce performance when there are a lot of versions of the same crate available as the extra filename won't be used to do an early rejection of crates before trying to load metadata, but it makes the logic to find the right filename a lot cleaner.

compiler/rustc_metadata/src/creader.rs

@@ -450,6 +450,7 @@ fn load_proc_macro<'b>(
         &self,
         locator: &mut CrateLocator<'b>,
         path_kind: PathKind,
+        host_hash: Option<Svh>,
     ) -> Result<Option<(LoadResult, Option<Library>)>, CrateError>
     where
         'a: 'b,
@@ -459,7 +460,7 @@ fn load_proc_macro<'b>(
         let mut proc_macro_locator = locator.clone();
 
         // Try to load a proc macro
-        proc_macro_locator.is_proc_macro = Some(true);
+        proc_macro_locator.is_proc_macro = true;
 
         // Load the proc macro crate for the target
         let (locator, target_result) = if self.sess.opts.debugging_opts.dual_proc_macros {
@@ -471,7 +472,7 @@ fn load_proc_macro<'b>(
                 Some(LoadResult::Loaded(library)) => Some(LoadResult::Loaded(library)),
                 None => return Ok(None),
             };
-            locator.hash = locator.host_hash;
+            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)
@@ -482,7 +483,7 @@ fn load_proc_macro<'b>(
         // Load the proc macro crate for the host
 
         locator.reset();
-        locator.is_proc_macro = Some(true);
+        locator.is_proc_macro = true;
         locator.target = &self.sess.host;
         locator.triple = TargetTriple::from_triple(config::host_triple());
         locator.filesearch = self.sess.host_filesearch(path_kind);
@@ -510,12 +511,9 @@ fn resolve_crate<'b>(
         name: Symbol,
         span: Span,
         dep_kind: CrateDepKind,
-        dep: Option<(&'b CratePaths, &'b CrateDep)>,
     ) -> CrateNum {
-        if dep.is_none() {
-            self.used_extern_options.insert(name);
-        }
-        self.maybe_resolve_crate(name, dep_kind, dep).unwrap_or_else(|err| {
+        self.used_extern_options.insert(name);
+        self.maybe_resolve_crate(name, dep_kind, None).unwrap_or_else(|err| {
             let missing_core =
                 self.maybe_resolve_crate(sym::core, CrateDepKind::Explicit, None).is_err();
             err.report(&self.sess, span, missing_core)
@@ -551,21 +549,18 @@ fn maybe_resolve_crate<'b>(
                 &*self.metadata_loader,
                 name,
                 hash,
-                host_hash,
                 extra_filename,
                 false, // is_host
                 path_kind,
-                root,
-                Some(false), // is_proc_macro
             );
 
             match self.load(&mut locator)? {
                 Some(res) => (res, None),
                 None => {
                     dep_kind = CrateDepKind::MacrosOnly;
-                    match self.load_proc_macro(&mut locator, path_kind)? {
+                    match self.load_proc_macro(&mut locator, path_kind, host_hash)? {
                         Some(res) => res,
-                        None => return Err(locator.into_error()),
+                        None => return Err(locator.into_error(root.cloned())),
                     }
                 }
             }
@@ -605,7 +600,7 @@ fn load(&self, locator: &mut CrateLocator<'_>) -> Result<Option<LoadResult>, Cra
         // FIXME: why is this condition necessary? It was adding in #33625 but I
         // don't know why and the original author doesn't remember ...
         let can_reuse_cratenum =
-            locator.triple == self.sess.opts.target_triple || locator.is_proc_macro == Some(true);
+            locator.triple == self.sess.opts.target_triple || locator.is_proc_macro;
         Ok(Some(if can_reuse_cratenum {
             let mut result = LoadResult::Loaded(library);
             self.cstore.iter_crate_data(|cnum, data| {
@@ -755,7 +750,7 @@ fn inject_panic_runtime(&mut self, krate: &ast::Crate) {
         };
         info!("panic runtime not found -- loading {}", name);
 
-        let cnum = self.resolve_crate(name, DUMMY_SP, CrateDepKind::Implicit, None);
+        let cnum = self.resolve_crate(name, DUMMY_SP, CrateDepKind::Implicit);
         let data = self.cstore.get_crate_data(cnum);
 
         // Sanity check the loaded crate to ensure it is indeed a panic runtime
@@ -795,7 +790,7 @@ fn inject_profiler_runtime(&mut self, krate: &ast::Crate) {
             );
         }
 
-        let cnum = self.resolve_crate(name, DUMMY_SP, CrateDepKind::Implicit, None);
+        let cnum = self.resolve_crate(name, DUMMY_SP, CrateDepKind::Implicit);
         let data = self.cstore.get_crate_data(cnum);
 
         // Sanity check the loaded crate to ensure it is indeed a profiler runtime
@@ -1015,7 +1010,7 @@ pub fn process_extern_crate(
                     CrateDepKind::Explicit
                 };
 
-                let cnum = self.resolve_crate(name, item.span, dep_kind, None);
+                let cnum = self.resolve_crate(name, item.span, dep_kind);
 
                 let path_len = definitions.def_path(def_id).data.len();
                 self.update_extern_crate(
@@ -1034,7 +1029,7 @@ pub fn process_extern_crate(
     }
 
     pub fn process_path_extern(&mut self, name: Symbol, span: Span) -> CrateNum {
-        let cnum = self.resolve_crate(name, span, CrateDepKind::Explicit, None);
+        let cnum = self.resolve_crate(name, span, CrateDepKind::Explicit);
 
         self.update_extern_crate(
             cnum,

compiler/rustc_metadata/src/locator.rs

@@ -240,27 +240,22 @@
 #[derive(Clone)]
 crate struct CrateLocator<'a> {
     // Immutable per-session configuration.
-    sess: &'a Session,
+    only_needs_metadata: bool,
+    sysroot: &'a Path,
     metadata_loader: &'a dyn MetadataLoader,
 
     // Immutable per-search configuration.
     crate_name: Symbol,
     exact_paths: Vec<CanonicalizedPath>,
     pub hash: Option<Svh>,
-    pub host_hash: Option<Svh>,
     extra_filename: Option<&'a str>,
     pub target: &'a Target,
     pub triple: TargetTriple,
     pub filesearch: FileSearch<'a>,
-    root: Option<&'a CratePaths>,
-    pub is_proc_macro: Option<bool>,
+    pub is_proc_macro: bool,
 
     // Mutable in-progress state or output.
-    rejected_via_hash: Vec<CrateMismatch>,
-    rejected_via_triple: Vec<CrateMismatch>,
-    rejected_via_kind: Vec<CrateMismatch>,
-    rejected_via_version: Vec<CrateMismatch>,
-    rejected_via_filename: Vec<CrateMismatch>,
+    crate_rejections: CrateRejections,
 }
 
 #[derive(Clone)]
@@ -298,15 +293,22 @@ impl<'a> CrateLocator<'a> {
         metadata_loader: &'a dyn MetadataLoader,
         crate_name: Symbol,
         hash: Option<Svh>,
-        host_hash: Option<Svh>,
         extra_filename: Option<&'a str>,
         is_host: bool,
         path_kind: PathKind,
-        root: Option<&'a CratePaths>,
-        is_proc_macro: Option<bool>,
     ) -> CrateLocator<'a> {
+        // The all loop is because `--crate-type=rlib --crate-type=rlib` is
+        // legal and produces both inside this type.
+        let is_rlib = sess.crate_types().iter().all(|c| *c == CrateType::Rlib);
+        let needs_object_code = sess.opts.output_types.should_codegen();
+        // If we're producing an rlib, then we don't need object code.
+        // Or, if we're not producing object code, then we don't need it either
+        // (e.g., if we're a cdylib but emitting just metadata).
+        let only_needs_metadata = is_rlib || !needs_object_code;
+
         CrateLocator {
-            sess,
+            only_needs_metadata,
+            sysroot: &sess.sysroot,
             metadata_loader,
             crate_name,
             exact_paths: if hash.is_none() {
@@ -324,7 +326,6 @@ impl<'a> CrateLocator<'a> {
                 Vec::new()
             },
             hash,
-            host_hash,
             extra_filename,
             target: if is_host { &sess.host } else { &sess.target },
             triple: if is_host {
@@ -337,22 +338,17 @@ impl<'a> CrateLocator<'a> {
             } else {
                 sess.target_filesearch(path_kind)
             },
-            root,
-            is_proc_macro,
-            rejected_via_hash: Vec::new(),
-            rejected_via_triple: Vec::new(),
-            rejected_via_kind: Vec::new(),
-            rejected_via_version: Vec::new(),
-            rejected_via_filename: Vec::new(),
+            is_proc_macro: false,
+            crate_rejections: CrateRejections::default(),
         }
     }
 
     crate fn reset(&mut self) {
-        self.rejected_via_hash.clear();
-        self.rejected_via_triple.clear();
-        self.rejected_via_kind.clear();
-        self.rejected_via_version.clear();
-        self.rejected_via_filename.clear();
+        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();
     }
 
     crate fn maybe_load_library_crate(&mut self) -> Result<Option<Library>, CrateError> {
@@ -435,7 +431,7 @@ fn find_library_crate(
             };
             FileMatches
         });
-        self.rejected_via_kind.extend(staticlibs);
+        self.crate_rejections.via_kind.extend(staticlibs);
 
         // We have now collected all known libraries into a set of candidates
         // keyed of the filename hash listed. For each filename, we also have a
@@ -480,18 +476,11 @@ fn extract_lib(
     }
 
     fn needs_crate_flavor(&self, flavor: CrateFlavor) -> bool {
-        if flavor == CrateFlavor::Dylib && self.is_proc_macro == Some(true) {
+        if flavor == CrateFlavor::Dylib && self.is_proc_macro {
             return true;
         }
 
-        // The all loop is because `--crate-type=rlib --crate-type=rlib` is
-        // legal and produces both inside this type.
-        let is_rlib = self.sess.crate_types().iter().all(|c| *c == CrateType::Rlib);
-        let needs_object_code = self.sess.opts.output_types.should_codegen();
-        // If we're producing an rlib, then we don't need object code.
-        // Or, if we're not producing object code, then we don't need it either
-        // (e.g., if we're a cdylib but emitting just metadata).
-        if is_rlib || !needs_object_code {
+        if self.only_needs_metadata {
             flavor == CrateFlavor::Rmeta
         } else {
             // we need all flavors (perhaps not true, but what we do for now)
@@ -591,7 +580,7 @@ fn extract_one(
             // candidates are all canonicalized, so we canonicalize the sysroot
             // as well.
             if let Some((prev, _)) = &ret {
-                let sysroot = &self.sess.sysroot;
+                let sysroot = self.sysroot;
                 let sysroot = sysroot.canonicalize().unwrap_or_else(|_| sysroot.to_path_buf());
                 if prev.starts_with(&sysroot) {
                     continue;
@@ -613,21 +602,20 @@ fn crate_matches(&mut self, metadata: &MetadataBlob, libpath: &Path) -> Option<S
         let found_version = metadata.get_rustc_version();
         if found_version != rustc_version {
             info!("Rejecting via version: expected {} got {}", rustc_version, found_version);
-            self.rejected_via_version
+            self.crate_rejections
+                .via_version
                 .push(CrateMismatch { path: libpath.to_path_buf(), got: found_version });
             return None;
         }
 
         let root = metadata.get_root();
-        if let Some(expected_is_proc_macro) = self.is_proc_macro {
-            let is_proc_macro = root.is_proc_macro_crate();
-            if is_proc_macro != expected_is_proc_macro {
-                info!(
-                    "Rejecting via proc macro: expected {} got {}",
-                    expected_is_proc_macro, is_proc_macro
-                );
-                return None;
-            }
+        if root.is_proc_macro_crate() != self.is_proc_macro {
+            info!(
+                "Rejecting via proc macro: expected {} got {}",
+                self.is_proc_macro,
+                root.is_proc_macro_crate(),
+            );
+            return None;
         }
 
         if self.exact_paths.is_empty() && self.crate_name != root.name() {
@@ -637,7 +625,7 @@ fn crate_matches(&mut self, metadata: &MetadataBlob, libpath: &Path) -> Option<S
 
         if root.triple() != &self.triple {
             info!("Rejecting via crate triple: expected {} got {}", self.triple, root.triple());
-            self.rejected_via_triple.push(CrateMismatch {
+            self.crate_rejections.via_triple.push(CrateMismatch {
                 path: libpath.to_path_buf(),
                 got: root.triple().to_string(),
             });
@@ -648,7 +636,8 @@ fn crate_matches(&mut self, metadata: &MetadataBlob, libpath: &Path) -> Option<S
         if let Some(expected_hash) = self.hash {
             if hash != expected_hash {
                 info!("Rejecting via hash: expected {} got {}", expected_hash, hash);
-                self.rejected_via_hash
+                self.crate_rejections
+                    .via_hash
                     .push(CrateMismatch { path: libpath.to_path_buf(), got: hash.to_string() });
                 return None;
             }
@@ -702,7 +691,8 @@ fn find_commandline_library(&mut self) -> Result<Option<Library>, CrateError> {
                     dylibs.insert(loc_canon, PathKind::ExternFlag);
                 }
             } else {
-                self.rejected_via_filename
+                self.crate_rejections
+                    .via_filename
                     .push(CrateMismatch { path: loc.original().clone(), got: String::new() });
             }
         }
@@ -711,18 +701,14 @@ fn find_commandline_library(&mut self) -> Result<Option<Library>, CrateError> {
         Ok(self.extract_lib(rlibs, rmetas, dylibs)?.map(|(_, lib)| lib))
     }
 
-    crate fn into_error(self) -> CrateError {
+    crate fn into_error(self, root: Option<CratePaths>) -> CrateError {
         CrateError::LocatorCombined(CombinedLocatorError {
             crate_name: self.crate_name,
-            root: self.root.cloned(),
+            root,
             triple: self.triple,
             dll_prefix: self.target.dll_prefix.clone(),
             dll_suffix: self.target.dll_suffix.clone(),
-            rejected_via_hash: self.rejected_via_hash,
-            rejected_via_triple: self.rejected_via_triple,
-            rejected_via_kind: self.rejected_via_kind,
-            rejected_via_version: self.rejected_via_version,
-            rejected_via_filename: self.rejected_via_filename,
+            crate_rejections: self.crate_rejections,
         })
     }
 }
@@ -806,12 +792,9 @@ fn find_plugin_registrar_impl<'a>(
         metadata_loader,
         name,
         None, // hash
-        None, // host_hash
         None, // extra_filename
         true, // is_host
         PathKind::Crate,
-        None, // root
-        None, // is_proc_macro
     );
 
     match locator.maybe_load_library_crate()? {
@@ -819,7 +802,7 @@ fn find_plugin_registrar_impl<'a>(
             Some(dylib) => Ok(dylib.0),
             None => Err(CrateError::NonDylibPlugin(name)),
         },
-        None => Err(locator.into_error()),
+        None => Err(locator.into_error(None)),
     }
 }
 
@@ -852,6 +835,15 @@ struct CrateMismatch {
     got: String,
 }
 
+#[derive(Clone, Default)]
+struct CrateRejections {
+    via_hash: Vec<CrateMismatch>,
+    via_triple: Vec<CrateMismatch>,
+    via_kind: Vec<CrateMismatch>,
+    via_version: Vec<CrateMismatch>,
+    via_filename: Vec<CrateMismatch>,
+}
+
 /// Candidate rejection reasons collected during crate search.
 /// If no candidate is accepted, then these reasons are presented to the user,
 /// otherwise they are ignored.
@@ -861,11 +853,7 @@ struct CrateMismatch {
     triple: TargetTriple,
     dll_prefix: String,
     dll_suffix: String,
-    rejected_via_hash: Vec<CrateMismatch>,
-    rejected_via_triple: Vec<CrateMismatch>,
-    rejected_via_kind: Vec<CrateMismatch>,
-    rejected_via_version: Vec<CrateMismatch>,
-    rejected_via_filename: Vec<CrateMismatch>,
+    crate_rejections: CrateRejections,
 }
 
 crate enum CrateError {
@@ -974,7 +962,7 @@ impl CrateError {
                     Some(r) => format!(" which `{}` depends on", r.name),
                 };
                 let mut msg = "the following crate versions were found:".to_string();
-                let mut err = if !locator.rejected_via_hash.is_empty() {
+                let mut err = if !locator.crate_rejections.via_hash.is_empty() {
                     let mut err = struct_span_err!(
                         sess,
                         span,
@@ -984,7 +972,7 @@ impl CrateError {
                         add,
                     );
                     err.note("perhaps that crate needs to be recompiled?");
-                    let mismatches = locator.rejected_via_hash.iter();
+                    let mismatches = locator.crate_rejections.via_hash.iter();
                     for CrateMismatch { path, .. } in mismatches {
                         msg.push_str(&format!("\ncrate `{}`: {}", crate_name, path.display()));
                     }
@@ -995,7 +983,7 @@ impl CrateError {
                     }
                     err.note(&msg);
                     err
-                } else if !locator.rejected_via_triple.is_empty() {
+                } else if !locator.crate_rejections.via_triple.is_empty() {
                     let mut err = struct_span_err!(
                         sess,
                         span,
@@ -1005,7 +993,7 @@ impl CrateError {
                         locator.triple,
                         add,
                     );
-                    let mismatches = locator.rejected_via_triple.iter();
+                    let mismatches = locator.crate_rejections.via_triple.iter();
                     for CrateMismatch { path, got } in mismatches {
                         msg.push_str(&format!(
                             "\ncrate `{}`, target triple {}: {}",
@@ -1016,7 +1004,7 @@ impl CrateError {
                     }
                     err.note(&msg);
                     err
-                } else if !locator.rejected_via_kind.is_empty() {
+                } else if !locator.crate_rejections.via_kind.is_empty() {
                     let mut err = struct_span_err!(
                         sess,
                         span,
@@ -1026,13 +1014,13 @@ impl CrateError {
                         add,
                     );
                     err.help("please recompile that crate using --crate-type lib");
-                    let mismatches = locator.rejected_via_kind.iter();
+                    let mismatches = locator.crate_rejections.via_kind.iter();
                     for CrateMismatch { path, .. } in mismatches {
                         msg.push_str(&format!("\ncrate `{}`: {}", crate_name, path.display()));
                     }
                     err.note(&msg);
                     err
-                } else if !locator.rejected_via_version.is_empty() {
+                } else if !locator.crate_rejections.via_version.is_empty() {
                     let mut err = struct_span_err!(
                         sess,
                         span,
@@ -1045,7 +1033,7 @@ impl CrateError {
                         "please recompile that crate using this compiler ({})",
                         rustc_version(),
                     ));
-                    let mismatches = locator.rejected_via_version.iter();
+                    let mismatches = locator.crate_rejections.via_version.iter();
                     for CrateMismatch { path, got } in mismatches {
                         msg.push_str(&format!(
                             "\ncrate `{}` compiled by {}: {}",
@@ -1112,8 +1100,8 @@ impl CrateError {
                     err
                 };
 
-                if !locator.rejected_via_filename.is_empty() {
-                    let mismatches = locator.rejected_via_filename.iter();
+                if !locator.crate_rejections.via_filename.is_empty() {
+                    let mismatches = locator.crate_rejections.via_filename.iter();
                     for CrateMismatch { path, .. } in mismatches {
                         err.note(&format!(
                             "extern location for {} is of an unknown type: {}",