refactor(snix/eval,snix/glue): add snix_eval::try_cek! macros

Fixes #146

Change-Id: I971fac0d9d18e4ea73a527e499ac7ac213658477
Reviewed-on: https://cl.snix.dev/c/snix/+/30638
Reviewed-by: Florian Klink <flokli@flokli.de>
Autosubmit: Axel Karjalainen <axel@axka.fi>
Tested-by: besadii

snix/eval/src/builtins/impure.rs

@@ -16,7 +16,10 @@ mod impure_builtins {
     use std::os::unix::ffi::OsStrExt;
 
     use super::*;
-    use crate::builtins::{coerce_value_to_path, hash::hash_nix_string};
+    use crate::{
+        builtins::{coerce_value_to_path, hash::hash_nix_string},
+        try_cek_to_value,
+    };
 
     #[builtin("getEnv")]
     async fn builtin_get_env(co: GenCo, var: Value) -> Result<Value, ErrorKind> {
@@ -28,27 +31,20 @@ mod impure_builtins {
 
     #[builtin("hashFile")]
     async fn builtin_hash_file(co: GenCo, algo: Value, path: Value) -> Result<Value, ErrorKind> {
-        let path = match coerce_value_to_path(&co, path).await? {
+        let path = try_cek_to_value!(coerce_value_to_path(&co, path).await?);
-            Err(cek) => return Ok(Value::from(cek)),
-            Ok(p) => p,
-        };
         let r = generators::request_open_file(&co, path).await;
         hash_nix_string(algo.to_str()?, r).map(Value::from)
     }
 
     #[builtin("pathExists")]
     async fn builtin_path_exists(co: GenCo, path: Value) -> Result<Value, ErrorKind> {
-        match coerce_value_to_path(&co, path).await? {
+        let path = try_cek_to_value!(coerce_value_to_path(&co, path).await?);
-            Err(cek) => Ok(Value::from(cek)),
+        Ok(generators::request_path_exists(&co, path).await)
-            Ok(path) => Ok(generators::request_path_exists(&co, path).await),
-        }
     }
 
     #[builtin("readDir")]
     async fn builtin_read_dir(co: GenCo, path: Value) -> Result<Value, ErrorKind> {
-        match coerce_value_to_path(&co, path).await? {
+        let path = try_cek_to_value!(coerce_value_to_path(&co, path).await?);
-            Err(cek) => Ok(Value::from(cek)),
-            Ok(path) => {
         let dir = generators::request_read_dir(&co, path).await;
         let res = dir.into_iter().map(|(name, ftype)| {
             (
@@ -62,33 +58,25 @@ mod impure_builtins {
 
         Ok(Value::attrs(NixAttrs::from_iter(res)))
     }
-        }
-    }
 
     #[builtin("readFile")]
     async fn builtin_read_file(co: GenCo, path: Value) -> Result<Value, ErrorKind> {
-        match coerce_value_to_path(&co, path).await? {
+        let path = try_cek_to_value!(coerce_value_to_path(&co, path).await?);
-            Err(cek) => Ok(Value::from(cek)),
-            Ok(path) => {
         let mut buf = Vec::new();
         generators::request_open_file(&co, path)
             .await
             .read_to_end(&mut buf)?;
         Ok(Value::from(buf))
     }
-        }
-    }
 
     #[builtin("readFileType")]
     async fn builtin_read_file_type(co: GenCo, path: Value) -> Result<Value, ErrorKind> {
-        match coerce_value_to_path(&co, path).await? {
+        let path = try_cek_to_value!(coerce_value_to_path(&co, path).await?);
-            Err(cek) => Ok(Value::from(cek)),
+        Ok(Value::from(
-            Ok(path) => Ok(Value::from(
             generators::request_read_file_type(&co, path)
                 .await
                 .to_string(),
-            )),
+        ))
-        }
     }
 }
 

snix/eval/src/builtins/mod.rs

@@ -14,7 +14,6 @@ use std::collections::VecDeque;
 use std::path::PathBuf;
 use std::sync::{Mutex, OnceLock};
 
-use crate::arithmetic_op;
 use crate::value::PointerEquality;
 use crate::vm::generators::{self, GenCo};
 use crate::warnings::WarningKind;
@@ -24,6 +23,7 @@ use crate::{
     errors::{CatchableErrorKind, ErrorKind},
     value::{CoercionKind, NixAttrs, NixList, NixString, Thunk, Value},
 };
+use crate::{arithmetic_op, try_cek};
 
 use self::versions::{VersionPart, VersionPartsIter};
 
@@ -59,7 +59,8 @@ pub async fn coerce_value_to_path(
         return Ok(Ok(*p));
     }
 
-    match generators::request_string_coerce(
+    let vs = try_cek!(
+        generators::request_string_coerce(
             co,
             value,
             CoercionKind {
@@ -68,8 +69,8 @@ pub async fn coerce_value_to_path(
             },
         )
         .await
-    {
+    );
-        Ok(vs) => {
+
     let path = vs.to_path()?.to_owned();
     if path.is_absolute() {
         Ok(Ok(path))
@@ -77,9 +78,6 @@ pub async fn coerce_value_to_path(
         Err(ErrorKind::NotAnAbsolutePath(path))
     }
 }
-        Err(cek) => Ok(Err(cek)),
-    }
-}
 
 static REGEX_CACHE: OnceLock<Mutex<FxHashMap<String, Regex>>> = OnceLock::new();
 
@@ -106,7 +104,9 @@ mod pure_builtins {
     use os_str_bytes::OsStringBytes;
     use rustc_hash::{FxHashMap, FxHashSet};
 
-    use crate::{AddContext, NixContext, NixContextElement, value::PointerEquality};
+    use crate::{
+        AddContext, NixContext, NixContextElement, try_cek_to_value, value::PointerEquality,
+    };
 
     use super::*;
 
@@ -302,7 +302,8 @@ mod pure_builtins {
             if i != 0 {
                 res.push_str(&separator);
             }
-            match generators::request_string_coerce(
+            let mut s = try_cek_to_value!(
+                generators::request_string_coerce(
                     &co,
                     val,
                     CoercionKind {
@@ -311,16 +312,12 @@ mod pure_builtins {
                     },
                 )
                 .await
-            {
+            );
-                Ok(mut s) => {
             res.push_str(&s);
             if let Some(other_context) = s.take_context() {
                 context.extend(other_context.into_iter());
             }
         }
-                Err(c) => return Ok(Value::Catchable(Box::new(c))),
-            }
-        }
         // FIXME: pass immediately the string res.
         Ok(NixString::new_context_from(context, res).into())
     }
@@ -372,11 +369,11 @@ mod pure_builtins {
     #[builtin("elem")]
     async fn builtin_elem(co: GenCo, x: Value, xs: Value) -> Result<Value, ErrorKind> {
         for val in xs.to_list()? {
-            match generators::check_equality(&co, x.clone(), val, PointerEquality::AllowAll).await?
+            match try_cek_to_value!(
-            {
+                generators::check_equality(&co, x.clone(), val, PointerEquality::AllowAll).await?
-                Ok(true) => return Ok(true.into()),
+            ) {
-                Ok(false) => continue,
+                true => return Ok(true.into()),
-                Err(cek) => return Ok(Value::from(cek)),
+                false => continue,
             }
         }
         Ok(false.into())
@@ -504,13 +501,10 @@ mod pure_builtins {
             let attrs = val.to_attrs()?;
             let key = attrs.select_required("key")?;
 
-            let value_missing = bgc_insert_key(&co, key.clone(), &mut done_keys).await?;
+            let value_missing =
+                try_cek_to_value!(bgc_insert_key(&co, key.clone(), &mut done_keys).await?);
 
-            if let Err(cek) = value_missing {
+            if !value_missing {
-                return Ok(Value::Catchable(Box::new(cek)));
-            }
-
-            if let Ok(false) = value_missing {
                 continue;
             }
 
@@ -909,10 +903,9 @@ mod pure_builtins {
     #[builtin("lessThan")]
     async fn builtin_less_than(co: GenCo, x: Value, y: Value) -> Result<Value, ErrorKind> {
         let span = generators::request_span(&co).await;
-        match x.nix_cmp_ordering(y, co, span).await? {
+        match try_cek_to_value!(x.nix_cmp_ordering(y, co, span).await?) {
-            Err(cek) => Ok(Value::from(cek)),
+            Ordering::Less => Ok(Value::Bool(true)),
-            Ok(Ordering::Less) => Ok(Value::Bool(true)),
+            _ => Ok(Value::Bool(false)),
-            Ok(_) => Ok(Value::Bool(false)),
         }
     }
 
@@ -1474,13 +1467,10 @@ mod pure_builtins {
             return Ok(s);
         }
 
-        match coerce_value_to_path(&co, s).await? {
+        let path = try_cek_to_value!(coerce_value_to_path(&co, s).await?);
-            Err(cek) => Ok(Value::from(cek)),
-            Ok(path) => {
         let path: Value = crate::value::canon_path(path).into();
         let span = generators::request_span(&co).await;
-                Ok(path
+        path.coerce_to_string(
-                    .coerce_to_string(
             co,
             CoercionKind {
                 strong: false,
@@ -1488,9 +1478,7 @@ mod pure_builtins {
             },
             span,
         )
-                    .await?)
+        .await
-            }
-        }
     }
 
     #[builtin("tryEval")]
@@ -1521,7 +1509,8 @@ async fn bgc_insert_key(
     done: &mut Vec<Value>,
 ) -> Result<Result<bool, CatchableErrorKind>, ErrorKind> {
     for existing in done.iter() {
-        match generators::check_equality(
+        if try_cek!(
+            generators::check_equality(
                 co,
                 existing.clone(),
                 key.clone(),
@@ -1529,10 +1518,8 @@ async fn bgc_insert_key(
                 PointerEquality::ForbidAll,
             )
             .await?
-        {
+        ) {
-            Ok(true) => return Ok(Ok(false)),
+            return Ok(Ok(false));
-            Ok(false) => (),
-            Err(cek) => return Ok(Err(cek)),
         }
     }
 

snix/eval/src/compiler/import.rs

@@ -14,6 +14,7 @@ use crate::{
     builtins::coerce_value_to_path,
     generators::pin_generator,
     observer::NoOpObserver,
+    try_cek_to_value,
     value::{Builtin, Thunk},
     vm::generators::{self, GenCo},
 };
@@ -25,10 +26,7 @@ async fn import_impl(
     mut args: Vec<Value>,
 ) -> Result<Value, ErrorKind> {
     // TODO(sterni): canon_path()?
-    let mut path = match coerce_value_to_path(&co, args.pop().unwrap()).await? {
+    let mut path = try_cek_to_value!(coerce_value_to_path(&co, args.pop().unwrap()).await?);
-        Err(cek) => return Ok(Value::Catchable(Box::new(cek))),
-        Ok(path) => path,
-    };
 
     if path.is_dir() {
         path.push("default.nix");

snix/eval/src/errors.rs

@@ -16,6 +16,81 @@ use crate::spans::ToSpan;
 use crate::value::{CoercionKind, NixString};
 use crate::{SourceCode, Value};
 
+/// Propagates a [catchable error](CatchableErrorKind) as `Ok(Err(_))` or returns the unwrapped value in `Ok`.
+///
+/// You should use the try operator (`?`) to propagate uncatchable errors before passing catchable
+/// errors to `try_cek`.
+///
+/// **Input type:** `Result<T, CatchableErrorKind>`
+///
+/// **Output type:** `T`
+///
+/// **Return type of containing function:** `Result<Result<_, CatchableErrorKind>, _>`
+///
+/// # Example
+///
+/// ```
+/// use snix_eval::{try_cek, CatchableErrorKind};
+///
+/// # #[derive(Debug)] struct MyUncatchableError;
+/// # fn example() -> Result<Result<(), CatchableErrorKind>, MyUncatchableError> {
+/// fn my_fn() -> Result<Result<i32, CatchableErrorKind>, MyUncatchableError> {
+///     Ok(Ok(42))
+/// }
+///
+/// let value: i32 = try_cek!(my_fn()?);
+/// assert_eq!(value, 42);
+///
+/// fn my_other_fn() -> Result<Result<i32, CatchableErrorKind>, MyUncatchableError> {
+///     Ok(Err(CatchableErrorKind::AssertionFailed))
+/// }
+///
+/// try_cek!(my_other_fn()?); // results in `return Ok(Err(cek))`
+/// unreachable!();
+///
+/// # Ok(Ok(()))
+/// # }
+/// # fn main() {
+/// #     pretty_assertions::assert_matches!(example().unwrap(), Err(CatchableErrorKind::AssertionFailed));
+/// # }
+/// ```
+#[macro_export]
+macro_rules! try_cek {
+    ($result:expr) => {
+        match $result {
+            Ok(s) => s,
+            Err(cek) => {
+                // Type-check to avoid accidental misuse
+                let cek: $crate::CatchableErrorKind = cek;
+                return Ok(Err(cek));
+            }
+        }
+    };
+}
+
+/// Propagates a [catchable error](CatchableErrorKind) as `Ok(Value::Catchable(_))` or returns the unwrapped value in `Ok`.
+///
+/// **Input type:** `Result<T, CatchableErrorKind>`
+///
+/// **Output type:** `T`
+///
+/// **Return type of containing function:** `Result<Value, _>`
+///
+/// See [`try_cek!`]'s documentation for more.
+#[macro_export]
+macro_rules! try_cek_to_value {
+    ($result:expr) => {
+        match $result {
+            Ok(s) => s,
+            Err(cek) => {
+                // Type-check to avoid accidental misuse
+                let cek: $crate::CatchableErrorKind = cek;
+                return Ok(Value::Catchable(Box::new(cek)));
+            }
+        }
+    };
+}
+
 /// "CatchableErrorKind" errors -- those which can be detected by
 /// `builtins.tryEval`.
 ///

snix/glue/src/builtins/derivation.rs

@@ -176,7 +176,7 @@ pub(crate) mod derivation_builtins {
 
     use nix_compat::store_path::hash_placeholder;
     use snix_eval::generators::Gen;
-    use snix_eval::{NixContext, NixContextElement, NixString};
+    use snix_eval::{NixContext, NixContextElement, NixString, try_cek_to_value};
 
     use crate::builtins::utils::{select_string, strong_importing_coerce_to_string};
     use crate::fetchurl::fetchurl_derivation_to_fetch;
@@ -279,15 +279,12 @@ pub(crate) mod derivation_builtins {
                 // These are only set in drv.arguments.
                 "args" => {
                     for arg in value.to_list()? {
-                        match strong_importing_coerce_to_string(&co, arg).await {
+                        let s =
-                            Err(cek) => return Ok(Value::from(cek)),
+                            try_cek_to_value!(strong_importing_coerce_to_string(&co, arg).await);
-                            Ok(s) => {
                         input_context.mimic(&s);
                         drv.arguments.push(s.to_str()?.to_owned())
                     }
                 }
-                    }
-                }
 
                 // If outputs is set, remove the original default `out` output,
                 // and replace it with the list of outputs.
@@ -338,9 +335,8 @@ pub(crate) mod derivation_builtins {
 
                 // handle builder and system.
                 "builder" | "system" => {
-                    match strong_importing_coerce_to_string(&co, value).await {
+                    let val_str =
-                        Err(cek) => return Ok(Value::from(cek)),
+                        try_cek_to_value!(strong_importing_coerce_to_string(&co, value).await);
-                        Ok(val_str) => {
                     input_context.mimic(&val_str);
 
                     if arg_name == "builder" {
@@ -352,16 +348,12 @@ pub(crate) mod derivation_builtins {
                     // Either populate drv.environment or structured_attrs.
                     if let Some(ref mut structured_attrs) = structured_attrs {
                         // No need to check for dups, we only iterate over every attribute name once
-                                structured_attrs.insert(
+                        structured_attrs
-                                    arg_name.to_owned(),
+                            .insert(arg_name.to_owned(), val_str.to_str()?.to_owned().into());
-                                    val_str.to_str()?.to_owned().into(),
-                                );
                     } else {
                         insert_env(&mut drv, arg_name, val_str.as_bytes().into())?;
                     }
                 }
-                    }
-                }
 
                 // Don't add STRUCTURED_ATTRS if enabled.
                 STRUCTURED_ATTRS if structured_attrs.is_some() => continue,
@@ -384,9 +376,8 @@ pub(crate) mod derivation_builtins {
                         // No need to check for dups, we only iterate over every attribute name once
                         structured_attrs.insert(arg_name.to_owned(), val_json);
                     } else {
-                        match strong_importing_coerce_to_string(&co, value).await {
+                        let val_str =
-                            Err(cek) => return Ok(Value::from(cek)),
+                            try_cek_to_value!(strong_importing_coerce_to_string(&co, value).await);
-                            Ok(val_str) => {
                         input_context.mimic(&val_str);
 
                         insert_env(&mut drv, arg_name, val_str.as_bytes().into())?;
@@ -394,33 +385,25 @@ pub(crate) mod derivation_builtins {
                 }
             }
         }
-            }
-        }
         // end of per-argument loop
 
         // Configure fixed-output derivations if required.
         {
-            let output_hash = match select_string(&co, &input, "outputHash")
+            let output_hash = try_cek_to_value!(
+                select_string(&co, &input, "outputHash")
                     .await
                     .context("evaluating the `outputHash` parameter")?
-            {
+            );
-                Err(cek) => return Ok(Value::from(cek)),
+            let output_hash_algo = try_cek_to_value!(
-                Ok(s) => s,
+                select_string(&co, &input, "outputHashAlgo")
-            };
-            let output_hash_algo = match select_string(&co, &input, "outputHashAlgo")
                     .await
                     .context("evaluating the `outputHashAlgo` parameter")?
-            {
+            );
-                Err(cek) => return Ok(Value::from(cek)),
+            let output_hash_mode = try_cek_to_value!(
-                Ok(s) => s,
+                select_string(&co, &input, "outputHashMode")
-            };
-            let output_hash_mode = match select_string(&co, &input, "outputHashMode")
                     .await
                     .context("evaluating the `outputHashMode` parameter")?
-            {
+            );
-                Err(cek) => return Ok(Value::from(cek)),
-                Ok(s) => s,
-            };
 
             if let Some(warning) =
                 handle_fixed_output(&mut drv, output_hash, output_hash_algo, output_hash_mode)?

snix/glue/src/builtins/fetchers.rs

@@ -9,7 +9,7 @@ use nix_compat::nixhash::{HashAlgo, NixHash};
 use snix_eval::builtin_macros::builtins;
 use snix_eval::generators::Gen;
 use snix_eval::generators::GenCo;
-use snix_eval::{CatchableErrorKind, ErrorKind, Value};
+use snix_eval::{CatchableErrorKind, ErrorKind, Value, try_cek};
 use std::rc::Rc;
 use url::Url;
 
@@ -56,18 +56,10 @@ async fn extract_fetch_args(
         ));
     }
 
-    let url_str = match select_string(co, &attrs, "url").await? {
+    let url_str = try_cek!(select_string(co, &attrs, "url").await?)
-        Ok(s) => s.ok_or_else(|| ErrorKind::AttributeNotFound { name: "url".into() })?,
+        .ok_or_else(|| ErrorKind::AttributeNotFound { name: "url".into() })?;
-        Err(cek) => return Ok(Err(cek)),
+    let name = try_cek!(select_string(co, &attrs, "name").await?);
-    };
+    let sha256_str = try_cek!(select_string(co, &attrs, "sha256").await?);
-    let name = match select_string(co, &attrs, "name").await? {
-        Ok(s) => s,
-        Err(cek) => return Ok(Err(cek)),
-    };
-    let sha256_str = match select_string(co, &attrs, "sha256").await? {
-        Ok(s) => s,
-        Err(cek) => return Ok(Err(cek)),
-    };
 
     Ok(Ok(NixFetchArgs {
         url: Url::parse(&url_str).map_err(|e| ErrorKind::SnixError(Rc::new(e)))?,
@@ -89,6 +81,7 @@ async fn extract_fetch_args(
 pub(crate) mod fetcher_builtins {
     use bstr::ByteSlice;
     use nix_compat::{flakeref, nixhash::NixHash};
+    use snix_eval::try_cek_to_value;
     use std::collections::BTreeMap;
 
     use super::*;
@@ -137,10 +130,7 @@ pub(crate) mod fetcher_builtins {
         co: GenCo,
         args: Value,
     ) -> Result<Value, ErrorKind> {
-        let args = match extract_fetch_args(&co, args).await? {
+        let args = try_cek_to_value!(extract_fetch_args(&co, args).await?);
-            Ok(args) => args,
-            Err(cek) => return Ok(Value::from(cek)),
-        };
 
         // Derive the name from the URL basename if not set explicitly.
         let name = args
@@ -163,10 +153,7 @@ pub(crate) mod fetcher_builtins {
         co: GenCo,
         args: Value,
     ) -> Result<Value, ErrorKind> {
-        let args = match extract_fetch_args(&co, args).await? {
+        let args = try_cek_to_value!(extract_fetch_args(&co, args).await?);
-            Ok(args) => args,
-            Err(cek) => return Ok(Value::from(cek)),
-        };
 
         // Name defaults to "source" if not set explicitly.
         const DEFAULT_NAME_FETCH_TARBALL: &str = "source";

snix/glue/src/builtins/utils.rs

@@ -2,6 +2,7 @@ use bstr::ByteSlice;
 use snix_eval::{
     CatchableErrorKind, CoercionKind, ErrorKind, NixAttrs, NixString, Value,
     generators::{self, GenCo},
+    try_cek,
 };
 
 pub(super) async fn strong_importing_coerce_to_string(
@@ -26,10 +27,8 @@ pub(super) async fn select_string(
     key: &str,
 ) -> Result<Result<Option<String>, CatchableErrorKind>, ErrorKind> {
     if let Some(attr) = attrs.select(key) {
-        match strong_importing_coerce_to_string(co, attr.clone()).await {
+        let str = try_cek!(strong_importing_coerce_to_string(co, attr.clone()).await);
-            Err(cek) => return Ok(Err(cek)),
+        return Ok(Ok(Some(str.to_str()?.to_owned())));
-            Ok(str) => return Ok(Ok(Some(str.to_str()?.to_owned()))),
-        }
     }
 
     Ok(Ok(None))