Auto merge of rust-lang#13260 - Veykril:simplify, r=Veykril

Simplify

crates/project-model/src/cargo_workspace.rs

@@ -272,8 +272,8 @@ impl CargoWorkspace {
         let target = config
             .target
             .clone()
-            .or_else(|| cargo_config_build_target(cargo_toml, config))
-            .or_else(|| rustc_discover_host_triple(cargo_toml, config));
+            .or_else(|| cargo_config_build_target(cargo_toml, &config.extra_env))
+            .or_else(|| rustc_discover_host_triple(cargo_toml, &config.extra_env));
 
         let mut meta = MetadataCommand::new();
         meta.cargo_path(toolchain::cargo());
@@ -304,12 +304,9 @@ impl CargoWorkspace {
         // unclear whether cargo itself supports it.
         progress("metadata".to_string());
 
-        fn exec_with_env(
-            command: &cargo_metadata::MetadataCommand,
-            extra_env: &FxHashMap<String, String>,
-        ) -> Result<cargo_metadata::Metadata, cargo_metadata::Error> {
-            let mut command = command.cargo_command();
-            command.envs(extra_env);
+        (|| -> Result<cargo_metadata::Metadata, cargo_metadata::Error> {
+            let mut command = meta.cargo_command();
+            command.envs(&config.extra_env);
             let output = command.output()?;
             if !output.status.success() {
                 return Err(cargo_metadata::Error::CargoMetadata {
@@ -321,12 +318,8 @@ impl CargoWorkspace {
                 .find(|line| line.starts_with('{'))
                 .ok_or(cargo_metadata::Error::NoJson)?;
             cargo_metadata::MetadataCommand::parse(stdout)
-        }
-
-        let meta = exec_with_env(&meta, &config.extra_env)
-            .with_context(|| format!("Failed to run `{:?}`", meta.cargo_command()))?;
-
-        Ok(meta)
+        })()
+        .with_context(|| format!("Failed to run `{:?}`", meta.cargo_command()))
     }
 
     pub fn new(mut meta: cargo_metadata::Metadata) -> CargoWorkspace {
@@ -395,32 +388,14 @@ impl CargoWorkspace {
         }
         let resolve = meta.resolve.expect("metadata executed with deps");
         for mut node in resolve.nodes {
-            let source = match pkg_by_id.get(&node.id) {
-                Some(&src) => src,
-                // FIXME: replace this and a similar branch below with `.unwrap`, once
-                // https://github.com/rust-lang/cargo/issues/7841
-                // is fixed and hits stable (around 1.43-is probably?).
-                None => {
-                    tracing::error!("Node id do not match in cargo metadata, ignoring {}", node.id);
-                    continue;
-                }
-            };
+            let &source = pkg_by_id.get(&node.id).unwrap();
             node.deps.sort_by(|a, b| a.pkg.cmp(&b.pkg));
-            for (dep_node, kind) in node
+            let dependencies = node
                 .deps
                 .iter()
-                .flat_map(|dep| DepKind::iter(&dep.dep_kinds).map(move |kind| (dep, kind)))
-            {
-                let pkg = match pkg_by_id.get(&dep_node.pkg) {
-                    Some(&pkg) => pkg,
-                    None => {
-                        tracing::error!(
-                            "Dep node id do not match in cargo metadata, ignoring {}",
-                            dep_node.pkg
-                        );
-                        continue;
-                    }
-                };
+                .flat_map(|dep| DepKind::iter(&dep.dep_kinds).map(move |kind| (dep, kind)));
+            for (dep_node, kind) in dependencies {
+                let &pkg = pkg_by_id.get(&dep_node.pkg).unwrap();
                 let dep = PackageDependency { name: dep_node.name.clone(), pkg, kind };
                 packages[source].dependencies.push(dep);
             }
@@ -465,10 +440,7 @@ impl CargoWorkspace {
                     found = true
                 }
                 self[pkg].dependencies.iter().find_map(|dep| {
-                    if &self[dep.pkg].manifest == manifest_path {
-                        return Some(self[pkg].manifest.clone());
-                    }
-                    None
+                    (&self[dep.pkg].manifest == manifest_path).then(|| self[pkg].manifest.clone())
                 })
             })
             .collect::<Vec<ManifestPath>>();
@@ -494,9 +466,12 @@ impl CargoWorkspace {
     }
 }
 
-fn rustc_discover_host_triple(cargo_toml: &ManifestPath, config: &CargoConfig) -> Option<String> {
+fn rustc_discover_host_triple(
+    cargo_toml: &ManifestPath,
+    extra_env: &FxHashMap<String, String>,
+) -> Option<String> {
     let mut rustc = Command::new(toolchain::rustc());
-    rustc.envs(&config.extra_env);
+    rustc.envs(extra_env);
     rustc.current_dir(cargo_toml.parent()).arg("-vV");
     tracing::debug!("Discovering host platform by {:?}", rustc);
     match utf8_stdout(rustc) {
@@ -518,9 +493,12 @@ fn rustc_discover_host_triple(cargo_toml: &ManifestPath, config: &CargoConfig) -
     }
 }
 
-fn cargo_config_build_target(cargo_toml: &ManifestPath, config: &CargoConfig) -> Option<String> {
+fn cargo_config_build_target(
+    cargo_toml: &ManifestPath,
+    extra_env: &FxHashMap<String, String>,
+) -> Option<String> {
     let mut cargo_config = Command::new(toolchain::cargo());
-    cargo_config.envs(&config.extra_env);
+    cargo_config.envs(extra_env);
     cargo_config
         .current_dir(cargo_toml.parent())
         .args(&["-Z", "unstable-options", "config", "get", "build.target"])

crates/project-model/src/project_json.rs

@@ -110,14 +110,17 @@ impl ProjectJson {
                 .collect::<Vec<_>>(),
         }
     }
+
     /// Returns the number of crates in the project.
     pub fn n_crates(&self) -> usize {
         self.crates.len()
     }
+
     /// Returns an iterator over the crates in the project.
     pub fn crates(&self) -> impl Iterator<Item = (CrateId, &Crate)> + '_ {
         self.crates.iter().enumerate().map(|(idx, krate)| (CrateId(idx as u32), krate))
     }
+
     /// Returns the path to the project's root folder.
     pub fn path(&self) -> &AbsPath {
         &self.project_root

crates/project-model/src/rustc_cfg.rs

@@ -3,13 +3,14 @@
 use std::process::Command;
 
 use anyhow::Result;
+use rustc_hash::FxHashMap;
 
-use crate::{cfg_flag::CfgFlag, utf8_stdout, CargoConfig, ManifestPath};
+use crate::{cfg_flag::CfgFlag, utf8_stdout, ManifestPath};
 
 pub(crate) fn get(
     cargo_toml: Option<&ManifestPath>,
     target: Option<&str>,
-    config: &CargoConfig,
+    extra_env: &FxHashMap<String, String>,
 ) -> Vec<CfgFlag> {
     let _p = profile::span("rustc_cfg::get");
     let mut res = Vec::with_capacity(6 * 2 + 1);
@@ -22,7 +23,7 @@ pub(crate) fn get(
         }
     }
 
-    match get_rust_cfgs(cargo_toml, target, config) {
+    match get_rust_cfgs(cargo_toml, target, extra_env) {
         Ok(rustc_cfgs) => {
             tracing::debug!(
                 "rustc cfgs found: {:?}",
@@ -42,11 +43,11 @@ pub(crate) fn get(
 fn get_rust_cfgs(
     cargo_toml: Option<&ManifestPath>,
     target: Option<&str>,
-    config: &CargoConfig,
+    extra_env: &FxHashMap<String, String>,
 ) -> Result<String> {
     if let Some(cargo_toml) = cargo_toml {
         let mut cargo_config = Command::new(toolchain::cargo());
-        cargo_config.envs(&config.extra_env);
+        cargo_config.envs(extra_env);
         cargo_config
             .current_dir(cargo_toml.parent())
             .args(&["-Z", "unstable-options", "rustc", "--print", "cfg"])
@@ -61,7 +62,7 @@ fn get_rust_cfgs(
     }
     // using unstable cargo features failed, fall back to using plain rustc
     let mut cmd = Command::new(toolchain::rustc());
-    cmd.envs(&config.extra_env);
+    cmd.envs(extra_env);
     cmd.args(&["--print", "cfg", "-O"]);
     if let Some(target) = target {
         cmd.args(&["--target", target]);

crates/project-model/src/sysroot.rs

@@ -9,8 +9,9 @@ use std::{env, fs, iter, ops, path::PathBuf, process::Command};
 use anyhow::{format_err, Result};
 use la_arena::{Arena, Idx};
 use paths::{AbsPath, AbsPathBuf};
+use rustc_hash::FxHashMap;
 
-use crate::{utf8_stdout, CargoConfig, ManifestPath};
+use crate::{utf8_stdout, ManifestPath};
 
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct Sysroot {
@@ -67,18 +68,21 @@ impl Sysroot {
         self.crates.iter().map(|(id, _data)| id)
     }
 
-    pub fn discover(dir: &AbsPath, config: &CargoConfig) -> Result<Sysroot> {
+    pub fn discover(dir: &AbsPath, extra_env: &FxHashMap<String, String>) -> Result<Sysroot> {
         tracing::debug!("Discovering sysroot for {}", dir.display());
-        let sysroot_dir = discover_sysroot_dir(dir, config)?;
-        let sysroot_src_dir = discover_sysroot_src_dir(&sysroot_dir, dir, config)?;
+        let sysroot_dir = discover_sysroot_dir(dir, extra_env)?;
+        let sysroot_src_dir = discover_sysroot_src_dir(&sysroot_dir, dir, extra_env)?;
         let res = Sysroot::load(sysroot_dir, sysroot_src_dir)?;
         Ok(res)
     }
 
-    pub fn discover_rustc(cargo_toml: &ManifestPath, config: &CargoConfig) -> Option<ManifestPath> {
+    pub fn discover_rustc(
+        cargo_toml: &ManifestPath,
+        extra_env: &FxHashMap<String, String>,
+    ) -> Option<ManifestPath> {
         tracing::debug!("Discovering rustc source for {}", cargo_toml.display());
         let current_dir = cargo_toml.parent();
-        discover_sysroot_dir(current_dir, config)
+        discover_sysroot_dir(current_dir, extra_env)
             .ok()
             .and_then(|sysroot_dir| get_rustc_src(&sysroot_dir))
     }
@@ -146,9 +150,12 @@ impl Sysroot {
     }
 }
 
-fn discover_sysroot_dir(current_dir: &AbsPath, config: &CargoConfig) -> Result<AbsPathBuf> {
+fn discover_sysroot_dir(
+    current_dir: &AbsPath,
+    extra_env: &FxHashMap<String, String>,
+) -> Result<AbsPathBuf> {
     let mut rustc = Command::new(toolchain::rustc());
-    rustc.envs(&config.extra_env);
+    rustc.envs(extra_env);
     rustc.current_dir(current_dir).args(&["--print", "sysroot"]);
     tracing::debug!("Discovering sysroot by {:?}", rustc);
     let stdout = utf8_stdout(rustc)?;
@@ -158,7 +165,7 @@ fn discover_sysroot_dir(current_dir: &AbsPath, config: &CargoConfig) -> Result<A
 fn discover_sysroot_src_dir(
     sysroot_path: &AbsPathBuf,
     current_dir: &AbsPath,
-    config: &CargoConfig,
+    extra_env: &FxHashMap<String, String>,
 ) -> Result<AbsPathBuf> {
     if let Ok(path) = env::var("RUST_SRC_PATH") {
         let path = AbsPathBuf::try_from(path.as_str())
@@ -174,7 +181,7 @@ fn discover_sysroot_src_dir(
     get_rust_src(sysroot_path)
         .or_else(|| {
             let mut rustup = Command::new(toolchain::rustup());
-            rustup.envs(&config.extra_env);
+            rustup.envs(extra_env);
             rustup.current_dir(current_dir).args(&["component", "add", "rust-src"]);
             utf8_stdout(rustup).ok()?;
             get_rust_src(sysroot_path)

crates/project-model/src/tests.rs

@@ -10,8 +10,8 @@ use paths::{AbsPath, AbsPathBuf};
 use serde::de::DeserializeOwned;
 
 use crate::{
-    CargoConfig, CargoWorkspace, CfgOverrides, ProjectJson, ProjectJsonData, ProjectWorkspace,
-    Sysroot, WorkspaceBuildScripts,
+    CargoWorkspace, CfgOverrides, ProjectJson, ProjectJsonData, ProjectWorkspace, Sysroot,
+    WorkspaceBuildScripts,
 };
 
 fn load_cargo(file: &str) -> CrateGraph {
@@ -101,7 +101,7 @@ fn to_crate_graph(project_workspace: ProjectWorkspace) -> CrateGraph {
                 Some(FileId(counter))
             }
         },
-        &CargoConfig::default(),
+        &Default::default(),
     )
 }
 

crates/project-model/src/workspace.rs

@@ -156,7 +156,11 @@ impl ProjectWorkspace {
                 })?;
                 let project_location = project_json.parent().to_path_buf();
                 let project_json = ProjectJson::new(&project_location, data);
-                ProjectWorkspace::load_inline(project_json, config.target.as_deref(), config)?
+                ProjectWorkspace::load_inline(
+                    project_json,
+                    config.target.as_deref(),
+                    &config.extra_env,
+                )?
             }
             ProjectManifest::CargoToml(cargo_toml) => {
                 let cargo_version = utf8_stdout({
@@ -187,17 +191,21 @@ impl ProjectWorkspace {
                 let sysroot = if config.no_sysroot {
                     None
                 } else {
-                    Some(Sysroot::discover(cargo_toml.parent(), config).with_context(|| {
-                        format!(
+                    Some(Sysroot::discover(cargo_toml.parent(), &config.extra_env).with_context(
+                        || {
+                            format!(
                             "Failed to find sysroot for Cargo.toml file {}. Is rust-src installed?",
                             cargo_toml.display()
                         )
-                    })?)
+                        },
+                    )?)
                 };
 
                 let rustc_dir = match &config.rustc_source {
                     Some(RustcSource::Path(path)) => ManifestPath::try_from(path.clone()).ok(),
-                    Some(RustcSource::Discover) => Sysroot::discover_rustc(&cargo_toml, config),
+                    Some(RustcSource::Discover) => {
+                        Sysroot::discover_rustc(&cargo_toml, &config.extra_env)
+                    }
                     None => None,
                 };
 
@@ -217,7 +225,8 @@ impl ProjectWorkspace {
                     None => None,
                 };
 
-                let rustc_cfg = rustc_cfg::get(Some(&cargo_toml), config.target.as_deref(), config);
+                let rustc_cfg =
+                    rustc_cfg::get(Some(&cargo_toml), config.target.as_deref(), &config.extra_env);
 
                 let cfg_overrides = config.cfg_overrides();
                 ProjectWorkspace::Cargo {
@@ -238,7 +247,7 @@ impl ProjectWorkspace {
     pub fn load_inline(
         project_json: ProjectJson,
         target: Option<&str>,
-        config: &CargoConfig,
+        extra_env: &FxHashMap<String, String>,
     ) -> Result<ProjectWorkspace> {
         let sysroot = match (project_json.sysroot.clone(), project_json.sysroot_src.clone()) {
             (Some(sysroot), Some(sysroot_src)) => Some(Sysroot::load(sysroot, sysroot_src)?),
@@ -260,7 +269,7 @@ impl ProjectWorkspace {
             (None, None) => None,
         };
 
-        let rustc_cfg = rustc_cfg::get(None, target, config);
+        let rustc_cfg = rustc_cfg::get(None, target, extra_env);
         Ok(ProjectWorkspace::Json { project: project_json, sysroot, rustc_cfg })
     }
 
@@ -270,9 +279,9 @@ impl ProjectWorkspace {
                 .first()
                 .and_then(|it| it.parent())
                 .ok_or_else(|| format_err!("No detached files to load"))?,
-            &CargoConfig::default(),
+            &Default::default(),
         )?;
-        let rustc_cfg = rustc_cfg::get(None, None, &CargoConfig::default());
+        let rustc_cfg = rustc_cfg::get(None, None, &Default::default());
         Ok(ProjectWorkspace::DetachedFiles { files: detached_files, sysroot, rustc_cfg })
     }
 
@@ -419,7 +428,7 @@ impl ProjectWorkspace {
         &self,
         load_proc_macro: &mut dyn FnMut(&str, &AbsPath) -> ProcMacroLoadResult,
         load: &mut dyn FnMut(&AbsPath) -> Option<FileId>,
-        config: &CargoConfig,
+        extra_env: &FxHashMap<String, String>,
     ) -> CrateGraph {
         let _p = profile::span("ProjectWorkspace::to_crate_graph");
 
@@ -430,7 +439,7 @@ impl ProjectWorkspace {
                 load,
                 project,
                 sysroot,
-                config,
+                extra_env,
             ),
             ProjectWorkspace::Cargo {
                 cargo,
@@ -469,7 +478,7 @@ fn project_json_to_crate_graph(
     load: &mut dyn FnMut(&AbsPath) -> Option<FileId>,
     project: &ProjectJson,
     sysroot: &Option<Sysroot>,
-    config: &CargoConfig,
+    extra_env: &FxHashMap<String, String>,
 ) -> CrateGraph {
     let mut crate_graph = CrateGraph::default();
     let sysroot_deps = sysroot
@@ -497,7 +506,7 @@ fn project_json_to_crate_graph(
             let target_cfgs = match krate.target.as_deref() {
                 Some(target) => cfg_cache
                     .entry(target)
-                    .or_insert_with(|| rustc_cfg::get(None, Some(target), config)),
+                    .or_insert_with(|| rustc_cfg::get(None, Some(target), extra_env)),
                 None => &rustc_cfg,
             };