From f12f938166e5b0c773f3553fb7e7422a58e83c6d Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Fri, 13 Jan 2023 14:18:01 +0300
Subject: refactor(tvix/eval): directly return builtin tuples from macro

All invocations of the builtin macro had to previously filter through
the `builtin_tuple` function, but it's more sensible to directly
return these from the macro.

Change-Id: I45600ba84d56c9528d3e92570461c319eea595ce
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7825
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
---
 tvix/eval/builtin-macros/src/lib.rs     |  4 ++--
 tvix/eval/builtin-macros/tests/tests.rs | 28 +++++++++++++++++-----------
 tvix/eval/src/builtins/impure.rs        |  5 +----
 tvix/eval/src/builtins/mod.rs           | 14 ++------------
 tvix/eval/src/tests/mod.rs              |  6 +-----
 5 files changed, 23 insertions(+), 34 deletions(-)

diff --git a/tvix/eval/builtin-macros/src/lib.rs b/tvix/eval/builtin-macros/src/lib.rs
index ff1c52e03fd8..5a54d499124d 100644
--- a/tvix/eval/builtin-macros/src/lib.rs
+++ b/tvix/eval/builtin-macros/src/lib.rs
@@ -209,8 +209,8 @@ pub fn builtins(_args: TokenStream, item: TokenStream) -> TokenStream {
     }
 
     items.push(parse_quote! {
-        pub fn builtins() -> Vec<crate::Builtin> {
-            vec![#(#builtins),*]
+        pub fn builtins() -> Vec<(&'static str, Value)> {
+            vec![#(#builtins),*].into_iter().map(|b| (b.name(), Value::Builtin(b))).collect()
         }
     });
 
diff --git a/tvix/eval/builtin-macros/tests/tests.rs b/tvix/eval/builtin-macros/tests/tests.rs
index 5705d9202f6e..fb062d34b99a 100644
--- a/tvix/eval/builtin-macros/tests/tests.rs
+++ b/tvix/eval/builtin-macros/tests/tests.rs
@@ -1,11 +1,9 @@
-pub use tvix_eval::internal;
-pub use tvix_eval::Value;
+pub use tvix_eval::{Builtin, BuiltinArgument, Value, VM};
 use tvix_eval_builtin_macros::builtins;
 
 #[builtins]
 mod builtins {
-    use tvix_eval::internal::VM;
-    use tvix_eval::{ErrorKind, Value};
+    use tvix_eval::{ErrorKind, Value, VM};
 
     /// Test docstring.
     ///
@@ -26,13 +24,21 @@ fn builtins() {
     let builtins = builtins::builtins();
     assert_eq!(builtins.len(), 2);
 
-    let identity = builtins.iter().find(|b| b.name() == "identity").unwrap();
-    assert_eq!(
-        identity.documentation(),
-        Some(
-            r#" Test docstring.
+    let (_, identity) = builtins
+        .iter()
+        .find(|(name, _)| *name == "identity")
+        .unwrap();
+
+    match identity {
+        Value::Builtin(identity) => assert_eq!(
+            identity.documentation(),
+            Some(
+                r#" Test docstring.
 
  It has multiple lines!"#
-        )
-    );
+            )
+        ),
+
+        _ => panic!("builtin was not a builtin"),
+    }
 }
diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs
index 0cebd5f6ab58..3eaebf101e3d 100644
--- a/tvix/eval/src/builtins/impure.rs
+++ b/tvix/eval/src/builtins/impure.rs
@@ -65,10 +65,7 @@ mod impure_builtins {
 /// Return all impure builtins, that is all builtins which may perform I/O
 /// outside of the VM and so cannot be used in all contexts (e.g. WASM).
 pub fn impure_builtins() -> Vec<(&'static str, Value)> {
-    let mut result = impure_builtins::builtins()
-        .into_iter()
-        .map(super::builtin_tuple)
-        .collect::<Vec<_>>();
+    let mut result = impure_builtins::builtins();
 
     result.push((
         "storeDir",
diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index 7614353f5bbb..b1ea408a8ca4 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -14,7 +14,7 @@ use crate::arithmetic_op;
 use crate::warnings::WarningKind;
 use crate::{
     errors::{ErrorKind, EvalResult},
-    value::{Builtin, CoercionKind, NixAttrs, NixList, NixString, Value},
+    value::{CoercionKind, NixAttrs, NixList, NixString, Value},
     vm::VM,
 };
 
@@ -968,17 +968,10 @@ mod pure_builtins {
     }
 }
 
-pub(crate) fn builtin_tuple(builtin: Builtin) -> (&'static str, Value) {
-    (builtin.name(), Value::Builtin(builtin))
-}
-
 /// The set of standard pure builtins in Nix, mostly concerned with
 /// data structure manipulation (string, attrs, list, etc. functions).
 pub fn pure_builtins() -> Vec<(&'static str, Value)> {
-    let mut result = pure_builtins::builtins()
-        .into_iter()
-        .map(builtin_tuple)
-        .collect::<Vec<_>>();
+    let mut result = pure_builtins::builtins();
 
     // Pure-value builtins
     result.push(("nixVersion", Value::String("2.3-compat-tvix-0.1".into())));
@@ -1038,7 +1031,4 @@ mod placeholder_builtins {
 
 pub fn placeholders() -> Vec<(&'static str, Value)> {
     placeholder_builtins::builtins()
-        .into_iter()
-        .map(builtin_tuple)
-        .collect()
 }
diff --git a/tvix/eval/src/tests/mod.rs b/tvix/eval/src/tests/mod.rs
index b8d9dfc6e086..98c93ba27084 100644
--- a/tvix/eval/src/tests/mod.rs
+++ b/tvix/eval/src/tests/mod.rs
@@ -48,11 +48,7 @@ fn eval_test(code_path: &str, expect_success: bool) {
     }
 
     let mut eval = crate::Evaluation::new_impure(&code, Some(code_path.into()));
-    eval.builtins.extend(
-        mock_builtins::builtins()
-            .into_iter()
-            .map(crate::builtins::builtin_tuple),
-    );
+    eval.builtins.extend(mock_builtins::builtins());
 
     let result = eval.evaluate();
 
-- 
cgit 1.4.1