docs(tvix/eval) comments for various fields

Change-Id: I8dcddf2b419761e475e71215c199eef2f7dc61dc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7028
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>

tvix/eval/src/opcode.rs

@@ -48,6 +48,8 @@ pub struct JumpOffset(pub usize);
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub struct Count(pub usize);
 
+/// All variants of this enum carry a bounded amount of data to
+/// ensure that no heap allocations are needed for an Opcode.
 #[warn(variant_size_differences)]
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum OpCode {
@@ -146,12 +148,17 @@ pub enum OpCode {
     /// given stack index after the scope is fully bound.
     OpFinalise(StackIdx),
 
-    // The closure and thunk creation instructions have a variable
+    // [`OpClosure`] and [`OpThunk`] have a variable number of
-    // number of arguments to the instruction, which is represented
+    // arguments to the instruction, which is represented here by
-    // here by making their data part of the opcodes.
+    // making their data part of the opcodes.  Each of these two
+    // opcodes has a `ConstantIdx`, which must reference a
+    // `Value::Blueprint(Lambda)`.  The `upvalue_count` field in
+    // that `Lambda` indicates the number of arguments it takes, and
+    // the `OpClosure` or `OpThunk` must be followed by exactly this
+    // number of `Data*` opcodes.  The VM skips over these by
+    // advancing the instruction pointer.
     //
-    // The VM skips over these by advancing the instruction pointer
+    // It is illegal for a `Data*` opcode to appear anywhere else.
-    // according to the count.
     DataLocalIdx(StackIdx),
     DataDeferredLocal(StackIdx),
     DataUpvalueIdx(UpvalueIdx),

tvix/eval/src/value/function.rs

@@ -9,10 +9,20 @@ use crate::{
     upvalues::{UpvalueCarrier, Upvalues},
 };
 
+/// The opcodes for a thunk or closure, plus the number of
+/// non-executable opcodes which are allowed after an OpClosure or
+/// OpThunk referencing it.  At runtime `Lambda` is usually wrapped
+/// in `Rc` to avoid copying the `Chunk` it holds (which can be
+/// quite large).
 #[derive(Debug, PartialEq)]
 pub struct Lambda {
     // name: Option<NixString>,
     pub(crate) chunk: Chunk,
+
+    /// Number of upvalues which the code in this Lambda closes
+    /// over, and which need to be initialised at
+    /// runtime.  Information about the variables is emitted using
+    /// data-carrying opcodes (see [`OpCode::DataLocalIdx`]).
     pub(crate) upvalue_count: usize,
 }
 

tvix/eval/src/value/mod.rs

@@ -43,8 +43,13 @@ pub enum Value {
     // Internal values that, while they technically exist at runtime,
     // are never returned to or created directly by users.
     Thunk(Thunk),
+
+    // See [`compiler::compile_select_or()`] for explanation
     AttrNotFound,
+
+    // this can only occur in Chunk::Constants and nowhere else
     Blueprint(Rc<Lambda>),
+
     DeferredUpvalue(StackIdx),
     UnresolvedPath(PathBuf),
 }