feat(tvix/eval): Box Value::Catchable

This is now the only enum variant for Value that is larger than 8
bytes (it's 16 bytes), so boxing it (especially since it's not
perf-critical) allows us to get the Value size down to only 16 bytes!

Change-Id: I98598e2b762944448bef982e8ff7da6d6683c4aa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10798
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: aspen <root@gws.fyi>

tvix/eval/src/builtins/impure.rs

@@ -33,7 +33,7 @@ mod impure_builtins {
     #[builtin("pathExists")]
     async fn builtin_path_exists(co: GenCo, path: Value) -> Result<Value, ErrorKind> {
         match coerce_value_to_path(&co, path).await? {
-            Err(cek) => Ok(Value::Catchable(cek)),
+            Err(cek) => Ok(Value::from(cek)),
             Ok(path) => Ok(generators::request_path_exists(&co, path).await),
         }
     }
@@ -41,7 +41,7 @@ mod impure_builtins {
     #[builtin("readDir")]
     async fn builtin_read_dir(co: GenCo, path: Value) -> Result<Value, ErrorKind> {
         match coerce_value_to_path(&co, path).await? {
-            Err(cek) => Ok(Value::Catchable(cek)),
+            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)| {
@@ -67,7 +67,7 @@ mod impure_builtins {
     #[builtin("readFile")]
     async fn builtin_read_file(co: GenCo, path: Value) -> Result<Value, ErrorKind> {
         match coerce_value_to_path(&co, path).await? {
-            Err(cek) => Ok(Value::Catchable(cek)),
+            Err(cek) => Ok(Value::from(cek)),
             Ok(path) => Ok(generators::request_read_to_string(&co, path).await),
         }
     }

tvix/eval/src/builtins/mod.rs

@@ -303,7 +303,7 @@ mod pure_builtins {
                         context = context.join(other_context);
                     }
                 }
-                Err(c) => return Ok(Value::Catchable(c)),
+                Err(c) => return Ok(Value::Catchable(Box::new(c))),
             }
         }
         // FIXME: pass immediately the string res.
@@ -365,7 +365,7 @@ mod pure_builtins {
             {
                 Ok(true) => return Ok(true.into()),
                 Ok(false) => continue,
-                Err(cek) => return Ok(Value::Catchable(cek)),
+                Err(cek) => return Ok(Value::from(cek)),
             }
         }
         Ok(false.into())
@@ -452,7 +452,7 @@ mod pure_builtins {
     #[builtin("toJSON")]
     async fn builtin_to_json(co: GenCo, val: Value) -> Result<Value, ErrorKind> {
         match val.into_json(&co).await? {
-            Err(cek) => Ok(Value::Catchable(cek)),
+            Err(cek) => Ok(Value::from(cek)),
             Ok(json_value) => {
                 let json_str = serde_json::to_string(&json_value)?;
                 Ok(json_str.into())
@@ -471,7 +471,7 @@ mod pure_builtins {
     #[allow(non_snake_case)]
     async fn builtin_filterSource(_co: GenCo, #[lazy] _e: Value) -> Result<Value, ErrorKind> {
         // TODO: implement for nixpkgs compatibility
-        Ok(Value::Catchable(CatchableErrorKind::UnimplementedFeature(
+        Ok(Value::from(CatchableErrorKind::UnimplementedFeature(
             "filterSource".into(),
         )))
     }
@@ -690,7 +690,7 @@ mod pure_builtins {
         _string: Value,
     ) -> Result<Value, ErrorKind> {
         // FIXME: propagate contexts here.
-        Ok(Value::Catchable(CatchableErrorKind::UnimplementedFeature(
+        Ok(Value::from(CatchableErrorKind::UnimplementedFeature(
             "hashString".into(),
         )))
     }
@@ -884,7 +884,7 @@ mod pure_builtins {
     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? {
-            Err(cek) => Ok(Value::Catchable(cek)),
+            Err(cek) => Ok(Value::from(cek)),
             Ok(Ordering::Less) => Ok(Value::Bool(true)),
             Ok(_) => Ok(Value::Bool(false)),
         }
@@ -1387,7 +1387,7 @@ mod pure_builtins {
         }
         // TODO(sterni): coerces to string
         // We do not care about the context here explicitly.
-        Ok(Value::Catchable(CatchableErrorKind::Throw(
+        Ok(Value::from(CatchableErrorKind::Throw(
             message.to_contextful_str()?.to_string().into(),
         )))
     }
@@ -1444,7 +1444,7 @@ mod pure_builtins {
         }
 
         match coerce_value_to_path(&co, s).await? {
-            Err(cek) => Ok(Value::Catchable(cek)),
+            Err(cek) => Ok(Value::from(cek)),
             Ok(path) => {
                 let path: Value = crate::value::canon_path(path).into();
                 let span = generators::request_span(&co).await;
@@ -1527,7 +1527,7 @@ pub fn pure_builtins() -> Vec<(&'static str, Value)> {
     // TODO: implement for nixpkgs compatibility
     result.push((
         "__curPos",
-        Value::Catchable(CatchableErrorKind::UnimplementedFeature("__curPos".into())),
+        Value::from(CatchableErrorKind::UnimplementedFeature("__curPos".into())),
     ));
 
     result

tvix/eval/src/compiler/import.rs

@@ -27,7 +27,7 @@ async fn import_impl(
 ) -> Result<Value, ErrorKind> {
     // TODO(sterni): canon_path()?
     let mut path = match coerce_value_to_path(&co, args.pop().unwrap()).await? {
-        Err(cek) => return Ok(Value::Catchable(cek)),
+        Err(cek) => return Ok(Value::Catchable(Box::new(cek))),
         Ok(path) => path,
     };
 

tvix/eval/src/compiler/mod.rs

@@ -399,9 +399,9 @@ impl Compiler<'_> {
             // TODO: decide what to do with findFile
             if raw_path.len() == 2 {
                 return self.emit_constant(
-                    Value::Catchable(CatchableErrorKind::NixPathResolution(
+                    Value::Catchable(Box::new(CatchableErrorKind::NixPathResolution(
                         "Empty <> path not allowed".into(),
-                    )),
+                    ))),
                     node,
                 );
             }

tvix/eval/src/value/attrs.rs

@@ -375,7 +375,7 @@ impl NixAttrs {
                     continue;
                 }
 
-                Value::Catchable(err) => return Ok(Err(err)),
+                Value::Catchable(err) => return Ok(Err(*err)),
 
                 other => return Err(ErrorKind::InvalidAttributeName(other)),
             }

tvix/eval/src/value/json.rs

@@ -61,7 +61,7 @@ impl Value {
                         )
                         .await?
                     {
-                        Value::Catchable(cek) => return Ok(Err(cek)),
+                        Value::Catchable(cek) => return Ok(Err(*cek)),
                         Value::String(s) => return Ok(Ok(Json::String(s.to_str()?.to_owned()))),
                         _ => panic!("Value::coerce_to_string_() returned a non-string!"),
                     }
@@ -88,7 +88,7 @@ impl Value {
                 Json::Object(out)
             }
 
-            Value::Catchable(c) => return Ok(Err(c)),
+            Value::Catchable(c) => return Ok(Err(*c)),
 
             val @ Value::Closure(_)
             | val @ Value::Thunk(_)
@@ -110,7 +110,7 @@ impl Value {
     /// Value::Json.
     pub(crate) async fn into_json_generator(self, co: GenCo) -> Result<Value, ErrorKind> {
         match self.into_json(&co).await? {
-            Err(cek) => Ok(Value::Catchable(cek)),
+            Err(cek) => Ok(Value::from(cek)),
             Ok(json) => Ok(Value::Json(Box::new(json))),
         }
     }

tvix/eval/src/value/mod.rs

@@ -84,12 +84,13 @@ pub enum Value {
     FinaliseRequest(bool),
 
     #[serde(skip)]
-    Catchable(CatchableErrorKind),
+    Catchable(Box<CatchableErrorKind>),
 }
 
 impl From<CatchableErrorKind> for Value {
+    #[inline]
     fn from(c: CatchableErrorKind) -> Value {
-        Value::Catchable(c)
+        Value::Catchable(Box::new(c))
     }
 }
 
@@ -97,8 +98,12 @@ impl<V> From<Result<V, CatchableErrorKind>> for Value
 where
     Value: From<V>,
 {
+    #[inline]
     fn from(v: Result<V, CatchableErrorKind>) -> Value {
-        v.map_or_else(Value::Catchable, |v| v.into())
+        match v {
+            Ok(v) => v.into(),
+            Err(e) => Value::Catchable(Box::new(e)),
+        }
     }
 }
 
@@ -776,8 +781,8 @@ impl Value {
                 b = b.force(&co, span.clone()).await?;
             }
             let result = match (a, b) {
-                (Value::Catchable(c), _) => return Ok(Err(c)),
-                (_, Value::Catchable(c)) => return Ok(Err(c)),
+                (Value::Catchable(c), _) => return Ok(Err(*c)),
+                (_, Value::Catchable(c)) => return Ok(Err(*c)),
                 // same types
                 (Value::Integer(i1), Value::Integer(i2)) => i1.cmp(&i2),
                 (Value::Float(f1), Value::Float(f2)) => f1.total_cmp(&f2),

tvix/eval/src/vm/generators.rs

@@ -609,7 +609,7 @@ pub async fn request_string_coerce(
     match val {
         Value::String(s) => Ok(*s),
         _ => match co.yield_(VMRequest::StringCoerce(val, kind)).await {
-            VMResponse::Value(Value::Catchable(c)) => Err(c),
+            VMResponse::Value(Value::Catchable(c)) => Err(*c),
             VMResponse::Value(value) => Ok(value
                 .to_contextful_str()
                 .expect("coerce_to_string always returns a string")),
@@ -644,7 +644,7 @@ pub(crate) async fn check_equality(
         .await
     {
         VMResponse::Value(Value::Bool(b)) => Ok(Ok(b)),
-        VMResponse::Value(Value::Catchable(cek)) => Ok(Err(cek)),
+        VMResponse::Value(Value::Catchable(cek)) => Ok(Err(*cek)),
         msg => panic!(
             "Tvix bug: VM responded with incorrect generator message: {}",
             msg
@@ -776,7 +776,7 @@ pub(crate) async fn request_to_json(
 ) -> Result<serde_json::Value, CatchableErrorKind> {
     match co.yield_(VMRequest::ToJson(value)).await {
         VMResponse::Value(Value::Json(json)) => Ok(*json),
-        VMResponse::Value(Value::Catchable(cek)) => Err(cek),
+        VMResponse::Value(Value::Catchable(cek)) => Err(*cek),
         msg => panic!(
             "Tvix bug: VM responded with incorrect generator message: {}",
             msg

tvix/eval/src/vm/macros.rs

@@ -44,7 +44,7 @@ macro_rules! cmp_op {
                     let span = generators::request_span(&co).await;
                     let ordering = a.nix_cmp_ordering(b, co, span).await?;
                     match ordering {
-                        Err(cek) => Ok(Value::Catchable(cek)),
+                        Err(cek) => Ok(Value::from(cek)),
                         Ok(ordering) => Ok(Value::Bool(cmp_op!(@order $op ordering))),
                     }
                 }

tvix/eval/src/vm/mod.rs

@@ -901,7 +901,7 @@ where
 
                 OpCode::OpAssertFail => {
                     self.stack
-                        .push(Value::Catchable(CatchableErrorKind::AssertionFailed));
+                        .push(Value::from(CatchableErrorKind::AssertionFailed));
                 }
 
                 // Data-carrying operands should never be executed,
@@ -1250,7 +1250,7 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> {
                     path.push(vs.to_os_str()?);
                     crate::value::canon_path(PathBuf::from(path)).into()
                 }
-                Err(c) => Value::Catchable(c),
+                Err(c) => Value::Catchable(Box::new(c)),
             }
         }
         (Value::String(s1), Value::String(s2)) => Value::String(Box::new(s1.concat(&s2))),
@@ -1288,8 +1288,8 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> {
             .await;
             match (r1, r2) {
                 (Ok(s1), Ok(s2)) => Value::String(Box::new(s1.concat(&s2))),
-                (Err(c), _) => return Ok(Value::Catchable(c)),
-                (_, Err(c)) => return Ok(Value::Catchable(c)),
+                (Err(c), _) => return Ok(Value::from(c)),
+                (_, Err(c)) => return Ok(Value::from(c)),
             }
         }
     };

tvix/glue/src/builtins/derivation.rs

@@ -248,7 +248,7 @@ pub(crate) mod derivation_builtins {
                 "args" => {
                     for arg in value.to_list()? {
                         match strong_importing_coerce_to_string(&co, arg).await {
-                            Err(cek) => return Ok(Value::Catchable(cek)),
+                            Err(cek) => return Ok(Value::from(cek)),
                             Ok(s) => {
                                 input_context.mimic(&s);
                                 drv.arguments.push(s.to_str()?.to_owned())
@@ -299,7 +299,7 @@ pub(crate) mod derivation_builtins {
                 // handle builder and system.
                 "builder" | "system" => {
                     match strong_importing_coerce_to_string(&co, value).await {
-                        Err(cek) => return Ok(Value::Catchable(cek)),
+                        Err(cek) => return Ok(Value::from(cek)),
                         Ok(val_str) => {
                             input_context.mimic(&val_str);
 
@@ -334,7 +334,7 @@ pub(crate) mod derivation_builtins {
                     // In non-SA case, coerce to string and add to env.
                     if let Some(ref mut structured_attrs) = structured_attrs {
                         let val = generators::request_force(&co, value).await;
-                        if matches!(val, Value::Catchable(_)) {
+                        if val.is_catchable() {
                             return Ok(val);
                         }
 
@@ -343,14 +343,14 @@ pub(crate) mod derivation_builtins {
 
                         let val_json = match val.into_json(&co).await? {
                             Ok(v) => v,
-                            Err(cek) => return Ok(Value::Catchable(cek)),
+                            Err(cek) => return Ok(Value::from(cek)),
                         };
 
                         // 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 {
-                            Err(cek) => return Ok(Value::Catchable(cek)),
+                            Err(cek) => return Ok(Value::from(cek)),
                             Ok(val_str) => {
                                 input_context.mimic(&val_str);
 
@@ -384,21 +384,21 @@ pub(crate) mod derivation_builtins {
                 .await
                 .context("evaluating the `outputHash` parameter")?
             {
-                Err(cek) => return Ok(Value::Catchable(cek)),
+                Err(cek) => return Ok(Value::from(cek)),
                 Ok(s) => s,
             };
             let output_hash_algo = match select_string(&co, &input, "outputHashAlgo")
                 .await
                 .context("evaluating the `outputHashAlgo` parameter")?
             {
-                Err(cek) => return Ok(Value::Catchable(cek)),
+                Err(cek) => return Ok(Value::from(cek)),
                 Ok(s) => s,
             };
             let output_hash_mode = match select_string(&co, &input, "outputHashMode")
                 .await
                 .context("evaluating the `outputHashMode` parameter")?
             {
-                Err(cek) => return Ok(Value::Catchable(cek)),
+                Err(cek) => return Ok(Value::from(cek)),
                 Ok(s) => s,
             };