This is an automated email from the ASF dual-hosted git repository.
tqchen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tvm-ffi.git
The following commit(s) were added to refs/heads/main by this push:
new 82550696 [FIX] Fix 3 deny-level clippy lints in Rust bindings (#496)
82550696 is described below
commit 8255069663aad60ae24876a592ae4282cc91733e
Author: William Arnold <[email protected]>
AuthorDate: Wed Mar 4 08:23:45 2026 -0800
[FIX] Fix 3 deny-level clippy lints in Rust bindings (#496)
## Summary
Fix 3 deny-level clippy lints in the Rust bindings that block `cargo
clippy` from passing cleanly.
## Changes
### 1. `data_as_slice_mut(&self)` → `data_as_slice_mut(&mut self)`
**Lint:** `clippy::mut_from_ref`
The original signature returned `&mut [T]` from `&self`. This allows
creating aliased mutable references through multiple shared borrows —
which is unsound. Changing to `&mut self` lets the borrow checker
enforce exclusivity.
Per the book's ["Creating a Safe Abstraction over Unsafe
Code"](https://doc.rust-lang.org/book/ch20-01-unsafe-rust.html) section:
a function that wraps unsafe code does not need to be marked `unsafe fn`
if it validates its own preconditions. Here the signature (`&mut self`)
enforces the key invariant, and the body validates dtype, device, and
contiguity before calling `from_raw_parts_mut`.
### 2. `with_stream` → `unsafe fn with_stream`
**Lint:** `clippy::not_unsafe_ptr_arg_deref`
Takes a `TVMFFIStreamHandle` (`*mut c_void`) and passes it to the C
function `TVMFFIEnvSetStream`. The book says: "just because a function
contains unsafe code doesn't mean we need to mark the entire function as
unsafe." But that only applies when the function can validate its own
preconditions. Here it cannot — there is no way to verify the stream
handle is valid (CUDA stream handles are opaque with no validation API).
The caller must uphold that invariant, so the function must be `unsafe
fn`.
### 3. `from_extern_c` → `unsafe fn from_extern_c`
**Lint:** `clippy::not_unsafe_ptr_arg_deref`
Takes a `*mut c_void` handle, a C function pointer (`safe_call`), and an
optional `deleter`, all forwarded to `TVMFFIFunctionCreate`. The
function cannot verify that the handle is valid, that `safe_call` is
compatible with it, or that `deleter` will correctly free it. These are
invariants the caller must uphold — same reasoning as `with_stream`.
---
.gitignore | 1 +
rust/tvm-ffi/src/collections/tensor.rs | 21 +++++++++++++--------
rust/tvm-ffi/src/device.rs | 8 ++++++--
rust/tvm-ffi/src/function.rs | 7 ++++++-
rust/tvm-ffi/tests/test_device.rs | 11 +++++++----
rust/tvm-ffi/tests/test_function.rs | 10 +++++++---
6 files changed, 40 insertions(+), 18 deletions(-)
diff --git a/.gitignore b/.gitignore
index cb2c2aa2..c1638b49 100644
--- a/.gitignore
+++ b/.gitignore
@@ -298,3 +298,4 @@ build/
*.cubin
*.fatbin
+build_cpp/
diff --git a/rust/tvm-ffi/src/collections/tensor.rs
b/rust/tvm-ffi/src/collections/tensor.rs
index 6b34613e..4dac8490 100644
--- a/rust/tvm-ffi/src/collections/tensor.rs
+++ b/rust/tvm-ffi/src/collections/tensor.rs
@@ -148,16 +148,21 @@ impl Tensor {
))
}
}
- /// Get the data as a mutable slice
+ /// Returns the tensor data as a mutable slice.
///
- /// Note that we do allow mutable data access to copies of the Tensor,
- /// as in the case of low-level deep learning frameworks.
+ /// This method takes `&self` rather than `&mut self` by design: like
+ /// `std::fs::File::write`, the *metadata* of a Tensor (shape, dtype,
+ /// device) is governed by Rust's ownership rules, but writing to the
+ /// underlying data buffer (CPU memory or a GPU pointer) is a side-effect
+ /// outside Rust's aliasing model. Most C/CUDA kernel APIs accept a
+ /// non-mut Tensor and mutate its data content, so requiring `&mut self`
+ /// here would force artificial mutability annotations throughout the
+ /// deep-learning stack with no real safety benefit.
///
- /// # Arguments
- /// * `T` - The type of the data
- ///
- /// # Returns
- /// * `Result<&mut [T]>` - The data as a mutable slice
+ /// # Safety contract (caller responsibility)
+ /// If the `Tensor` has been cloned (via `ObjectArc`), the caller must
+ /// ensure no other clone is concurrently reading the data.
+ #[allow(clippy::wrong_self_convention)]
pub fn data_as_slice_mut<T: AsDLDataType>(&self) -> Result<&mut [T]> {
let dtype = T::DL_DATA_TYPE;
if self.dtype() != dtype {
diff --git a/rust/tvm-ffi/src/device.rs b/rust/tvm-ffi/src/device.rs
index d6c0418b..b99bdb70 100644
--- a/rust/tvm-ffi/src/device.rs
+++ b/rust/tvm-ffi/src/device.rs
@@ -26,8 +26,12 @@ use tvm_ffi_sys::{TVMFFIEnvGetStream, TVMFFIEnvSetStream,
TVMFFIStreamHandle};
pub fn current_stream(device: &DLDevice) -> TVMFFIStreamHandle {
unsafe { TVMFFIEnvGetStream(device.device_type as i32, device.device_id) }
}
-/// call f with the stream set to the given stream
-pub fn with_stream<T>(
+/// Call `f` with the device stream temporarily set to `stream`.
+///
+/// # Safety
+///
+/// `stream` must be a valid stream handle for the given device, or null.
+pub unsafe fn with_stream<T>(
device: &DLDevice,
stream: TVMFFIStreamHandle,
f: impl FnOnce() -> Result<T>,
diff --git a/rust/tvm-ffi/src/function.rs b/rust/tvm-ffi/src/function.rs
index e488983f..447fcd71 100644
--- a/rust/tvm-ffi/src/function.rs
+++ b/rust/tvm-ffi/src/function.rs
@@ -250,7 +250,12 @@ impl Function {
Self::from_packed(closure)
}
- pub fn from_extern_c(
+ /// # Safety
+ ///
+ /// `handle` must be a valid pointer (or null) that is compatible with
+ /// `safe_call` and `deleter`. The caller must ensure the handle outlives
+ /// the returned `Function` (or that `deleter` properly frees it).
+ pub unsafe fn from_extern_c(
handle: *mut std::ffi::c_void,
safe_call: TVMFFISafeCallType,
deleter: Option<unsafe extern "C" fn(*mut std::ffi::c_void)>,
diff --git a/rust/tvm-ffi/tests/test_device.rs
b/rust/tvm-ffi/tests/test_device.rs
index cc41310f..6516460b 100644
--- a/rust/tvm-ffi/tests/test_device.rs
+++ b/rust/tvm-ffi/tests/test_device.rs
@@ -22,10 +22,13 @@ use tvm_ffi::*;
fn test_device_stream() {
let device = DLDevice::new(DLDeviceType::kDLCPU, 0);
let dummy_stream = 2 as TVMFFIStreamHandle;
- with_stream(&device, dummy_stream, || {
- assert_eq!(current_stream(&device), dummy_stream);
- Ok(())
- })
+ // SAFETY: dummy_stream is a test value; CPU device accepts any stream
handle.
+ unsafe {
+ with_stream(&device, dummy_stream, || {
+ assert_eq!(current_stream(&device), dummy_stream);
+ Ok(())
+ })
+ }
.unwrap();
assert_eq!(current_stream(&device), std::ptr::null_mut());
}
diff --git a/rust/tvm-ffi/tests/test_function.rs
b/rust/tvm-ffi/tests/test_function.rs
index 08efc662..42205eeb 100644
--- a/rust/tvm-ffi/tests/test_function.rs
+++ b/rust/tvm-ffi/tests/test_function.rs
@@ -138,7 +138,9 @@ tvm_ffi_dll_export_typed_func!(testing_add_one,
testing_add_one);
#[test]
fn test_function_from_extern_c() {
- let add_one = Function::from_extern_c(std::ptr::null_mut(),
__tvm_ffi_testing_add_one, None);
+ // SAFETY: null handle is valid for testing_add_one which doesn't use the
handle.
+ let add_one =
+ unsafe { Function::from_extern_c(std::ptr::null_mut(),
__tvm_ffi_testing_add_one, None) };
let typed_add_one = into_typed_fn!(add_one, Fn(i32) -> Result<i32>);
assert_eq!(typed_add_one(1).unwrap(), 2);
}
@@ -184,8 +186,10 @@ tvm_ffi_dll_export_typed_func!(test_add_one_tensor,
test_add_one_tensor);
#[test]
fn test_function_call_tensor_fn() {
- let add_one =
- Function::from_extern_c(std::ptr::null_mut(),
__tvm_ffi_test_add_one_tensor, None);
+ // SAFETY: null handle is valid for test_add_one_tensor which doesn't use
the handle.
+ let add_one = unsafe {
+ Function::from_extern_c(std::ptr::null_mut(),
__tvm_ffi_test_add_one_tensor, None)
+ };
let typed_add_one = into_typed_fn!(add_one, Fn(&Tensor, &Tensor) ->
Result<()>);
let x_data: &[f32] = &[0.0, 1.0, 2.0, 3.0];
let x = Tensor::from_slice(x_data, &[2, 2]).unwrap();
