[target-spec] make TargetSpec::eval no longer return errors

Now that all checking happens at parse time, this can be done.

target-spec/src/evaluator.rs

@@ -11,21 +11,19 @@ use std::{error, fmt};
 
 /// An error that occurred during target evaluation.
 #[derive(PartialEq)]
+#[non_exhaustive]
 pub enum EvalError {
     /// An invalid target spec was specified.
     InvalidSpec(ParseError),
     /// The platform was not found in the database.
     PlatformNotFound,
-    /// The target family wasn't recognized.
-    UnknownOption(String),
 }
 
 impl fmt::Display for EvalError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
             EvalError::InvalidSpec(_) => write!(f, "invalid target spec"),
             EvalError::PlatformNotFound => write!(f, "platform not found in database"),
-            EvalError::UnknownOption(ref opt) => write!(f, "target family not recognized: {}", opt),
         }
     }
 }
@@ -40,7 +38,7 @@ impl error::Error for EvalError {
     fn source(&self) -> Option<&(dyn error::Error + 'static)> {
         match self {
             EvalError::InvalidSpec(err) => Some(err),
-            EvalError::PlatformNotFound | EvalError::UnknownOption(_) => None,
+            EvalError::PlatformNotFound => None,
         }
     }
 }
@@ -60,25 +58,19 @@ pub fn eval(spec_or_triple: &str, platform: &str) -> Result<Option<bool>, EvalEr
         .map_err(EvalError::InvalidSpec)?;
     match Platform::new(platform, TargetFeatures::Unknown) {
         None => Err(EvalError::PlatformNotFound),
-        Some(platform) => target_spec.eval(&platform),
+        Some(platform) => Ok(target_spec.eval(&platform)),
     }
 }
 
