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 9288282f [Rust] Append visit error context in place (#940)
9288282f is described below
commit 9288282fee38e0ba681411b80bb36d211d436e60
Author: Tianqi Chen <[email protected]>
AuthorDate: Fri Nov 6 00:15:44 2026 -0500
[Rust] Append visit error context in place (#940)
Rust structural visit and map annotate an escaping error with a
`VisitErrorContext` breadcrumb per frame. Today
`with_visit_error_context` reads the whole pattern through
`ffi.ArraySize`/`ffi.ArrayGetItem`, then rebuilds the Array, the context
(`ffi.MakeObjectFromPackedArgs`) and the Error at every frame, which is
quadratic in the unwind depth.
This follows C++ `detail::UpdateVisitErrorContext` instead, entirely in
Rust: copy-on-write at each level, so an Error owned by the unwinding
frame has its pattern appended in place, while a shared Error, context
or pattern is copied before the write. The same-frame dedupe is kept,
and a fresh context keeps the previous `extra_context` as
`prev_error_context`.
- A crate-internal `#[repr(C)]` binding of `ffi.VisitErrorContext` with
the C++ field layout (`reverse_visit_pattern`, `prev_error_context`),
checked against the registered field offsets in a unit test.
- A crate-internal `Error::extra_context_mut`, which first replaces a
shared Error by a shallow copy.
- A crate-internal copy-on-write `Array::push` that writes in place when
unique with room and otherwise moves or copies into a buffer that grows
geometrically. To support it, a Rust-allocated `ArrayObj` now sizes its
inline buffer by `capacity`, and releases its elements when dropped
(previously they were never released).
No public API changes.
Also adds `String::is_empty` and `Bytes::is_empty` (next to `len`, as in
std) and `impl Default for Shape` (the empty shape), so they are part of
the API before stabilization.
---
rust/tvm-ffi/src/collections/array.rs | 62 +++++++++-
rust/tvm-ffi/src/collections/shape.rs | 8 ++
rust/tvm-ffi/src/error.rs | 21 ++++
rust/tvm-ffi/src/extra/structural_common.rs | 164 ++++++++++++++-------------
rust/tvm-ffi/src/string.rs | 10 ++
rust/tvm-ffi/tests/test_structural_mutate.rs | 69 ++++-------
6 files changed, 207 insertions(+), 127 deletions(-)
diff --git a/rust/tvm-ffi/src/collections/array.rs
b/rust/tvm-ffi/src/collections/array.rs
index 6a6f989e..930458d6 100644
--- a/rust/tvm-ffi/src/collections/array.rs
+++ b/rust/tvm-ffi/src/collections/array.rs
@@ -43,10 +43,24 @@ pub struct ArrayObj {
pub data_deleter: Option<unsafe extern "C" fn(*mut core::ffi::c_void)>,
}
+// A Rust-allocated array holds `capacity` cells inline, of which the first
`size`
+// are initialized.
unsafe impl ObjectCoreWithExtraItems for ArrayObj {
type ExtraItem = TVMFFIAny;
fn extra_items_count(this: &Self) -> usize {
- this.size as usize
+ this.capacity as usize
+ }
+}
+
+// Runs only for a Rust-allocated array; one allocated in C++ is released by
its
+// own deleter.
+impl Drop for ArrayObj {
+ fn drop(&mut self) {
+ let cells = self.data as *const TVMFFIAny;
+ for i in 0..self.size as usize {
+ // SAFETY: the first `size` cells are initialized and owned by the
array.
+ drop(unsafe { Any::from_raw_ffi_any(*cells.add(i)) });
+ }
}
}
@@ -199,7 +213,7 @@ impl<T: ContainerElement + Clone> Array<T> {
let raw_ptr = ObjectArc::as_ptr(&arc) as *mut ArrayObj;
let container = &mut *raw_ptr;
- let base_ptr = ArrayObj::extra_items_mut(container).as_ptr() as
*mut TVMFFIAny;
+ let base_ptr = ArrayObj::extra_items_mut(container).as_mut_ptr();
container.data = base_ptr as *mut _;
for (i, item) in items.into_iter().enumerate() {
@@ -212,6 +226,50 @@ impl<T: ContainerElement + Clone> Array<T> {
unsafe { Self::from_data(arc) }
}
+ /// Append `value`, copy-on-write like C++ `Array::push_back`: in place
when
+ /// this handle is the only reference and the buffer has room, otherwise
into
+ /// a new buffer that grows geometrically when full.
+ pub(crate) fn push(&mut self, value: T) {
+ let size = self.len();
+ let capacity = self.data.capacity as usize;
+ let unique = ObjectArc::strong_count(&self.data) == 1;
+ if !unique || size == capacity {
+ let capacity = if size == capacity {
+ (capacity * 2).max(size + 1)
+ } else {
+ capacity
+ };
+ let mut grown = Self::new_with_capacity(Vec::new(), capacity);
+ let source = self.as_container().data as *const TVMFFIAny;
+ // SAFETY: `grown` is fresh with room for `size` cells. A unique
source
+ // moves its cells and forgets them; a shared one keeps its cells,
so each
+ // copy takes its own reference.
+ unsafe {
+ let target = ObjectArc::as_mut_ptr(&mut grown.data);
+ let cells = (*target).data as *mut TVMFFIAny;
+ if unique {
+ std::ptr::copy_nonoverlapping(source, cells, size);
+ (*ObjectArc::as_mut_ptr(&mut self.data)).size = 0;
+ } else {
+ for i in 0..size {
+ let copy =
Any::from(AnyView::from_raw_ffi_any(*source.add(i)));
+ std::ptr::write(cells.add(i),
Any::into_raw_ffi_any(copy));
+ }
+ }
+ (*target).size = size as i64;
+ }
+ *self = grown;
+ }
+ // SAFETY: this handle is now the only reference, with room for one
more cell.
+ unsafe {
+ let container = ObjectArc::as_mut_ptr(&mut self.data);
+ let mut cell = TVMFFIAny::new();
+ T::container_move_to_any(value, &mut cell);
+ std::ptr::write(((*container).data as *mut TVMFFIAny).add(size),
cell);
+ (*container).size += 1;
+ }
+ }
+
/// Return the number of elements.
///
/// # Returns
diff --git a/rust/tvm-ffi/src/collections/shape.rs
b/rust/tvm-ffi/src/collections/shape.rs
index 0600a4a4..7abfb7ff 100644
--- a/rust/tvm-ffi/src/collections/shape.rs
+++ b/rust/tvm-ffi/src/collections/shape.rs
@@ -58,6 +58,14 @@ pub struct Shape {
unsafe impl Send for Shape {}
unsafe impl Sync for Shape {}
+impl Default for Shape {
+ /// The empty shape, like [`Shape::new`].
+ #[inline]
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
impl Shape {
/// Create a new empty Shape
pub fn new() -> Self {
diff --git a/rust/tvm-ffi/src/error.rs b/rust/tvm-ffi/src/error.rs
index be0b7675..42c3a053 100644
--- a/rust/tvm-ffi/src/error.rs
+++ b/rust/tvm-ffi/src/error.rs
@@ -323,6 +323,27 @@ impl Error {
Some(unsafe { &*(handle as *const _ as *const object::ObjectRef) })
}
+ /// The `extra_context` slot, to update in place. A shared error is first
+ /// replaced by a shallow copy, so the update reaches no other holder.
+ pub(crate) fn extra_context_mut(&mut self) -> &mut
Option<object::ObjectRef> {
+ if ObjectArc::strong_count(&self.data) != 1 {
+ *self = Self::new_with_cause_and_extra_context(
+ self.kind(),
+ self.message(),
+ self.backtrace(),
+ self.cause_chain(),
+ self.extra_context(),
+ );
+ }
+ // SAFETY: this handle is the only reference, and the slot owns a
nullable
+ // object handle, which is exactly an `Option<ObjectRef>`.
+ unsafe {
+ let error = ObjectArc::as_mut_ptr(&mut self.data);
+ &mut *std::ptr::addr_of_mut!((*error).cell.extra_context)
+ .cast::<Option<object::ObjectRef>>()
+ }
+ }
+
/// Get the traceback of the error in the order of most recent call last,
as
/// Python prints it.
///
diff --git a/rust/tvm-ffi/src/extra/structural_common.rs
b/rust/tvm-ffi/src/extra/structural_common.rs
index 058e8672..0c1ed75f 100644
--- a/rust/tvm-ffi/src/extra/structural_common.rs
+++ b/rust/tvm-ffi/src/extra/structural_common.rs
@@ -18,13 +18,12 @@
*/
use crate::any::{Any, AnyView};
+use crate::collections::array::Array;
+use crate::derive::Object;
use crate::error::{Error, Result, RUNTIME_ERROR};
-use crate::function::Function;
-use crate::object::{self, ObjectRefCore};
-use crate::reflection::FieldGetter;
+use crate::object::{self, Object, ObjectArc, ObjectRef, ObjectRefCore};
use crate::tvm_ffi_sys::{
- TVMFFIAny, TVMFFIAnyDataUnion, TVMFFIByteArray, TVMFFIGetTypeInfo,
TVMFFITypeIndex,
- TVMFFITypeKeyToIndex,
+ TVMFFIAny, TVMFFIAnyDataUnion, TVMFFIGetTypeInfo, TVMFFIObject,
TVMFFITypeIndex,
};
//
-----------------------------------------------------------------------------
@@ -108,88 +107,95 @@ pub(crate) fn with_structural_error_context(error: Error,
operation: &str, frame
Error::with_appended_backtrace(error, &format!("[native structural
{operation}] {frame}\n"))
}
+/// Rust binding of C++ `VisitErrorContextObj`, the payload that
+/// `Error::extra_context` carries to name the nodes a failed visit unwound
through.
+#[repr(C)]
+#[derive(Object)]
+#[type_key = "ffi.VisitErrorContext"]
+#[type_final]
+pub(crate) struct VisitErrorContextObj {
+ base: Object,
+ /// Visited nodes, innermost first; C++ `ObjectRef` is nullable.
+ reverse_visit_pattern: Array<Option<ObjectRef>>,
+ /// The `extra_context` the error carried before this context replaced it.
+ prev_error_context: Option<ObjectRef>,
+}
+
+/// Name the visited object in `raw` in the error's visit context, like C++
+/// `detail::UpdateVisitErrorContext`. Copy-on-write at each level: the Error,
its
+/// context and the context's pattern are each copied only when shared, so an
error
+/// owned by the unwinding frame is appended in place.
#[cold]
-pub(crate) fn with_visit_error_context(error: Error, raw: TVMFFIAny) -> Error {
+pub(crate) fn with_visit_error_context(mut error: Error, raw: TVMFFIAny) ->
Error {
if raw.type_index < TVMFFITypeIndex::kTVMFFIStaticObjectBegin as i32 {
return error;
}
- let context = (|| {
- let mut context_type = 0;
+ // SAFETY: the traversal input remains live during annotation. Conversion
+ // acquires ownership before the temporary view ends; raw is an object tag.
+ let Ok(node) = ObjectRef::try_from(unsafe { AnyView::from_raw_ffi_any(raw)
}) else {
+ return error;
+ };
+ let as_context = |context: &ObjectRef| {
+ let object = ObjectArc::as_ptr(ObjectRef::data(context));
+ // SAFETY: `context` keeps its object alive; the type is checked first.
unsafe {
- crate::check_safe_call!(TVMFFITypeKeyToIndex(
- &TVMFFIByteArray::from_str("ffi.VisitErrorContext"),
- &mut context_type,
- ))
- .ok()?;
+ object::is_instance_of::<VisitErrorContextObj>(
+ (*object.cast::<TVMFFIObject>()).type_index,
+ )
+ .then(|| &*object.cast::<VisitErrorContextObj>())
}
- // SAFETY: the traversal input remains live during annotation.
Conversion
- // acquires ownership before the temporary view ends; raw is an object
tag.
- let node = object::ObjectRef::try_from(unsafe {
AnyView::from_raw_ffi_any(raw) }).ok()?;
- let mut previous = error.extra_context().cloned();
- let mut nodes = Vec::new();
- if let Some(prior) = previous.as_ref() {
- if AnyView::from(prior).type_index() == context_type {
- let object = &**object::ObjectRef::data(prior);
- let records = FieldGetter::new(context_type,
"reverse_visit_pattern")
- .ok()?
- .get_any(object)
- .ok()?;
- let size = Function::get_global("ffi.ArraySize")
- .ok()?
- .call_tuple((records.clone(),))
- .ok()?
- .downcast_ref::<i64>()
- .copied()?;
- let get_item = Function::get_global("ffi.ArrayGetItem").ok()?;
- if size > 0 {
- let last: Option<object::ObjectRef> = get_item
- .call_tuple((records.clone(), size - 1))
- .ok()?
- .try_into()
- .ok();
- // Callback, policy and default descent can report the
same frame.
- if last.is_some_and(|last| last.same_as(&node)) {
- return None;
- }
- }
- for i in 0..size {
- nodes.push(get_item.call_tuple((records.clone(),
i)).ok()?);
- }
- previous = FieldGetter::new(context_type, "prev_error_context")
- .ok()?
- .get::<_, Option<object::ObjectRef>>(object)
- .ok()?;
- }
+ };
+ // Callback, policy and default descent can report the same frame.
+ if let Some(context) = error.extra_context().and_then(as_context) {
+ if context
+ .reverse_visit_pattern
+ .iter()
+ .next_back()
+ .and_then(Option::as_ref)
+ .is_some_and(|last| last.same_as(&node))
+ {
+ return error;
}
- nodes.push(Any::from(node));
- let records = Function::get_global("ffi.Array")
- .ok()?
- .call_packed(&nodes.iter().map(AnyView::from).collect::<Vec<_>>())
- .ok()?;
- Function::get_global("ffi.MakeObjectFromPackedArgs")
- .ok()?
- .call_tuple((
- context_type,
- crate::String::from("reverse_visit_pattern"),
- records,
- crate::String::from("prev_error_context"),
- previous,
- ))
- .ok()?
- .try_into()
- .ok()
- })();
- // Diagnostic enrichment must not replace the original error on failure.
- match context {
- Some(context) => Error::new_with_cause_and_extra_context(
- error.kind(),
- error.message(),
- error.backtrace(),
- error.cause_chain(),
- Some(&context),
- ),
- None => error,
}
+ let slot = error.extra_context_mut();
+ let previous = slot.take();
+ let context = match previous {
+ Some(previous) if as_context(&previous).is_some() => {
+ // SAFETY: `previous` was checked to be a `VisitErrorContextObj`.
+ let mut context = unsafe {
+ ObjectArc::from_raw(
+ ObjectArc::into_raw(ObjectRef::into_data(previous))
+ .cast::<VisitErrorContextObj>(),
+ )
+ };
+ if ObjectArc::strong_count(&context) != 1 {
+ context = ObjectArc::new(VisitErrorContextObj {
+ base: Object::new(),
+ reverse_visit_pattern:
context.reverse_visit_pattern.clone(),
+ prev_error_context: context.prev_error_context.clone(),
+ });
+ }
+ // SAFETY: this handle is the context's only reference.
+ unsafe {
+ (*ObjectArc::as_mut_ptr(&mut context))
+ .reverse_visit_pattern
+ .push(Some(node))
+ };
+ context
+ }
+ previous => ObjectArc::new(VisitErrorContextObj {
+ base: Object::new(),
+ reverse_visit_pattern: Array::from_vec(vec![Some(node)]),
+ prev_error_context: previous,
+ }),
+ };
+ // SAFETY: every object starts with the root `Object` header.
+ *slot = Some(unsafe {
+ ObjectRef::from_data(ObjectArc::from_raw(
+ ObjectArc::into_raw(context).cast::<Object>(),
+ ))
+ });
+ error
}
// Generate the tuple arities supported by the standard library (1 through 12).
diff --git a/rust/tvm-ffi/src/string.rs b/rust/tvm-ffi/src/string.rs
index ec55ea5a..d9c6a7a5 100644
--- a/rust/tvm-ffi/src/string.rs
+++ b/rust/tvm-ffi/src/string.rs
@@ -76,6 +76,11 @@ impl Bytes {
self.as_slice().len()
}
+ /// Return `true` if there are no bytes.
+ pub fn is_empty(&self) -> bool {
+ self.len() == 0
+ }
+
/// Get the bytes as a slice
pub fn as_slice(&self) -> &[u8] {
unsafe {
@@ -300,6 +305,11 @@ impl String {
self.as_bytes().len()
}
+ /// Return `true` if the string is empty.
+ pub fn is_empty(&self) -> bool {
+ self.len() == 0
+ }
+
/// Get the string as a byte slice
pub fn as_bytes(&self) -> &[u8] {
unsafe {
diff --git a/rust/tvm-ffi/tests/test_structural_mutate.rs
b/rust/tvm-ffi/tests/test_structural_mutate.rs
index 00c454d5..1059b79f 100644
--- a/rust/tvm-ffi/tests/test_structural_mutate.rs
+++ b/rust/tvm-ffi/tests/test_structural_mutate.rs
@@ -279,6 +279,12 @@ fn list_item(list: &Any, index: i64) -> i64 {
.unwrap()
}
+type Pattern = Array<Option<ObjectRef>>;
+
+fn pattern_pointer(records: &Pattern, index: usize) -> *const Object {
+ ObjectArc::as_ptr(ObjectRef::data(&records.get(index).unwrap().unwrap()))
+}
+
fn array_item(array: &Any, index: i64) -> Any {
Function::get_global("ffi.ArrayGetItem")
.unwrap()
@@ -1167,20 +1173,11 @@ fn
callback_errors_preserve_message_and_add_object_context() {
assert!(error.cause_chain().unwrap().same_as(&cause));
let context = Any::from(error.extra_context().unwrap().clone());
assert!(reflected_field::<ObjectRef>(&context,
"prev_error_context").same_as(&payload));
- let records = Any::from(reflected_field::<ObjectRef>(
- &context,
- "reverse_visit_pattern",
- ));
- let size = i64::try_from(call_global("ffi.ArraySize",
&[records.clone()])).unwrap();
- assert_eq!(size, 3);
- assert_eq!(
- call_global("ffi.ArrayGetItem", &[records.clone(),
0_i64.into()]).type_index(),
- TypeIndex::kTVMFFINone as i32
- );
- let innermost = call_global("ffi.ArrayGetItem", &[records.clone(),
1_i64.into()]);
- assert_eq!(any_object_pointer(&innermost), any_object_pointer(&child));
- let outermost = call_global("ffi.ArrayGetItem", &[records, (size -
1).into()]);
- assert_eq!(any_object_pointer(&outermost), any_object_pointer(&root));
+ let records: Pattern = reflected_field(&context,
"reverse_visit_pattern");
+ assert_eq!(records.len(), 3);
+ assert!(records.get(0).unwrap().is_none());
+ assert_eq!(pattern_pointer(&records, 1), any_object_pointer(&child));
+ assert_eq!(pattern_pointer(&records, 2), any_object_pointer(&root));
let paths = call_global(
"ffi.VisitErrorContext.FindAccessPaths",
&[root.clone(), context, false.into()],
@@ -1196,14 +1193,8 @@ fn
callback_errors_preserve_message_and_add_object_context() {
"<root>.value"
);
}
- let records = Any::from(reflected_field::<ObjectRef>(
- &context,
- "reverse_visit_pattern",
- ));
- assert_eq!(
- i64::try_from(call_global("ffi.ArraySize", &[records])).unwrap(),
- 1
- );
+ let records: Pattern = reflected_field(&context, "reverse_visit_pattern");
+ assert_eq!(records.len(), 1);
assert_eq!(source.backtrace(), "origin");
// Keep nonconsecutive occurrences: child -> root -> child is a distinct
path.
@@ -1215,20 +1206,13 @@ fn
callback_errors_preserve_message_and_add_object_context() {
.err()
.unwrap();
let context = Any::from(error.extra_context().unwrap().clone());
- let records = Any::from(reflected_field::<ObjectRef>(
- &context,
- "reverse_visit_pattern",
- ));
- assert_eq!(
- i64::try_from(call_global("ffi.ArraySize",
&[records.clone()])).unwrap(),
- 4
- );
+ let records: Pattern = reflected_field(&context, "reverse_visit_pattern");
+ assert_eq!(records.len(), 4);
for (i, expected) in [&child, &root, &child].into_iter().enumerate() {
- let node = call_global(
- "ffi.ArrayGetItem",
- &[records.clone(), (i as i64 + 1).into()],
+ assert_eq!(
+ pattern_pointer(&records, i + 1),
+ any_object_pointer(expected)
);
- assert_eq!(any_object_pointer(&node), any_object_pointer(expected));
}
// The failing node may be a pre-order replacement or a post-order rebuilt
parent.
@@ -1264,20 +1248,13 @@ fn
callback_errors_preserve_message_and_add_object_context() {
.err()
.unwrap();
let context = Any::from(error.extra_context().unwrap().clone());
- let records = Any::from(reflected_field::<ObjectRef>(
- &context,
- "reverse_visit_pattern",
- ));
- let node = call_global("ffi.ArrayGetItem", &[records.clone(),
0i64.into()]);
- assert!(node
- .downcast_ref::<ObjectRef>()
- .cloned()
+ let records: Pattern = reflected_field(&context,
"reverse_visit_pattern");
+ assert!(records
+ .get(0)
+ .unwrap()
.unwrap()
.same_as(&failed_node.unwrap()));
- assert_eq!(
- i64::try_from(call_global("ffi.ArraySize", &[records])).unwrap(),
- 1
- );
+ assert_eq!(records.len(), 1);
assert_eq!(root.get(0).unwrap(), 0);
}
}
