feat(tvix/value): introduce string representation with &'static str r/4457

For cases where the strings are statically known (such as the
oft-occuring name/value), this can be a useful optimisation.

It's also much more convenient in tests.

Change-Id: Ie462b684805bd4986ea5e85ca4bff663bc2d3c3c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6111
Tested-by: BuildkiteCI
Reviewed-by: eta <tvl@eta.st>

4 files changed, 44 insertions, 26 deletions

diff --git a/tvix/eval/src/compiler.rs b/tvix/eval/src/compiler.rs
index 5b6f748dc7..668ec842e8 100644
--- a/tvix/eval/src/compiler.rs
+++ b/tvix/eval/src/compiler.rs
@@ -111,7 +111,7 @@ impl Compiler {
                 rnix::StrPart::Ast(node) => self.compile(node)?,
 
                 rnix::StrPart::Literal(lit) => {
-                    let idx = self.chunk.add_constant(Value::String(NixString(lit)));
+                    let idx = self.chunk.add_constant(Value::String(lit.into()));
                     self.chunk.add_op(OpCode::OpConstant(idx));
                 }
             }
@@ -206,9 +206,9 @@ impl Compiler {
                         let ident = rnix::types::Ident::cast(fragment).unwrap();
 
                         // TODO(tazjin): intern!
-                        let idx = self
-                            .chunk
-                            .add_constant(Value::String(NixString(ident.as_str().to_string())));
+                        let idx = self.chunk.add_constant(Value::String(NixString::Heap(
+                            ident.as_str().to_string(),
+                        )));
                         self.chunk.add_op(OpCode::OpConstant(idx));
                     }
 
diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs
index 75ade6a89a..7e3b5f231d 100644
--- a/tvix/eval/src/value/attrs.rs
+++ b/tvix/eval/src/value/attrs.rs
@@ -71,14 +71,8 @@ impl NixAttrs {
 
             NixAttrs::KV { name, value } => {
                 *self = NixAttrs::Map(BTreeMap::from([
-                    (
-                        NixString("name".into()),
-                        std::mem::replace(name, Value::Blackhole),
-                    ),
-                    (
-                        NixString("value".into()),
-                        std::mem::replace(value, Value::Blackhole),
-                    ),
+                    ("name".into(), std::mem::replace(name, Value::Blackhole)),
+                    ("value".into(), std::mem::replace(value, Value::Blackhole)),
                 ]));
                 self.map_mut()
             }
@@ -155,14 +149,14 @@ impl NixAttrs {
 fn attempt_optimise_kv(slice: &mut [Value]) -> Option<NixAttrs> {
     let (name_idx, value_idx) = {
         match (&slice[2], &slice[0]) {
-            (Value::String(NixString(s1)), Value::String(NixString(s2)))
-                if (s1 == "name" && s2 == "value") =>
+            (Value::String(s1), Value::String(s2))
+                if (*s1 == NixString::NAME && *s2 == NixString::VALUE) =>
             {
                 (3, 1)
             }
 
-            (Value::String(NixString(s1)), Value::String(NixString(s2)))
-                if (s1 == "value" && s2 == "name") =>
+            (Value::String(s1), Value::String(s2))
+                if (*s1 == NixString::VALUE && *s2 == NixString::NAME) =>
             {
                 (1, 3)
             }
@@ -189,7 +183,7 @@ fn set_attr(attrs: &mut NixAttrs, key: NixString, value: Value) -> EvalResult<()
     match entry {
         std::collections::btree_map::Entry::Occupied(entry) => {
             return Err(Error::DuplicateAttrsKey {
-                key: entry.key().0.clone(),
+                key: entry.key().as_str().to_string(),
             })
         }
 
@@ -254,7 +248,7 @@ fn set_nested_attr(
 
             _ => {
                 return Err(Error::DuplicateAttrsKey {
-                    key: entry.key().0.clone(),
+                    key: entry.key().as_str().to_string(),
                 })
             }
         },
diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs
index d4776caea4..47661e03ad 100644
--- a/tvix/eval/src/value/string.rs
+++ b/tvix/eval/src/value/string.rs
@@ -4,16 +4,40 @@ use std::fmt::Display;
 /// backing implementations.
 
 #[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
-pub struct NixString(pub String);
+pub enum NixString {
+    Static(&'static str),
+    Heap(String),
+}
 
 impl Display for NixString {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str(self.0.as_str())
+        match self {
+            NixString::Static(s) => f.write_str(s),
+            NixString::Heap(s) => f.write_str(s),
+        }
+    }
+}
+
+impl From<&'static str> for NixString {
+    fn from(s: &'static str) -> Self {
+        NixString::Static(s)
     }
 }
 
-impl From<&str> for NixString {
-    fn from(s: &str) -> Self {
-        NixString(s.to_string())
+impl From<String> for NixString {
+    fn from(s: String) -> Self {
+        NixString::Heap(s)
+    }
+}
+
+impl NixString {
+    pub const NAME: Self = NixString::Static("name");
+    pub const VALUE: Self = NixString::Static("value");
+
+    pub fn as_str(&self) -> &str {
+        match self {
+            NixString::Static(s) => s,
+            NixString::Heap(s) => s,
+        }
     }
 }
diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs
index a58d77cc27..a529c5a799 100644
--- a/tvix/eval/src/vm.rs
+++ b/tvix/eval/src/vm.rs
@@ -7,7 +7,7 @@ use crate::{
     chunk::Chunk,
     errors::{Error, EvalResult},
     opcode::OpCode,
-    value::{NixAttrs, NixList, NixString, Value},
+    value::{NixAttrs, NixList, Value},
 };
 
 pub struct VM {
@@ -159,10 +159,10 @@ impl VM {
         let mut out = String::new();
 
         for _ in 0..count {
-            out.push_str(&self.pop().as_string()?.0);
+            out.push_str(&self.pop().as_string()?.as_str());
         }
 
-        self.push(Value::String(NixString(out)));
+        self.push(Value::String(out.into()));
         Ok(())
     }