refactor(tvix/value): hide internal string representation r/4482

Wraps the string representation in an additional newtype struct with a
private field in order to hide the representation from other modules.

This is done in order to avoid accidental leakage of the internals
outside of value::string.

In fact, this caught a mistake in the compiler module which was
directly constructing an internal variant.

Change-Id: If4b627d3cff7ab9cd50ca1a3ac73245d4dcf7aef
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6147
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>

2 files changed, 16 insertions, 13 deletions

diff --git a/tvix/eval/src/compiler.rs b/tvix/eval/src/compiler.rs
index 91eb1bda12eb..4a3a652af5c5 100644
--- a/tvix/eval/src/compiler.rs
+++ b/tvix/eval/src/compiler.rs
@@ -4,7 +4,7 @@
 use crate::chunk::Chunk;
 use crate::errors::EvalResult;
 use crate::opcode::OpCode;
-use crate::value::{NixString, Value};
+use crate::value::Value;
 
 use rnix;
 use rnix::types::{EntryHolder, TokenWrapper, TypedNode, Wrapper};
@@ -221,9 +221,9 @@ impl Compiler {
                         let ident = rnix::types::Ident::cast(fragment).unwrap();
 
                         // TODO(tazjin): intern!
-                        let idx = self.chunk.add_constant(Value::String(NixString::Heap(
-                            ident.as_str().to_string(),
-                        )));
+                        let idx = self
+                            .chunk
+                            .add_constant(Value::String(ident.as_str().to_string().into()));
                         self.chunk.add_op(OpCode::OpConstant(idx));
                     }
 
diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs
index 3816e89cd1b1..302566821881 100644
--- a/tvix/eval/src/value/string.rs
+++ b/tvix/eval/src/value/string.rs
@@ -5,11 +5,14 @@ use std::{borrow::Cow, fmt::Display};
 /// backing implementations.
 
 #[derive(Clone, Debug)]
-pub enum NixString {
+enum StringRepr {
     Static(&'static str),
     Heap(String),
 }
 
+#[derive(Clone, Debug)]
+pub struct NixString(StringRepr);
+
 impl PartialEq for NixString {
     fn eq(&self, other: &Self) -> bool {
         self.as_str() == other.as_str()
@@ -32,13 +35,13 @@ impl Ord for NixString {
 
 impl From<&'static str> for NixString {
     fn from(s: &'static str) -> Self {
-        NixString::Static(s)
+        NixString(StringRepr::Static(s))
     }
 }
 
 impl From<String> for NixString {
     fn from(s: String) -> Self {
-        NixString::Heap(s)
+        NixString(StringRepr::Heap(s))
     }
 }
 
@@ -49,13 +52,13 @@ impl Hash for NixString {
 }
 
 impl NixString {
-    pub const NAME: Self = NixString::Static("name");
-    pub const VALUE: Self = NixString::Static("value");
+    pub const NAME: Self = NixString(StringRepr::Static("name"));
+    pub const VALUE: Self = NixString(StringRepr::Static("value"));
 
     pub fn as_str(&self) -> &str {
-        match self {
-            NixString::Static(s) => s,
-            NixString::Heap(s) => s,
+        match &self.0 {
+            StringRepr::Static(s) => s,
+            StringRepr::Heap(s) => s,
         }
     }
 
@@ -82,7 +85,7 @@ impl NixString {
     pub fn concat(&self, other: &Self) -> Self {
         let mut s = self.as_str().to_owned();
         s.push_str(other.as_str());
-        NixString::Heap(s)
+        NixString(StringRepr::Heap(s))
     }
 }