Auto merge of #13984 - Veykril:target-data-layout, r=Veykril

bors · bors · commit 56fb0cad6da9 · 2023-01-19T20:45:29.000Z
fix: Fix target-data-layout fetching incorrectly passing 'rustc' to rustc
diff --git a/crates/base-db/src/fixture.rs b/crates/base-db/src/fixture.rs
@@ -163,7 +163,10 @@ impl ChangeFixture {
                     Ok(Vec::new()),
                     false,
                     origin,
-                    meta.target_data_layout.as_deref().map(Arc::from),
+                    meta.target_data_layout
+                        .as_deref()
+                        .map(Arc::from)
+                        .ok_or_else(|| "target_data_layout unset".into()),
                 );
                 let prev = crates.insert(crate_name.clone(), crate_id);
                 assert!(prev.is_none());
@@ -200,7 +203,9 @@ impl ChangeFixture {
                 Ok(Vec::new()),
                 false,
                 CrateOrigin::CratesIo { repo: None, name: None },
-                default_target_data_layout.map(|x| x.into()),
+                default_target_data_layout
+                    .map(|x| x.into())
+                    .ok_or_else(|| "target_data_layout unset".into()),
             );
         } else {
             for (from, to, prelude) in crate_deps {
@@ -214,8 +219,10 @@ impl ChangeFixture {
                     .unwrap();
             }
         }
-        let target_layout =
-            crate_graph.iter().next().and_then(|it| crate_graph[it].target_layout.clone());
+        let target_layout = crate_graph.iter().next().map_or_else(
+            || Err("target_data_layout unset".into()),
+            |it| crate_graph[it].target_layout.clone(),
+        );
 
         if let Some(mini_core) = mini_core {
             let core_file = file_id;
diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs
@@ -243,6 +243,7 @@ pub enum ProcMacroExpansionError {
 }
 
 pub type ProcMacroLoadResult = Result<Vec<ProcMacro>, String>;
+pub type TargetLayoutLoadResult = Result<Arc<str>, Arc<str>>;
 
 #[derive(Debug, Clone)]
 pub struct ProcMacro {
@@ -265,7 +266,7 @@ pub struct CrateData {
     pub display_name: Option<CrateDisplayName>,
     pub cfg_options: CfgOptions,
     pub potential_cfg_options: CfgOptions,
-    pub target_layout: Option<Arc<str>>,
+    pub target_layout: TargetLayoutLoadResult,
     pub env: Env,
     pub dependencies: Vec<Dependency>,
     pub proc_macro: ProcMacroLoadResult,
@@ -324,7 +325,7 @@ impl CrateGraph {
         proc_macro: ProcMacroLoadResult,
         is_proc_macro: bool,
         origin: CrateOrigin,
-        target_layout: Option<Arc<str>>,
+        target_layout: Result<Arc<str>, Arc<str>>,
     ) -> CrateId {
         let data = CrateData {
             root_file_id,
@@ -647,7 +648,7 @@ mod tests {
             Ok(Vec::new()),
             false,
             CrateOrigin::CratesIo { repo: None, name: None },
-            None,
+            Err("".into()),
         );
         let crate2 = graph.add_crate_root(
             FileId(2u32),
@@ -660,7 +661,7 @@ mod tests {
             Ok(Vec::new()),
             false,
             CrateOrigin::CratesIo { repo: None, name: None },
-            None,
+            Err("".into()),
         );
         let crate3 = graph.add_crate_root(
             FileId(3u32),
@@ -673,7 +674,7 @@ mod tests {
             Ok(Vec::new()),
             false,
             CrateOrigin::CratesIo { repo: None, name: None },
-            None,
+            Err("".into()),
         );
         assert!(graph
             .add_dep(crate1, Dependency::new(CrateName::new("crate2").unwrap(), crate2))
@@ -700,7 +701,7 @@ mod tests {
             Ok(Vec::new()),
             false,
             CrateOrigin::CratesIo { repo: None, name: None },
-            None,
+            Err("".into()),
         );
         let crate2 = graph.add_crate_root(
             FileId(2u32),
@@ -713,7 +714,7 @@ mod tests {
             Ok(Vec::new()),
             false,
             CrateOrigin::CratesIo { repo: None, name: None },
-            None,
+            Err("".into()),
         );
         assert!(graph
             .add_dep(crate1, Dependency::new(CrateName::new("crate2").unwrap(), crate2))
@@ -737,7 +738,7 @@ mod tests {
             Ok(Vec::new()),
             false,
             CrateOrigin::CratesIo { repo: None, name: None },
-            None,
+            Err("".into()),
         );
         let crate2 = graph.add_crate_root(
             FileId(2u32),
@@ -750,7 +751,7 @@ mod tests {
             Ok(Vec::new()),
             false,
             CrateOrigin::CratesIo { repo: None, name: None },
-            None,
+            Err("".into()),
         );
         let crate3 = graph.add_crate_root(
             FileId(3u32),
@@ -763,7 +764,7 @@ mod tests {
             Ok(Vec::new()),
             false,
             CrateOrigin::CratesIo { repo: None, name: None },
-            None,
+            Err("".into()),
         );
         assert!(graph
             .add_dep(crate1, Dependency::new(CrateName::new("crate2").unwrap(), crate2))
@@ -787,7 +788,7 @@ mod tests {
             Ok(Vec::new()),
             false,
             CrateOrigin::CratesIo { repo: None, name: None },
-            None,
+            Err("".into()),
         );
         let crate2 = graph.add_crate_root(
             FileId(2u32),
@@ -800,7 +801,7 @@ mod tests {
             Ok(Vec::new()),
             false,
             CrateOrigin::CratesIo { repo: None, name: None },
-            None,
+            Err("".into()),
         );
         assert!(graph
             .add_dep(
diff --git a/crates/base-db/src/lib.rs b/crates/base-db/src/lib.rs
@@ -17,6 +17,7 @@ pub use crate::{
         CrateData, CrateDisplayName, CrateGraph, CrateId, CrateName, CrateOrigin, Dependency,
         Edition, Env, LangCrateOrigin, ProcMacro, ProcMacroExpander, ProcMacroExpansionError,
         ProcMacroId, ProcMacroKind, ProcMacroLoadResult, SourceRoot, SourceRootId,
+        TargetLayoutLoadResult,
     },
 };
 pub use salsa::{self, Cancelled};
diff --git a/crates/hir-ty/src/layout/target.rs b/crates/hir-ty/src/layout/target.rs
@@ -12,6 +12,13 @@ pub fn target_data_layout_query(
     krate: CrateId,
 ) -> Option<Arc<TargetDataLayout>> {
     let crate_graph = db.crate_graph();
-    let target_layout = crate_graph[krate].target_layout.as_ref()?;
-    Some(Arc::new(TargetDataLayout::parse_from_llvm_datalayout_string(&target_layout).ok()?))
+    let target_layout = crate_graph[krate].target_layout.as_ref().ok()?;
+    let res = TargetDataLayout::parse_from_llvm_datalayout_string(&target_layout);
+    if let Err(_e) = &res {
+        // FIXME: Print the error here once it implements debug/display
+        // also logging here is somewhat wrong, but unfortunately this is the earliest place we can
+        // parse that doesn't impose a dependency to the rust-abi crate for project-model
+        tracing::error!("Failed to parse target data layout for {krate:?}");
+    }
+    res.ok().map(Arc::new)
 }
diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs
@@ -237,7 +237,7 @@ impl Analysis {
             Ok(Vec::new()),
             false,
             CrateOrigin::CratesIo { repo: None, name: None },
-            None,
+            Err("Analysis::from_single_file has no target layout".into()),
         );
         change.change_file(file_id, Some(Arc::new(text)));
         change.set_crate_graph(crate_graph);
diff --git a/crates/project-model/src/target_data_layout.rs b/crates/project-model/src/target_data_layout.rs
@@ -1,6 +1,7 @@
 //! Runs `rustc --print target-spec-json` to get the target_data_layout.
 use std::process::Command;
 
+use anyhow::Result;
 use rustc_hash::FxHashMap;
 
 use crate::{utf8_stdout, ManifestPath};
@@ -9,7 +10,7 @@ pub(super) fn get(
     cargo_toml: Option<&ManifestPath>,
     target: Option<&str>,
     extra_env: &FxHashMap<String, String>,
-) -> Option<String> {
+) -> Result<String> {
     let output = (|| {
         if let Some(cargo_toml) = cargo_toml {
             let mut cmd = Command::new(toolchain::rustc());
@@ -28,13 +29,13 @@ pub(super) fn get(
         // using unstable cargo features failed, fall back to using plain rustc
         let mut cmd = Command::new(toolchain::rustc());
         cmd.envs(extra_env)
-            .args(["-Z", "unstable-options", "rustc", "--print", "target-spec-json"])
+            .args(["-Z", "unstable-options", "--print", "target-spec-json"])
             .env("RUSTC_BOOTSTRAP", "1");
         if let Some(target) = target {
             cmd.args(["--target", target]);
         }
         utf8_stdout(cmd)
-    })()
-    .ok()?;
-    Some(output.split_once(r#""data-layout": ""#)?.1.split_once('"')?.0.to_owned())
+    })()?;
+    (|| Some(output.split_once(r#""data-layout": ""#)?.1.split_once('"')?.0.to_owned()))()
+        .ok_or_else(|| anyhow::format_err!("could not fetch target-spec-json from command output"))
 }
diff --git a/crates/project-model/src/tests.rs b/crates/project-model/src/tests.rs
diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs
