Remove unsafe_op_in_unsafe_fn from vortex-ffi (#3243)

joseph-isaacs · web-flow · commit 526fb1f3618e · 2025-05-07T19:13:21.000+01:00
We should remove this lint ignore as per https://doc.rust-lang.org/edition-guide/rust-2024/unsafe-op-in-unsafe-fn.html
diff --git a/vortex-ffi/src/array.rs b/vortex-ffi/src/array.rs
@@ -25,7 +25,10 @@ pub struct vx_array {
 /// Get the length of the array.
 #[unsafe(no_mangle)]
 pub unsafe extern "C-unwind" fn vx_array_len(array: *const vx_array) -> u64 {
-    array.as_ref().vortex_expect("array null").inner.len() as u64
+    unsafe { array.as_ref() }
+        .vortex_expect("array null")
+        .inner
+        .len() as u64
 }
 
 /// Get a pointer to the data type for an array.
@@ -34,7 +37,10 @@ pub unsafe extern "C-unwind" fn vx_array_len(array: *const vx_array) -> u64 {
 /// for ensuring that it is never dereferenced after the array has been freed.
 #[unsafe(no_mangle)]
 pub unsafe extern "C-unwind" fn vx_array_dtype(array: *const vx_array) -> *const DType {
-    array.as_ref().vortex_expect("array null").inner.dtype()
+    unsafe { array.as_ref() }
+        .vortex_expect("array null")
+        .inner
+        .dtype()
 }
 
 #[unsafe(no_mangle)]
@@ -44,7 +50,7 @@ pub unsafe extern "C-unwind" fn vx_array_get_field(
     error: *mut *mut vx_error,
 ) -> *const vx_array {
     try_or(error, ptr::null(), || {
-        let array = array.as_ref().vortex_expect("array null");
+        let array = unsafe { array.as_ref() }.vortex_expect("array null");
 
         let field_array = array
             .inner
@@ -75,7 +81,7 @@ pub unsafe extern "C-unwind" fn vx_array_slice(
     stop: u32,
     error: *mut *mut vx_error,
 ) -> *const vx_array {
-    let array = array.as_ref().vortex_expect("array null");
+    let array = unsafe { array.as_ref() }.vortex_expect("array null");
     try_or(error, ptr::null_mut(), || {
         let sliced = array.inner.as_ref().slice(start as usize, stop as usize)?;
         Ok(Box::into_raw(Box::new(vx_array { inner: sliced })))
@@ -88,7 +94,7 @@ pub unsafe extern "C-unwind" fn vx_array_is_null(
     index: u32,
     error: *mut *mut vx_error,
 ) -> bool {
-    let array = array.as_ref().vortex_expect("array null");
+    let array = unsafe { array.as_ref() }.vortex_expect("array null");
     try_or(error, false, || array.inner.is_invalid(index as usize))
 }
 
@@ -97,7 +103,7 @@ pub unsafe extern "C-unwind" fn vx_array_null_count(
     array: *const vx_array,
     error: *mut *mut vx_error,
 ) -> u32 {
-    let array = array.as_ref().vortex_expect("array null");
+    let array = unsafe { array.as_ref() }.vortex_expect("array null");
     try_or(error, 0, || {
         Ok(array.inner.as_ref().invalid_count()?.try_into()?)
     })
@@ -108,7 +114,7 @@ macro_rules! ffiarray_get_ptype {
         paste::paste! {
             #[unsafe(no_mangle)]
             pub unsafe extern "C-unwind" fn [<vx_array_get_ $ptype>](array: *const vx_array, index: u32) -> $ptype {
-                let array = array.as_ref().vortex_expect("array null");
+                let array = unsafe { array.as_ref() } .vortex_expect("array null");
                 let value = array.inner.scalar_at(index as usize).vortex_expect("scalar_at");
                 value.as_primitive()
                     .as_::<$ptype>()
@@ -118,7 +124,7 @@ macro_rules! ffiarray_get_ptype {
 
             #[unsafe(no_mangle)]
             pub unsafe extern "C-unwind" fn [<vx_array_get_storage_ $ptype>](array: *const vx_array, index: u32) -> $ptype {
-                let array = array.as_ref().vortex_expect("array null");
+                let array = unsafe { array.as_ref() }.vortex_expect("array null");
                 let value = array.inner.scalar_at(index as usize).vortex_expect("scalar_at");
                 value.as_extension()
                     .storage()
@@ -152,7 +158,7 @@ pub unsafe extern "C-unwind" fn vx_array_get_utf8(
     dst: *mut c_void,
     len: *mut c_int,
 ) {
-    let array = array.as_ref().vortex_expect("array null");
+    let array = unsafe { array.as_ref() }.vortex_expect("array null");
     let value = array
         .inner
         .as_ref()
@@ -161,9 +167,9 @@ pub unsafe extern "C-unwind" fn vx_array_get_utf8(
     let utf8_scalar = value.as_utf8();
     if let Some(buffer) = utf8_scalar.value() {
         let bytes = buffer.as_bytes();
-        let dst = std::slice::from_raw_parts_mut(dst as *mut u8, bytes.len());
+        let dst = unsafe { std::slice::from_raw_parts_mut(dst as *mut u8, bytes.len()) };
         dst.copy_from_slice(bytes);
-        *len = bytes.len().try_into().vortex_unwrap();
+        unsafe { *len = bytes.len().try_into().vortex_unwrap() };
     }
 }
 
@@ -176,16 +182,16 @@ pub unsafe extern "C-unwind" fn vx_array_get_binary(
     dst: *mut c_void,
     len: *mut c_int,
 ) {
-    let array = array.as_ref().vortex_expect("array null");
+    let array = unsafe { array.as_ref() }.vortex_expect("array null");
     let value = array
         .inner
         .scalar_at(index as usize)
         .vortex_expect("scalar_at");
     let utf8_scalar = value.as_binary();
     if let Some(bytes) = utf8_scalar.value() {
-        let dst = std::slice::from_raw_parts_mut(dst as *mut u8, bytes.len());
+        let dst = unsafe { std::slice::from_raw_parts_mut(dst as *mut u8, bytes.len()) };
         dst.copy_from_slice(&bytes);
-        *len = bytes.len().try_into().vortex_unwrap();
+        unsafe { *len = bytes.len().try_into().vortex_unwrap() };
     }
 }
 
@@ -203,20 +209,20 @@ mod tests {
     fn test_simple() {
         unsafe {
             let primitive = PrimitiveArray::new(buffer![1i32, 2i32, 3i32], Validity::NonNullable);
-            let ffi_array = Box::new(vx_array {
+            let vx_array = Box::new(vx_array {
                 inner: primitive.to_array(),
             });
 
-            assert_eq!(vx_array_len(&*ffi_array), 3);
+            assert_eq!(vx_array_len(&*vx_array), 3);
 
-            let array_dtype = vx_array_dtype(&*ffi_array);
+            let array_dtype = vx_array_dtype(&*vx_array);
             assert_eq!(vx_dtype_get(array_dtype), DTYPE_PRIMITIVE_I32);
 
-            assert_eq!(vx_array_get_i32(&*ffi_array, 0), 1);
-            assert_eq!(vx_array_get_i32(&*ffi_array, 1), 2);
-            assert_eq!(vx_array_get_i32(&*ffi_array, 2), 3);
+            assert_eq!(vx_array_get_i32(&*vx_array, 0), 1);
+            assert_eq!(vx_array_get_i32(&*vx_array, 1), 2);
+            assert_eq!(vx_array_get_i32(&*vx_array, 2), 3);
 
-            vx_array_free(Box::into_raw(ffi_array));
+            vx_array_free(Box::into_raw(vx_array));
         }
     }
 }
diff --git a/vortex-ffi/src/dtype.rs b/vortex-ffi/src/dtype.rs
@@ -1,3 +1,6 @@
+// TODO(joe): remove me
+#![allow(clippy::panic)]
+
 use std::ffi::{CStr, c_char, c_int, c_void};
 use std::ptr;
 use std::sync::Arc;
@@ -51,10 +54,8 @@ pub unsafe extern "C-unwind" fn vx_dtype_new_list(
     element: *mut DType,
     nullable: bool,
 ) -> *mut DType {
-    assert!(!element.is_null(), "DType_new_list: null ptr");
-
-    let element_type = *Box::from_raw(element);
-    let element_dtype = Arc::new(element_type);
+    let element_type = unsafe { element.as_ref() }.vortex_expect("null dtype");
+    let element_dtype = Arc::new(element_type.clone());
     let list_dtype = DType::List(element_dtype, nullable.into());
 
     Box::into_raw(Box::new(list_dtype))
@@ -72,13 +73,15 @@ pub unsafe extern "C-unwind" fn vx_dtype_new_struct(
     let mut field_dtypes = Vec::with_capacity(len as usize);
 
     for i in 0..len {
-        let name_ptr = *names.add(i as usize);
-        let name: Arc<str> = CStr::from_ptr(name_ptr).to_string_lossy().into();
-        let dtype = Box::from_raw(*dtypes.add(i as usize));
-        let dtype = *dtype;
+        unsafe {
+            let name_ptr = *names.add(i as usize);
+            let name: Arc<str> = CStr::from_ptr(name_ptr).to_string_lossy().into();
+            let dtype = Box::from_raw(*dtypes.add(i as usize));
+            let dtype = *dtype;
 
-        rust_names.push(name);
-        field_dtypes.push(dtype);
+            rust_names.push(name);
+            field_dtypes.push(dtype);
+        }
     }
 
     let field_names = FieldNames::from(rust_names);
@@ -90,13 +93,13 @@ pub unsafe extern "C-unwind" fn vx_dtype_new_struct(
 /// Free an [`DType`] and all associated resources.
 #[unsafe(no_mangle)]
 pub unsafe extern "C-unwind" fn vx_dtype_free(dtype: *mut DType) {
-    drop(Box::from_raw(dtype));
+    drop(unsafe { Box::from_raw(dtype) });
 }
 
 /// Get the dtype variant tag for an [`DType`].
 #[unsafe(no_mangle)]
 pub unsafe extern "C-unwind" fn vx_dtype_get(dtype: *const DType) -> u8 {
-    match dtype.as_ref().vortex_expect("null dtype") {
+    match unsafe { dtype.as_ref() }.vortex_expect("null dtype") {
         DType::Null => DTYPE_NULL,
         DType::Bool(_) => DTYPE_BOOL,
         DType::Primitive(ptype, _) => match ptype {
@@ -159,10 +162,10 @@ pub unsafe extern "C-unwind" fn vx_dtype_field_name(
     let field_name = struct_dtype.names()[index as usize].as_ref();
     let bytes = field_name.as_bytes();
 
-    let dst_slice = std::slice::from_raw_parts_mut(dst as *mut u8, bytes.len());
+    let dst_slice = unsafe { std::slice::from_raw_parts_mut(dst as *mut u8, bytes.len()) };
     dst_slice.copy_from_slice(bytes);
 
-    *len = bytes.len().try_into().vortex_unwrap();
+    unsafe { *len = bytes.len().try_into().vortex_unwrap() };
 }
 
 /// Get the dtype of a field in a `DTYPE_STRUCT` variant DType.
@@ -268,12 +271,12 @@ pub unsafe extern "C-unwind" fn vx_dtype_time_zone(
         TemporalMetadata::Timestamp(_, zone) => {
             if let Some(zone) = zone {
                 let bytes = zone.as_bytes();
-                let dst = std::slice::from_raw_parts_mut(dst as *mut u8, bytes.len());
+                let dst = unsafe { std::slice::from_raw_parts_mut(dst as *mut u8, bytes.len()) };
                 dst.copy_from_slice(bytes);
-                *len = bytes.len().try_into().vortex_unwrap();
+                unsafe { *len = bytes.len().try_into().vortex_unwrap() };
             } else {
                 // No time zone, using local timestamps.
-                *len = 0;
+                unsafe { *len = 0 };
             }
         }
         _ => panic!("DType_time_zone: not a timestamp metadata: {:?}", ext_dtype),
diff --git a/vortex-ffi/src/duckdb/mod.rs b/vortex-ffi/src/duckdb/mod.rs
@@ -45,14 +45,14 @@ pub unsafe extern "C-unwind" fn vx_duckdb_logical_type_to_dtype(
 ) -> *mut DType {
     try_or(error, ptr::null_mut(), || {
         let field_names: Vec<Arc<str>> = (0..column_count)
-            .map(|idx| to_string(*column_names.offset(idx as isize)))
+            .map(|idx| unsafe { to_string(*column_names.offset(idx as isize)) })
             .map(Arc::from)
             .collect();
 
         let types = (0..column_count)
-            .map(|idx| {
+            .map(|idx| unsafe {
                 (
-                    LogicalTypeHandle::new_unowned(unsafe { *column_types.offset(idx as isize) }),
+                    LogicalTypeHandle::new_unowned(*column_types.offset(idx as isize)),
                     *column_nullable.offset(idx as isize) != 0,
                 )
             })
@@ -130,7 +130,7 @@ pub unsafe extern "C-unwind" fn vx_duckdb_chunk_to_array(
             .collect_vec();
 
         let array = ArrayRef::from_duckdb(&NamedDataChunk {
-            chunk: &DataChunkHandle::new_unowned(chunk),
+            chunk: &unsafe { DataChunkHandle::new_unowned(chunk) },
             nullable: Some(&nullable),
             names: Some(struct_type.names().clone()),
         })?;
diff --git a/vortex-ffi/src/file.rs b/vortex-ffi/src/file.rs
@@ -77,11 +77,11 @@ pub unsafe extern "C-unwind" fn vx_file_open_reader(
         if options.uri.is_null() {
             vortex_bail!("null uri")
         }
-        let uri = CStr::from_ptr(options.uri).to_string_lossy();
+        let uri = unsafe { CStr::from_ptr(options.uri) }.to_string_lossy();
         let uri: Url = uri.parse().vortex_expect("File_open: parse uri");
 
-        let prop_keys = to_string_vec(options.property_keys, options.property_len);
-        let prop_vals = to_string_vec(options.property_vals, options.property_len);
+        let prop_keys = unsafe { to_string_vec(options.property_keys, options.property_len) };
+        let prop_vals = unsafe { to_string_vec(options.property_vals, options.property_len) };
 
         let object_store = make_object_store(&uri, &prop_keys, &prop_vals)?;
 
@@ -106,8 +106,7 @@ pub unsafe extern "C-unwind" fn vx_file_extract_statistics(
     file: *mut vx_file_reader,
 ) -> *mut vx_file_statistics {
     Box::into_raw(Box::new(vx_file_statistics {
-        num_rows: file
-            .as_ref()
+        num_rows: unsafe { file.as_ref() }
             .vortex_expect("null file ptr")
             .inner
             .row_count(),
@@ -117,14 +116,14 @@ pub unsafe extern "C-unwind" fn vx_file_extract_statistics(
 #[unsafe(no_mangle)]
 pub unsafe extern "C-unwind" fn vx_file_statistics_free(stat: *mut vx_file_statistics) {
     assert!(!stat.is_null());
-    drop(Box::from_raw(stat));
+    drop(unsafe { Box::from_raw(stat) });
 }
 
 /// Get the DType of the data inside of the file.
 #[unsafe(no_mangle)]
 pub unsafe extern "C-unwind" fn vx_file_dtype(file: *const vx_file_reader) -> *mut DType {
     Box::into_raw(Box::new(
-        file.as_ref()
+        unsafe { file.as_ref() }
             .vortex_expect("null file")
             .inner
             .dtype()
@@ -143,11 +142,11 @@ pub unsafe extern "C-unwind" fn vx_file_scan(
         let file = unsafe { file.as_ref().vortex_expect("null file") };
         let mut stream = file.inner.scan().vortex_expect("create scan");
 
-        if let Some(opts) = opts.as_ref() {
+        if let Some(opts) = unsafe { opts.as_ref() } {
             let mut field_names = Vec::new();
             for i in 0..opts.projection_len {
                 let col_name = unsafe { *opts.projection.offset(i as isize) };
-                let col_name: Arc<str> = to_string(col_name).into();
+                let col_name: Arc<str> = unsafe { to_string(col_name) }.into();
                 field_names.push(col_name);
             }
             let expr_str = opts.filter_expression;
@@ -188,7 +187,7 @@ pub unsafe extern "C-unwind" fn vx_file_scan(
 /// this file.
 #[unsafe(no_mangle)]
 pub unsafe extern "C-unwind" fn vx_file_reader_free(file: *mut vx_file_reader) {
-    drop(Box::from_raw(file));
+    drop(unsafe { Box::from_raw(file) });
 }
 
 fn make_object_store(
diff --git a/vortex-ffi/src/lib.rs b/vortex-ffi/src/lib.rs
@@ -1,4 +1,4 @@
-#![allow(unsafe_op_in_unsafe_fn, clippy::missing_safety_doc, clippy::panic)]
+#![allow(clippy::missing_safety_doc)]
 
 //! Native interface to Vortex arrays, types, files and streams.
 
@@ -32,12 +32,12 @@ static RUNTIME: LazyLock<Runtime> = LazyLock::new(|| {
 });
 
 pub(crate) unsafe fn to_string(ptr: *const c_char) -> String {
-    let c_str = CStr::from_ptr(ptr);
+    let c_str = unsafe { CStr::from_ptr(ptr) };
     c_str.to_string_lossy().into_owned()
 }
 
 pub(crate) unsafe fn to_string_vec(ptr: *const *const c_char, len: c_int) -> Vec<String> {
     (0..len)
-        .map(|i| to_string(*ptr.offset(i as isize)))
+        .map(|i| unsafe { to_string(*ptr.offset(i as isize)) })
         .collect()
 }
diff --git a/vortex-ffi/src/sink.rs b/vortex-ffi/src/sink.rs
@@ -78,7 +78,7 @@ pub unsafe extern "C-unwind" fn vx_array_sink_close(
     error: *mut *mut vx_error,
 ) {
     try_or(error, (), || {
-        let vx_array_sink { sink, writer } = *Box::from_raw(sink);
+        let vx_array_sink { sink, writer } = *unsafe { Box::from_raw(sink) };
         drop(sink);
 
         RUNTIME.block_on(async {
