From 9407af5684ca5602df51c4edddc428db7fc98417 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Thu, 11 Aug 2022 11:55:59 +0300
Subject: [PATCH] refactor(tvix/eval): encapsulate list construction in
 value::list

Ensuring that the implementation is not leaking out of the module lets
us keep things open for optimisations (e.g. empty list or pairs
through tuples).

Change-Id: I18fd9b7740f28c55736471e16c6b4095a05dd6d0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6145
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
---
 tvix/eval/src/value/list.rs | 13 ++++++++++++-
 tvix/eval/src/vm.rs         | 21 +++++----------------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/tvix/eval/src/value/list.rs b/tvix/eval/src/value/list.rs
index f9d852294..546d2c88a 100644
--- a/tvix/eval/src/value/list.rs
+++ b/tvix/eval/src/value/list.rs
@@ -4,7 +4,7 @@ use std::fmt::Display;
 use super::Value;
 
 #[derive(Clone, Debug, PartialEq)]
-pub struct NixList(pub Vec<Value>);
+pub struct NixList(Vec<Value>);
 
 impl Display for NixList {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -26,4 +26,15 @@ impl NixList {
         lhs.0.append(&mut rhs.0);
         lhs
     }
+
+    pub fn construct(count: usize, stack_slice: Vec<Value>) -> Self {
+        debug_assert!(
+            count == stack_slice.len(),
+            "NixList::construct called with count == {}, but slice.len() == {}",
+            count,
+            stack_slice.len(),
+        );
+
+        NixList(stack_slice)
+    }
 }
diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs
index 540845d2c..a64e0b337 100644
--- a/tvix/eval/src/vm.rs
+++ b/tvix/eval/src/vm.rs
@@ -150,7 +150,11 @@ impl VM {
                     self.push(Value::Attrs(Rc::new(lhs.update(&rhs))))
                 }
 
-                OpCode::OpList(count) => self.run_list(count)?,
+                OpCode::OpList(count) => {
+                    let list =
+                        NixList::construct(count, self.stack.split_off(self.stack.len() - count));
+                    self.push(Value::List(list));
+                }
 
                 OpCode::OpConcat => {
                     let rhs = self.pop().as_list()?;
@@ -204,21 +208,6 @@ impl VM {
         self.push(Value::String(out.into()));
         Ok(())
     }
-
-    // Construct runtime representation of a list. Because the list
-    // items are on the stack in reverse order, the vector is created
-    // initialised and elements are directly assigned to their
-    // respective indices.
-    fn run_list(&mut self, count: usize) -> EvalResult<()> {
-        let mut list = vec![Value::Null; count];
-
-        for idx in 0..count {
-            list[count - idx - 1] = self.pop();
-        }
-
-        self.push(Value::List(NixList(list)));
-        Ok(())
-    }
 }
 
 pub fn run_chunk(chunk: Chunk) -> EvalResult<Value> {