no in-place mutation for str and bytes (#231)

Author: davidhewitt

crates/monty/src/bytecode/vm/binary.rs

@@ -223,7 +223,7 @@ impl<T: ResourceTracker> VM<'_, '_, T> {
         let (lhs, this) = lhs_guard.as_parts_mut();
 
         // Try in-place operation first (for mutable types like lists)
-        if lhs.py_iadd(rhs.clone_with_heap(this.heap), this.heap, lhs.ref_id(), this.interns)? {
+        if lhs.py_iadd(rhs, this.heap, lhs.ref_id(), this.interns)? {
             // In-place operation succeeded - push lhs back
             let (lhs, this) = lhs_guard.into_parts();
             this.push(lhs);

crates/monty/src/heap.rs

@@ -589,22 +589,15 @@ impl PyTrait for HeapData {
 
     fn py_iadd(
         &mut self,
-        other: Value,
+        other: &Value,
         heap: &mut Heap<impl ResourceTracker>,
         self_id: Option<HeapId>,
         interns: &Interns,
     ) -> Result<bool, crate::resource::ResourceError> {
         match self {
-            Self::Str(s) => s.py_iadd(other, heap, self_id, interns),
-            Self::Bytes(b) => b.py_iadd(other, heap, self_id, interns),
             Self::List(l) => l.py_iadd(other, heap, self_id, interns),
-            Self::Tuple(t) => t.py_iadd(other, heap, self_id, interns),
             Self::Dict(d) => d.py_iadd(other, heap, self_id, interns),
-            _ => {
-                // Drop other if it's a Ref (ensure proper refcounting for unsupported types)
-                other.drop_with_heap(heap);
-                Ok(false)
-            }
+            _ => Ok(false),
         }
     }
 

crates/monty/src/heap_data.rs

@@ -618,27 +618,6 @@ impl PyTrait for HeapDataMut<'_> {
         }
     }
 
-    fn py_iadd(
-        &mut self,
-        other: Value,
-        heap: &mut Heap<impl ResourceTracker>,
-        self_id: Option<HeapId>,
-        interns: &Interns,
-    ) -> Result<bool, crate::resource::ResourceError> {
-        match self {
-            Self::Str(s) => s.py_iadd(other, heap, self_id, interns),
-            Self::Bytes(b) => b.py_iadd(other, heap, self_id, interns),
-            Self::List(l) => l.py_iadd(other, heap, self_id, interns),
-            Self::Tuple(t) => t.py_iadd(other, heap, self_id, interns),
-            Self::Dict(d) => d.py_iadd(other, heap, self_id, interns),
-            _ => {
-                // Drop other if it's a Ref (ensure proper refcounting for unsupported types)
-                other.drop_with_heap(heap);
-                Ok(false)
-            }
-        }
-    }
-
     fn py_call_attr(
         &mut self,
         self_id: HeapId,

crates/monty/src/types/bytes.rs

@@ -177,11 +177,6 @@ impl Bytes {
         &self.0
     }
 
-    /// Returns a mutable reference to the inner byte vector.
-    pub fn as_vec_mut(&mut self) -> &mut Vec<u8> {
-        &mut self.0
-    }
-
     /// Creates bytes from the `bytes()` constructor call.
     ///
     /// - `bytes()` with no args returns empty bytes

crates/monty/src/types/list.rs

@@ -366,13 +366,13 @@ impl PyTrait for List {
 
     fn py_iadd(
         &mut self,
-        other: Value,
+        other: &Value,
         heap: &mut Heap<impl ResourceTracker>,
         self_id: Option<HeapId>,
         _interns: &Interns,
     ) -> Result<bool, crate::resource::ResourceError> {
         // Extract the value ID first, keeping `other` around to drop later
-        let Value::Ref(other_id) = &other else { return Ok(false) };
+        let Value::Ref(other_id) = other else { return Ok(false) };
 
         if Some(*other_id) == self_id {
             // Self-extend: clone our own items with proper refcounting
@@ -408,8 +408,6 @@ impl PyTrait for List {
             }
         }
 
-        // Drop the other value - we've extracted its contents and are done with the temporary reference
-        other.drop_with_heap(heap);
         Ok(true)
     }
 

crates/monty/src/types/py_trait.rs

@@ -241,13 +241,11 @@ pub trait PyTrait {
     /// The `interns` parameter provides access to interned string content for InternString/InternBytes.
     fn py_iadd(
         &mut self,
-        other: Value,
-        heap: &mut Heap<impl ResourceTracker>,
+        _other: &Value,
+        _heap: &mut Heap<impl ResourceTracker>,
         _self_id: Option<HeapId>,
         _interns: &Interns,
     ) -> Result<bool, ResourceError> {
-        // Drop other if it's a Ref (ensure proper refcounting for unsupported types)
-        other.drop_with_heap(heap);
         Ok(false)
     }
 

crates/monty/src/types/str.rs

@@ -26,13 +26,13 @@ use crate::{
 /// Wraps a Rust `String` and provides Python-compatible operations.
 /// `len()` returns the number of Unicode codepoints (characters), matching Python semantics.
 #[derive(Debug, Clone, PartialEq, Default, serde::Serialize, serde::Deserialize)]
-pub(crate) struct Str(String);
+pub(crate) struct Str(Box<str>);
 
 impl Str {
     /// Creates a new Str from a Rust String.
     #[must_use]
     pub fn new(s: String) -> Self {
-        Self(s)
+        Self(s.into())
     }
 
     /// Returns a reference to the inner string.
@@ -41,11 +41,6 @@ impl Str {
         &self.0
     }
 
-    /// Returns a mutable reference to the inner string.
-    pub fn as_string_mut(&mut self) -> &mut String {
-        &mut self.0
-    }
-
     /// Creates a string from the `str()` constructor call.
     ///
     /// - `str()` with no args returns an empty string
@@ -81,19 +76,19 @@ impl Str {
 
 impl From<String> for Str {
     fn from(s: String) -> Self {
-        Self(s)
+        Self(s.into())
     }
 }
 
 impl From<&str> for Str {
     fn from(s: &str) -> Self {
-        Self(s.to_string())
+        Self(s.into())
     }
 }
 
 impl From<Str> for String {
     fn from(value: Str) -> Self {
-        value.0
+        value.0.into_string()
     }
 }
 
@@ -202,7 +197,7 @@ pub(crate) fn get_str_slice(s: &str, start: usize, stop: usize, step: i64) -> St
 }
 
 impl std::ops::Deref for Str {
-    type Target = String;
+    type Target = str;
 
     fn deref(&self) -> &Self::Target {
         &self.0
@@ -270,7 +265,7 @@ impl PyTrait for Str {
     }
 
     fn py_str(&self, _heap: &Heap<impl ResourceTracker>, _interns: &Interns) -> Cow<'static, str> {
-        self.0.clone().into()
+        self.0.clone().into_string().into()
     }
 
     fn py_add(
@@ -284,35 +279,6 @@ impl PyTrait for Str {
         Ok(Some(Value::Ref(id)))
     }
 
-    fn py_iadd(
-        &mut self,
-        other: Value,
-        heap: &mut Heap<impl ResourceTracker>,
-        self_id: Option<HeapId>,
-        interns: &Interns,
-    ) -> Result<bool, crate::resource::ResourceError> {
-        match &other {
-            Value::Ref(other_id) => {
-                if Some(*other_id) == self_id {
-                    let rhs = self.0.clone();
-                    self.0.push_str(&rhs);
-                } else if let HeapData::Str(rhs) = heap.get(*other_id) {
-                    self.0.push_str(rhs.as_str());
-                } else {
-                    return Ok(false);
-                }
-                // Drop the other value - we've consumed it
-                other.drop_with_heap(heap);
-                Ok(true)
-            }
-            Value::InternString(string_id) => {
-                self.0.push_str(interns.get_str(*string_id));
-                Ok(true)
-            }
-            _ => Ok(false),
-        }
-    }
-
     fn py_call_attr(
         &mut self,
         _self_id: HeapId,

crates/monty/src/value.rs

@@ -642,12 +642,12 @@ impl PyTrait for Value {
 
     fn py_iadd(
         &mut self,
-        other: Self,
+        other: &Self,
         heap: &mut Heap<impl ResourceTracker>,
         _self_id: Option<HeapId>,
         interns: &Interns,
     ) -> Result<bool, crate::resource::ResourceError> {
-        match (&self, &other) {
+        match (&self, other) {
             (Self::Int(v1), Self::Int(v2)) => {
                 if let Some(result) = v1.checked_add(*v2) {
                     *self = Self::Int(result);
@@ -675,18 +675,8 @@ impl PyTrait for Value {
                 } else {
                     false
                 };
-                // Drop the other value - we've consumed it
-                other.drop_with_heap(heap);
                 Ok(result)
             }
-            (Self::Ref(id1), Self::InternString(string_id)) => {
-                if let HeapDataMut::Str(s1) = heap.get_mut(*id1) {
-                    s1.as_string_mut().push_str(interns.get_str(*string_id));
-                    Ok(true)
-                } else {
-                    Ok(false)
-                }
-            }
             // same for bytes
             (Self::InternBytes(b1), Self::InternBytes(b2)) => {
                 let bytes1 = interns.get_bytes(*b1);
@@ -708,26 +698,12 @@ impl PyTrait for Value {
                 } else {
                     false
                 };
-                // Drop the other value - we've consumed it
-                other.drop_with_heap(heap);
                 Ok(result)
             }
-            (Self::Ref(id1), Self::InternBytes(bytes_id)) => {
-                if let HeapDataMut::Bytes(b1) = heap.get_mut(*id1) {
-                    b1.as_vec_mut().extend_from_slice(interns.get_bytes(*bytes_id));
-                    Ok(true)
-                } else {
-                    Ok(false)
-                }
-            }
             (Self::Ref(id), Self::Ref(_)) => {
                 heap.with_entry_mut(*id, |heap, mut data| data.py_iadd(other, heap, Some(*id), interns))
             }
-            _ => {
-                // Drop other if it's a Ref (ensure proper refcounting for unsupported type combinations)
-                other.drop_with_heap(heap);
-                Ok(false)
-            }
+            _ => Ok(false),
         }
     }