Merge pull request #537 from theseus-rs/fix-object-clone

fix: correct java.lang.Object.clone()

Author: brianheineman

ristretto_classloader/src/object.rs

@@ -306,22 +306,6 @@ impl Object {
         std::ptr::eq(self, other)
     }
 
-    /// Deep clone the object.
-    ///
-    /// # Errors
-    ///
-    /// if the fields cannot be cloned.
-    pub fn deep_clone(&self) -> Result<Self> {
-        let mut values = Vec::with_capacity(self.values.len());
-        for value in &self.values {
-            values.push(value.deep_clone()?);
-        }
-        Ok(Self {
-            class: self.class.clone(),
-            values,
-        })
-    }
-
     /// Convert the object to a bool value.
     ///
     /// # Errors
@@ -747,20 +731,6 @@ mod tests {
         Ok(())
     }
 
-    #[tokio::test]
-    async fn test_deep_clone() -> Result<()> {
-        let class = load_class("java.lang.Integer").await?;
-        let mut object = Object::new(class)?;
-        object.set_value("value", Value::Int(1))?;
-        let mut clone = object.deep_clone()?;
-        assert_eq!(object, clone);
-        assert!(!object.ptr_eq(&clone));
-
-        clone.set_value("value", Value::Int(2))?;
-        assert_ne!(object, clone);
-        Ok(())
-    }
-
     #[tokio::test]
     async fn test_eq_same_references() -> Result<()> {
         let class = load_class("java.lang.Integer").await?;

ristretto_classloader/src/reference.rs

@@ -331,70 +331,6 @@ impl Reference {
         }
     }
 
-    /// Returns a deep clone of the reference.
-    ///
-    /// # Errors
-    ///
-    /// if the reference cannot be cloned.
-    pub fn deep_clone(&self) -> Result<Reference> {
-        let value = match self {
-            Reference::ByteArray(array) => {
-                let array = array.read();
-                Reference::ByteArray(Gc::new(RwLock::new(array.clone())))
-            }
-            Reference::CharArray(array) => {
-                let array = array.read();
-                Reference::CharArray(Gc::new(RwLock::new(array.to_vec())))
-            }
-            Reference::ShortArray(array) => {
-                let array = array.read();
-                Reference::ShortArray(Gc::new(RwLock::new(array.to_vec())))
-            }
-            Reference::IntArray(array) => {
-                let array = array.read();
-                Reference::IntArray(Gc::new(RwLock::new(array.to_vec())))
-            }
-            Reference::LongArray(array) => {
-                let array = array.read();
-                Reference::LongArray(Gc::new(RwLock::new(array.to_vec())))
-            }
-            Reference::FloatArray(array) => {
-                let array = array.read();
-                Reference::FloatArray(Gc::new(RwLock::new(array.to_vec())))
-            }
-            Reference::DoubleArray(array) => {
-                let array = array.read();
-                Reference::DoubleArray(Gc::new(RwLock::new(array.to_vec())))
-            }
-            Reference::Array(object_array) => {
-                let array = object_array.elements.read();
-                let array = array.to_vec();
-                let mut cloned_values = Vec::with_capacity(array.len());
-                for value in array {
-                    match value {
-                        Some(reference) => cloned_values.push(Some(reference.deep_clone()?)),
-                        None => cloned_values.push(value),
-                    }
-                }
-                let object_array = ObjectArray {
-                    class: object_array.class.clone(),
-                    elements: Gc::new(RwLock::new(cloned_values)),
-                };
-                Reference::Array(object_array)
-            }
-            Reference::Object(object) => {
-                let object = object.read();
-                if object.class().name() == "java/lang/Class" {
-                    // Special case for Class objects, which should not be deep cloned.
-                    self.clone()
-                } else {
-                    Reference::Object(Gc::new(RwLock::new(object.deep_clone()?)))
-                }
-            }
-        };
-        Ok(value)
-    }
-
     /// Convert the object to a bool value.
     ///
     /// # Errors
@@ -1369,22 +1305,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn test_deep_clone_byte_array() -> Result<()> {
-        let reference = Reference::from(vec![1i8]);
-        let clone = reference.deep_clone()?;
-        assert_eq!(reference, clone);
-
-        {
-            let mut array = clone.as_byte_vec_mut()?;
-            if let Some(element) = array.get_mut(0) {
-                *element = 2;
-            }
-        }
-        assert_ne!(reference, clone);
-        Ok(())
-    }
-
     #[test]
     fn test_hash_code_char_array() {
         let reference = Reference::from(vec![1 as char]);
@@ -1430,22 +1350,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn test_deep_clone_char_array() -> Result<()> {
-        let reference = Reference::from(vec![1 as char]);
-        let clone = reference.deep_clone()?;
-        assert_eq!(reference, clone);
-
-        {
-            let mut array = clone.as_char_vec_mut()?;
-            if let Some(element) = array.get_mut(0) {
-                *element = 2;
-            }
-        }
-        assert_ne!(reference, clone);
-        Ok(())
-    }
-
     #[test]
     fn test_hash_code_short_array() {
         let reference = Reference::from(vec![1i16]);
@@ -1491,22 +1395,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn test_deep_clone_short_array() -> Result<()> {
-        let reference = Reference::from(vec![1i16]);
-        let clone = reference.deep_clone()?;
-        assert_eq!(reference, clone);
-
-        {
-            let mut array = clone.as_short_vec_mut()?;
-            if let Some(element) = array.get_mut(0) {
-                *element = 2;
-            }
-        }
-        assert_ne!(reference, clone);
-        Ok(())
-    }
-
     #[test]
     fn test_hash_code_int_array() {
         let reference = Reference::from(vec![1i32]);
@@ -1552,22 +1440,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn test_deep_clone_int_array() -> Result<()> {
-        let reference = Reference::from(vec![1i32]);
-        let clone = reference.deep_clone()?;
-        assert_eq!(reference, clone);
-
-        {
-            let mut array = clone.as_int_vec_mut()?;
-            if let Some(element) = array.get_mut(0) {
-                *element = 2;
-            }
-        }
-        assert_ne!(reference, clone);
-        Ok(())
-    }
-
     #[test]
     fn test_hash_code_long_array() {
         let reference = Reference::from(vec![1i64]);
@@ -1613,22 +1485,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn test_deep_clone_long_array() -> Result<()> {
-        let reference = Reference::from(vec![1i64]);
-        let clone = reference.deep_clone()?;
-        assert_eq!(reference, clone);
-
-        {
-            let mut array = clone.as_long_vec_mut()?;
-            if let Some(element) = array.get_mut(0) {
-                *element = 2;
-            }
-        }
-        assert_ne!(reference, clone);
-        Ok(())
-    }
-
     #[test]
     fn test_hash_code_float_array() {
         let reference = Reference::from(vec![1.0f32]);
@@ -1674,22 +1530,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn test_deep_clone_float_array() -> Result<()> {
-        let reference = Reference::from(vec![1.0f32]);
-        let clone = reference.deep_clone()?;
-        assert_eq!(reference, clone);
-
-        {
-            let mut array = clone.as_float_vec_mut()?;
-            if let Some(element) = array.get_mut(0) {
-                *element = 2.0;
-            }
-        }
-        assert_ne!(reference, clone);
-        Ok(())
-    }
-
     #[test]
     fn test_hash_code_double_array() {
         let reference = Reference::from(vec![1.0f64]);
@@ -1735,22 +1575,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn test_deep_clone_double_array() -> Result<()> {
-        let reference = Reference::from(vec![1.0f64]);
-        let clone = reference.deep_clone()?;
-        assert_eq!(reference, clone);
-
-        {
-            let mut array = clone.as_double_vec_mut()?;
-            if let Some(element) = array.get_mut(0) {
-                *element = 2.0;
-            }
-        }
-        assert_ne!(reference, clone);
-        Ok(())
-    }
-
     #[tokio::test]
     async fn test_hash_code_reference_array() -> Result<()> {
         let class = load_class("java.lang.Object").await?;
@@ -1803,23 +1627,6 @@ mod tests {
         Ok(())
     }
 
-    #[tokio::test]
-    async fn test_deep_clone_reference_array() -> Result<()> {
-        let class = load_class("java.lang.Object").await?;
-        let reference = Reference::from((class.clone(), vec![None]));
-        let clone = reference.deep_clone()?;
-        assert_eq!(reference, clone);
-
-        {
-            let (_class, mut array) = clone.as_class_vec_mut()?;
-            if let Some(element) = array.get_mut(0) {
-                *element = Some(Reference::from(vec![1i8]));
-            }
-        }
-        assert_ne!(reference, clone);
-        Ok(())
-    }
-
     #[tokio::test]
     async fn test_hash_code_object() -> Result<()> {
         let class = load_class("java.lang.Object").await?;
@@ -1870,23 +1677,6 @@ mod tests {
         Ok(())
     }
 
-    #[tokio::test]
-    async fn test_deep_clone_object() -> Result<()> {
-        let class = load_class("java.lang.Integer").await?;
-        let mut object = Object::new(class)?;
-        object.set_value("value", Value::Int(1))?;
-        let reference = Reference::from(object);
-        let clone = reference.deep_clone()?;
-        assert_eq!(reference, clone);
-
-        {
-            let mut cloned_object = clone.as_object_mut()?;
-            cloned_object.set_value("value", Value::Int(2))?;
-        }
-        assert_ne!(reference, clone);
-        Ok(())
-    }
-
     #[tokio::test]
     async fn test_array_eq() -> Result<()> {
         let class = load_class("java.lang.Object").await?;

ristretto_classloader/src/value.rs

@@ -245,20 +245,6 @@ impl Value {
         matches!(self, Value::Long(_) | Value::Double(_))
     }
 
-    /// Returns a deep clone of the value.
-    ///
-    /// # Errors
-    ///
-    /// if the value cannot be cloned.
-    pub fn deep_clone(&self) -> Result<Self> {
-        let value = match self {
-            Value::Object(Some(reference)) => Value::Object(Some(reference.deep_clone()?)),
-            _ => self.clone(),
-        };
-
-        Ok(value)
-    }
-
     /// Returns the reference to `Vec<i8>`.
     ///
     /// # Errors
@@ -888,26 +874,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn test_deep_clone() -> Result<()> {
-        let value = Value::from(vec![1i32]);
-        let clone = value.deep_clone()?;
-        assert_eq!(value, clone);
-
-        let Value::Object(Some(ref reference)) = clone else {
-            unreachable!("Expected an IntArray reference");
-        };
-
-        {
-            let mut array = reference.as_int_vec_mut()?;
-            if let Some(element) = array.get_mut(0) {
-                *element = 2;
-            }
-        }
-        assert_ne!(value, clone);
-        Ok(())
-    }
-
     #[test]
     fn test_hash_int() {
         let mut hasher = DefaultHasher::new();