target enum: make SpirvTargetEnv::parse_triple() return a Result

Author: Firestar99

Cargo.lock

@@ -10,6 +10,9 @@ repository.workspace = true
 [features]
 include_str = []
 dir_path = []
+serde = ["dep:serde"]
 
 [dependencies]
 strum = { version = "0.26.3", features = ["derive"] }
+thiserror = "2.0.12"
+serde = { version = "1.0", features = ["derive"], optional = true }

crates/rustc_codegen_spirv-target-specs/Cargo.toml

@@ -1,6 +1,9 @@
 #![doc = include_str!("../README.md")]
 
+use core::str::FromStr;
+use std::fmt::{Debug, Display, Formatter};
 use strum::{Display, EnumIter, EnumString, IntoStaticStr};
+use thiserror::Error;
 
 /// directory with all the `target-specs` jsons for our codegen backend
 #[cfg(feature = "dir_path")]
@@ -11,10 +14,12 @@ mod include_str;
 
 pub const SPIRV_ARCH: &str = "spirv";
 pub const SPIRV_VENDOR: &str = "unknown";
+pub const SPIRV_TARGET_PREFIX: &str = "spirv-unknown-";
 
-/// All target envs rust-gpu supports
+/// All target envs rust-gpu supports. The corresponding target tripple is `spirv-unknown-ENV` with `ENV` replaced by any of the values below.
 #[allow(non_camel_case_types, clippy::upper_case_acronyms)]
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, EnumString, IntoStaticStr, EnumIter, Display)]
+#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 pub enum SpirvTargetEnv {
     #[strum(to_string = "opengl4.0")]
     OpenGL_4_0,
@@ -54,29 +59,46 @@ pub enum SpirvTargetEnv {
     Vulkan_1_4,
 }
 