-pub(crate) fn eval_target(
-    target: &Target,
-    platform: &Platform<'_>,
-) -> Result<Option<bool>, EvalError> {
+pub(crate) fn eval_target(target: &Target, platform: &Platform<'_>) -> Option<bool> {
     match target {
-        Target::TargetInfo(ref target_info) => Ok(Some(platform.triple() == target_info.triple)),
+        Target::TargetInfo(ref target_info) => Some(platform.triple() == target_info.triple),
         Target::Spec(ref expr) => eval_expr(expr, platform),
     }
 }
 
-fn eval_expr(spec: &Arc<Expression>, platform: &Platform<'_>) -> Result<Option<bool>, EvalError> {
-    // Expression::eval doesn't support returning errors, so have an Option at the top to set errors
-    // into.
-    let mut err = None;
-    let res: Option<bool> = spec.eval(|pred| {
+fn eval_expr(spec: &Arc<Expression>, platform: &Platform<'_>) -> Option<bool> {
+    spec.eval(|pred| {
         match pred {
             Predicate::Target(target) => Some(target.matches(platform.target_info())),
             Predicate::TargetFeature(feature) => platform.target_features().matches(feature),
@@ -92,23 +84,11 @@ fn eval_expr(spec: &Arc<Expression>, platform: &Platform<'_>) -> Result<Option<b
                 // https://github.com/rust-lang/cargo/issues/7442 for more details.
                 Some(false)
             }
-            Predicate::KeyValue { key, .. } => {
-                err.replace(EvalError::UnknownOption((*key).to_string()));
-                Some(false)
-            }
-            Predicate::Flag(other) => {
-                // cfg_expr turns "windows" and "unix" into target families, so they don't need to
-                // be handled explicitly.
-                err.replace(EvalError::UnknownOption((*other).to_string()));
-                Some(false)
+            Predicate::KeyValue { .. } | Predicate::Flag(_) => {
+                unreachable!("these predicates are disallowed at TargetSpec construction time")
             }
         }
-    });
-
-    match err {
-        Some(err) => Err(err),
-        None => Ok(res),
-    }
+    })
 }
 
 #[cfg(test)]
@@ -215,7 +195,7 @@ mod tests {
             Ok(None),
         );
 
-        fn eval_ext(spec: &str, platform: &str) -> Result<Option<bool>, EvalError> {
+        fn eval_ext(spec: &str, platform: &str) -> Option<bool> {
             let platform = Platform::new(platform, TargetFeatures::features(&["sse", "sse2"]))
                 .expect("platform should be found");
             let spec: TargetSpec = spec.parse().unwrap();
@@ -224,25 +204,25 @@ mod tests {
 
         assert_eq!(
             eval_ext("cfg(target_feature = \"sse\")", "x86_64-unknown-linux-gnu"),
-            Ok(Some(true)),
+            Some(true),
         );
         assert_eq!(
             eval_ext(
                 "cfg(not(target_feature = \"sse\"))",
                 "x86_64-unknown-linux-gnu",
             ),
-            Ok(Some(false)),
+            Some(false),
         );
         assert_eq!(
             eval_ext("cfg(target_feature = \"fxsr\")", "x86_64-unknown-linux-gnu"),
-            Ok(Some(false)),
+            Some(false),
         );
         assert_eq!(
             eval_ext(
                 "cfg(not(target_feature = \"fxsr\"))",
                 "x86_64-unknown-linux-gnu",
             ),
-            Ok(Some(true)),
+            Some(true),
         );
     }
 }

target-spec/src/parser.rs

@@ -1,7 +1,7 @@
 // Copyright (c) The cargo-guppy Contributors
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
-use crate::{eval_target, EvalError, Platform};
+use crate::{eval_target, Platform};
 use cfg_expr::targets::{get_target_by_triple, TargetInfo};
 use cfg_expr::{Expression, Predicate};
 use std::str::FromStr;
@@ -22,25 +22,27 @@ use std::{error, fmt};
 /// let i686_linux = Platform::new("i686-unknown-linux-gnu", TargetFeatures::features(&["sse2"])).unwrap();
 ///
 /// let spec: TargetSpec = "cfg(any(windows, target_arch = \"x86_64\"))".parse().unwrap();
-/// assert_eq!(spec.eval(&i686_windows).unwrap(), Some(true), "i686 Windows");
-/// assert_eq!(spec.eval(&x86_64_mac).unwrap(), Some(true), "x86_64 MacOS");
-/// assert_eq!(spec.eval(&i686_linux).unwrap(), Some(false), "i686 Linux (should not match)");
+/// assert_eq!(spec.eval(&i686_windows), Some(true), "i686 Windows");
+/// assert_eq!(spec.eval(&x86_64_mac), Some(true), "x86_64 MacOS");
+/// assert_eq!(spec.eval(&i686_linux), Some(false), "i686 Linux (should not match)");
 ///
 /// let spec: TargetSpec = "cfg(any(target_feature = \"sse2\", target_feature = \"sse\"))".parse().unwrap();
-/// assert_eq!(spec.eval(&i686_windows).unwrap(), None, "i686 Windows features are unknown");
-/// assert_eq!(spec.eval(&x86_64_mac).unwrap(), Some(false), "x86_64 MacOS matches no features");
-/// assert_eq!(spec.eval(&i686_linux).unwrap(), Some(true), "i686 Linux matches some features");
+/// assert_eq!(spec.eval(&i686_windows), None, "i686 Windows features are unknown");
+/// assert_eq!(spec.eval(&x86_64_mac), Some(false), "x86_64 MacOS matches no features");
+/// assert_eq!(spec.eval(&i686_linux), Some(true), "i686 Linux matches some features");
 /// ```
 #[derive(Clone, Debug)]
 pub struct TargetSpec {
     target: Target,
 }
 
 impl TargetSpec {
-    /// Evaluates this specification against the given platform triple, defaulting to accepting all
-    /// target features.
+    /// Evaluates this specification against the given platform triple.
+    ///
+    /// Returns `Some(true)` if there's a match, `Some(false)` if there's none, or `None` if the
+    /// result of the evaluation is unknown (typically found if target families are involved).
     #[inline]
-    pub fn eval(&self, platform: &Platform<'_>) -> Result<Option<bool>, EvalError> {
+    pub fn eval(&self, platform: &Platform<'_>) -> Option<bool> {
         eval_target(&self.target, platform)
     }
 }