target enum: propagate parsing error to build() to reduce breakage

Author: Firestar99

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

@@ -12,7 +12,9 @@ pub const TARGET_SPEC_DIR_PATH: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tar
 #[cfg(feature = "include_str")]
 mod include_str;
 #[cfg(feature = "serde")]
-mod serde;
+mod serde_feature;
+#[cfg(feature = "serde")]
+pub use serde_feature::*;
 
 pub const SPIRV_ARCH: &str = "spirv";
 pub const SPIRV_VENDOR: &str = "unknown";
@@ -60,7 +62,8 @@ pub enum SpirvTargetEnv {
     Vulkan_1_4,
 }
 
-#[derive(Error)]
+#[derive(Clone, Error)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum SpirvTargetParseError {
     #[error("Expected `rustc_codegen_spirv` target with prefix `{SPIRV_TARGET_PREFIX}`, got `{0}`")]
     WrongPrefix(String),
@@ -125,6 +128,12 @@ impl IntoSpirvTarget for &str {
     }
 }
 
+impl IntoSpirvTarget for String {
+    fn to_spirv_target_env(&self) -> Result<SpirvTargetEnv, SpirvTargetParseError> {
+        SpirvTargetEnv::parse_triple(self)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/rustc_codegen_spirv-target-specs/src/serde.rs renamed to crates/rustc_codegen_spirv-target-specs/src/serde_feature.rs

@@ -1,4 +1,5 @@
-use crate::SpirvTargetEnv;
+use crate::{SpirvTargetEnv, SpirvTargetParseError};
+use serde::ser::Error;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
 impl Serialize for SpirvTargetEnv {
@@ -20,6 +21,34 @@ impl<'de> Deserialize<'de> for SpirvTargetEnv {
     }
 }
 
+pub fn serialize_target<S>(
+    target: &Option<Result<SpirvTargetEnv, SpirvTargetParseError>>,
+    serializer: S,
+) -> Result<S::Ok, S::Error>
+where
+    S: Serializer,
+{
+    // cannot use `transpose()` due to target being a ref, not a value
+    match target {
+        Some(Ok(_)) | None => Serialize::serialize(target, serializer),
+        Some(Err(_e)) => Err(Error::custom(
+            "cannot serialize `target` that failed to parse",
+        )),
+    }
+}
+
+pub fn deserialize_target<'de, D>(
+    deserializer: D,
+) -> Result<Option<Result<SpirvTargetEnv, SpirvTargetParseError>>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    Ok(Ok(<Option<SpirvTargetEnv> as Deserialize>::deserialize(
+        deserializer,
+    )?)
+    .transpose())
+}
+
 #[cfg(test)]
 mod tests {
     use crate::SpirvTargetEnv;

crates/spirv-builder/src/lib.rs

@@ -93,6 +93,7 @@ use thiserror::Error;
 pub use rustc_codegen_spirv_target_specs::{
     IntoSpirvTarget, SpirvTargetEnv, SpirvTargetParseError,
 };
+pub use rustc_codegen_spirv_target_specs::{deserialize_target, serialize_target};
 pub use rustc_codegen_spirv_types::*;
 
 #[cfg(feature = "include-target-specs")]
@@ -101,8 +102,10 @@ pub use rustc_codegen_spirv_target_specs::TARGET_SPEC_DIR_PATH;
 #[derive(Debug, Error)]
 #[non_exhaustive]
 pub enum SpirvBuilderError {
-    #[error("`target` must be set or was invalid, for example `spirv-unknown-vulkan1.2`")]
+    #[error("`target` must be set, for example `spirv-unknown-vulkan1.2`")]
     MissingTarget,
+    #[error("Error parsing target: {0}")]
+    SpirvTargetParseError(#[from] SpirvTargetParseError),
     #[error("`path_to_crate` must be set")]
     MissingCratePath,
     #[error("crate path '{0}' does not exist")]
@@ -409,10 +412,14 @@ pub struct SpirvBuilder {
         clap(
             long,
             default_value = "spirv-unknown-vulkan1.2",
-            value_parser = SpirvTargetEnv::parse_triple
+            value_parser = Self::parse_target
         )
     )]
-    pub target: Option<SpirvTargetEnv>,
+    #[serde(
+        serialize_with = "serialize_target",
+        deserialize_with = "deserialize_target"
+    )]
+    pub target: Option<Result<SpirvTargetEnv, SpirvTargetParseError>>,
     /// Cargo features specification for building the shader crate.
     #[cfg_attr(feature = "clap", clap(flatten))]
     #[serde(flatten)]
@@ -481,6 +488,12 @@ pub struct SpirvBuilder {
 
 #[cfg(feature = "clap")]
 impl SpirvBuilder {
+    fn parse_target(
+        target: &str,
+    ) -> Result<Result<SpirvTargetEnv, SpirvTargetParseError>, clap::Error> {
+        Ok(SpirvTargetEnv::parse_triple(target))
+    }
+
     /// Clap value parser for `Capability`.
     fn parse_spirv_capability(capability: &str) -> Result<Capability, clap::Error> {
         use core::str::FromStr;
@@ -521,7 +534,7 @@ impl SpirvBuilder {
     pub fn new(path_to_crate: impl AsRef<Path>, target: impl IntoSpirvTarget) -> Self {
         Self {
             path_to_crate: Some(path_to_crate.as_ref().to_owned()),
-            target: target.to_spirv_target_env().ok(),
+            target: Some(target.to_spirv_target_env()),
             ..SpirvBuilder::default()
         }
     }
@@ -788,7 +801,10 @@ fn join_checking_for_separators(strings: Vec<impl Borrow<str>>, sep: &str) -> St
 
 // Returns path to the metadata json.
 fn invoke_rustc(builder: &SpirvBuilder) -> Result<PathBuf, SpirvBuilderError> {
-    let target = builder.target.ok_or(SpirvBuilderError::MissingTarget)?;
+    let target = builder
+        .target
+        .clone()
+        .ok_or(SpirvBuilderError::MissingTarget)??;
     let path_to_crate = builder
         .path_to_crate
         .as_ref()

crates/spirv-builder/src/tests.rs

@@ -22,6 +22,6 @@ mod clap {
         let builder = SpirvBuilder::try_parse_from(["", "--target", &env.target_triple()])
             .map_err(|e| e.to_string())
             .unwrap();
-        assert_eq!(builder.target, Some(env));
+        assert_eq!(builder.target.unwrap().unwrap(), env);
     }
 }