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 c244bc5f [Rust] Final-audit fixes: allocation overflow, immutable
subtypes, dtype naming, API follow-ups (#960)
c244bc5f is described below
commit c244bc5fcbaac54bbc6f0563fccf302cb570388a
Author: Tianqi Chen <[email protected]>
AuthorDate: Sat Nov 7 15:28:42 2026 -0500
[Rust] Final-audit fixes: allocation overflow, immutable subtypes, dtype
naming, API follow-ups (#960)
Follow-up fixes to the Rust bindings after #959.
Soundness:
- `List::reserve` and list growth reject a capacity whose byte size
exceeds `isize::MAX` with a `ValueError` (the check sits in the cold
grow path).
- Trailing-item layouts (`Array`, `Shape`, `Tensor`) are built with
`Layout::array`/`Layout::extend` and panic with "capacity overflow", as
`Vec` does; `ObjectArc::new_with_extra_items` leaks rather than drops
the object on that panic.
- The object derive rejects at compile time a subtype of an
`ImmutableObject` type that is not itself immutable, and an immutable
type that inherits a `#[reflect(writable)]` field. The Rust stub
generator marks a subtype with no registered mutability immutable when
its parent binding is.
- `Tensor::from_nd_alloc` and `Tensor::from_env_alloc` panic when the
shape length does not fit the `i32` ndim, instead of truncating it and
overflowing the trailing shape buffer.
- `Tensor::from_nd_alloc` computes row-major strides with wrapping
products, as C++ does, so a zero-element tensor with large extents no
longer panics in debug builds while the object is half built.
Correctness:
- The `Module` binding is no longer `final` (`ffi.Module` has subtypes).
- `DLDataTypeExt::to_string` falls back to `dtype(code=.., bits=..,
lanes=..)` for a type the runtime cannot name, so
`Tensor::data_as_slice` returns its `TypeError` instead of panicking.
- Generated `type_index_available` calls `::core::mem::drop`.
API:
- `DefRegionKind`, `InplaceMode`, `StructuralVisitorView` and
`StructuralMutatorView` are re-exported at the crate root (module paths
stay).
- `Debug` for `Any` and `AnyView`: the type key and the repr, such as
`Any(int: 1)`.
Docs: `structural_mutate`/`structural_map` state that a panicking
callback aborts; the crate-level FFI contracts list the structural hook
contract; `rs_object` states that structural equality and hashing see
the fields only under an `s_eq_hash_kind`.
Source-breaking:
- A binding or `rs_object` type whose parent is immutable must be marked
`#[type_immutable]` / `immutable`, for example an enum type under
`ffi.Enum`, or a Rust-defined type under an immutable IR node.
---
docs/guides/rust_lang_guide.rst | 16 ++++++------
python/tvm_ffi/stub/rust_generator/codegen.py | 20 ++++++++++-----
rust/tvm-ffi-macros/src/object_macros.rs | 37 ++++++++++++++++++---------
rust/tvm-ffi/src/any.rs | 17 ++++++++++++
rust/tvm-ffi/src/collections/list.rs | 18 ++++++++++---
rust/tvm-ffi/src/collections/shape.rs | 5 ++--
rust/tvm-ffi/src/collections/tensor.rs | 6 +++--
rust/tvm-ffi/src/dtype.rs | 20 ++++++++++-----
rust/tvm-ffi/src/enum_ref.rs | 1 +
rust/tvm-ffi/src/extra/mod.rs | 8 +++---
rust/tvm-ffi/src/extra/module.rs | 1 -
rust/tvm-ffi/src/extra/structural_map.rs | 3 +++
rust/tvm-ffi/src/extra/structural_mutate.rs | 7 +++--
rust/tvm-ffi/src/extra/structural_visit.rs | 2 +-
rust/tvm-ffi/src/lib.rs | 19 ++++++++++----
rust/tvm-ffi/src/object.rs | 25 +++++++++++++++---
rust/tvm-ffi/src/reflection/enum_def.rs | 1 +
rust/tvm-ffi/src/reflection/registry.rs | 5 ++--
rust/tvm-ffi/tests/test_cast.rs | 1 +
rust/tvm-ffi/tests/test_enum.rs | 1 +
rust/tvm-ffi/tests/test_match_any.rs | 6 +++--
21 files changed, 158 insertions(+), 61 deletions(-)
diff --git a/docs/guides/rust_lang_guide.rst b/docs/guides/rust_lang_guide.rst
index 818749de..1f4439a2 100644
--- a/docs/guides/rust_lang_guide.rst
+++ b/docs/guides/rust_lang_guide.rst
@@ -685,7 +685,7 @@ Walk handlers may take an optional trailing
``DefRegionKind`` argument:
.. code-block:: rust
use tvm_ffi::{dispatch, structural_walk, Array, Result, WalkOrder,
WalkResult};
- use tvm_ffi::extra::structural_visit::DefRegionKind;
+ use tvm_ffi::DefRegionKind;
#[derive(Default)]
struct Probe {
@@ -726,7 +726,7 @@ take a trailing ``DefRegionKind`` argument:
.. code-block:: rust
use tvm_ffi::{structural_walk, Array, Object, WalkOrder, WalkResult};
- use tvm_ffi::extra::structural_visit::DefRegionKind;
+ use tvm_ffi::DefRegionKind;
let values = Array::from_vec(vec![1_i64, 2, 3]);
@@ -792,7 +792,7 @@ dispatchers and closures.
.. code-block:: rust
use tvm_ffi::{dispatch, structural_visit, Array, Result, VisitInterrupt};
- use tvm_ffi::extra::structural_visit::StructuralVisitorView;
+ use tvm_ffi::StructuralVisitorView;
#[derive(Default)]
struct Stats {
@@ -832,7 +832,7 @@ argument tuple and return type supply the engine signature
directly:
.. code-block:: rust
use tvm_ffi::{dispatch, structural_visit, AnyView, Array, Result,
VisitInterrupt};
- use tvm_ffi::extra::structural_visit::StructuralVisitorView;
+ use tvm_ffi::StructuralVisitorView;
#[derive(Default)]
struct Depth {
@@ -894,7 +894,7 @@ catch-all and should therefore come last:
use tvm_ffi::{
dispatch, structural_map, Any, Array, AnyView, Result, WalkOrder,
UnchangedOr, Unchanged,
};
- use tvm_ffi::extra::structural_visit::DefRegionKind;
+ use tvm_ffi::DefRegionKind;
#[derive(Default)]
struct Increment {
@@ -988,7 +988,7 @@ engines, ``DefRegionKind`` and ``InplaceMode`` live in
.. code-block:: rust
use tvm_ffi::{dispatch, structural_mutate, Any, Array, MutationView,
Result, UnchangedOr};
- use tvm_ffi::extra::structural_mutate::StructuralMutatorView;
+ use tvm_ffi::StructuralMutatorView;
#[derive(Default)]
struct Stats {
@@ -1045,7 +1045,7 @@ other handler consumes it:
.. code-block:: rust
use tvm_ffi::{dispatch, structural_mutate, Any, Array, Result, UnchangedOr};
- use tvm_ffi::extra::structural_mutate::StructuralMutatorView;
+ use tvm_ffi::StructuralMutatorView;
#[derive(Default)]
struct Increment {
@@ -1074,7 +1074,7 @@ while retaining typed scalar replacements:
.. code-block:: rust
use tvm_ffi::{dispatch, structural_mutate, Any, Array, MutationView,
Result, UnchangedOr};
- use tvm_ffi::extra::structural_mutate::{InplaceMode, StructuralMutatorView};
+ use tvm_ffi::{InplaceMode, StructuralMutatorView};
struct Increment;
diff --git a/python/tvm_ffi/stub/rust_generator/codegen.py
b/python/tvm_ffi/stub/rust_generator/codegen.py
index b31e760b..95b9a900 100644
--- a/python/tvm_ffi/stub/rust_generator/codegen.py
+++ b/python/tvm_ffi/stub/rust_generator/codegen.py
@@ -733,22 +733,30 @@ class _ObjectRenderer:
)
return sections
- def _is_immutable(self) -> bool:
+ def _is_immutable(self, info: ObjectInfo | None = None) -> bool:
"""Whether C++ declares the class not ``_type_mutable`` and no field
along its chain writable.
Such a class only hands out const access and changes through
copy-on-write,
so its handles may cross threads when its fields can. A mutable class,
one
- whose mutability is not registered, or one with a ``def_rw`` field
stays
- single-threaded.
+ with a ``def_rw`` field, or one whose mutability is not registered
stays
+ single-threaded, except that an unregistered subtype of an immutable
class
+ is immutable too, as the derive requires.
"""
- if self.info.is_mutable is not False:
+ info = self.info if info is None else info
+ parent = info.parent_type_key
+ if info.is_mutable is None and parent is not None:
+ if parent == C_RUST.RUST_ENUM_TYPE_KEY:
+ return True
+ if self._provider(parent) == "generated":
+ return self._is_immutable(object_info_from_type_key(parent))
+ if info.is_mutable is not False:
return False
chain = [
object_info_from_type_key(key)
- for key in self.info.ancestors
+ for key in info.ancestors
if self._provider(key) in ("generated", "mapped")
]
- return all(field.frozen for info in [*chain, self.info] for field in
info.fields)
+ return all(field.frozen for each in [*chain, info] for field in
each.fields)
def _is_rs_parent(self, verdict: Verdict) -> bool:
"""Whether the object derives ``ShallowCopy`` and ``UnsafeInit``
(``#[type_rs_parent]``).
diff --git a/rust/tvm-ffi-macros/src/object_macros.rs
b/rust/tvm-ffi-macros/src/object_macros.rs
index b2e8dd92..7d8a08bb 100644
--- a/rust/tvm-ffi-macros/src/object_macros.rs
+++ b/rust/tvm-ffi-macros/src/object_macros.rs
@@ -124,18 +124,17 @@ pub(crate) fn expand_object(
.expect("First field must be `<base_name>: <ObjectCoreType>`");
let reflection = Reflection::parse(derive_input, rs_object);
- if type_immutable {
- let writable = reflection
- .iter()
- .flat_map(|reflection| &reflection.reflected)
- .find(|field| field.flags.iter().any(|flag| flag == "writable"));
- if let Some(field) = writable {
- abort!(
- field.id,
- "an immutable type cannot have a #[reflect(writable)] field"
- );
- }
+ let writable = reflection
+ .iter()
+ .flat_map(|reflection| &reflection.reflected)
+ .find(|field| field.flags.iter().any(|flag| flag == "writable"));
+ if let (true, Some(field)) = (type_immutable, writable) {
+ abort!(
+ field.id,
+ "an immutable type cannot have a #[reflect(writable)] field"
+ );
}
+ let has_writable_field = writable.is_some();
let immutable_impl = if type_immutable {
quote! {
unsafe impl #tvm_ffi_crate::object::ImmutableObject for
#struct_name {}
@@ -282,7 +281,7 @@ pub(crate) fn expand_object(
};
if !found {
// Discard the error the failed lookup raised.
- drop(#tvm_ffi_crate::Error::from_raised());
+
::core::mem::drop(#tvm_ffi_crate::Error::from_raised());
}
found
}
@@ -323,6 +322,16 @@ pub(crate) fn expand_object(
!<#base_ty as
#tvm_ffi_crate::object::ObjectCore>::TYPE_FINAL,
"an object type cannot derive from a final parent"
);
+ ::core::assert!(
+ #type_immutable
+ || !<#base_ty as
#tvm_ffi_crate::object::ObjectCore>::TYPE_IMMUTABLE,
+ "a subtype of an immutable object type must be
immutable"
+ );
+ ::core::assert!(
+ !#type_immutable
+ || !<#base_ty as
#tvm_ffi_crate::object::ObjectCore>::TYPE_HAS_WRITABLE_FIELD,
+ "an immutable type cannot inherit a
#[reflect(writable)] field"
+ );
};
}
}),
@@ -366,6 +375,10 @@ pub(crate) fn expand_object(
unsafe impl #tvm_ffi_crate::object::ObjectCore for #struct_name {
const TYPE_KEY: &'static str = #type_key;
const TYPE_FINAL: bool = #type_final;
+ const TYPE_IMMUTABLE: bool = #type_immutable;
+ const TYPE_HAS_WRITABLE_FIELD: bool =
+ <#base_ty as
#tvm_ffi_crate::object::ObjectCore>::TYPE_HAS_WRITABLE_FIELD
+ || #has_writable_field;
#type_index_tokens
diff --git a/rust/tvm-ffi/src/any.rs b/rust/tvm-ffi/src/any.rs
index 7fd54c80..a5f05243 100644
--- a/rust/tvm-ffi/src/any.rs
+++ b/rust/tvm-ffi/src/any.rs
@@ -444,6 +444,23 @@ impl std::fmt::Display for Any {
}
}
+/// Formats the type key and the `repr`, such as `AnyView(int: 1)`.
+impl std::fmt::Debug for AnyView<'_> {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ let key = object::type_key_of_index(self.type_index());
+ write!(f, "AnyView({key}: {self})")
+ }
+}
+
+/// Formats the type key and the `repr`, such as `Any(int: 1)`.
+impl std::fmt::Debug for Any {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ let view = AnyView::from(self);
+ let key = object::type_key_of_index(view.type_index());
+ write!(f, "Any({key}: {view})")
+ }
+}
+
/// Whether a `TVMFFIAny` cell owns everything it holds, so its owning form is
a
/// bitwise copy.
#[inline]
diff --git a/rust/tvm-ffi/src/collections/list.rs
b/rust/tvm-ffi/src/collections/list.rs
index 10bc5ff3..91241a68 100644
--- a/rust/tvm-ffi/src/collections/list.rs
+++ b/rust/tvm-ffi/src/collections/list.rs
@@ -130,6 +130,9 @@ unsafe fn write_cell<T: ContainerElement>(cell: *mut
TVMFFIAny, value: T) {
ptr::write(cell, raw);
}
+/// The most elements a list buffer can hold: its byte size must fit in
`isize`.
+const MAX_CAPACITY: usize = isize::MAX as usize /
std::mem::size_of::<TVMFFIAny>();
+
// The error paths stay out of line so the hot paths are small enough to
inline.
#[cold]
#[inline(never)]
@@ -474,7 +477,7 @@ impl<T: ContainerElement> List<T> {
/// Makes room for at least `additional` more elements without
/// reallocating again, like `Vec::reserve`.
pub fn reserve(&self, additional: usize) -> Result<()> {
- let total = self.len() + additional;
+ let total = self.len().saturating_add(additional);
if total > self.capacity() {
self.grow_to(total)?;
}
@@ -486,8 +489,9 @@ impl<T: ContainerElement> List<T> {
#[inline]
fn reserve_for(&self, extra: usize) -> Result<()> {
let (size, capacity) = (self.len(), self.capacity());
- if size + extra > capacity {
- self.grow_to((capacity * 2).max(size + extra))?;
+ let needed = size.saturating_add(extra);
+ if needed > capacity {
+ self.grow_to((capacity * 2).min(MAX_CAPACITY).max(needed))?;
}
Ok(())
}
@@ -497,6 +501,14 @@ impl<T: ContainerElement> List<T> {
#[cold]
#[inline(never)]
fn grow_to(&self, capacity: usize) -> Result<()> {
+ if capacity > MAX_CAPACITY {
+ crate::bail!(
+ crate::error::VALUE_ERROR,
+ "List capacity {} exceeds the maximum {}",
+ capacity,
+ MAX_CAPACITY
+ );
+ }
crate::cached_global_func!("ffi.ListReserve")
.call_packed(&[AnyView::from(self), AnyView::from(&(capacity as
i64))])?;
Ok(())
diff --git a/rust/tvm-ffi/src/collections/shape.rs
b/rust/tvm-ffi/src/collections/shape.rs
index 8cd2c776..6bbd5c43 100644
--- a/rust/tvm-ffi/src/collections/shape.rs
+++ b/rust/tvm-ffi/src/collections/shape.rs
@@ -106,7 +106,8 @@ impl Shape {
}
}
- /// Fill `strides` with the row-major strides of `shape`.
+ /// Fill `strides` with the row-major strides of `shape`, wrapping on
+ /// overflow as C++ does: only a zero-element tensor gets that far.
pub(crate) fn fill_strides_from_shape<T>(shape: T, strides: &mut [i64])
where
T: AsRef<[i64]>,
@@ -115,7 +116,7 @@ impl Shape {
let mut stride = 1;
for i in (0..shape.len()).rev() {
strides[i] = stride;
- stride *= shape[i];
+ stride = stride.wrapping_mul(shape[i]);
}
}
}
diff --git a/rust/tvm-ffi/src/collections/tensor.rs
b/rust/tvm-ffi/src/collections/tensor.rs
index d1bd2968..df39460a 100644
--- a/rust/tvm-ffi/src/collections/tensor.rs
+++ b/rust/tvm-ffi/src/collections/tensor.rs
@@ -336,10 +336,11 @@ impl Tensor {
where
TNDAlloc: NDAllocator,
{
+ let ndim = i32::try_from(shape.len()).expect("tensor ndim exceeds
i32");
let dltensor = DLTensor {
data: std::ptr::null_mut(),
device,
- ndim: shape.len() as i32,
+ ndim,
dtype,
shape: shape.as_ptr() as *mut i64,
strides: std::ptr::null_mut(),
@@ -397,10 +398,11 @@ impl Tensor {
/// The error the allocator raises, or a `RuntimeError` when no allocator
/// is set.
pub fn from_env_alloc(shape: &[i64], dtype: DLDataType, device: DLDevice)
-> Result<Self> {
+ let ndim = i32::try_from(shape.len()).expect("tensor ndim exceeds
i32");
let mut prototype = DLTensor {
data: std::ptr::null_mut(),
device,
- ndim: shape.len() as i32,
+ ndim,
dtype,
shape: shape.as_ptr() as *mut i64,
strides: std::ptr::null_mut(),
diff --git a/rust/tvm-ffi/src/dtype.rs b/rust/tvm-ffi/src/dtype.rs
index d926fd6e..cf00c082 100644
--- a/rust/tvm-ffi/src/dtype.rs
+++ b/rust/tvm-ffi/src/dtype.rs
@@ -27,7 +27,8 @@ use tvm_ffi_sys::{TVMFFIAny, TVMFFIByteArray,
TVMFFIDataTypeFromString, TVMFFIDa
/// Name conversions of a `DLDataType`.
pub trait DLDataTypeExt: Sized {
/// Return the name of the data type, such as `"int32"`, `"float64"` or
- /// `"int8x4"` (bits and lanes), the name every language uses for it.
+ /// `"int8x4"` (bits and lanes), the name every language uses for it, or
+ /// `"dtype(code=.., bits=.., lanes=..)"` for one the runtime cannot name.
///
/// ```
/// use tvm_ffi::{DLDataType, DLDataTypeExt};
@@ -53,13 +54,18 @@ pub trait DLDataTypeExt: Sized {
impl DLDataTypeExt for DLDataType {
fn to_string(&self) -> std::string::String {
- unsafe {
+ let name = unsafe {
let mut ffi_any = TVMFFIAny::new();
- crate::check_safe_call!(TVMFFIDataTypeToString(self, &mut
ffi_any)).unwrap();
- let name: crate::string::String =
crate::any::Any::from_raw_ffi_any(ffi_any)
- .try_into()
- .unwrap();
- name.as_str().to_owned()
+ crate::check_safe_call!(TVMFFIDataTypeToString(self, &mut
ffi_any)).and_then(|()| {
+
crate::string::String::try_from(crate::any::Any::from_raw_ffi_any(ffi_any))
+ })
+ };
+ match name {
+ Ok(name) => name.as_str().to_owned(),
+ Err(_) => format!(
+ "dtype(code={}, bits={}, lanes={})",
+ self.code, self.bits, self.lanes
+ ),
}
}
diff --git a/rust/tvm-ffi/src/enum_ref.rs b/rust/tvm-ffi/src/enum_ref.rs
index eb548de1..1d41a0ee 100644
--- a/rust/tvm-ffi/src/enum_ref.rs
+++ b/rust/tvm-ffi/src/enum_ref.rs
@@ -80,6 +80,7 @@ impl EnumObj {
/// #[type_key = "my_ext.Color"]
/// #[type_register]
/// #[type_final]
+/// #[type_immutable]
/// pub struct ColorObj {
/// base_: EnumObj,
/// }
diff --git a/rust/tvm-ffi/src/extra/mod.rs b/rust/tvm-ffi/src/extra/mod.rs
index bc405301..babc87b6 100644
--- a/rust/tvm-ffi/src/extra/mod.rs
+++ b/rust/tvm-ffi/src/extra/mod.rs
@@ -51,7 +51,7 @@ mod visit_error_context;
///
/// ```compile_fail,E0277
/// use tvm_ffi::Any;
-/// use tvm_ffi::extra::structural_visit::StructuralVisitorView;
+/// use tvm_ffi::StructuralVisitorView;
/// fn retain<D>(view: &StructuralVisitorView<'_, D>) -> Any {
/// Any::from(view.base())
/// }
@@ -60,7 +60,7 @@ mod visit_error_context;
/// A node borrowed through a copied `MutationView` cannot survive consumption
of the view:
/// ```compile_fail,E0505
/// use tvm_ffi::MutationView;
-/// use tvm_ffi::extra::structural_mutate::StructuralMutatorView;
+/// use tvm_ffi::StructuralMutatorView;
/// fn invalid<D>(value: MutationView<'_>, dispatch: &mut D, mutator: &mut
StructuralMutatorView<'_, D>) {
/// let view = value.as_any_view();
/// let copy = view;
@@ -73,7 +73,7 @@ mod visit_error_context;
/// Moving the handle into a nested callback cannot bypass that borrow:
/// ```compile_fail,E0505
/// use tvm_ffi::{dispatch, structural_mutate, Any, MutationView, Result,
UnchangedOr};
-/// use tvm_ffi::extra::structural_mutate::StructuralMutatorView;
+/// use tvm_ffi::StructuralMutatorView;
/// struct Inner<'a>(Option<MutationView<'a>>);
/// #[dispatch(mutate, (MutationView<'_>, &mut StructuralMutatorView<'_,
Self>), Result<UnchangedOr<Any>>)]
/// impl<'a> Inner<'a> {
@@ -93,7 +93,7 @@ mod visit_error_context;
/// A copied inspection view also cannot survive low-level default mutation:
/// ```compile_fail,E0505
/// use tvm_ffi::{dispatch, Any, MutationView, Result, Unchanged, UnchangedOr};
-/// use tvm_ffi::extra::structural_mutate::StructuralMutatorView;
+/// use tvm_ffi::StructuralMutatorView;
/// struct Pass;
/// #[dispatch(mutate, (MutationView<'_>, &mut StructuralMutatorView<'_,
Self>), Result<UnchangedOr<Any>>)]
/// impl Pass {
diff --git a/rust/tvm-ffi/src/extra/module.rs b/rust/tvm-ffi/src/extra/module.rs
index 46a898e1..aff890dd 100644
--- a/rust/tvm-ffi/src/extra/module.rs
+++ b/rust/tvm-ffi/src/extra/module.rs
@@ -34,7 +34,6 @@ use tvm_ffi_sys::TVMFFITypeIndex as TypeIndex;
#[derive(Object)]
#[type_key = "ffi.Module"]
#[type_index(unsafe TypeIndex::kTVMFFIModule)]
-#[type_final]
pub struct ModuleObj {
object: Object,
}
diff --git a/rust/tvm-ffi/src/extra/structural_map.rs
b/rust/tvm-ffi/src/extra/structural_map.rs
index 2f4872e9..db55e99f 100644
--- a/rust/tvm-ffi/src/extra/structural_map.rs
+++ b/rust/tvm-ffi/src/extra/structural_map.rs
@@ -178,6 +178,9 @@ where
/// this function consumes `root`, an error does not return the partly mapped
/// root to the caller.
///
+/// Callbacks must report failures through [`Result`] and must not panic: a
panic
+/// reaching the non-unwinding native callback boundary aborts the process.
+///
/// A callback must not modify an `ffi.List` or `ffi.Dict` that holds the
/// value it receives or one of that value's ancestors: the traversal borrows
/// their cells while the callback runs. A callback that returns an `Error`
diff --git a/rust/tvm-ffi/src/extra/structural_mutate.rs
b/rust/tvm-ffi/src/extra/structural_mutate.rs
index 749f3b74..54913571 100644
--- a/rust/tvm-ffi/src/extra/structural_mutate.rs
+++ b/rust/tvm-ffi/src/extra/structural_mutate.rs
@@ -519,7 +519,7 @@ impl<T: object::ObjectRefCore> MutationView<'_, T> {
///
/// ```
/// # use tvm_ffi::*;
- /// # use tvm_ffi::extra::structural_mutate::StructuralMutatorView;
+ /// # use tvm_ffi::StructuralMutatorView;
/// # #[rs_object("my_ext.MyPair", final)]
/// # pub struct MyPairObj { base_: Object, pub first: Array<i64>, pub
second: Array<i64> }
/// # #[derive(Clone, tvm_ffi::derive::ObjectRef)]
@@ -794,6 +794,9 @@ impl<D, Base: StructuralMutatorBase>
StructuralMutatorView<'_, D, Base> {
/// in-place mutation. A matching callback supplies the final value; unmatched
/// values use default mutation.
///
+/// Callbacks must report failures through [`Result`] and must not panic: a
panic
+/// reaching the non-unwinding native callback boundary aborts the process.
+///
/// A callback must not modify an `ffi.List` or `ffi.Dict` that holds the
/// value it receives or one of that value's ancestors: the traversal borrows
/// their cells while the callback runs. A callback that returns an `Error`
@@ -815,7 +818,7 @@ impl<D, Base: StructuralMutatorBase>
StructuralMutatorView<'_, D, Base> {
///
/// ```
/// use tvm_ffi::{dispatch, structural_mutate, Any, Array, MutationView,
Result, UnchangedOr};
-/// use tvm_ffi::extra::structural_mutate::StructuralMutatorView;
+/// use tvm_ffi::StructuralMutatorView;
///
/// struct Double;
///
diff --git a/rust/tvm-ffi/src/extra/structural_visit.rs
b/rust/tvm-ffi/src/extra/structural_visit.rs
index 910b45f0..74b70293 100644
--- a/rust/tvm-ffi/src/extra/structural_visit.rs
+++ b/rust/tvm-ffi/src/extra/structural_visit.rs
@@ -447,7 +447,7 @@ impl<D, Base: StructuralVisitorBase>
StructuralVisitorView<'_, D, Base> {
/// ```
/// # fn main() -> tvm_ffi::Result<()> {
/// use tvm_ffi::{dispatch, structural_visit, AnyView, Array, Result,
VisitInterrupt};
-/// use tvm_ffi::extra::structural_visit::StructuralVisitorView;
+/// use tvm_ffi::StructuralVisitorView;
///
/// struct Sum(i64);
/// #[dispatch(visit, (AnyView<'_>, &mut StructuralVisitorView<'_, Self>),
Result<Option<VisitInterrupt>>)]
diff --git a/rust/tvm-ffi/src/lib.rs b/rust/tvm-ffi/src/lib.rs
index 577d5e94..ed3fd393 100644
--- a/rust/tvm-ffi/src/lib.rs
+++ b/rust/tvm-ffi/src/lib.rs
@@ -53,6 +53,9 @@
//! * A [`TensorView`] received as a function argument is valid only during
//! the call, and so is an [`Any`] holding a `DLTensor*` converted from it
//! or from an [`AnyView`].
+//! * A registered structural visit or mutate hook uses the visitor or
+//! mutator only through its arguments, during the call, and does not keep
+//! it.
#![warn(missing_docs)]
// Lets macro-generated `::tvm_ffi` paths resolve inside this crate too.
@@ -105,8 +108,12 @@ pub use crate::extra::structural_eq_hash::{
structural_equal_with, structural_hash, structural_hash_with,
StructuralOptions,
};
pub use crate::extra::structural_map::structural_map;
-pub use crate::extra::structural_mutate::{structural_mutate, MutationView};
-pub use crate::extra::structural_visit::{structural_visit, VisitInterrupt};
+pub use crate::extra::structural_mutate::{
+ structural_mutate, InplaceMode, MutationView, StructuralMutatorView,
+};
+pub use crate::extra::structural_visit::{
+ structural_visit, DefRegionKind, StructuralVisitorView, VisitInterrupt,
+};
pub use crate::extra::structural_walk::{structural_walk, IntoWalkResult,
WalkOrder, WalkResult};
pub use crate::extra::unchanged::{Unchanged, UnchangedOr};
pub use crate::function::Function;
@@ -262,10 +269,12 @@ pub use tvm_ffi_macros::match_any;
/// * `final` - The type has no subtypes.
/// * `immutable` - Shared objects of the type are never mutated, so its
/// references cross threads when its fields can
-/// ([`ImmutableObject`](crate::object::ImmutableObject)); a field cannot be
-/// `writable`.
+/// ([`ImmutableObject`](crate::object::ImmutableObject)); a field, own or
+/// inherited, cannot be `writable`. Every subtype must be `immutable` too.
/// * `s_eq_hash_kind = "TreeNode"` - The
[`SEqHashKind`](crate::reflection::SEqHashKind) of structural
-/// equality; by default the parent's.
+/// equality; by default the parent's. Structural equality and hashing see
+/// the fields only when the type or an ancestor has one, and otherwise fail
+/// with a `TypeError`.
/// * `child_slots = N`, `child_slots_can_overflow = bool` - Type indices kept
/// for subtypes; 0 and `true` by default.
/// * `reflection = path` - A `fn(&mut ObjectDef<Self>)` that registers the
diff --git a/rust/tvm-ffi/src/object.rs b/rust/tvm-ffi/src/object.rs
index bc97fa17..70234aaf 100644
--- a/rust/tvm-ffi/src/object.rs
+++ b/rust/tvm-ffi/src/object.rs
@@ -164,6 +164,9 @@ impl<T: ObjectCore> ObjectArc<T> {
where
T: ObjectCoreWithExtraItems<ExtraItem = U>,
{
+ // `data` may describe items it does not hold yet, so a panic before
+ // the write leaks it rather than dropping it.
+ let data = std::mem::ManuallyDrop::new(data);
unsafe {
// ensure strict alignment requirements
// so we can have { T, U*extra_items } layout
@@ -176,7 +179,7 @@ impl<T: ObjectCore> ObjectArc<T> {
std::alloc::handle_alloc_error(layout);
}
let ptr = raw_data_ptr as *mut T;
- std::ptr::write(ptr, data);
+ std::ptr::write(ptr, std::mem::ManuallyDrop::into_inner(data));
// now override the header directly
std::ptr::write(
ptr as *mut TVMFFIObject,
@@ -340,7 +343,9 @@ impl<T: ObjectCore> Clone for ObjectArc<T> {
/// copy-on-write, in place only on a uniquely held instance (one strong
/// reference, no weak ones). This holds for every type not registered as
/// mutable (`_type_mutable`), in any language. Every subclass must also be
-/// immutable while shared, with thread-safe fields.
+/// immutable while shared, with thread-safe fields; the derives reject a
+/// subtype that is not `immutable` at compile time. A hand-written impl must
+/// also set `ObjectCore::TYPE_IMMUTABLE` and keep every subtype immutable.
pub unsafe trait ImmutableObject: ObjectCore {}
// SAFETY: `T` is never mutated while shared, its fields are thread-safe, and
the
@@ -390,6 +395,12 @@ pub unsafe trait ObjectCore: Sized + 'static {
/// A final type has no separately registered object-system subtype.
#[doc(hidden)]
const TYPE_FINAL: bool = false;
+ /// Whether `Self` implements [`ImmutableObject`].
+ #[doc(hidden)]
+ const TYPE_IMMUTABLE: bool = false;
+ /// Whether `Self` or an ancestor declares a `writable` reflected field.
+ #[doc(hidden)]
+ const TYPE_HAS_WRITABLE_FIELD: bool = false;
/// The runtime type index of this type.
fn type_index() -> i32;
/// Whether [`type_index`](Self::type_index) can return now: false while
@@ -1162,10 +1173,16 @@ pub mod unsafe_ {
/// Layout of a `T` followed by `count` extra items, kept at least one
word past the
/// header: once a weak reference outlives the data, the deleter stores
`count` there.
+ /// Panics with "capacity overflow" (like `Vec`) when the size does not
fit in `isize`.
pub(crate) fn extra_items_layout<T, U>(count: usize) -> std::alloc::Layout
{
- let size = std::mem::size_of::<T>() + count * std::mem::size_of::<U>();
+ use std::alloc::Layout;
let min_size = std::mem::size_of::<TVMFFIObject>() +
std::mem::size_of::<u64>();
- std::alloc::Layout::from_size_align(size.max(min_size),
std::mem::align_of::<T>()).unwrap()
+ Layout::array::<U>(count)
+ .and_then(|items| Layout::new::<T>().extend(items))
+ .and_then(|(layout, _)| {
+ Layout::from_size_align(layout.size().max(min_size),
layout.align())
+ })
+ .expect("capacity overflow")
}
pub(crate) unsafe extern "C" fn object_deleter_for_new_with_extra_items<T,
U>(
diff --git a/rust/tvm-ffi/src/reflection/enum_def.rs
b/rust/tvm-ffi/src/reflection/enum_def.rs
index 46b00185..15996a1d 100644
--- a/rust/tvm-ffi/src/reflection/enum_def.rs
+++ b/rust/tvm-ffi/src/reflection/enum_def.rs
@@ -60,6 +60,7 @@ use crate::{Any, AnyView, EnumRef, Error, List, Result,
String, TYPE_ERROR, VALU
/// #[type_key = "my_ext.Color"]
/// #[type_register]
/// #[type_final]
+/// #[type_immutable]
/// pub struct ColorObj {
/// base_: EnumObj,
/// }
diff --git a/rust/tvm-ffi/src/reflection/registry.rs
b/rust/tvm-ffi/src/reflection/registry.rs
index 6d0fd3cf..36d61ed2 100644
--- a/rust/tvm-ffi/src/reflection/registry.rs
+++ b/rust/tvm-ffi/src/reflection/registry.rs
@@ -435,8 +435,9 @@ impl<N: ObjectCore> ObjectDef<N> {
return Ok(());
}
if let Some(creator) = self.creator {
- // Objects from this creator start zeroed: another language must
set
- // every field before Rust code reads it.
+ // Objects from this creator start zero-filled, in the `UnsafeInit`
+ // state: whoever creates one, in any language, must set every
+ // field before it is used.
let new = Function::from_typed(move || -> Result<ObjectRef> {
let mut handle: TVMFFIObjectHandle = std::ptr::null_mut();
crate::check_safe_call!(unsafe { creator(&mut handle) })?;
diff --git a/rust/tvm-ffi/tests/test_cast.rs b/rust/tvm-ffi/tests/test_cast.rs
index f509e8e3..a1399c59 100644
--- a/rust/tvm-ffi/tests/test_cast.rs
+++ b/rust/tvm-ffi/tests/test_cast.rs
@@ -55,6 +55,7 @@ struct TestBase {
#[type_key = "testing.rust.CastDerived"]
#[type_register]
#[type_final]
+#[type_immutable]
struct TestDerivedObj {
base_: TestBaseObj,
extra: i64,
diff --git a/rust/tvm-ffi/tests/test_enum.rs b/rust/tvm-ffi/tests/test_enum.rs
index 524e10e5..7aef0428 100644
--- a/rust/tvm-ffi/tests/test_enum.rs
+++ b/rust/tvm-ffi/tests/test_enum.rs
@@ -29,6 +29,7 @@ use tvm_ffi::{EnumRef, ObjectArc, ObjectRefCore, Result,
VALUE_ERROR};
#[derive(Object)]
#[type_key = "testing.TestEnumVariant"]
#[type_final]
+#[type_immutable]
struct TestEnumVariantObj {
base_: EnumObj,
}
diff --git a/rust/tvm-ffi/tests/test_match_any.rs
b/rust/tvm-ffi/tests/test_match_any.rs
index 463889dd..8999cd80 100644
--- a/rust/tvm-ffi/tests/test_match_any.rs
+++ b/rust/tvm-ffi/tests/test_match_any.rs
@@ -20,7 +20,9 @@
use std::any::TypeId;
use tvm_ffi::match_any_internal::{ArmId, LeafLookupTable, LeafPatternMetadata,
LeafPatternProbe};
-use tvm_ffi::{match_any, Any, AnyView, Array, Function, Map, Module, Shape,
Tensor, TypeIndex};
+use tvm_ffi::{
+ match_any, Any, AnyView, Array, Function, Map, Module, Shape, Tensor,
TypeIndex, VisitInterrupt,
+};
type CustomModuleMatcher = Module;
@@ -416,7 +418,7 @@ fn
direct_lookup_table_maps_runtime_indices_to_local_arm_ids() {
// `(&probe)` mirrors the receiver `match_any!` generates for autoref
specialization.
#[allow(clippy::needless_borrow)]
fn metadata_only_accepts_exact_leaf_patterns() {
- type Leaf = (Module, ());
+ type Leaf = (VisitInterrupt, ());
let leaf = LeafPatternProbe::<Leaf>::new();
let mut type_indices = [0; 1];
assert!((&leaf).leaf_pattern_list_id().is_some());
