From 8c1c9aee3cab52befe55d6f41e1c9acaef1b1843 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Sun, 14 Aug 2022 20:12:20 +0300
Subject: feat(tvix/eval): implement `inherit` in attribute set literals

Straightforward implementation, evaluating the elements of an inherit
and preparing the stack so that `OpAttrs` sees all relevant values
when constructing the attribute set itself.

The emitted instructions for inheriting a lot of values from the same
attribute set are inefficient, but it's too early to say whether this
actually matters.

Change-Id: Icb55a20936d4ef77173f34433811c5fa5d2c9ecc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6214
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
---
 tvix/eval/src/compiler.rs                          | 45 ++++++++++++++++------
 .../tvix_tests/eval-okay-attrs-inherit-literal.exp |  1 +
 .../tvix_tests/eval-okay-attrs-inherit-literal.nix |  2 +
 .../tests/tvix_tests/eval-okay-attrs-inherit.exp   |  1 +
 .../tests/tvix_tests/eval-okay-attrs-inherit.nix   |  2 +
 5 files changed, 39 insertions(+), 12 deletions(-)
 create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit-literal.exp
 create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit-literal.nix
 create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit.exp
 create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit.nix

(limited to 'tvix/eval')

diff --git a/tvix/eval/src/compiler.rs b/tvix/eval/src/compiler.rs
index 94d91159f8..11bd649f71 100644
--- a/tvix/eval/src/compiler.rs
+++ b/tvix/eval/src/compiler.rs
@@ -152,10 +152,7 @@ impl Compiler {
     fn compile_with_literal_ident(&mut self, node: rnix::SyntaxNode) -> EvalResult<()> {
         if node.kind() == rnix::SyntaxKind::NODE_IDENT {
             let ident = rnix::types::Ident::cast(node).unwrap();
-            let idx = self
-                .chunk
-                .push_constant(Value::String(ident.as_str().into()));
-            self.chunk.push_op(OpCode::OpConstant(idx));
+            self.emit_literal_ident(&ident);
             return Ok(());
         }
 
@@ -363,17 +360,31 @@ impl Compiler {
         // inherit "from the outside").
         for inherit in node.inherits() {
             match inherit.from() {
-                Some(_from) => todo!("inherit from attrs not implemented"),
-                None => {
+                Some(from) => {
                     for ident in inherit.idents() {
                         count += 1;
 
-                        // Leave the identifier on the stack (never
-                        // nested in case of inherits!)
-                        let idx = self
-                            .chunk
-                            .push_constant(Value::String(ident.as_str().into()));
-                        self.chunk.push_op(OpCode::OpConstant(idx));
+                        // First emit the identifier itself
+                        self.emit_literal_ident(&ident);
+
+                        // Then emit the node that we're inheriting
+                        // from.
+                        //
+                        // TODO: Likely significant optimisation
+                        // potential in having a multi-select
+                        // instruction followed by a merge, rather
+                        // than pushing/popping the same attrs
+                        // potentially a lot of times.
+                        self.compile(from.inner().unwrap())?;
+                        self.emit_literal_ident(&ident);
+                        self.chunk.push_op(OpCode::OpAttrsSelect);
+                    }
+                }
+
+                None => {
+                    for ident in inherit.idents() {
+                        count += 1;
+                        self.emit_literal_ident(&ident);
 
                         match self.resolve_local(ident.as_str()) {
                             Some(idx) => self.chunk.push_op(OpCode::OpGetLocal(idx)),
@@ -729,6 +740,16 @@ impl Compiler {
         Ok(())
     }
 
+    // Emit the literal string value of an identifier. Required for
+    // several operations related to attribute sets, where identifiers
+    // are used as string keys.
+    fn emit_literal_ident(&mut self, ident: &rnix::types::Ident) {
+        let idx = self
+            .chunk
+            .push_constant(Value::String(ident.as_str().into()));
+        self.chunk.push_op(OpCode::OpConstant(idx));
+    }
+
     fn patch_jump(&mut self, idx: CodeIdx) {
         let offset = self.chunk.code.len() - 1 - idx.0;
 
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit-literal.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit-literal.exp
new file mode 100644
index 0000000000..60d3b2f4a4
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit-literal.exp
@@ -0,0 +1 @@
+15
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit-literal.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit-literal.nix
new file mode 100644
index 0000000000..587aec8933
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit-literal.nix
@@ -0,0 +1,2 @@
+# the 'from' part of an `inherit` can be any expression.
+{ inherit ({a = 15;}) a; }.a
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit.exp
new file mode 100644
index 0000000000..9ea79851a1
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit.exp
@@ -0,0 +1 @@
+{ a = 15; }
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit.nix
new file mode 100644
index 0000000000..6d045643cc
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-attrs-inherit.nix
@@ -0,0 +1,2 @@
+let a = 15;
+in { inherit a; }
-- 
cgit 1.4.1