From 058e77bab20db90347ce1d91c41076ef56b61b26 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Wed, 10 Aug 2022 21:01:15 +0300 Subject: feat(tvix/eval): implement attrset update (`//`) operator The underlying implementation does a few tricks based on which pair of attrset representations is encountered. Particularly the effect of short-circuiting the empty cases might be relevant in nixpkgs/NixOS, due to the use of lib.optionalAttrs. Change-Id: I22b978b1c69af12926489a71087c6a6219c012f3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6140 Reviewed-by: sterni Tested-by: BuildkiteCI --- tvix/eval/src/compiler.rs | 5 ++ tvix/eval/src/opcode.rs | 1 + .../eval-okay-attrs-update-empty-lhs.exp | 1 + .../eval-okay-attrs-update-empty-lhs.nix | 1 + .../eval-okay-attrs-update-empty-rhs.exp | 1 + .../eval-okay-attrs-update-empty-rhs.nix | 1 + .../tvix_tests/eval-okay-attrs-update-kv-lhs.exp | 1 + .../tvix_tests/eval-okay-attrs-update-kv-lhs.nix | 1 + .../tests/tvix_tests/eval-okay-attrs-update.exp | 1 + .../tests/tvix_tests/eval-okay-attrs-update.nix | 1 + tvix/eval/src/value/attrs.rs | 55 +++++++++++++++++++++- tvix/eval/src/value/mod.rs | 10 ++++ tvix/eval/src/vm.rs | 9 ++++ 13 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-lhs.exp create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-lhs.nix create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-rhs.exp create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-rhs.nix create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-kv-lhs.exp create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-kv-lhs.nix create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update.exp create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update.nix diff --git a/tvix/eval/src/compiler.rs b/tvix/eval/src/compiler.rs index 668ec842e8..4aadfaba80 100644 --- a/tvix/eval/src/compiler.rs +++ b/tvix/eval/src/compiler.rs @@ -136,6 +136,7 @@ impl Compiler { BinOpKind::Mul => OpCode::OpMul, BinOpKind::Div => OpCode::OpDiv, BinOpKind::Equal => OpCode::OpEqual, + BinOpKind::Update => OpCode::OpAttrsUpdate, _ => todo!(), }; @@ -187,6 +188,10 @@ impl Compiler { // 2. Keys can refer to nested attribute sets. // 3. Attribute sets can (optionally) be recursive. fn compile_attr_set(&mut self, node: rnix::types::AttrSet) -> EvalResult<()> { + if node.recursive() { + todo!("recursive attribute sets are not yet implemented") + } + let mut count = 0; for kv in node.entries() { diff --git a/tvix/eval/src/opcode.rs b/tvix/eval/src/opcode.rs index 622a02ac85..f682cfc8b8 100644 --- a/tvix/eval/src/opcode.rs +++ b/tvix/eval/src/opcode.rs @@ -33,6 +33,7 @@ pub enum OpCode { // Attribute sets OpAttrs(usize), OpAttrPath(usize), + OpAttrsUpdate, // Lists OpList(usize), diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-lhs.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-lhs.exp new file mode 100644 index 0000000000..fedf8f25a6 --- /dev/null +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-lhs.exp @@ -0,0 +1 @@ +{ a = "ok"; } diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-lhs.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-lhs.nix new file mode 100644 index 0000000000..9596be22b8 --- /dev/null +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-lhs.nix @@ -0,0 +1 @@ +{} // { a = "ok"; } diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-rhs.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-rhs.exp new file mode 100644 index 0000000000..fedf8f25a6 --- /dev/null +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-rhs.exp @@ -0,0 +1 @@ +{ a = "ok"; } diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-rhs.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-rhs.nix new file mode 100644 index 0000000000..117c011413 --- /dev/null +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-empty-rhs.nix @@ -0,0 +1 @@ +{ a = "ok"; } // {} diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-kv-lhs.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-kv-lhs.exp new file mode 100644 index 0000000000..c2234a47e2 --- /dev/null +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-kv-lhs.exp @@ -0,0 +1 @@ +{ name = "foo"; other = 42; value = "bar"; } diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-kv-lhs.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-kv-lhs.nix new file mode 100644 index 0000000000..6f71684902 --- /dev/null +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update-kv-lhs.nix @@ -0,0 +1 @@ +{ name = "foo"; value = "bar"; } // { other = 42; } diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update.exp new file mode 100644 index 0000000000..57f4d541bd --- /dev/null +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update.exp @@ -0,0 +1 @@ +{ a = 15; b = "works"; } diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update.nix new file mode 100644 index 0000000000..735602fe02 --- /dev/null +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-update.nix @@ -0,0 +1 @@ +{ a = 15; } // { b = "works"; } diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index 51f4795c59..e7da6ee621 100644 --- a/tvix/eval/src/value/attrs.rs +++ b/tvix/eval/src/value/attrs.rs @@ -35,7 +35,7 @@ impl Display for NixAttrs { } NixAttrs::Map(map) => { - for (name, value) in map { + for (name, value) in map.iter() { f.write_fmt(format_args!("{} = {}; ", name.ident_str(), value))?; } } @@ -54,6 +54,59 @@ impl PartialEq for NixAttrs { } impl NixAttrs { + // Update one attribute set with the values of the other. + pub fn update(&self, other: &Self) -> Self { + match (self, other) { + // Short-circuit on some optimal cases: + (NixAttrs::Empty, NixAttrs::Empty) => NixAttrs::Empty, + (NixAttrs::Empty, _) => other.clone(), + (_, NixAttrs::Empty) => self.clone(), + (NixAttrs::KV { .. }, NixAttrs::KV { .. }) => other.clone(), + + // Slightly more advanced, but still optimised updates + (NixAttrs::Map(m), NixAttrs::KV { name, value }) => { + let mut m = m.clone(); + m.insert(NixString::NAME, name.clone()); + m.insert(NixString::VALUE, value.clone()); + NixAttrs::Map(m) + } + + (NixAttrs::KV { name, value }, NixAttrs::Map(m)) => { + let mut m = m.clone(); + + match m.entry(NixString::NAME) { + std::collections::btree_map::Entry::Vacant(e) => { + e.insert(name.clone()); + } + + std::collections::btree_map::Entry::Occupied(_) => { + /* name from `m` has precedence */ + } + }; + + match m.entry(NixString::VALUE) { + std::collections::btree_map::Entry::Vacant(e) => { + e.insert(value.clone()); + } + + std::collections::btree_map::Entry::Occupied(_) => { + /* value from `m` has precedence */ + } + }; + + NixAttrs::Map(m) + } + + // Plain merge of maps. + (NixAttrs::Map(m1), NixAttrs::Map(m2)) => { + let mut m1 = m1.clone(); + let mut m2 = m2.clone(); + m1.append(&mut m2); + NixAttrs::Map(m1) + } + } + } + /// Retrieve reference to a mutable map inside of an attrs, /// optionally changing the representation if required. fn map_mut(&mut self) -> &mut BTreeMap { diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index 0a430ae08c..2a89f1c358 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -71,6 +71,16 @@ impl Value { }), } } + + pub fn as_attrs(self) -> EvalResult> { + match self { + Value::Attrs(s) => Ok(s), + other => Err(Error::TypeError { + expected: "set", + actual: other.type_of(), + }), + } + } } impl Display for Value { diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs index 9a65668caa..7a1082344e 100644 --- a/tvix/eval/src/vm.rs +++ b/tvix/eval/src/vm.rs @@ -117,8 +117,17 @@ impl VM { OpCode::OpNull => self.push(Value::Null), OpCode::OpTrue => self.push(Value::Bool(true)), OpCode::OpFalse => self.push(Value::Bool(false)), + OpCode::OpAttrs(count) => self.run_attrset(count)?, OpCode::OpAttrPath(count) => self.run_attr_path(count)?, + + OpCode::OpAttrsUpdate => { + let rhs = self.pop().as_attrs()?; + let lhs = self.pop().as_attrs()?; + + self.push(Value::Attrs(Rc::new(lhs.update(&rhs)))) + } + OpCode::OpList(count) => self.run_list(count)?, OpCode::OpInterpolate(count) => self.run_interpolate(count)?, } -- cgit 1.4.1