Merge pull request #224 from theseus-rs/remove-tryinto-clone

fix: remove unnecessary clone() for Value::TryInto and Reference::TryInto
theseus-rs · Jan 20, 2025 · 9799c75 · 9799c75
2 parents 5bdfce6 + e809721
commit 9799c75
Show file tree

Hide file tree

Showing 11 changed files with 172 additions and 60 deletions.
diff --git a/ristretto_classloader/src/reference.rs b/ristretto_classloader/src/reference.rs
@@ -154,7 +154,7 @@ impl Reference {
     pub fn to_object(&self) -> Result<Object> {
         match self {
             Reference::Object(object) => Ok(object.clone()),
-            _ => Err(InvalidValueType("Expected array".to_string())),
+            _ => Err(InvalidValueType("Expected object".to_string())),
         }
     }
 
@@ -491,135 +491,159 @@ impl TryInto<bool> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<bool> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<char> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<char> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<i8> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<i8> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<u8> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<u8> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<i16> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<i16> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<u16> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<u16> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<i32> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<i32> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<u32> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<u32> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<i64> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<i64> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<u64> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<u64> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<isize> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<isize> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<usize> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<usize> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<f32> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<f32> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<f64> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<f64> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<Object> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<Object> {
-        self.to_object()
+        match self {
+            Reference::Object(object) => Ok(object),
+            _ => Err(InvalidValueType("Expected object".to_string())),
+        }
     }
 }
 
 impl TryInto<String> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<String> {
-        self.to_object()?.try_into()
+        let object: Object = self.try_into()?;
+        object.try_into()
     }
 }
 
 impl TryInto<Arc<Class>> for Reference {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<Arc<Class>> {
-        self.class()
+        match self {
+            Reference::Object(object) => {
+                let class: Arc<Class> = object.try_into()?;
+                Ok(class)
+            }
+            _ => self.class(),
+        }
     }
 }
 
@@ -1732,6 +1756,15 @@ mod tests {
         Ok(())
     }
 
+    #[tokio::test]
+    async fn test_try_into_object_error() -> Result<()> {
+        let array = vec![42];
+        let reference = Reference::from(array);
+        let result: Result<Object> = reference.try_into();
+        assert!(matches!(result, Err(InvalidValueType(_))));
+        Ok(())
+    }
+
     #[expect(clippy::cast_possible_wrap)]
     #[tokio::test]
     async fn test_try_into_string() -> Result<()> {
@@ -1756,4 +1789,13 @@ mod tests {
         assert_eq!("java/lang/Integer", value.name());
         Ok(())
     }
+
+    #[tokio::test]
+    async fn test_try_into_class_array() -> Result<()> {
+        let array = vec![42];
+        let reference = Reference::from(array);
+        let value: Arc<Class> = reference.try_into()?;
+        assert_eq!("[I", value.name());
+        Ok(())
+    }
 }
diff --git a/ristretto_classloader/src/value.rs b/ristretto_classloader/src/value.rs
@@ -625,8 +625,10 @@ impl TryInto<Reference> for Value {
     type Error = crate::Error;
 
     fn try_into(self) -> Result<Reference> {
-        self.to_reference()?
-            .ok_or(InvalidValueType("Expected a reference value".to_string()))
+        match self {
+            Value::Object(Some(reference)) => Ok(reference),
+            _ => Err(InvalidValueType("Expected a reference value".to_string())),
+        }
     }
 }
 

diff --git a/ristretto_vm/src/native_methods/com/sun/java/util/jar/pack/nativeunpack.rs b/ristretto_vm/src/native_methods/com/sun/java/util/jar/pack/nativeunpack.rs
@@ -118,6 +118,14 @@ mod tests {
         let _ = get_unused_input(thread, Arguments::default()).await;
     }
 
+    #[tokio::test]
+    async fn test_init_ids() -> Result<()> {
+        let (_vm, thread) = crate::test::thread().await.expect("thread");
+        let result = init_ids(thread, Arguments::default()).await?;
+        assert_eq!(result, None);
+        Ok(())
+    }
+
     #[tokio::test]
     #[should_panic(
         expected = "not yet implemented: com.sun.java.util.jar.pack.NativeUnpack.setOption(Ljava/lang/String;Ljava/lang/String;)Z"

diff --git a/ristretto_vm/src/native_methods/java/io/filedescriptor.rs b/ristretto_vm/src/native_methods/java/io/filedescriptor.rs
@@ -65,13 +65,13 @@ async fn init_ids(_thread: Arc<Thread>, _arguments: Arguments) -> Result<Option<
 }
 
 #[async_recursion(?Send)]
-async fn sync(_thread: Arc<Thread>, _arguments: Arguments) -> Result<Option<Value>> {
-    todo!("java.io.FileDescriptor.sync()V")
+async fn sync(thread: Arc<Thread>, arguments: Arguments) -> Result<Option<Value>> {
+    sync_0(thread, arguments).await
 }
 
 #[async_recursion(?Send)]
-async fn sync_0(thread: Arc<Thread>, arguments: Arguments) -> Result<Option<Value>> {
-    sync(thread, arguments).await
+async fn sync_0(_thread: Arc<Thread>, _arguments: Arguments) -> Result<Option<Value>> {
+    todo!("java.io.FileDescriptor.sync0()V")
 }
 
 #[cfg(test)]
@@ -116,9 +116,16 @@ mod tests {
     }
 
     #[tokio::test]
-    #[should_panic(expected = "not yet implemented: java.io.FileDescriptor.sync()V")]
+    #[should_panic(expected = "not yet implemented: java.io.FileDescriptor.sync0()V")]
     async fn test_sync() {
         let (_vm, thread) = crate::test::thread().await.expect("thread");
         let _ = sync(thread, Arguments::default()).await;
     }
+
+    #[tokio::test]
+    #[should_panic(expected = "not yet implemented: java.io.FileDescriptor.sync0()V")]
+    async fn test_sync_0() {
+        let (_vm, thread) = crate::test::thread().await.expect("thread");
+        let _ = sync_0(thread, Arguments::default()).await;
+    }
 }
diff --git a/ristretto_vm/src/native_methods/java/io/randomaccessfile.rs b/ristretto_vm/src/native_methods/java/io/randomaccessfile.rs
@@ -113,6 +113,13 @@ async fn write_bytes_0(_thread: Arc<Thread>, _arguments: Arguments) -> Result<Op
 mod tests {
     use super::*;
 
+    #[tokio::test]
+    #[should_panic(expected = "not yet implemented: java.io.RandomAccessFile.close0()V")]
+    async fn test_close_0() {
+        let (_vm, thread) = crate::test::thread().await.expect("thread");
+        let _ = close_0(thread, Arguments::default()).await;
+    }
+
     #[tokio::test]
     #[should_panic(expected = "not yet implemented: java.io.RandomAccessFile.length()J")]
     async fn test_length() {