From a1015ba1d7c2228224847f1931118da473815de3 Mon Sep 17 00:00:00 2001 From: Griffin Smith Date: Sun, 6 Nov 2022 10:28:34 -0500 Subject: feat(tvix/eval): Give names to builtin arguments 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 Tested-by: BuildkiteCI --- tvix/eval/builtin-macros/src/lib.rs | 65 ++++++++++++++++++++++++----------- tvix/eval/src/builtins/impure.rs | 7 ++-- tvix/eval/src/builtins/mod.rs | 68 ++++++++++++++++++++++++------------- tvix/eval/src/lib.rs | 2 +- tvix/eval/src/value/builtin.rs | 24 ++++++++----- tvix/eval/src/value/mod.rs | 2 +- 6 files changed, 111 insertions(+), 57 deletions(-) diff --git a/tvix/eval/builtin-macros/src/lib.rs b/tvix/eval/builtin-macros/src/lib.rs index 8e2c4b3f87..8276e421c5 100644 --- a/tvix/eval/builtin-macros/src/lib.rs +++ b/tvix/eval/builtin-macros/src/lib.rs @@ -5,7 +5,9 @@ use proc_macro2::Span; use quote::{quote_spanned, ToTokens}; use syn::parse::Parse; use syn::spanned::Spanned; -use syn::{parse_macro_input, parse_quote, FnArg, Ident, Item, ItemMod, LitStr, PatType}; +use syn::{ + parse_macro_input, parse_quote, FnArg, Ident, Item, ItemMod, LitStr, Pat, PatIdent, PatType, +}; struct BuiltinArgs { name: LitStr, @@ -89,31 +91,54 @@ pub fn builtins(_args: TokenStream, item: TokenStream) -> TokenStream { .into(); } - let strictness = f + let builtin_arguments = f .sig .inputs .iter_mut() .skip(1) - .map(|input| { - let mut lazy = false; - if let FnArg::Typed(PatType { attrs, .. }) = input { - attrs.retain(|attr| { - attr.path.get_ident().into_iter().any(|id| { - if id == "lazy" { - lazy = true; - false - } else { - true - } - }) - }); - } - !lazy + .map(|arg| { + let mut strict = true; + let name = match arg { + FnArg::Receiver(_) => { + return Err(quote_spanned!(arg.span() => { + compile_error!("Unexpected receiver argument in builtin") + })) + } + FnArg::Typed(PatType { attrs, pat, .. }) => { + attrs.retain(|attr| { + attr.path.get_ident().into_iter().any(|id| { + if id == "lazy" { + strict = false; + false + } else { + true + } + }) + }); + match pat.as_ref() { + Pat::Ident(PatIdent { ident, .. }) => ident.to_string(), + _ => "unknown".to_string(), + } + } + }; + + Ok(quote_spanned!(arg.span() => { + crate::internal::BuiltinArgument { + strict: #strict, + name: #name, + } + })) }) - .collect::>(); + .collect::, _>>(); + + let builtin_arguments = match builtin_arguments { + Ok(args) => args, + Err(err) => return err.into(), + }; let fn_name = f.sig.ident.clone(); - let args = (0..(f.sig.inputs.len() - 1)) + let num_args = f.sig.inputs.len() - 1; + let args = (0..num_args) .map(|n| Ident::new(&format!("arg_{n}"), Span::call_site())) .collect::>(); let mut reversed_args = args.clone(); @@ -122,7 +147,7 @@ pub fn builtins(_args: TokenStream, item: TokenStream) -> TokenStream { builtins.push(quote_spanned! { builtin_attr.span() => { crate::internal::Builtin::new( #name, - &[#(#strictness),*], + &[#(#builtin_arguments),*], |mut args: Vec, vm: &mut crate::internal::VM| { #(let #reversed_args = args.pop().unwrap();)* #fn_name(vm, #(#args),*) diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs index 006cd9d7ac..d4e7c41703 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, source: SourceCode) -> Builti Builtin::new( "import", - &[true], + &[BuiltinArgument { + strict: true, + name: "path", + }], move |mut args: Vec, 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 caa575fcf3..9b6e29db0a 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 { vec![ Builtin::new( "addErrorContext", - &[false, false], + &[ + BuiltinArgument { + strict: false, + name: "context", + }, + BuiltinArgument { + strict: false, + name: "value", + }, + ], |mut args: Vec, vm: &mut VM| { vm.emit_warning(WarningKind::NotImplemented("builtins.addErrorContext")); Ok(args.pop().unwrap()) @@ -915,7 +925,10 @@ fn placeholders() -> Vec { ), Builtin::new( "unsafeDiscardStringContext", - &[true], + &[BuiltinArgument { + strict: true, + name: "s", + }], |mut args: Vec, vm: &mut VM| { vm.emit_warning(WarningKind::NotImplemented( "builtins.unsafeDiscardStringContext", @@ -923,28 +936,35 @@ fn placeholders() -> Vec { Ok(args.pop().unwrap()) }, ), - Builtin::new("derivation", &[true], |args: Vec, 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, 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 diff --git a/tvix/eval/src/lib.rs b/tvix/eval/src/lib.rs index 106f7d851c..3e160fa92c 100644 --- a/tvix/eval/src/lib.rs +++ b/tvix/eval/src/lib.rs @@ -37,7 +37,7 @@ pub use crate::vm::run_lambda; /// Internal-only parts of `tvix-eval`, exported for use in macros, but not part of the public /// interface of the crate. pub mod internal { - pub use crate::value::Builtin; + pub use crate::value::{Builtin, BuiltinArgument}; pub use crate::vm::VM; } diff --git a/tvix/eval/src/value/builtin.rs b/tvix/eval/src/value/builtin.rs index fc80431762..944efd85eb 100644 --- a/tvix/eval/src/value/builtin.rs +++ b/tvix/eval/src/value/builtin.rs @@ -25,6 +25,14 @@ use std::{ pub trait BuiltinFn: Fn(Vec, &mut VM) -> Result {} impl, &mut VM) -> Result> BuiltinFn for F {} +/// Description of a single argument passed to a builtin +pub struct BuiltinArgument { + /// Whether the argument should be forced before the underlying builtin function is called + pub strict: bool, + /// The name of the argument, to be used in docstrings and error messages + pub name: &'static str, +} + /// Represents a single built-in function which directly executes Rust /// code that operates on a Nix value. /// @@ -40,10 +48,8 @@ impl, &mut VM) -> Result> BuiltinFn for F {} #[derive(Clone)] pub struct Builtin { name: &'static str, - /// Array reference that describes how many arguments there are (usually 1 - /// or 2) and whether they need to be forced. `true` causes the - /// corresponding argument to be forced before `func` is called. - strict_args: &'static [bool], + /// Array of arguments to the builtin. + arguments: &'static [BuiltinArgument], func: Rc, /// Partially applied function arguments. @@ -53,12 +59,12 @@ pub struct Builtin { impl Builtin { pub fn new( name: &'static str, - strict_args: &'static [bool], + arguments: &'static [BuiltinArgument], func: F, ) -> Self { Builtin { name, - strict_args, + arguments, func: Rc::new(func), partials: vec![], } @@ -74,9 +80,9 @@ impl Builtin { pub fn apply(mut self, vm: &mut VM, arg: Value) -> Result { self.partials.push(arg); - if self.partials.len() == self.strict_args.len() { - for (idx, force) in self.strict_args.iter().enumerate() { - if *force { + if self.partials.len() == self.arguments.len() { + for (idx, BuiltinArgument { strict, .. }) in self.arguments.iter().enumerate() { + if *strict { self.partials[idx].force(vm)?; } } diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index db72081e8d..f0bb3628d8 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -20,7 +20,7 @@ use crate::errors::ErrorKind; use crate::opcode::StackIdx; use crate::vm::VM; pub use attrs::NixAttrs; -pub use builtin::Builtin; +pub use builtin::{Builtin, BuiltinArgument}; pub(crate) use function::Formals; pub use function::{Closure, Lambda}; pub use list::NixList; -- cgit 1.4.1