about summary refs log tree commit diff
path: root/tvix/eval
diff options
context:
space:
mode:
Diffstat (limited to 'tvix/eval')
-rw-r--r--tvix/eval/src/compiler/mod.rs2
-rw-r--r--tvix/eval/src/value/arbitrary.rs2
-rw-r--r--tvix/eval/src/value/attrs.rs11
-rw-r--r--tvix/eval/src/value/mod.rs12
-rw-r--r--tvix/eval/src/value/string.rs389
-rw-r--r--tvix/eval/src/vm/generators.rs2
-rw-r--r--tvix/eval/src/vm/mod.rs10
7 files changed, 365 insertions, 63 deletions
diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 0f9accc13e77..60c55dda27b4 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -1362,7 +1362,7 @@ impl Compiler<'_, '_> {
     /// several operations related to attribute sets, where
     /// identifiers are used as string keys.
     fn emit_literal_ident(&mut self, ident: &ast::Ident) {
-        self.emit_constant(Value::String(Box::new(ident.clone().into())), ident);
+        self.emit_constant(Value::String(ident.clone().into()), ident);
     }
 
     /// Patch the jump instruction at the given index, setting its
diff --git a/tvix/eval/src/value/arbitrary.rs b/tvix/eval/src/value/arbitrary.rs
index c14fbae6cb71..bf53f4fcb28a 100644
--- a/tvix/eval/src/value/arbitrary.rs
+++ b/tvix/eval/src/value/arbitrary.rs
@@ -91,7 +91,7 @@ fn leaf_value() -> impl Strategy<Value = Value> {
         any::<bool>().prop_map(Bool),
         any::<i64>().prop_map(Integer),
         any::<f64>().prop_map(Float),
-        any::<Box<NixString>>().prop_map(String),
+        any::<NixString>().prop_map(String),
         any::<OsString>().prop_map(|s| Path(Box::new(s.into()))),
     ]
 }
diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs
index c9f04b54cc57..33259c8058eb 100644
--- a/tvix/eval/src/value/attrs.rs
+++ b/tvix/eval/src/value/attrs.rs
@@ -366,7 +366,7 @@ impl NixAttrs {
             let key = stack_slice.pop().unwrap();
 
             match key {
-                Value::String(ks) => set_attr(&mut attrs, *ks, value)?,
+                Value::String(ks) => set_attr(&mut attrs, ks, value)?,
 
                 Value::Null => {
                     // This is in fact valid, but leads to the value
@@ -422,13 +422,8 @@ impl IntoIterator for NixAttrs {
 fn attempt_optimise_kv(slice: &mut [Value]) -> Option<NixAttrs> {
     let (name_idx, value_idx) = {
         match (&slice[2], &slice[0]) {
-            (Value::String(s1), Value::String(s2)) if (**s1 == *NAME_S && **s2 == *VALUE_S) => {
-                (3, 1)
-            }
-
-            (Value::String(s1), Value::String(s2)) if (**s1 == *VALUE_S && **s2 == *NAME_S) => {
-                (1, 3)
-            }
+            (Value::String(s1), Value::String(s2)) if (*s1 == *NAME_S && *s2 == *VALUE_S) => (3, 1),
+            (Value::String(s1), Value::String(s2)) if (*s1 == *VALUE_S && *s2 == *NAME_S) => (1, 3),
 
             // Technically this branch lets type errors pass,
             // but they will be caught during normal attribute
diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index 095bc269d497..e8e27e5968de 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -47,7 +47,7 @@ pub enum Value {
     Bool(bool),
     Integer(i64),
     Float(f64),
-    String(Box<NixString>),
+    String(NixString),
 
     #[serde(skip)]
     Path(Box<PathBuf>),
@@ -192,7 +192,7 @@ where
     T: Into<NixString>,
 {
     fn from(t: T) -> Self {
-        Self::String(Box::new(t.into()))
+        Self::String(t.into())
     }
 }
 
@@ -333,9 +333,7 @@ impl Value {
             let value = if let Some(v) = vals.pop() {
                 v.force(co, span.clone()).await?
             } else {
-                return Ok(Value::String(Box::new(NixString::new_context_from(
-                    context, result,
-                ))));
+                return Ok(Value::String(NixString::new_context_from(context, result)));
             };
             let coerced: Result<BString, _> = match (value, kind) {
                 // coercions that are always done
@@ -700,7 +698,7 @@ impl Value {
     /// everytime you want a string.
     pub fn to_str(&self) -> Result<NixString, ErrorKind> {
         match self {
-            Value::String(s) if !s.has_context() => Ok((**s).clone()),
+            Value::String(s) if !s.has_context() => Ok((*s).clone()),
             Value::Thunk(thunk) => Self::to_str(&thunk.value()),
             other => Err(type_error("contextless strings", other)),
         }
@@ -711,7 +709,7 @@ impl Value {
         NixString,
         "contextful string",
         Value::String(s),
-        (**s).clone()
+        (*s).clone()
     );
     gen_cast!(to_path, Box<PathBuf>, "path", Value::Path(p), p.clone());
     gen_cast!(to_attrs, Box<NixAttrs>, "set", Value::Attrs(a), a.clone());
diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs
index 5a7783b8fd72..2f1592546b85 100644
--- a/tvix/eval/src/value/string.rs
+++ b/tvix/eval/src/value/string.rs
@@ -5,11 +5,15 @@
 //! paying the cost when creating new strings.
 use bstr::{BStr, BString, ByteSlice, Chars};
 use rnix::ast;
+use std::alloc::{alloc, dealloc, handle_alloc_error, Layout};
 use std::borrow::{Borrow, Cow};
 use std::collections::HashSet;
-use std::fmt::Display;
+use std::ffi::c_void;
+use std::fmt::{self, Debug, Display};
 use std::hash::Hash;
 use std::ops::Deref;
+use std::ptr::{self, NonNull};
+use std::slice;
 
 use serde::de::{Deserializer, Visitor};
 use serde::{Deserialize, Serialize};
@@ -80,7 +84,7 @@ impl NixContext {
     /// Copies from another [NixString] its context strings
     /// in this context.
     pub fn mimic(&mut self, other: &NixString) {
-        if let Some(ref context) = other.1 {
+        if let Some(context) = other.context() {
             self.0.extend(context.iter().cloned());
         }
     }
@@ -144,9 +148,274 @@ impl NixContext {
     }
 }
 
-// FIXME: when serializing, ignore the context?
-#[derive(Clone, Debug, Serialize)]
-pub struct NixString(Box<BStr>, Option<NixContext>);
+/// This type is never instantiated, but serves to document the memory layout of the actual heap
+/// allocation for Nix strings.
+#[allow(dead_code)]
+struct NixStringInner {
+    /// The string context, if any.  Note that this is boxed to take advantage of the null pointer
+    /// niche, otherwise this field ends up being very large:
+    ///
+    /// ```notrust
+    /// >> std::mem::size_of::<Option<HashSet<String>>>()
+    /// 48
+    ///
+    /// >> std::mem::size_of::<Option<Box<HashSet<String>>>>()
+    /// 8
+    /// ```
+    context: Option<Box<NixContext>>,
+    /// The length of the data, stored *inline in the allocation*
+    length: usize,
+    /// The actual data for the string itself. Will always be `length` bytes long
+    data: [u8],
+}
+
+#[allow(clippy::zst_offset)]
+impl NixStringInner {
+    /// Construct a [`Layout`] for a nix string allocation with the given length.
+    ///
+    /// Returns a tuple of:
+    /// 1. The layout itself.
+    /// 2. The offset of [`Self::length`] within the allocation, assuming the allocation starts at 0
+    /// 3. The offset of the data array within the allocation, assuming the allocation starts at 0
+    fn layout(len: usize) -> (Layout, usize, usize) {
+        let layout = Layout::new::<Option<Box<NixContext>>>();
+        let (layout, len_offset) = layout.extend(Layout::new::<usize>()).unwrap();
+        let (layout, data_offset) = layout.extend(Layout::array::<u8>(len).unwrap()).unwrap();
+        (layout, len_offset, data_offset)
+    }
+
+    /// Returns the [`Layout`] for an *already-allocated* nix string, loading the length from the
+    /// pointer.
+    ///
+    /// Returns a tuple of:
+    /// 1. The layout itself.
+    /// 2. The offset of [`Self::length`] within the allocation, assuming the allocation starts at 0
+    /// 3. The offset of the data array within the allocation, assuming the allocation starts at 0
+    ///
+    /// # Safety
+    ///
+    /// This function must only be called on a pointer that has been properly initialized with
+    /// [`Self::alloc`]. The data buffer may not necessarily be initialized
+    unsafe fn layout_of(this: NonNull<c_void>) -> (Layout, usize, usize) {
+        let layout = Layout::new::<Option<Box<NixContext>>>();
+        let (_, len_offset) = layout.extend(Layout::new::<usize>()).unwrap();
+        // SAFETY: Layouts are linear, so even though we haven't involved data at all yet, we know
+        // the len_offset is a valid offset into the second field of the allocation
+        let len = *(this.as_ptr().add(len_offset) as *const usize);
+        Self::layout(len)
+    }
+
+    /// Allocate an *uninitialized* nix string with the given length. Writes the length to the
+    /// length value in the pointer, but leaves both context and data uninitialized
+    ///
+    /// This function is safe to call (as constructing pointers of any sort of validity is always
+    /// safe in Rust) but it is unsafe to use the resulting pointer to do anything other than
+    ///
+    /// 1. Read the length
+    /// 2. Write the context
+    /// 3. Write the data
+    ///
+    /// until the string is fully initialized
+    fn alloc(len: usize) -> NonNull<c_void> {
+        let (layout, len_offset, _data_offset) = Self::layout(len);
+        debug_assert_ne!(layout.size(), 0);
+        unsafe {
+            // SAFETY: Layout has non-zero size, since the layout of the context and the
+            // layout of the len both have non-zero size
+            let ptr = alloc(layout);
+
+            if let Some(this) = NonNull::new(ptr as *mut _) {
+                // SAFETY: We've allocated with a layout that causes the len_offset to be in-bounds
+                // and writeable, and if the allocation succeeded it won't wrap
+                ((this.as_ptr() as *mut u8).add(len_offset) as *mut usize).write(len);
+                debug_assert_eq!(Self::len(this), len);
+                this
+            } else {
+                handle_alloc_error(layout);
+            }
+        }
+    }
+
+    /// Deallocate the Nix string at the given pointer
+    ///
+    /// # Safety
+    ///
+    /// This function must only be called with a pointer that has been properly initialized with
+    /// [`Self::alloc`]
+    unsafe fn dealloc(this: NonNull<c_void>) {
+        let (layout, _, _) = Self::layout_of(this);
+        // SAFETY: okay because of the safety guarantees of this method
+        dealloc(this.as_ptr() as *mut u8, layout)
+    }
+
+    /// Return the length of the Nix string at the given pointer
+    ///
+    /// # Safety
+    ///
+    /// This function must only be called with a pointer that has been properly initialized with
+    /// [`Self::alloc`]
+    unsafe fn len(this: NonNull<c_void>) -> usize {
+        let (_, len_offset, _) = Self::layout_of(this);
+        // SAFETY: As long as the safety guarantees of this method are upheld, we've allocated with
+        // a layout that causes the len_offset to be in-bounds and writeable, and if the allocation
+        // succeeded it won't wrap
+        *(this.as_ptr().add(len_offset) as *const usize)
+    }
+
+    /// Return a pointer to the context value within the given Nix string pointer
+    ///
+    /// # Safety
+    ///
+    /// This function must only be called with a pointer that has been properly initialized with
+    /// [`Self::alloc`]
+    unsafe fn context_ptr(this: NonNull<c_void>) -> *mut Option<Box<NixContext>> {
+        // SAFETY: The context is the first field in the layout of the allocation
+        this.as_ptr() as *mut Option<Box<NixContext>>
+    }
+
+    /// Construct a shared reference to the context value within the given Nix string pointer
+    ///
+    /// # Safety
+    ///
+    /// This function must only be called with a pointer that has been properly initialized with
+    /// [`Self::alloc`], and where the context has been properly initialized (by writing to the
+    /// pointer returned from [`Self::context_ptr`]).
+    ///
+    /// Also, all the normal Rust rules about pointer-to-reference conversion apply. See
+    /// [`NonNull::as_ref`] for more.
+    unsafe fn context_ref<'a>(this: NonNull<c_void>) -> &'a Option<Box<NixContext>> {
+        Self::context_ptr(this).as_ref().unwrap()
+    }
+
+    /// Construct a mutable reference to the context value within the given Nix string pointer
+    ///
+    /// # Safety
+    ///
+    /// This function must only be called with a pointer that has been properly initialized with
+    /// [`Self::alloc`], and where the context has been properly initialized (by writing to the
+    /// pointer returned from [`Self::context_ptr`]).
+    ///
+    /// Also, all the normal Rust rules about pointer-to-reference conversion apply. See
+    /// [`NonNull::as_mut`] for more.
+    unsafe fn context_mut<'a>(this: NonNull<c_void>) -> &'a mut Option<Box<NixContext>> {
+        Self::context_ptr(this).as_mut().unwrap()
+    }
+
+    /// Return a pointer to the data array within the given Nix string pointer
+    ///
+    /// # Safety
+    ///
+    /// This function must only be called with a pointer that has been properly initialized with
+    /// [`Self::alloc`]
+    unsafe fn data_ptr(this: NonNull<c_void>) -> *mut u8 {
+        let (_, _, data_offset) = Self::layout_of(this);
+        // SAFETY: data is the third field in the layout of the allocation
+        this.as_ptr().add(data_offset) as *mut u8
+    }
+
+    /// Construct a shared reference to the data slice within the given Nix string pointer
+    ///
+    /// # Safety
+    ///
+    /// This function must only be called with a pointer that has been properly initialized with
+    /// [`Self::alloc`], and where the data array has been properly initialized (by writing to the
+    /// pointer returned from [`Self::data_ptr`]).
+    ///
+    /// Also, all the normal Rust rules about pointer-to-reference conversion apply. See
+    /// [`slice::from_raw_parts`] for more.
+    unsafe fn data_slice<'a>(this: NonNull<c_void>) -> &'a [u8] {
+        let len = Self::len(this);
+        let data = Self::data_ptr(this);
+        slice::from_raw_parts(data, len)
+    }
+
+    /// Construct a mutable reference to the data slice within the given Nix string pointer
+    ///
+    /// # Safety
+    ///
+    /// This function must only be called with a pointer that has been properly initialized with
+    /// [`Self::alloc`], and where the data array has been properly initialized (by writing to the
+    /// pointer returned from [`Self::data_ptr`]).
+    ///
+    /// Also, all the normal Rust rules about pointer-to-reference conversion apply. See
+    /// [`slice::from_raw_parts_mut`] for more.
+    #[allow(dead_code)]
+    unsafe fn data_slice_mut<'a>(this: NonNull<c_void>) -> &'a mut [u8] {
+        let len = Self::len(this);
+        let data = Self::data_ptr(this);
+        slice::from_raw_parts_mut(data, len)
+    }
+
+    /// Clone the Nix string pointed to by this pointer, and return a pointer to a new Nix string
+    /// containing the same data and context.
+    ///
+    /// # Safety
+    ///
+    /// This function must only be called with a pointer that has been properly initialized with
+    /// [`Self::alloc`], and where the context has been properly initialized (by writing to the
+    /// pointer returned from [`Self::context_ptr`]), and the data array has been properly
+    /// initialized (by writing to the pointer returned from [`Self::data_ptr`]).
+    unsafe fn clone(this: NonNull<c_void>) -> NonNull<c_void> {
+        let (layout, _, _) = Self::layout_of(this);
+        let ptr = alloc(layout);
+        if let Some(new) = NonNull::new(ptr as *mut _) {
+            ptr::copy_nonoverlapping(this.as_ptr(), new.as_ptr(), layout.size());
+            Self::context_ptr(new).write(Self::context_ref(this).clone());
+            new
+        } else {
+            handle_alloc_error(layout);
+        }
+    }
+}
+
+/// Nix string values
+///
+/// # Internals
+///
+/// For performance reasons (to keep allocations small, and to avoid indirections), [`NixString`] is
+/// represented as a single *thin* pointer to a packed data structure containing the
+/// [context][NixContext] and the string data itself (which is a raw byte array, to match the Nix
+/// string semantics that allow any array of bytes to be represented by a string).
+
+/// This memory representation is documented in [`NixStringInner`], but since Rust prefers to deal
+/// with slices via *fat pointers* (pointers that include the length in the *pointer*, not in the
+/// heap allocation), we have to do mostly manual layout management and allocation for this
+/// representation. See the documentation for the methods of [`NixStringInner`] for more information
+pub struct NixString(NonNull<c_void>);
+
+unsafe impl Send for NixString {}
+unsafe impl Sync for NixString {}
+
+impl Drop for NixString {
+    fn drop(&mut self) {
+        // SAFETY: There's no way to construct a NixString that doesn't leave the allocation correct
+        // according to the rules of dealloc
+        unsafe {
+            NixStringInner::dealloc(self.0);
+        }
+    }
+}
+
+impl Clone for NixString {
+    fn clone(&self) -> Self {
+        // SAFETY: There's no way to construct a NixString that doesn't leave the allocation correct
+        // according to the rules of clone
+        unsafe { Self(NixStringInner::clone(self.0)) }
+    }
+}
+
+impl Debug for NixString {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        if let Some(ctx) = self.context() {
+            f.debug_struct("NixString")
+                .field("context", ctx)
+                .field("data", &self.as_bstr())
+                .finish()
+        } else {
+            write!(f, "{:?}", self.as_bstr())
+        }
+    }
+}
 
 impl PartialEq for NixString {
     fn eq(&self, other: &Self) -> bool {
@@ -182,13 +451,13 @@ impl Ord for NixString {
 
 impl From<Box<BStr>> for NixString {
     fn from(value: Box<BStr>) -> Self {
-        Self(value, None)
+        Self::new(&value, None)
     }
 }
 
 impl From<BString> for NixString {
     fn from(value: BString) -> Self {
-        Self(Vec::<u8>::from(value).into_boxed_slice().into(), None)
+        Self::new(&value, None)
     }
 }
 
@@ -212,7 +481,7 @@ impl From<Vec<u8>> for NixString {
 
 impl From<Box<[u8]>> for NixString {
     fn from(value: Box<[u8]>) -> Self {
-        Self(value.into(), None)
+        Self::new(&value, None)
     }
 }
 
@@ -228,12 +497,12 @@ impl From<String> for NixString {
     }
 }
 
-impl<T> From<(T, Option<NixContext>)> for NixString
+impl<T> From<(T, Option<Box<NixContext>>)> for NixString
 where
     NixString: From<T>,
 {
-    fn from((s, ctx): (T, Option<NixContext>)) -> Self {
-        NixString(NixString::from(s).0, ctx)
+    fn from((s, ctx): (T, Option<Box<NixContext>>)) -> Self {
+        Self::new(NixString::from(s).as_ref(), ctx)
     }
 }
 
@@ -251,25 +520,19 @@ impl From<ast::Ident> for NixString {
 
 impl<'a> From<&'a NixString> for &'a BStr {
     fn from(s: &'a NixString) -> Self {
-        BStr::new(&*s.0)
-    }
-}
-
-impl From<NixString> for Box<BStr> {
-    fn from(s: NixString) -> Self {
-        s.0
+        s.as_bstr()
     }
 }
 
 impl From<NixString> for BString {
     fn from(s: NixString) -> Self {
-        s.0.to_vec().into()
+        s.as_bstr().to_owned()
     }
 }
 
 impl AsRef<[u8]> for NixString {
     fn as_ref(&self) -> &[u8] {
-        &self.0
+        self.as_bytes()
     }
 }
 
@@ -322,7 +585,7 @@ impl Deref for NixString {
     type Target = BStr;
 
     fn deref(&self) -> &Self::Target {
-        &self.0
+        self.as_bstr()
     }
 }
 
@@ -344,37 +607,61 @@ mod arbitrary {
 }
 
 impl NixString {
+    fn new(contents: &[u8], context: Option<Box<NixContext>>) -> Self {
+        // SAFETY: We're always fully initializing a NixString here:
+        //
+        // 1. NixStringInner::alloc sets up the len for us
+        // 2. We set the context, using ptr::write to make sure that the uninitialized memory isn't
+        //    read or dropped
+        // 3. We set the data, using copy_from_nonoverlapping to make sure that the uninitialized
+        //    memory isn't read or dropped
+        //
+        // Only *then* can we construct a NixString
+        unsafe {
+            let inner = NixStringInner::alloc(contents.len());
+            NixStringInner::context_ptr(inner).write(context);
+            NixStringInner::data_ptr(inner)
+                .copy_from_nonoverlapping(contents.as_ptr(), contents.len());
+            Self(inner)
+        }
+    }
+
     pub fn new_inherit_context_from<T>(other: &NixString, new_contents: T) -> Self
     where
         NixString: From<T>,
     {
-        Self(Self::from(new_contents).0, other.1.clone())
+        Self::new(
+            Self::from(new_contents).as_ref(),
+            other.context().map(|c| Box::new(c.clone())),
+        )
     }
 
     pub fn new_context_from<T>(context: NixContext, contents: T) -> Self
     where
         NixString: From<T>,
     {
-        Self(
-            Self::from(contents).0,
+        Self::new(
+            Self::from(contents).as_ref(),
             if context.is_empty() {
                 None
             } else {
-                Some(context)
+                Some(Box::new(context))
             },
         )
     }
 
     pub fn as_bstr(&self) -> &BStr {
-        self
+        BStr::new(self.as_bytes())
     }
 
     pub fn as_bytes(&self) -> &[u8] {
-        &self.0
+        // SAFETY: There's no way to construct an uninitialized NixString (see the SAFETY comment in
+        // `new`)
+        unsafe { NixStringInner::data_slice(self.0) }
     }
 
     pub fn into_bstring(self) -> BString {
-        (*self.0).to_owned()
+        self.as_bstr().to_owned()
     }
 
     /// Return a displayable representation of the string as an
@@ -409,7 +696,7 @@ impl NixString {
         let mut s = self.to_vec();
         s.extend(&(***other));
 
-        let context = [&self.1, &other.1]
+        let context = [self.context(), other.context()]
             .into_iter()
             .flatten()
             .fold(NixContext::new(), |acc_ctx, new_ctx| {
@@ -418,42 +705,59 @@ impl NixString {
         Self::new_context_from(context, s)
     }
 
+    pub(crate) fn context(&self) -> Option<&NixContext> {
+        // SAFETY: There's no way to construct an uninitialized or invalid NixString (see the SAFETY
+        // comment in `new`).
+        //
+        // Also, we're using the same lifetime and mutability as self, to fit the
+        // pointer-to-reference conversion rules
+        unsafe { NixStringInner::context_ref(self.0).as_deref() }
+    }
+
     pub(crate) fn context_mut(&mut self) -> Option<&mut NixContext> {
-        return self.1.as_mut();
+        // SAFETY: There's no way to construct an uninitialized or invalid NixString (see the SAFETY
+        // comment in `new`).
+        //
+        // Also, we're using the same lifetime and mutability as self, to fit the
+        // pointer-to-reference conversion rules
+        unsafe { NixStringInner::context_mut(self.0).as_deref_mut() }
     }
 
     pub fn iter_context(&self) -> impl Iterator<Item = &NixContext> {
-        return self.1.iter();
+        self.context().into_iter()
     }
 
     pub fn iter_plain(&self) -> impl Iterator<Item = &str> {
-        return self.1.iter().flat_map(|context| context.iter_plain());
+        self.iter_context().flat_map(|context| context.iter_plain())
     }
 
     pub fn iter_derivation(&self) -> impl Iterator<Item = &str> {
-        return self.1.iter().flat_map(|context| context.iter_derivation());
+        return self
+            .iter_context()
+            .flat_map(|context| context.iter_derivation());
     }
 
     pub fn iter_single_outputs(&self) -> impl Iterator<Item = (&str, &str)> {
         return self
-            .1
-            .iter()
+            .iter_context()
             .flat_map(|context| context.iter_single_outputs());
     }
 
     /// Returns whether this Nix string possess a context or not.
     pub fn has_context(&self) -> bool {
-        self.1.is_some()
+        self.context().is_some()
     }
 
     /// This clears the context of that string, losing
     /// all dependency tracking information.
     pub fn clear_context(&mut self) {
-        self.1 = None;
+        // SAFETY: There's no way to construct an uninitialized or invalid NixString (see the SAFETY
+        // comment in `new`).
+        *unsafe { NixStringInner::context_mut(self.0) } = None;
     }
 
     pub fn chars(&self) -> Chars<'_> {
-        self.0.chars()
+        self.as_bstr().chars()
     }
 }
 
@@ -539,13 +843,20 @@ impl Display for NixString {
 
 #[cfg(test)]
 mod tests {
+    use test_strategy::proptest;
+
     use super::*;
 
     use crate::properties::{eq_laws, hash_laws, ord_laws};
 
     #[test]
     fn size() {
-        assert_eq!(std::mem::size_of::<NixString>(), 64);
+        assert_eq!(std::mem::size_of::<NixString>(), 8);
+    }
+
+    #[proptest]
+    fn clone_strings(s: NixString) {
+        drop(s.clone())
     }
 
     eq_laws!(NixString);
diff --git a/tvix/eval/src/vm/generators.rs b/tvix/eval/src/vm/generators.rs
index 2a2710dc34af..4dacdef0dd08 100644
--- a/tvix/eval/src/vm/generators.rs
+++ b/tvix/eval/src/vm/generators.rs
@@ -607,7 +607,7 @@ pub async fn request_string_coerce(
     kind: CoercionKind,
 ) -> Result<NixString, CatchableErrorKind> {
     match val {
-        Value::String(s) => Ok(*s),
+        Value::String(s) => Ok(s),
         _ => match co.yield_(VMRequest::StringCoerce(val, kind)).await {
             VMResponse::Value(Value::Catchable(c)) => Err(*c),
             VMResponse::Value(value) => Ok(value
diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs
index 32f0d0456f14..c10b79cd992e 100644
--- a/tvix/eval/src/vm/mod.rs
+++ b/tvix/eval/src/vm/mod.rs
@@ -1000,9 +1000,7 @@ where
         }
 
         self.stack
-            .push(Value::String(Box::new(NixString::new_context_from(
-                context, out,
-            ))));
+            .push(Value::String(NixString::new_context_from(context, out)));
         Ok(())
     }
 
@@ -1263,7 +1261,7 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> {
                 Err(c) => Value::Catchable(Box::new(c)),
             }
         }
-        (Value::String(s1), Value::String(s2)) => Value::String(Box::new(s1.concat(&s2))),
+        (Value::String(s1), Value::String(s2)) => Value::String(s1.concat(&s2)),
         (Value::String(s1), v) => generators::request_string_coerce(
             &co,
             v,
@@ -1274,7 +1272,7 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> {
             },
         )
         .await
-        .map(|s2| Value::String(Box::new(s1.concat(&s2))))
+        .map(|s2| Value::String(s1.concat(&s2)))
         .into(),
         (a @ Value::Integer(_), b) | (a @ Value::Float(_), b) => arithmetic_op!(&a, &b, +)?,
         (a, b) => {
@@ -1297,7 +1295,7 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> {
             )
             .await;
             match (r1, r2) {
-                (Ok(s1), Ok(s2)) => Value::String(Box::new(s1.concat(&s2))),
+                (Ok(s1), Ok(s2)) => Value::String(s1.concat(&s2)),
                 (Err(c), _) => return Ok(Value::from(c)),
                 (_, Err(c)) => return Ok(Value::from(c)),
             }