feat(tvix/eval): Give names to builtin arguments r/5268

Refactor the arguments of a Builtin to be a vec of a new BuiltinArgument
struct, which contains the old strictness boolean and also a static
`name` str - this is automatically determined via the ident for the
corresponding function argument in the proc-macro case, and passed in in
the cases where we're still manually calling Builtin::new.

Currently this name is unused, but in the future this can be used as
part of a documentation system for builtins.

Change-Id: Ib9dadb15b69bf8c9ea1983a4f4f197294a2394a6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7204
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI

2 files changed, 49 insertions, 26 deletions

diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs
index 006cd9d7ac93..d4e7c4170364 100644
--- a/tvix/eval/src/builtins/impure.rs
+++ b/tvix/eval/src/builtins/impure.rs
@@ -12,7 +12,7 @@ use crate::{
     compiler::GlobalsMap,
     errors::ErrorKind,
     observer::NoOpObserver,
-    value::{Builtin, NixAttrs, Thunk},
+    value::{Builtin, BuiltinArgument, NixAttrs, Thunk},
     vm::VM,
     SourceCode, Value,
 };
@@ -111,7 +111,10 @@ pub fn builtins_import(globals: &Weak<GlobalsMap>, source: SourceCode) -> Builti
 
     Builtin::new(
         "import",
-        &[true],
+        &[BuiltinArgument {
+            strict: true,
+            name: "path",
+        }],
         move |mut args: Vec<Value>, vm: &mut VM| {
             let mut path = super::coerce_value_to_path(&args.pop().unwrap(), vm)?;
             if path.is_dir() {
diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index caa575fcf3bc..9b6e29db0a3d 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -5,6 +5,7 @@
 
 use crate::compiler::{GlobalsMap, GlobalsMapFunc};
 use crate::source::SourceCode;
+use crate::value::BuiltinArgument;
 use std::cmp::{self, Ordering};
 use std::collections::{BTreeMap, HashMap, HashSet};
 use std::path::PathBuf;
@@ -907,7 +908,16 @@ fn placeholders() -> Vec<Builtin> {
     vec![
         Builtin::new(
             "addErrorContext",
-            &[false, false],
+            &[
+                BuiltinArgument {
+                    strict: false,
+                    name: "context",
+                },
+                BuiltinArgument {
+                    strict: false,
+                    name: "value",
+                },
+            ],
             |mut args: Vec<Value>, vm: &mut VM| {
                 vm.emit_warning(WarningKind::NotImplemented("builtins.addErrorContext"));
                 Ok(args.pop().unwrap())
@@ -915,7 +925,10 @@ fn placeholders() -> Vec<Builtin> {
         ),
         Builtin::new(
             "unsafeDiscardStringContext",
-            &[true],
+            &[BuiltinArgument {
+                strict: true,
+                name: "s",
+            }],
             |mut args: Vec<Value>, vm: &mut VM| {
                 vm.emit_warning(WarningKind::NotImplemented(
                     "builtins.unsafeDiscardStringContext",
@@ -923,28 +936,35 @@ fn placeholders() -> Vec<Builtin> {
                 Ok(args.pop().unwrap())
             },
         ),
-        Builtin::new("derivation", &[true], |args: Vec<Value>, vm: &mut VM| {
-            vm.emit_warning(WarningKind::NotImplemented("builtins.derivation"));
-
-            // We do not implement derivations yet, so this function sets mock
-            // values on the fields that a real derivation would contain.
-            //
-            // Crucially this means we do not yet *validate* the values either.
-            let attrs = unwrap_or_clone_rc(args[0].to_attrs()?);
-            let attrs = attrs.update(NixAttrs::from_map(BTreeMap::from([
-                (
-                    "outPath".into(),
-                    "/nix/store/00000000000000000000000000000000-mock".into(),
-                ),
-                (
-                    "drvPath".into(),
-                    "/nix/store/00000000000000000000000000000000-mock.drv".into(),
-                ),
-                ("type".into(), "derivation".into()),
-            ])));
-
-            Ok(Value::Attrs(Rc::new(attrs)))
-        }),
+        Builtin::new(
+            "derivation",
+            &[BuiltinArgument {
+                strict: true,
+                name: "attrs",
+            }],
+            |args: Vec<Value>, vm: &mut VM| {
+                vm.emit_warning(WarningKind::NotImplemented("builtins.derivation"));
+
+                // We do not implement derivations yet, so this function sets mock
+                // values on the fields that a real derivation would contain.
+                //
+                // Crucially this means we do not yet *validate* the values either.
+                let attrs = unwrap_or_clone_rc(args[0].to_attrs()?);
+                let attrs = attrs.update(NixAttrs::from_map(BTreeMap::from([
+                    (
+                        "outPath".into(),
+                        "/nix/store/00000000000000000000000000000000-mock".into(),
+                    ),
+                    (
+                        "drvPath".into(),
+                        "/nix/store/00000000000000000000000000000000-mock.drv".into(),
+                    ),
+                    ("type".into(), "derivation".into()),
+                ])));
+
+                Ok(Value::Attrs(Rc::new(attrs)))
+            },
+        ),
     ]
 }
 // we set TVIX_CURRENT_SYSTEM in build.rs