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 12c6fc51 [EXTRA] VisitErrorContext: Array with copy-on-write append
(#939)
12c6fc51 is described below
commit 12c6fc51e71bc23bf3042258503826135abb41f3
Author: Tianqi Chen <[email protected]>
AuthorDate: Thu Nov 5 23:46:01 2026 -0500
[EXTRA] VisitErrorContext: Array with copy-on-write append (#939)
`VisitErrorContextObj` kept its visit pattern in a mutable `List`, and
`UpdateVisitErrorContext` appended to it and rewrote the `ErrorObj`'s
`extra_context` in place. That was safe only while the unwinding frame
was the Error's sole owner, and `Error`'s `Send + Sync` claim depended
on that assumption.
This PR makes the update copy-on-write at each level:
- `reverse_visit_pattern` is now an `Array<ObjectRef>`, and
`VisitErrorContextObj` is no longer `_type_mutable`.
- `UpdateVisitErrorContext` swaps `err` for a shallow copy (kind,
message, backtrace, cause chain, extra context) if the `ErrorObj` is
shared. If the error's `VisitErrorContext` is shared, it gets swapped
for a copy too. Then `Array::push_back` copies its own storage if that
is shared. Each level is borrowed through its parent's field, so when
the unwinding frame is the only owner, the update still happens in
place.
- The raw-result and `Expected` overloads used by the visit and mutate
engines now move the Error out of the result and put it back. Before,
they annotated a second handle. That second handle would now trigger a
copy, so moving keeps the engine's sole-owner path in place.
- Every `UpdateVisitErrorContext` overload, and
`StructuralMutatorObj::UpdateVisitErrorContext`, takes the Error or
result by pointer, so call sites show that it may be replaced.
- On the Rust side, structural annotation reads and builds the pattern
through the `ffi.Array*` globals.
The Error C ABI is unchanged; `extra_context` stays an opaque handle.
Rust stub generation now emits just the `ImmutableObject` impl line,
with no SAFETY comment, and the checked-in `rust_stubgen` example now
includes that line for `IntPairObj` (it was missing after #938).
---
.../rust/src/generated/rust_stubgen/mod.rs | 2 +
include/tvm/ffi/extra/structural_mutate.h | 25 ++---
include/tvm/ffi/extra/structural_visit.h | 8 +-
include/tvm/ffi/extra/visit_error_context.h | 102 ++++++++++++---------
python/tvm_ffi/stub/rust_generator/codegen.py | 6 +-
rust/tvm-ffi/src/extra/structural_common.rs | 6 +-
rust/tvm-ffi/tests/test_structural_mutate.rs | 23 +++--
src/ffi/extra/structural_mutate.cc | 4 +-
src/ffi/extra/visit_error_context.cc | 2 +-
tests/cpp/extra/test_structural_mutate.cc | 2 +-
tests/cpp/extra/test_visit_error_context.cc | 69 +++++++++-----
11 files changed, 141 insertions(+), 108 deletions(-)
diff --git a/examples/rust_stubgen/rust/src/generated/rust_stubgen/mod.rs
b/examples/rust_stubgen/rust/src/generated/rust_stubgen/mod.rs
index d32c0829..3d660241 100644
--- a/examples/rust_stubgen/rust/src/generated/rust_stubgen/mod.rs
+++ b/examples/rust_stubgen/rust/src/generated/rust_stubgen/mod.rs
@@ -108,6 +108,8 @@ impl IntPair {
Self { base: ObjectArc::new(obj) }
}
}
+
+unsafe impl tvm_ffi::object::ImmutableObject for IntPairObj {}
// tvm-ffi-stubgen(end)
// Hand-written code lives outside the blocks; the generator never touches it.
diff --git a/include/tvm/ffi/extra/structural_mutate.h
b/include/tvm/ffi/extra/structural_mutate.h
index afc1a628..8f86e2ab 100644
--- a/include/tvm/ffi/extra/structural_mutate.h
+++ b/include/tvm/ffi/extra/structural_mutate.h
@@ -675,7 +675,7 @@ class StructuralMutatorObj : public Object {
/*! \brief Keep address-taking for error decoration off the successful
raw-result path. */
TVM_FFI_COLD_CODE static TVMFFIAny AttachVisitErrorContextRaw(TVMFFIAny
result,
AnyView value)
noexcept {
- detail::UpdateVisitErrorContext(result, value);
+ detail::UpdateVisitErrorContext(&result, value);
return result;
}
@@ -788,14 +788,9 @@ class StructuralMutatorObj : public Object {
* \param result The failed result whose Error is annotated.
* \param node The borrowed node to name in the context.
*/
- TVM_FFI_COLD_CODE static void UpdateVisitErrorContext(const Expected<Any>&
result,
+ TVM_FFI_COLD_CODE static void UpdateVisitErrorContext(Expected<Any>* result,
// NOLINT(*)
AnyView node) noexcept
{
- // The Error is refcounted, so annotating the local handle annotates the
object the result
- // holds. A non-object node has no context to add.
- if (node.type_index() >= TypeIndex::kTVMFFIStaticObjectBegin) {
- Error err = result.error();
- ::tvm::ffi::detail::UpdateVisitErrorContext(err, node.cast<ObjectRef>());
- }
+ ::tvm::ffi::detail::UpdateVisitErrorContext(result, node);
}
/*!
@@ -1323,7 +1318,7 @@ class StructuralMapEngine : public
detail::StructuralMutatorEngineParent<Parent>
}
}();
if (TVM_FFI_PREDICT_FALSE(callback_result.is_err())) {
- this->UpdateVisitErrorContext(callback_result, value);
+ this->UpdateVisitErrorContext(&callback_result, value);
*out = std::move(callback_result);
return true;
}
@@ -1373,7 +1368,7 @@ class StructuralMapEngine : public
detail::StructuralMutatorEngineParent<Parent>
}
}();
if (TVM_FFI_PREDICT_FALSE(callback_result.is_err())) {
- this->UpdateVisitErrorContext(callback_result, mapped_view);
+ this->UpdateVisitErrorContext(&callback_result, mapped_view);
*out = std::move(callback_result);
return true;
}
@@ -1550,7 +1545,7 @@ class StructuralMapDynEngine : public
detail::StructuralMutatorEngineParent<Pare
Expected<Any> callback_result =
InvokeLink(*matched, with_kind, value, this->def_region_kind());
if (TVM_FFI_PREDICT_FALSE(callback_result.is_err())) {
- this->UpdateVisitErrorContext(callback_result, value);
+ this->UpdateVisitErrorContext(&callback_result, value);
*out = std::move(callback_result);
return true;
}
@@ -1591,7 +1586,7 @@ class StructuralMapDynEngine : public
detail::StructuralMutatorEngineParent<Pare
Expected<Any> callback_result =
InvokeLink(*matched, with_kind, mapped_view,
this->def_region_kind());
if (TVM_FFI_PREDICT_FALSE(callback_result.is_err())) {
- this->UpdateVisitErrorContext(callback_result, mapped_view);
+ this->UpdateVisitErrorContext(&callback_result, mapped_view);
*out = std::move(callback_result);
return true;
}
@@ -1707,7 +1702,7 @@ class StructuralMutateEngine : public
detail::StructuralMutatorEngineParent<Pare
if (TVM_FFI_PREDICT_FALSE(result.is_err())) {
// Keep callback-boundary context in addition to the default-descent
// context: a callback may return a rebuilt value, so the two nodes
can differ.
- Parent::UpdateVisitErrorContext(result, value);
+ Parent::UpdateVisitErrorContext(&result, value);
}
return detail::ExpectedUnsafe::MoveToTVMFFIAny(std::move(result));
}
@@ -1722,7 +1717,7 @@ class StructuralMutateEngine : public
detail::StructuralMutatorEngineParent<Pare
if (TVM_FFI_PREDICT_FALSE(result.is_err())) {
// Keep callback-boundary context in addition to the default-descent
// context: a callback may return a rebuilt value, so the two nodes
can differ.
- Parent::UpdateVisitErrorContext(result, value);
+ Parent::UpdateVisitErrorContext(&result, value);
}
return detail::ExpectedUnsafe::MoveToTVMFFIAny(std::move(result));
}
@@ -1763,7 +1758,7 @@ class StructuralMutateEngine : public
detail::StructuralMutatorEngineParent<Pare
/*! \brief Materialize the callback result only on the cold Parent
error-decoration path. */
TVM_FFI_COLD_CODE TVMFFIAny AnnotateCallbackErrorRaw(TVMFFIAny raw, AnyView
value) noexcept {
Expected<Any> result = detail::ExpectedUnsafe::MoveFromTVMFFIAny<Any>(raw);
- Parent::UpdateVisitErrorContext(result, value);
+ Parent::UpdateVisitErrorContext(&result, value);
return detail::ExpectedUnsafe::MoveToTVMFFIAny(std::move(result));
}
diff --git a/include/tvm/ffi/extra/structural_visit.h
b/include/tvm/ffi/extra/structural_visit.h
index 24b056ec..92c70721 100644
--- a/include/tvm/ffi/extra/structural_visit.h
+++ b/include/tvm/ffi/extra/structural_visit.h
@@ -369,7 +369,7 @@ TVM_FFI_INLINE bool StructuralVisitRawNeedEarlyReturn(const
TVMFFIAny& result) n
// Keep the raw result in registers on success; only error decoration takes
its address.
TVM_FFI_COLD_CODE inline TVMFFIAny
AttachStructuralVisitErrorContextRaw(TVMFFIAny result,
AnyView value) noexcept {
- UpdateVisitErrorContext(result, value);
+ UpdateVisitErrorContext(&result, value);
return result;
}
@@ -697,8 +697,7 @@ class StructuralWalkEngine : public Parent {
TryLinks(value, &result, std::index_sequence_for<Callbacks...>{});
if
(TVM_FFI_PREDICT_FALSE(detail::StructuralVisitNeedEarlyReturn(result))) {
if (TVM_FFI_PREDICT_FALSE(result.is_err())) {
- Error err = result.error();
- detail::UpdateVisitErrorContext(err, value);
+ detail::UpdateVisitErrorContext(&result, value);
}
return detail::ExpectedUnsafe::MoveToTVMFFIAny(std::move(result));
}
@@ -731,8 +730,7 @@ class StructuralWalkEngine : public Parent {
TryLinks(value, &result, std::index_sequence_for<Callbacks...>{});
if
(TVM_FFI_PREDICT_FALSE(detail::StructuralVisitNeedEarlyReturn(result))) {
if (TVM_FFI_PREDICT_FALSE(result.is_err())) {
- Error err = result.error();
- detail::UpdateVisitErrorContext(err, value);
+ detail::UpdateVisitErrorContext(&result, value);
}
return detail::ExpectedUnsafe::MoveToTVMFFIAny(std::move(result));
}
diff --git a/include/tvm/ffi/extra/visit_error_context.h
b/include/tvm/ffi/extra/visit_error_context.h
index 249eca50..4d5bde91 100644
--- a/include/tvm/ffi/extra/visit_error_context.h
+++ b/include/tvm/ffi/extra/visit_error_context.h
@@ -25,7 +25,6 @@
#define TVM_FFI_EXTRA_VISIT_ERROR_CONTEXT_H_
#include <tvm/ffi/container/array.h>
-#include <tvm/ffi/container/list.h>
#include <tvm/ffi/error.h>
#include <tvm/ffi/extra/base.h>
#include <tvm/ffi/function.h>
@@ -69,7 +68,7 @@ class VisitErrorContextObj : public Object {
* \brief Visit records that get populated, which include the object visit
* path pattern in innermost-first order. Best-effort — not
exhaustive.
*/
- List<ObjectRef> reverse_visit_pattern;
+ Array<ObjectRef> reverse_visit_pattern;
/*!
* \brief Pre-existing Error::extra_context payload before we placed the
@@ -78,7 +77,6 @@ class VisitErrorContextObj : public Object {
Optional<ObjectRef> prev_error_context;
/// \cond Doxygen_Suppress
- static constexpr bool _type_mutable = true;
static constexpr TVMFFISEqHashKind _type_s_eq_hash_kind =
kTVMFFISEqHashKindUnsupported;
TVM_FFI_DECLARE_OBJECT_INFO_FINAL("ffi.VisitErrorContext",
VisitErrorContextObj, Object);
/// \endcond
@@ -204,11 +202,11 @@ class VisitErrorContext : public ObjectRef {
* \param node The ObjectRef at the current visit level (appended to the
* error context's reverse_visit_pattern on exception).
*/
-#define TVM_FFI_VISIT_END(node) \
- } \
- catch (::tvm::ffi::Error & _tvm_ffi_visit_err_) { \
- ::tvm::ffi::detail::UpdateVisitErrorContext(_tvm_ffi_visit_err_, (node)); \
- throw; \
+#define TVM_FFI_VISIT_END(node)
\
+ }
\
+ catch (::tvm::ffi::Error & _tvm_ffi_visit_err_) {
\
+ ::tvm::ffi::detail::UpdateVisitErrorContext(&_tvm_ffi_visit_err_, (node));
\
+ throw;
\
}
/*!
@@ -220,11 +218,11 @@ class VisitErrorContext : public ObjectRef {
* \param node The current visit value. Object-backed values are appended to
the
* error context's reverse_visit_pattern on exception.
*/
-#define TVM_FFI_VISIT_END_RETURN_EXPECTED(node) \
- } \
- catch (::tvm::ffi::Error & _tvm_ffi_visit_err_) { \
- ::tvm::ffi::detail::UpdateVisitErrorContext(_tvm_ffi_visit_err_, (node)); \
- return ::tvm::ffi::Unexpected(_tvm_ffi_visit_err_); \
+#define TVM_FFI_VISIT_END_RETURN_EXPECTED(node)
\
+ }
\
+ catch (::tvm::ffi::Error & _tvm_ffi_visit_err_) {
\
+ ::tvm::ffi::detail::UpdateVisitErrorContext(&_tvm_ffi_visit_err_, (node));
\
+ return ::tvm::ffi::Unexpected(_tvm_ffi_visit_err_);
\
}
/*!
@@ -282,7 +280,7 @@ namespace detail {
TVM_FFI_COLD_CODE
inline VisitErrorContext MakeVisitErrorContext(const ObjectRef& node) {
ObjectPtr<VisitErrorContextObj> obj = make_object<VisitErrorContextObj>();
- obj->reverse_visit_pattern = List<ObjectRef>{node};
+ obj->reverse_visit_pattern = Array<ObjectRef>{node};
return VisitErrorContext(std::move(obj));
}
@@ -291,28 +289,35 @@ inline VisitErrorContext MakeVisitErrorContext(const
ObjectRef& node) {
* Calling convention may change; do not call directly from user code.
*/
TVM_FFI_COLD_CODE
-inline void UpdateVisitErrorContext(Error& err, const ObjectRef& node) { //
NOLINT(*)
- // NOTE: This function mutates the ErrorObj in place via ObjectUnsafe.
- // Expected to run only inside the exception throw chain, where the Error
- // is single-owned by this thread. The tradeoff avoids reallocating a
- // fresh Error per catch frame; the immutability invariant returns once
- // the unwind window closes.
- std::optional<ObjectRef> extra_context = err.extra_context();
- if (extra_context) {
- Optional<VisitErrorContext> visit_context =
extra_context->as<VisitErrorContext>();
- if (visit_context) {
- visit_context.value()->reverse_visit_pattern.push_back(node);
- return;
+inline void UpdateVisitErrorContext(Error* err, const ObjectRef& node) { //
NOLINT(*)
+ // Copy-on-write at each level: the Error is replaced by a shallow copy when
shared, and its
+ // VisitErrorContext likewise. Each level is borrowed through its parent's
field, so an Error
+ // owned only by the unwinding frame is updated in place.
+ if (!err->unique()) {
+ *err = Error(err->kind(), err->message(), err->backtrace(),
err->cause_chain(),
+ err->extra_context());
+ }
+ ErrorObj* error_obj =
+
static_cast<ErrorObj*>(detail::ObjectUnsafe::RawObjectPtrFromObjectRef(*err));
+ Object* extra_context = static_cast<Object*>(error_obj->extra_context);
+ if (extra_context != nullptr &&
extra_context->IsInstance<VisitErrorContextObj>()) {
+ VisitErrorContextObj* visit_context =
static_cast<VisitErrorContextObj*>(extra_context);
+ if (!visit_context->unique()) {
+ ObjectPtr<VisitErrorContextObj> copy =
make_object<VisitErrorContextObj>(*visit_context);
+ visit_context = copy.get();
+ detail::ObjectUnsafe::DecRefObjectHandle(error_obj->extra_context);
+ error_obj->extra_context =
+
detail::ObjectUnsafe::MoveObjectPtrToTVMFFIObjectPtr(std::move(copy));
}
+ visit_context->reverse_visit_pattern.push_back(node);
+ return;
}
// Build a fresh VisitErrorContext, preserving any pre-existing payload.
ObjectPtr<VisitErrorContextObj> new_context =
make_object<VisitErrorContextObj>();
- new_context->reverse_visit_pattern = List<ObjectRef>{node};
- if (extra_context) new_context->prev_error_context = *extra_context;
-
- ErrorObj* error_obj =
-
static_cast<ErrorObj*>(detail::ObjectUnsafe::RawObjectPtrFromObjectRef(err));
- if (error_obj->extra_context != nullptr) {
+ new_context->reverse_visit_pattern = Array<ObjectRef>{node};
+ if (extra_context != nullptr) {
+ new_context->prev_error_context =
detail::ObjectUnsafe::ObjectRefFromObjectPtr<ObjectRef>(
+ detail::ObjectUnsafe::ObjectPtrFromUnowned<Object>(extra_context));
detail::ObjectUnsafe::DecRefObjectHandle(error_obj->extra_context);
}
error_obj->extra_context =
@@ -323,17 +328,19 @@ inline void UpdateVisitErrorContext(Error& err, const
ObjectRef& node) { // NOL
* \brief Name \p node in a raw failed result's visit error context.
*
* The overload the visit and mutate engines use at a hook boundary: hooks
propagate errors
- * untouched, and the engine, which knows the node it dispatched on, adds that
frame. The Error
- * is refcounted, so annotating this handle annotates the object \p result
carries.
+ * untouched, and the engine, which knows the node it dispatched on, adds that
frame.
*
* \param result A raw result already known to hold an Error.
* \param node The node to name; a non-object node has no frame to add.
*/
-TVM_FFI_COLD_CODE inline void UpdateVisitErrorContext(const TVMFFIAny& result,
- AnyView node) noexcept {
+TVM_FFI_COLD_CODE inline void UpdateVisitErrorContext(TVMFFIAny* result,
AnyView node) noexcept {
if (node.type_index() >= TypeIndex::kTVMFFIStaticObjectBegin) {
- Error err = AnyView::CopyFromTVMFFIAny(result).cast<Error>();
- UpdateVisitErrorContext(err, node.cast<ObjectRef>());
+ // Take the Error out of the result and store it back, so one owned only
by the result is
+ // updated in place.
+ Error err = detail::ObjectUnsafe::ObjectRefFromObjectPtr<Error>(
+ detail::ObjectUnsafe::ObjectPtrFromOwned<Object>(result->v_obj));
+ UpdateVisitErrorContext(&err, node.cast<ObjectRef>());
+ result->v_obj =
detail::ObjectUnsafe::MoveObjectRefToTVMFFIObjectPtr(std::move(err));
}
}
@@ -346,16 +353,29 @@ TVM_FFI_COLD_CODE inline void
UpdateVisitErrorContext(const TVMFFIAny& result,
* `noexcept`, so an unguarded cast would terminate rather than propagate;
skipping the frame is
* correct because a non-object node has no identity to name in the context.
*
- * \param err The Error to annotate. Refcounted, so annotating this handle
annotates the shared
- * object the caller's result carries.
+ * \param err The Error to annotate; replaced by an annotated copy when shared.
* \param node The node to name; skipped when it is not object-backed.
*/
-TVM_FFI_COLD_CODE inline void UpdateVisitErrorContext(Error& err,
+TVM_FFI_COLD_CODE inline void UpdateVisitErrorContext(Error* err,
AnyView node) noexcept {
// NOLINT(*)
if (node.type_index() >= TypeIndex::kTVMFFIStaticObjectBegin) {
UpdateVisitErrorContext(err, node.cast<ObjectRef>());
}
}
+
+/*!
+ * \brief Name \p node in a failed Expected result's visit error context.
+ *
+ * \param result A result already known to hold an Error.
+ * \param node The node to name; skipped when it is not object-backed.
+ */
+template <typename T>
+TVM_FFI_COLD_CODE inline void UpdateVisitErrorContext(Expected<T>* result,
AnyView node) noexcept {
+ // Move the Error out and back, so one owned only by the result is updated
in place.
+ Error err = std::move(*result).error();
+ UpdateVisitErrorContext(&err, node);
+ *result = Unexpected<Error>(std::move(err));
+}
} // namespace detail
} // namespace ffi
diff --git a/python/tvm_ffi/stub/rust_generator/codegen.py
b/python/tvm_ffi/stub/rust_generator/codegen.py
index 5ed67467..a8bf35c4 100644
--- a/python/tvm_ffi/stub/rust_generator/codegen.py
+++ b/python/tvm_ffi/stub/rust_generator/codegen.py
@@ -583,11 +583,7 @@ class _ObjectRenderer:
"""
if self.info.is_mutable is not False:
return None
- return [
- f"// SAFETY: `{self.type_key}` is not `_type_mutable`: a shared
instance is never",
- "// mutated, as C++ changes it only through copy-on-write.",
- f"unsafe impl tvm_ffi::object::ImmutableObject for
{self.obj_struct} {{}}",
- ]
+ return [f"unsafe impl tvm_ffi::object::ImmutableObject for
{self.obj_struct} {{}}"]
def _shallow_copy_lines(self) -> list[str] | None:
"""``ShallowCopy``, which a subtype defined in Rust needs of its
parent's object.
diff --git a/rust/tvm-ffi/src/extra/structural_common.rs
b/rust/tvm-ffi/src/extra/structural_common.rs
index f7d45ac3..058e8672 100644
--- a/rust/tvm-ffi/src/extra/structural_common.rs
+++ b/rust/tvm-ffi/src/extra/structural_common.rs
@@ -134,13 +134,13 @@ pub(crate) fn with_visit_error_context(error: Error, raw:
TVMFFIAny) -> Error {
.ok()?
.get_any(object)
.ok()?;
- let size = Function::get_global("ffi.ListSize")
+ let size = Function::get_global("ffi.ArraySize")
.ok()?
.call_tuple((records.clone(),))
.ok()?
.downcast_ref::<i64>()
.copied()?;
- let get_item = Function::get_global("ffi.ListGetItem").ok()?;
+ 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))
@@ -162,7 +162,7 @@ pub(crate) fn with_visit_error_context(error: Error, raw:
TVMFFIAny) -> Error {
}
}
nodes.push(Any::from(node));
- let records = Function::get_global("ffi.List")
+ let records = Function::get_global("ffi.Array")
.ok()?
.call_packed(&nodes.iter().map(AnyView::from).collect::<Vec<_>>())
.ok()?;
diff --git a/rust/tvm-ffi/tests/test_structural_mutate.rs
b/rust/tvm-ffi/tests/test_structural_mutate.rs
index e69053cf..00c454d5 100644
--- a/rust/tvm-ffi/tests/test_structural_mutate.rs
+++ b/rust/tvm-ffi/tests/test_structural_mutate.rs
@@ -1091,7 +1091,7 @@ fn
callback_errors_preserve_message_and_add_object_context() {
],
);
let payload =
ObjectRef::try_from(Any::from(Array::from_vec(vec![7i64]))).unwrap();
- let records = call_global("ffi.List", &[Any::new()]);
+ let records = call_global("ffi.Array", &[Any::new()]);
let context = call_global(
"ffi.MakeObjectFromPackedArgs",
&[
@@ -1171,15 +1171,15 @@ fn
callback_errors_preserve_message_and_add_object_context() {
&context,
"reverse_visit_pattern",
));
- let size = i64::try_from(call_global("ffi.ListSize",
&[records.clone()])).unwrap();
+ let size = i64::try_from(call_global("ffi.ArraySize",
&[records.clone()])).unwrap();
assert_eq!(size, 3);
assert_eq!(
- call_global("ffi.ListGetItem", &[records.clone(),
0_i64.into()]).type_index(),
+ call_global("ffi.ArrayGetItem", &[records.clone(),
0_i64.into()]).type_index(),
TypeIndex::kTVMFFINone as i32
);
- let innermost = call_global("ffi.ListGetItem", &[records.clone(),
1_i64.into()]);
+ 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.ListGetItem", &[records, (size -
1).into()]);
+ let outermost = call_global("ffi.ArrayGetItem", &[records, (size -
1).into()]);
assert_eq!(any_object_pointer(&outermost), any_object_pointer(&root));
let paths = call_global(
"ffi.VisitErrorContext.FindAccessPaths",
@@ -1201,7 +1201,7 @@ fn
callback_errors_preserve_message_and_add_object_context() {
"reverse_visit_pattern",
));
assert_eq!(
- i64::try_from(call_global("ffi.ListSize", &[records])).unwrap(),
+ i64::try_from(call_global("ffi.ArraySize", &[records])).unwrap(),
1
);
assert_eq!(source.backtrace(), "origin");
@@ -1220,11 +1220,14 @@ fn
callback_errors_preserve_message_and_add_object_context() {
"reverse_visit_pattern",
));
assert_eq!(
- i64::try_from(call_global("ffi.ListSize",
&[records.clone()])).unwrap(),
+ i64::try_from(call_global("ffi.ArraySize",
&[records.clone()])).unwrap(),
4
);
for (i, expected) in [&child, &root, &child].into_iter().enumerate() {
- let node = call_global("ffi.ListGetItem", &[records.clone(), (i as i64
+ 1).into()]);
+ let node = call_global(
+ "ffi.ArrayGetItem",
+ &[records.clone(), (i as i64 + 1).into()],
+ );
assert_eq!(any_object_pointer(&node), any_object_pointer(expected));
}
@@ -1265,14 +1268,14 @@ fn
callback_errors_preserve_message_and_add_object_context() {
&context,
"reverse_visit_pattern",
));
- let node = call_global("ffi.ListGetItem", &[records.clone(),
0i64.into()]);
+ let node = call_global("ffi.ArrayGetItem", &[records.clone(),
0i64.into()]);
assert!(node
.downcast_ref::<ObjectRef>()
.cloned()
.unwrap()
.same_as(&failed_node.unwrap()));
assert_eq!(
- i64::try_from(call_global("ffi.ListSize", &[records])).unwrap(),
+ i64::try_from(call_global("ffi.ArraySize", &[records])).unwrap(),
1
);
assert_eq!(root.get(0).unwrap(), 0);
diff --git a/src/ffi/extra/structural_mutate.cc
b/src/ffi/extra/structural_mutate.cc
index 72ffc6ad..1747f9a5 100644
--- a/src/ffi/extra/structural_mutate.cc
+++ b/src/ffi/extra/structural_mutate.cc
@@ -121,7 +121,7 @@ class StructuralMutateDynEngine : public
StructuralMutatorEngineParent<Parent> {
if (std::optional<Expected<Any>> matched = DispatchCallback(value,
InplaceMode::kDisallow)) {
Expected<Any> result = *std::move(matched);
if (TVM_FFI_PREDICT_FALSE(result.is_err())) {
- Parent::UpdateVisitErrorContext(result, value);
+ Parent::UpdateVisitErrorContext(&result, value);
}
return ExpectedUnsafe::MoveToTVMFFIAny(std::move(result));
}
@@ -132,7 +132,7 @@ class StructuralMutateDynEngine : public
StructuralMutatorEngineParent<Parent> {
if (std::optional<Expected<Any>> matched = DispatchCallback(value,
InplaceMode::kAllow)) {
Expected<Any> result = *std::move(matched);
if (TVM_FFI_PREDICT_FALSE(result.is_err())) {
- Parent::UpdateVisitErrorContext(result, value);
+ Parent::UpdateVisitErrorContext(&result, value);
}
return ExpectedUnsafe::MoveToTVMFFIAny(std::move(result));
}
diff --git a/src/ffi/extra/visit_error_context.cc
b/src/ffi/extra/visit_error_context.cc
index f57da780..74d499f8 100644
--- a/src/ffi/extra/visit_error_context.cc
+++ b/src/ffi/extra/visit_error_context.cc
@@ -63,7 +63,7 @@ class VisitErrorAccessPathFinder {
// The matcher operates on `records_` from here on; the original
// `context_->reverse_visit_pattern` is left untouched so callers that
// inspect the context for debugging still see the raw history.
- const List<ObjectRef>& raw = context_->reverse_visit_pattern;
+ const Array<ObjectRef>& raw = context_->reverse_visit_pattern;
records_.reserve(raw.size());
for (const ObjectRef& entry : raw) {
if (!entry.defined()) continue;
diff --git a/tests/cpp/extra/test_structural_mutate.cc
b/tests/cpp/extra/test_structural_mutate.cc
index 0acf0257..1d7bbfec 100644
--- a/tests/cpp/extra/test_structural_mutate.cc
+++ b/tests/cpp/extra/test_structural_mutate.cc
@@ -837,7 +837,7 @@ TEST(StructuralMap, RegisteredMutateHookUsesAssignOrReturn)
{
ASSERT_TRUE(failed.is_err());
Optional<VisitErrorContext> context =
VisitErrorContext::TryGetFromError(failed.error());
ASSERT_TRUE(context.has_value());
- const List<ObjectRef>& reverse_pattern =
context.value()->reverse_visit_pattern;
+ const Array<ObjectRef>& reverse_pattern =
context.value()->reverse_visit_pattern;
ASSERT_EQ(reverse_pattern.size(), 2U);
EXPECT_TRUE(reverse_pattern[0].same_as(lhs));
EXPECT_TRUE(reverse_pattern[1].same_as(root));
diff --git a/tests/cpp/extra/test_visit_error_context.cc
b/tests/cpp/extra/test_visit_error_context.cc
index e10b9b81..8c99ab86 100644
--- a/tests/cpp/extra/test_visit_error_context.cc
+++ b/tests/cpp/extra/test_visit_error_context.cc
@@ -129,7 +129,7 @@ TEST(VisitErrorContext, MacroBuildsChain) {
// reverse_visit_pattern is innermost-first: leaf pushed first (inner catch
// fires first during unwind), root pushed last.
- const List<ObjectRef>& chain = visit_context.value()->reverse_visit_pattern;
+ const Array<ObjectRef>& chain = visit_context.value()->reverse_visit_pattern;
EXPECT_GE(chain.size(), 2u);
// Innermost entry is leaf (the throw site).
EXPECT_TRUE(chain[0].same_as(leaf));
@@ -137,7 +137,7 @@ TEST(VisitErrorContext, MacroBuildsChain) {
EXPECT_TRUE(chain[chain.size() - 1].same_as(root));
// Verify order structurally: [leaf, ..., root].
- List<ObjectRef> expected_chain = {leaf, root};
+ Array<ObjectRef> expected_chain = {leaf, root};
EXPECT_TRUE(StructuralEqual::Equal(expected_chain, chain));
}
@@ -145,7 +145,7 @@ TEST(VisitErrorContext, MacroBuildsChain) {
// MacroPreExistingPayloadWrap: when UpdateVisitErrorContext is called on
// an error that already has a non-context extra_context payload, the existing
// payload is stashed in prev_error_context and the visit chain starts
-// fresh. Subsequent pushes append to the same context object;
prev_error_context
+// fresh. Subsequent pushes append to the context; prev_error_context
// is not modified.
// ---------------------------------------------------------------------------
@@ -161,7 +161,7 @@ TEST(VisitErrorContext, MacroPreExistingPayloadWrap) {
ASSERT_TRUE(err.extra_context().has_value());
// First push: existing payload is not a VisitErrorContext → wrapped.
- tvm::ffi::detail::UpdateVisitErrorContext(err, leaf);
+ tvm::ffi::detail::UpdateVisitErrorContext(&err, leaf);
Optional<VisitErrorContext> visit_context =
VisitErrorContext::TryGetFromError(err);
ASSERT_TRUE(visit_context.has_value());
@@ -171,12 +171,12 @@ TEST(VisitErrorContext, MacroPreExistingPayloadWrap) {
EXPECT_TRUE(visit_context.value()->prev_error_context.value().same_as(existing_payload));
// Second push: context already exists → node appended; prev_error_context
unchanged.
- tvm::ffi::detail::UpdateVisitErrorContext(err, root);
+ tvm::ffi::detail::UpdateVisitErrorContext(&err, root);
Optional<VisitErrorContext> visit_context_after =
VisitErrorContext::TryGetFromError(err);
ASSERT_TRUE(visit_context_after.has_value());
- List<ObjectRef> expected_chain = {leaf, root};
+ Array<ObjectRef> expected_chain = {leaf, root};
EXPECT_TRUE(
StructuralEqual::Equal(expected_chain,
visit_context_after.value()->reverse_visit_pattern));
@@ -184,6 +184,25 @@ TEST(VisitErrorContext, MacroPreExistingPayloadWrap) {
EXPECT_TRUE(visit_context_after.value()->prev_error_context.value().same_as(existing_payload));
}
+// Appending to a shared Error or context copies it; other holders keep their
pattern.
+TEST(VisitErrorContext, UpdateCopiesSharedError) {
+ Array<ObjectRef> empty;
+ TPairRef leaf(empty, empty);
+ TPairRef root(leaf, empty);
+ Error err("RuntimeError", "test", "");
+ tvm::ffi::detail::UpdateVisitErrorContext(&err, leaf);
+ Error shared = err;
+ VisitErrorContext context = VisitErrorContext::TryGetFromError(err).value();
+
+ tvm::ffi::detail::UpdateVisitErrorContext(&err, root);
+ EXPECT_FALSE(err.same_as(shared));
+ EXPECT_EQ(err.kind(), "RuntimeError");
+ EXPECT_EQ(err.message(), "test");
+
EXPECT_EQ(VisitErrorContext::TryGetFromError(err).value()->reverse_visit_pattern.size(),
2U);
+
EXPECT_TRUE(VisitErrorContext::TryGetFromError(shared).value().same_as(context));
+ EXPECT_EQ(context->reverse_visit_pattern.size(), 1U);
+}
+
// ---------------------------------------------------------------------------
// MacroThrowBuildsChain: TVM_FFI_VISIT_THROW seeds the
// VisitErrorContext with the throw-site node directly (no separate
@@ -219,7 +238,7 @@ TEST(VisitErrorContext, MacroThrowBuildsChain) {
Optional<VisitErrorContext> visit_context =
VisitErrorContext::TryGetFromError(caught);
ASSERT_TRUE(visit_context.has_value());
- const List<ObjectRef>& chain =
visit_context.value()->reverse_visit_pattern;
+ const Array<ObjectRef>& chain =
visit_context.value()->reverse_visit_pattern;
ASSERT_EQ(chain.size(), 1u);
EXPECT_TRUE(chain[0].same_as(throw_site));
}
@@ -242,9 +261,9 @@ TEST(VisitErrorContext, MacroThrowBuildsChain) {
Optional<VisitErrorContext> visit_context =
VisitErrorContext::TryGetFromError(caught);
ASSERT_TRUE(visit_context.has_value());
- const List<ObjectRef>& chain =
visit_context.value()->reverse_visit_pattern;
+ const Array<ObjectRef>& chain =
visit_context.value()->reverse_visit_pattern;
// THROW seeds with throw_site; END appends root.
- List<ObjectRef> expected = {throw_site, root};
+ Array<ObjectRef> expected = {throw_site, root};
EXPECT_TRUE(StructuralEqual::Equal(expected, chain));
}
}
@@ -289,7 +308,7 @@ TEST(FindAccessPaths, BasicMatch) {
TPairRef root(mid, empty);
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{leaf, mid, root};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{leaf, mid, root};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths = VisitErrorContext::FindAccessPaths(root,
ctx);
@@ -305,7 +324,7 @@ TEST(FindAccessPaths, BasicMatch) {
TPairRef root(shared, shared);
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{shared, root};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{shared, root};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths = VisitErrorContext::FindAccessPaths(root,
ctx);
@@ -354,7 +373,7 @@ TEST(FindAccessPaths, AccessKindCoverage) {
TPairRef root(mid, empty);
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{leaf, root};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{leaf, root};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths = VisitErrorContext::FindAccessPaths(root,
ctx);
@@ -372,7 +391,7 @@ TEST(FindAccessPaths, AccessKindCoverage) {
TPairRef root(arr, empty);
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{leaf1, root};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{leaf1, root};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths = VisitErrorContext::FindAccessPaths(root,
ctx);
@@ -390,7 +409,7 @@ TEST(FindAccessPaths, AccessKindCoverage) {
TPairRef root(m, empty);
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{target, root};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{target, root};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths = VisitErrorContext::FindAccessPaths(root,
ctx);
@@ -443,7 +462,7 @@ TEST(FindAccessPaths, SparsePatternAnchors) {
// Scenario 1: pattern = [m_inner, root] → 2 paths (one per branch).
{
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{m_inner, root};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{m_inner, root};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths = VisitErrorContext::FindAccessPaths(root,
ctx);
@@ -453,7 +472,7 @@ TEST(FindAccessPaths, SparsePatternAnchors) {
// Scenario 2: pattern = [m_inner, m_outer, root] → 1 path through branch_b.
{
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{m_inner, m_outer, root};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{m_inner, m_outer, root};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths = VisitErrorContext::FindAccessPaths(root,
ctx);
@@ -487,7 +506,7 @@ TEST(FindAccessPaths, PartialChain) {
TPairRef root(child, empty);
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{unreachable, child, root};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{unreachable, child, root};
VisitErrorContext ctx(std::move(ctx_obj));
// Sub-scenario A: strict mode — no full match.
@@ -541,7 +560,7 @@ TEST(FindAccessPaths, EdgeCases) {
TPairRef root(empty, empty);
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{root};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{root};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths = VisitErrorContext::FindAccessPaths(root,
ctx);
@@ -555,7 +574,7 @@ TEST(FindAccessPaths, EdgeCases) {
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
ObjectRef null_ref;
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{null_ref, null_ref};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{null_ref, null_ref};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths;
@@ -585,7 +604,7 @@ TEST(FindAccessPaths, RecordsCleanup) {
// Raw pattern: [leaf, leaf, root, root]; effective: [leaf, root].
{
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{leaf, leaf, root, root};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{leaf, leaf, root, root};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths = VisitErrorContext::FindAccessPaths(root,
ctx);
@@ -597,7 +616,7 @@ TEST(FindAccessPaths, RecordsCleanup) {
// Raw pattern: [null, leaf, null, root, null]; effective: [leaf, root].
{
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{null_ref, leaf, null_ref,
root, null_ref};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{null_ref, leaf,
null_ref, root, null_ref};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths = VisitErrorContext::FindAccessPaths(root,
ctx);
@@ -609,7 +628,7 @@ TEST(FindAccessPaths, RecordsCleanup) {
// Raw pattern: [leaf, null, leaf, root, root, null]; effective: [leaf,
root].
{
ObjectPtr<VisitErrorContextObj> ctx_obj =
make_object<VisitErrorContextObj>();
- ctx_obj->reverse_visit_pattern = List<ObjectRef>{leaf, null_ref, leaf,
root, root, null_ref};
+ ctx_obj->reverse_visit_pattern = Array<ObjectRef>{leaf, null_ref, leaf,
root, root, null_ref};
VisitErrorContext ctx(std::move(ctx_obj));
Array<refl::AccessPath> paths = VisitErrorContext::FindAccessPaths(root,
ctx);
@@ -635,8 +654,8 @@ TEST(VisitErrorContext, TryGetFromError) {
EXPECT_FALSE(no_context.has_value());
// Attach context via UpdateVisitErrorContext.
- tvm::ffi::detail::UpdateVisitErrorContext(err, leaf);
- tvm::ffi::detail::UpdateVisitErrorContext(err, root);
+ tvm::ffi::detail::UpdateVisitErrorContext(&err, leaf);
+ tvm::ffi::detail::UpdateVisitErrorContext(&err, root);
Optional<VisitErrorContext> visit_context =
VisitErrorContext::TryGetFromError(err);
ASSERT_TRUE(visit_context.has_value());
@@ -654,7 +673,7 @@ TEST(VisitErrorContext,
EnrichmentLeavesContextFreeErrorsUnchanged) {
Array<ObjectRef> root;
EXPECT_TRUE(VisitErrorContext::EnrichVisitError(original,
root).same_as(original));
auto contextual = Error("ValueError", "contextual failure", "trace");
- detail::UpdateVisitErrorContext(contextual, root);
+ detail::UpdateVisitErrorContext(&contextual, root);
EXPECT_TRUE(VisitErrorContext::EnrichVisitError(contextual,
root).same_as(contextual));
EXPECT_TRUE(VisitErrorContext::EnrichVisitError(contextual,
ObjectRef()).same_as(contextual));
}