+#[derive(Error)]
+pub enum SpirvTargetParseError {
+    #[error("Expected `rustc_codegen_spirv` target with prefix `{SPIRV_TARGET_PREFIX}`, got `{0}`")]
+    WrongPrefix(String),
+    #[error(
+        "Target `{SPIRV_TARGET_PREFIX}{0}` not supported by `rustc_codegen_spirv`, see `enum SpirvTargetEnv` for possible env values"
+    )]
+    UnknownEnv(String),
+}
+
+impl Debug for SpirvTargetParseError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        Display::fmt(self, f)
+    }
+}
+
 impl SpirvTargetEnv {
-    pub fn parse_triple(target: &str) -> Option<Self> {
-        target
-            .strip_prefix(&format!("{SPIRV_ARCH}-{SPIRV_VENDOR}-"))
-            .and_then(|env| TryFrom::try_from(env).ok())
+    pub fn parse_triple(target: &str) -> Result<Self, SpirvTargetParseError> {
+        let env = target
+            .strip_prefix(SPIRV_TARGET_PREFIX)
+            .ok_or_else(|| SpirvTargetParseError::WrongPrefix(target.to_string()))?;
+        FromStr::from_str(env).map_err(|_| SpirvTargetParseError::UnknownEnv(env.to_string()))
     }
 
     pub fn as_str(&self) -> &'static str {
         self.into()
     }
 
     pub fn target_triple(&self) -> String {
-        format!("{SPIRV_ARCH}-{SPIRV_VENDOR}-{}", self.as_str())
+        format!("{SPIRV_TARGET_PREFIX}{}", self.as_str())
     }
 
     pub fn target_json_file_name(&self) -> String {
-        format!("{SPIRV_ARCH}-{SPIRV_VENDOR}-{}.json", self.as_str())
+        format!("{SPIRV_TARGET_PREFIX}{}.json", self.as_str())
     }
 
     #[cfg(feature = "dir_path")]
     pub fn target_json_path(&self) -> String {
         format!(
-            "{TARGET_SPEC_DIR_PATH}/{SPIRV_ARCH}-{SPIRV_VENDOR}-{}.json",
+            "{TARGET_SPEC_DIR_PATH}/{SPIRV_TARGET_PREFIX}{}.json",
             self.as_str()
         )
     }
@@ -91,9 +113,17 @@ mod tests {
     use super::*;
 
     #[test]
-    pub fn test_parse_as_str_loop() {
+    pub fn test_prefix() {
+        assert_eq!(
+            SPIRV_TARGET_PREFIX,
+            &format!("{SPIRV_ARCH}-{SPIRV_VENDOR}-")
+        );
+    }
+
+    #[test]
+    pub fn test_triple_parse_loop() {
         for target in SpirvTargetEnv::iter() {
-            let parsed = SpirvTargetEnv::parse_triple(target.as_str()).unwrap();
+            let parsed = SpirvTargetEnv::parse_triple(&target.target_triple()).unwrap();
             assert_eq!(target, parsed);
         }
     }
@@ -103,7 +133,7 @@ mod tests {
     pub fn test_target_json_path() {
         for target in SpirvTargetEnv::iter() {
             let file = std::path::PathBuf::from(target.target_json_path());
-            assert!(file.is_file());
+            assert!(file.is_file(), "{}", file.display());
         }
     }
 

crates/rustc_codegen_spirv-target-specs/src/lib.rs

@@ -100,16 +100,7 @@ impl<'tcx> CodegenCx<'tcx> {
     pub fn new(tcx: TyCtxt<'tcx>, codegen_unit: &'tcx CodegenUnit<'tcx>) -> Self {
         // Validate the target spec, as the backend doesn't control `--target`.
         let target_tuple = tcx.sess.opts.target_triple.tuple();
-        let target = SpirvTargetEnv::parse_triple(target_tuple).unwrap_or_else(|| {
-            let qualifier = if !target_tuple.starts_with("spirv-") {
-                "non-SPIR-V "
-            } else {
-                ""
-            };
-            tcx.dcx().fatal(format!(
-                "{qualifier}target `{target_tuple}` not supported by `rustc_codegen_spirv`",
-            ))
-        });
+        let target = SpirvTargetEnv::parse_triple(target_tuple).unwrap();
         let target_spec_mismatched_jsons = {
             use rustc_target::json::ToJson;
 

crates/rustc_codegen_spirv/src/codegen_cx/mod.rs

@@ -77,7 +77,6 @@ mod depfile;
 mod watch;
 
 use raw_string::{RawStr, RawString};
-use rustc_codegen_spirv_target_specs::SpirvTargetEnv;
 use semver::Version;
 use serde::Deserialize;
 use std::borrow::Borrow;
@@ -89,6 +88,7 @@ use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 use thiserror::Error;
 
+pub use rustc_codegen_spirv_target_specs::{SpirvTargetEnv, SpirvTargetParseError};
 pub use rustc_codegen_spirv_types::*;
 
 #[cfg(feature = "include-target-specs")]
@@ -99,8 +99,8 @@ pub use rustc_codegen_spirv_target_specs::TARGET_SPEC_DIR_PATH;
 pub enum SpirvBuilderError {
     #[error("`target` must be set, for example `spirv-unknown-vulkan1.2`")]
     MissingTarget,
-    #[error("Unknown target `{target}`")]
-    UnknownTarget { target: String },
+    #[error("Error parsing target: {0}")]
+    SpirvTargetParseError(#[from] SpirvTargetParseError),
     #[error("`path_to_crate` must be set")]
     MissingCratePath,
     #[error("crate path '{0}' does not exist")]
@@ -760,9 +760,7 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result<PathBuf, SpirvBuilderError> {
         .target
         .as_ref()
         .ok_or(SpirvBuilderError::MissingTarget)?;
-    let target = SpirvTargetEnv::parse_triple(target).ok_or(SpirvBuilderError::UnknownTarget {
-        target: target.clone(),
-    })?;
+    let target = SpirvTargetEnv::parse_triple(target)?;
     let path_to_crate = builder
         .path_to_crate
         .as_ref()

crates/spirv-builder/src/lib.rs

@@ -138,7 +138,7 @@ impl Runner {
             } else {
                 SpirvTargetEnv::parse_triple(&format!("spirv-unknown-{target}"))
             }
-            .unwrap_or_else(|| panic!("unknown target {}", target));
+            .unwrap();
 
             // HACK(eddyb) in order to allow *some* tests to have separate output
             // in different testing variations (i.e. experimental features), while