From 69d8f17a26db50e7b76297dbf588aafc23425c71 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Sat, 27 Aug 2022 00:21:08 +0300
Subject: feat(tvix/eval): compile creation of closure objects

Fully implements the instructions for compiling closure
objects (without runtime handling yet).

Closure (and thunk) objects are created at runtime by capturing all
known upvalues. To represent this, the instructions for creating them
need to have a variable number of arguments. Due to that, this commit
introduces new variants in OpCode that are not actually operations,
but data.

If the VM is implemented correctly, the instruction pointer should
never point at these. Due to this, the VM will panic if it sees a data
operand during an execution run.

Change-Id: Ic56b49b3a42736dc437751e76df0e89c8d0619c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6291
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
---
 tvix/eval/src/compiler/mod.rs   | 24 +++++++++++++++++++++---
 tvix/eval/src/opcode.rs         | 12 +++++++++++-
 tvix/eval/src/value/function.rs | 12 +++++++++++-
 tvix/eval/src/vm.rs             |  9 ++++++++-
 4 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 3ade6831c2..0c41f06cbd 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -856,9 +856,27 @@ impl Compiler {
             crate::disassembler::disassemble_chunk(&compiled.lambda.chunk);
         }
 
-        self.emit_constant(Value::Closure(Closure {
-            lambda: compiled.lambda,
-        }));
+        // If the function is not a closure, just emit it directly and
+        // move on.
+        if compiled.lambda.upvalue_count == 0 {
+            self.emit_constant(Value::Closure(Closure::new(compiled.lambda)));
+            return;
+        }
+
+        // If the function is a closure, we need to emit the variable
+        // number of operands that allow the runtime to close over the
+        // upvalues.
+        let closure_idx = self
+            .chunk()
+            .push_constant(Value::Closure(Closure::new(compiled.lambda)));
+
+        self.chunk().push_op(OpCode::OpClosure(closure_idx));
+        for upvalue in compiled.scope.upvalues {
+            match upvalue {
+                Upvalue::Stack(idx) => self.chunk().push_op(OpCode::DataLocalIdx(idx)),
+                Upvalue::Upvalue(idx) => self.chunk().push_op(OpCode::DataUpvalueIdx(idx)),
+            };
+        }
     }
 
     fn compile_apply(&mut self, node: ast::Apply) {
diff --git a/tvix/eval/src/opcode.rs b/tvix/eval/src/opcode.rs
index 6cce7ea9b1..ca99602862 100644
--- a/tvix/eval/src/opcode.rs
+++ b/tvix/eval/src/opcode.rs
@@ -102,7 +102,17 @@ pub enum OpCode {
     // Asserts stack top is a boolean, and true.
     OpAssert,
 
-    // Lambdas
+    // Lambdas & closures
     OpCall,
     OpGetUpvalue(UpvalueIdx),
+    OpClosure(ConstantIdx),
+
+    // The closure and thunk creation instructions have a variable
+    // number of arguments to the instruction, which is represented
+    // here by making their data part of the opcodes.
+    //
+    // The VM skips over these by advancing the instruction pointer
+    // according to the count.
+    DataLocalIdx(StackIdx),
+    DataUpvalueIdx(UpvalueIdx),
 }
diff --git a/tvix/eval/src/value/function.rs b/tvix/eval/src/value/function.rs
index 5d7247416e..2b5fcf6c98 100644
--- a/tvix/eval/src/value/function.rs
+++ b/tvix/eval/src/value/function.rs
@@ -1,7 +1,7 @@
 //! This module implements the runtime representation of functions.
 use std::rc::Rc;
 
-use crate::chunk::Chunk;
+use crate::{chunk::Chunk, Value};
 
 #[derive(Clone, Debug)]
 pub struct Lambda {
@@ -27,4 +27,14 @@ impl Lambda {
 #[derive(Clone, Debug)]
 pub struct Closure {
     pub lambda: Lambda,
+    pub upvalues: Vec<Value>,
+}
+
+impl Closure {
+    pub fn new(lambda: Lambda) -> Self {
+        Closure {
+            upvalues: Vec::with_capacity(lambda.upvalue_count),
+            lambda,
+        }
+    }
 }
diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs
index 1c7772f2a7..5ebc95e7f1 100644
--- a/tvix/eval/src/vm.rs
+++ b/tvix/eval/src/vm.rs
@@ -357,7 +357,7 @@ impl VM {
                 OpCode::OpCall => {
                     let callable = self.pop();
                     match callable {
-                        Value::Closure(Closure { lambda }) => self.call(lambda, 1),
+                        Value::Closure(Closure { lambda, .. }) => self.call(lambda, 1),
                         Value::Builtin(builtin) => {
                             let arg = self.pop();
                             let result = builtin.apply(arg)?;
@@ -368,6 +368,13 @@ impl VM {
                 }
 
                 OpCode::OpGetUpvalue(_) => todo!("getting upvalues"),
+                OpCode::OpClosure(_) => todo!("creating closure objects"),
+
+                // Data-carrying operands should never be executed,
+                // that is a critical error in the VM.
+                OpCode::DataLocalIdx(_) | OpCode::DataUpvalueIdx(_) => {
+                    panic!("VM bug: attempted to execute data-carrying operand")
+                }
             }
 
             #[cfg(feature = "disassembler")]
-- 
cgit 1.4.1