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 b2ec7c93 [Rust] Structural errors carry only the visit context; enrich
at public boundaries (#967)
b2ec7c93 is described below
commit b2ec7c93a9bb30bb44df537669090f6312cb838a
Author: Tianqi Chen <[email protected]>
AuthorDate: Sun Nov 8 16:27:19 2026 -0500
[Rust] Structural errors carry only the visit context; enrich at public
boundaries (#967)
Rust structural traversal errors now carry only the structured visit
context, the same way C++ `detail::UpdateVisitErrorContext` does. This
PR removes the Rust-only `[native structural {visit,mutate,map}]
object|field ...` text traceback frames for both objects and fields. C++
appends no such frames, they repeat what the visit context already
records, and custom-base engines reported each ancestor twice.
The public structural entry points (`structural_visit`,
`structural_walk`, `structural_mutate`, `structural_map`, and the
custom-base `StructuralVisitEngine::visit_root` /
`StructuralMutateEngine::mutate_root`) now pass the error from a failed
traversal to the root type's `__ffi_enrich_visit_error__` hook, the same
way C++ `VisitErrorContext::EnrichVisitError` does. The hook is called
only on error, only for an object root whose error carries a visit
context, and the lookup uses the root's exact type. The hook is looked
up on each error, so one registered later is found. If the hook is
missing or fails, the original error is kept.
- Removed `with_structural_error_context`,
`with_structural_field_error_context`, the visit-side
`with_field_error_context` / `annotate_visit_error`, and
`with_mutation_error_context`. Callers now call
`update_visit_error_context` directly.
- Added the crate-internal `enrich_visit_error`, called from the shared
`run_visitor` / `run_mutator` roots.
- Added `tvm_ffi::extra::visit_error_context::find_access_paths(root,
error)`, which returns the access paths from `root` to where a failed
traversal stopped (as C++ `VisitErrorContext::FindAccessPaths`), so
callers can locate an error when the root type has no hook.
- Tests that checked the text frames now check the visit context.
Rust stubgen:
- Added the `tvm-ffi-stubgen(no-new): <type_key>` directive. It skips
the wrapper's generated allocator, and for a final type also
`<Leaf>Obj::new`, which only that allocator uses. Downstream projects
use it for abstract bases whose field-wise `new` builds a bare base
object.
- Generated docs read natively: the wrapper allocator is documented as
"Create a `X` from its field values.", and the layout notes on
`<Leaf>Obj` (`Complete: ...` / `Opaque: ...`) are plain comments, not
docs.
---
.../rust/src/generated/rust_stubgen/mod.rs | 5 +-
python/tvm_ffi/stub/rust_generator/codegen.py | 16 +++--
python/tvm_ffi/stub/rust_generator/consts.py | 1 +
python/tvm_ffi/stub/rust_generator/directives.py | 8 ++-
rust/tvm-ffi/src/extra/mod.rs | 2 +-
rust/tvm-ffi/src/extra/structural_common.rs | 19 -----
rust/tvm-ffi/src/extra/structural_map.rs | 10 +--
rust/tvm-ffi/src/extra/structural_mutate.rs | 54 ++++-----------
rust/tvm-ffi/src/extra/structural_visit.rs | 55 +++------------
rust/tvm-ffi/src/extra/visit_error_context.rs | 81 +++++++++++++++++++---
rust/tvm-ffi/tests/test_structural_mutate.rs | 12 ++--
rust/tvm-ffi/tests/test_structural_visit.rs | 24 +++++--
tests/python/test_stubgen_rust.py | 43 ++++++------
13 files changed, 163 insertions(+), 167 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 2183fd4e..1c7e1ad2 100644
--- a/examples/rust_stubgen/rust/src/generated/rust_stubgen/mod.rs
+++ b/examples/rust_stubgen/rust/src/generated/rust_stubgen/mod.rs
@@ -70,8 +70,7 @@ impl TryFrom<i64> for PairKind {
}
/// The object behind [`IntPair`].
-///
-/// Complete: reflected fields fill [24, 48) exactly (alignment padding [44,
48)).
+// Complete: reflected fields fill [24, 48) exactly (alignment padding [44,
48)).
#[repr(C)]
#[derive(tvm_ffi::derive::Object)]
#[object(type_key = "rust_stubgen.IntPair", final, immutable)]
@@ -95,7 +94,7 @@ impl IntPairObj {
}
impl IntPair {
- /// Lossless complete-field allocation.
+ /// Create a `IntPair` from its field values.
pub fn from_fields(a: i64, b: i64, kind: impl Into<PairKind>) -> Self {
let obj = IntPairObj::new(a, b, kind.into());
Self { data_: ObjectArc::new(obj) }
diff --git a/python/tvm_ffi/stub/rust_generator/codegen.py
b/python/tvm_ffi/stub/rust_generator/codegen.py
index d7128168..708dae3e 100644
--- a/python/tvm_ffi/stub/rust_generator/codegen.py
+++ b/python/tvm_ffi/stub/rust_generator/codegen.py
@@ -606,11 +606,11 @@ class _ObjectRenderer:
f"pub struct {self.obj_struct} {{",
f" base_: {base},", # a reflected `base_` is rejected
(`rust_ident`)
]
- summary = [f"/// The object behind [`{self.leaf}`].", "///"]
+ summary = [f"/// The object behind [`{self.leaf}`]."]
if not verdict.is_complete:
return [
*summary,
- f"/// Opaque: {verdict.detail}. Fields are read through the C
ABI getters.",
+ f"// Opaque: {verdict.detail}. Fields are read through the C
ABI getters.",
*header,
"}",
]
@@ -622,7 +622,7 @@ class _ObjectRenderer:
members.append(f" {vis} {rust_ident(field.name)}: {mirror},")
return [
*summary,
- f"/// Complete: {verdict.detail}.",
+ f"// Complete: {verdict.detail}.",
*_allow_non_snake_case(rust_ident(f.name) for f in
self.info.fields),
*header,
*members,
@@ -727,7 +727,8 @@ class _ObjectRenderer:
field as its own type, as ``#[rs_object]``'s ``new`` does, an object
reference as
``impl IntoArg`` and any other as ``impl Into`` (:func:`_param_type`),
and
converts it on the way to ``<Leaf>Obj::new``.
- ``custom-new`` names the wrapper's allocator ``from_fields`` instead.
+ ``custom-new`` names the wrapper's allocator ``from_fields`` instead;
``no-new``
+ skips it, and ``<Leaf>Obj::new`` of a final type with it.
``<Leaf>Obj::new`` of a type that is not final is public: a subtype
defined in Rust
builds its parent's object with it.
"""
@@ -746,6 +747,9 @@ class _ObjectRenderer:
own = [rust_ident(p.name) for p in params[len(inherited) :]]
forward = [_forward(p) for p in params]
allow = self._allow_missing_docs()
+ no_new = self.type_key in self.imports.directives.no_new
+ if no_new and self.info.is_final:
+ return []
sections = [
[
*allow,
@@ -759,12 +763,14 @@ class _ObjectRenderer:
"}",
]
]
+ if no_new:
+ return sections
custom = self.type_key in self.imports.directives.custom_new
sections.append(
[
*allow,
f"impl {self.leaf} {{",
- " /// Lossless complete-field allocation.",
+ f" /// Create a `{self.leaf}` from its field values.",
*self._fn_lines(
"pub fn from_fields" if custom else "pub fn new",
params,
diff --git a/python/tvm_ffi/stub/rust_generator/consts.py
b/python/tvm_ffi/stub/rust_generator/consts.py
index 2f66c5d1..01e1d521 100644
--- a/python/tvm_ffi/stub/rust_generator/consts.py
+++ b/python/tvm_ffi/stub/rust_generator/consts.py
@@ -29,6 +29,7 @@ RUST_DIRECTIVE_KINDS = frozenset(
"opaque",
"upcast",
"custom-new",
+ "no-new",
"missing-docs",
}
)
diff --git a/python/tvm_ffi/stub/rust_generator/directives.py
b/python/tvm_ffi/stub/rust_generator/directives.py
index c6a8535e..4ca394a4 100644
--- a/python/tvm_ffi/stub/rust_generator/directives.py
+++ b/python/tvm_ffi/stub/rust_generator/directives.py
@@ -26,6 +26,7 @@ and one applies to the whole file::
// tvm-ffi-stubgen(opaque): ir.SourceName
// tvm-ffi-stubgen(upcast): tirx.Add -> PrimExpr
// tvm-ffi-stubgen(custom-new): tirx.Add
+ // tvm-ffi-stubgen(no-new): ir.Expr
// tvm-ffi-stubgen(missing-docs): allow
``field`` sets the field's Rust type (a name in scope, or a ``::`` path to
@@ -36,7 +37,9 @@ integer newtype for it, or with ``closed`` before the member
list a
``#[repr(<int>)]`` enum of exactly those members; ``opaque`` keeps a type
opaque
even when its layout is reproducible. ``upcast`` adds an upcast to a reference
outside the ancestor chain; ``custom-new`` says the wrapper's ``new`` is
-hand-written; the generated one is named ``from_fields`` instead.
+hand-written; the generated one is named ``from_fields`` instead. ``no-new``
+skips the wrapper's generated allocator, and for a final type
``<Leaf>Obj::new``
+too, which only that allocator uses.
``missing-docs: allow`` puts ``#[allow(missing_docs)]`` on the file's generated
types and ``impl`` blocks.
"""
@@ -79,6 +82,7 @@ class Directives:
opaque: set[str] = dataclasses.field(default_factory=set)
upcasts: dict[str, list[str]] = dataclasses.field(default_factory=dict)
custom_new: set[str] = dataclasses.field(default_factory=set)
+ no_new: set[str] = dataclasses.field(default_factory=set)
allow_missing_docs: bool = False
def add(self, name: str, payload: str, lineno: int) -> None:
@@ -103,6 +107,8 @@ class Directives:
self.upcasts.setdefault(_type_target(name, lhs, lineno),
[]).append(rust_type)
elif name == "custom-new":
self.custom_new.add(_type_target(name, payload, lineno))
+ elif name == "no-new":
+ self.no_new.add(_type_target(name, payload, lineno))
elif name == "missing-docs":
if payload.strip() != "allow": # the one level so far
raise _invalid(name, lineno, "allow")
diff --git a/rust/tvm-ffi/src/extra/mod.rs b/rust/tvm-ffi/src/extra/mod.rs
index babc87b6..87ec6b2d 100644
--- a/rust/tvm-ffi/src/extra/mod.rs
+++ b/rust/tvm-ffi/src/extra/mod.rs
@@ -35,7 +35,7 @@ pub mod structural_mutate;
pub mod structural_visit;
pub mod structural_walk;
pub mod unchanged;
-mod visit_error_context;
+pub mod visit_error_context;
/// Borrow guarantees of the structural views, checked as `compile_fail`
doctests.
///
diff --git a/rust/tvm-ffi/src/extra/structural_common.rs
b/rust/tvm-ffi/src/extra/structural_common.rs
index b1ba171c..ed1b812b 100644
--- a/rust/tvm-ffi/src/extra/structural_common.rs
+++ b/rust/tvm-ffi/src/extra/structural_common.rs
@@ -56,16 +56,6 @@ pub(super) fn missing_field_getter_error(operation: &str,
name: &str) -> Error {
))
}
-#[cold]
-#[inline(never)]
-pub(super) fn with_structural_field_error_context(
- error: Error,
- operation: &str,
- name: &str,
-) -> Error {
- with_structural_error_context(error, operation, &format!("field `{name}`"))
-}
-
/// Consume an owning error carrier while keeping checked-conversion scratch
/// inside the cold path instead of successful recursive traversal frames.
///
@@ -82,15 +72,6 @@ pub(super) unsafe fn structural_error_from_raw(raw:
TVMFFIAny) -> Error {
}
}
-// Perf note: cold marks these error-composition paths as uncommon; inlining
-// within error paths is compiler-selected. Use explicit inline(never) at
boundaries
-// where scratch must stay outside recursive callers.
-/// Add one structural traversal frame to an error's backtrace.
-#[cold]
-pub(crate) fn with_structural_error_context(error: Error, operation: &str,
frame: &str) -> Error {
- Error::with_appended_backtrace(error, &format!("[native structural
{operation}] {frame}\n"))
-}
-
//
-----------------------------------------------------------------------------
// Raw carrier utilities
//
-----------------------------------------------------------------------------
diff --git a/rust/tvm-ffi/src/extra/structural_map.rs
b/rust/tvm-ffi/src/extra/structural_map.rs
index b9dafe85..141d38d8 100644
--- a/rust/tvm-ffi/src/extra/structural_map.rs
+++ b/rust/tvm-ffi/src/extra/structural_map.rs
@@ -22,11 +22,11 @@
use super::structural_common::raw_ffi_any_same_as;
use super::structural_mutate::{
IntoMutateResult, MutationView, StructuralMutateEngine,
StructuralMutatorView,
- with_mutation_error_context,
};
use super::structural_visit::DefRegionKind;
use super::structural_walk::WalkOrder;
use super::unchanged::{UnchangedOr, is_unchanged};
+use super::visit_error_context::update_visit_error_context;
use crate::{Any, AnyView, Dispatcher, IntoDispatcher, Result};
use std::borrow::BorrowMut;
@@ -126,7 +126,7 @@ where
return mutator.default_mutate(self, value).map(Into::into);
};
let mapped: Any = callback_result
- .map_err(|error| with_mutation_error_context(error, raw,
"map"))?
+ .map_err(|error| update_visit_error_context(error, raw))?
.into();
if is_unchanged(&mapped) || raw_ffi_any_same_as(raw,
*mapped.as_raw_ffi_any()) {
// Drop temporary ownership before default descent checks
uniqueness.
@@ -154,9 +154,9 @@ where
};
match self.dispatch.dispatch_tuple((current, kind)) {
Some(Ok(result)) if result.is_unchanged() => Ok(mapped),
- Some(result) => result.map(Any::from).map_err(|error| {
- with_mutation_error_context(error,
*current.as_raw_ffi_any(), "map")
- }),
+ Some(result) => result
+ .map(Any::from)
+ .map_err(|error| update_visit_error_context(error,
*current.as_raw_ffi_any())),
None => Ok(mapped),
}
}
diff --git a/rust/tvm-ffi/src/extra/structural_mutate.rs
b/rust/tvm-ffi/src/extra/structural_mutate.rs
index bd791ceb..5b50be3a 100644
--- a/rust/tvm-ffi/src/extra/structural_mutate.rs
+++ b/rust/tvm-ffi/src/extra/structural_mutate.rs
@@ -53,13 +53,12 @@ use super::structural_common::{
FLAG_SEQ_HASH_IGNORE, FLAG_SETTER_IS_FUNCTION, field_def_region,
for_each_field_info,
free_var_child_region, missing_field_getter_error, raw_ffi_any_into_result,
raw_ffi_any_same_as, registered_type_index, reject_error_value,
result_into_raw_ffi_any,
- runtime_error, seq_hash_kind, with_structural_error_context,
- with_structural_field_error_context, wrong_dispatcher,
+ runtime_error, seq_hash_kind, wrong_dispatcher,
};
use super::structural_visit::DefRegionKind;
use super::unchanged::is_unchanged;
use super::unchanged::{Unchanged, UnchangedOr};
-use super::visit_error_context::update_visit_error_context;
+use super::visit_error_context::{enrich_visit_error,
update_visit_error_context};
const STRUCTURAL_MUTATE_ATTR: &str = "__s_mutate__";
const STRUCTURAL_MAYBE_INPLACE_MUTATE_ATTR: &str =
"__s_maybe_inplace_mutate__";
@@ -1025,7 +1024,7 @@ impl<Base: StructuralMutatorBase>
StructuralMutateEngine<Base> {
Some(result) => {
let result =
result.map(Any::from).and_then(reject_error_value);
result_into_raw_ffi_any(if CALLBACK_CONTEXT {
- result.map_err(|error| with_mutation_error_context(error,
raw, "mutate"))
+ result.map_err(|error| update_visit_error_context(error,
raw))
} else {
result
})
@@ -1176,7 +1175,11 @@ impl<Base: StructuralMutatorBase>
StructuralMutateEngine<Base> {
StructuralMutatorObj::call_mutator(runtime,
*root.as_raw_ffi_any(), InplaceMode::Allow);
// Native hooks release temporary runtime owners before this
synchronous run ends.
drop(engine);
- result.map(|value| if is_unchanged(&value) { root } else { value })
+ match result {
+ Ok(value) => Ok(if is_unchanged(&value) { root } else { value }),
+ // The public traversal boundary, as the C++
`StructuralMutateExpected`.
+ Err(error) => Err(enrich_visit_error(error,
*root.as_raw_ffi_any())),
+ }
}
}
@@ -1215,7 +1218,7 @@ impl StructuralMutatorObj {
unsafe {
result_into_raw_ffi_any(
raw_ffi_any_into_result(result)
- .map_err(|error| with_mutation_error_context(error, raw,
"mutate")),
+ .map_err(|error| update_visit_error_context(error, raw)),
)
}
}
@@ -1305,13 +1308,7 @@ impl StructuralMutatorObj {
// populate an owning result and still report an error.
let mut child = Any::new();
if unsafe { getter(source_address, Any::as_raw_ffi_any_mut(&mut
child)) } != 0 {
- return Err(unsafe {
- with_structural_field_error_context(
- Error::from_raised(),
- "mutate",
- field.name.as_str(),
- )
- });
+ return Err(Error::from_raised());
}
// Reflection getters return owning values. Keep the child alive for
// the complete recursive call, then let normal Drop release it.
@@ -1319,19 +1316,12 @@ impl StructuralMutatorObj {
let child_region = field_def_region(field, inherited_region);
let mapped = Self::with_def_region_kind(runtime, child_region, |_| {
Self::call_mutator(runtime, child_raw, InplaceMode::Disallow)
- })
- .map_err(|error| unsafe {
- with_structural_field_error_context(error, "mutate",
field.name.as_str())
})?;
if is_unchanged(&mapped) || raw_ffi_any_same_as(child_raw,
*mapped.as_raw_ffi_any()) {
return Ok(());
}
- call_field_setter(field, source_address,
mapped.as_raw_ffi_any()).map_err(
- |error| unsafe {
- with_structural_field_error_context(error, "mutate",
field.name.as_str())
- },
- )?;
+ call_field_setter(field, source_address, mapped.as_raw_ffi_any())?;
*field_changed = true;
Ok(())
}
@@ -1424,9 +1414,7 @@ impl StructuralMutatorObj {
Err(invalid_mutation_hook_error())
};
// Preserve successful Error-valued returns until the public
conversion.
- result_into_raw_ffi_any(
- result.map_err(|error| with_mutation_error_context(error, raw,
"mutate")),
- )
+ result_into_raw_ffi_any(result.map_err(|error|
update_visit_error_context(error, raw)))
}
/// Borrow the live `runtime` as an object argument, without a reference
count.
@@ -1494,7 +1482,7 @@ impl StructuralMutatorObj {
unsafe { std::mem::transmute::<i32,
DefRegionKind>(*(*runtime).def_region_mode.get()) };
result_into_raw_ffi_any(
Self::mutate_without_hook(runtime, raw, def_region_kind)
- .map_err(|error| with_mutation_error_context(error, raw,
"mutate")),
+ .map_err(|error| update_visit_error_context(error, raw)),
)
}
@@ -1816,19 +1804,3 @@ fn unregistered_type_error(type_index: i32) -> Error {
"native structural mutate: unregistered type index {type_index}"
))
}
-
-#[cold]
-pub(super) fn with_mutation_error_context(error: Error, raw: TVMFFIAny,
operation: &str) -> Error {
- if raw.type_index < TVMFFITypeIndex::kTVMFFIStaticObjectBegin as i32 {
- error
- } else {
- with_structural_error_context(
- update_visit_error_context(error, raw),
- operation,
- &format!(
- "object `{}`",
- crate::object::type_key_of_index(raw.type_index)
- ),
- )
- }
-}
diff --git a/rust/tvm-ffi/src/extra/structural_visit.rs
b/rust/tvm-ffi/src/extra/structural_visit.rs
index 4b9387b1..9da218e2 100644
--- a/rust/tvm-ffi/src/extra/structural_visit.rs
+++ b/rust/tvm-ffi/src/extra/structural_visit.rs
@@ -74,9 +74,9 @@ use crate::tvm_ffi_sys::{
use super::structural_common::{
FLAG_SEQ_HASH_IGNORE, field_def_region, for_each_field_info,
free_var_child_region,
missing_field_getter_error, registered_type_index, runtime_error,
seq_hash_kind,
- with_structural_error_context, with_structural_field_error_context,
wrong_dispatcher,
+ wrong_dispatcher,
};
-use super::visit_error_context::update_visit_error_context;
+use super::visit_error_context::{enrich_visit_error,
update_visit_error_context};
const STRUCTURAL_VISIT_ATTR: &str = "__s_visit__";
@@ -723,7 +723,8 @@ impl<Base: StructuralVisitorBase>
StructuralVisitEngine<Base> {
unsafe {
Any::from_raw_ffi_any(StructuralVisitorObj::call_visitor_raw(runtime, root)) };
// Trusted hooks have released every temporary strong/weak runtime
reference.
drop(engine);
- visit_result_from_any(result)
+ // The public traversal boundary, as the C++ `StructuralVisitExpected`.
+ visit_result_from_any(result).map_err(|error|
enrich_visit_error(error, root))
}
}
@@ -758,7 +759,7 @@ impl StructuralVisitorObj {
// SAFETY: the Error tag establishes the owned object's
representation.
unsafe {
let error = take_error(Any::from_raw_ffi_any(result));
- Any::into_raw_ffi_any(annotate_visit_error(error,
value).into())
+ Any::into_raw_ffi_any(update_visit_error_context(error,
value).into())
}
}
}
@@ -843,24 +844,14 @@ impl StructuralVisitorObj {
// Own the getter result so partial writes and recursive borrows drop
safely.
let mut child = Any::new();
if unsafe { getter(address, Any::as_raw_ffi_any_mut(&mut child)) } !=
0 {
- return unsafe {
- with_field_error_context(Any::from(Error::from_raised()),
field.name.as_str())
- };
+ return Any::from(Error::from_raised());
}
let borrowed = AnyView::from(&child);
let child_region = field_def_region(field, inherited_region);
- unsafe {
- with_field_error_context(
- Self::with_def_region_kind(runtime, child_region, || {
- Any::from_raw_ffi_any(Self::call_visitor_raw(
- runtime,
- *borrowed.as_raw_ffi_any(),
- ))
- }),
- field.name.as_str(),
- )
- }
+ Self::with_def_region_kind(runtime, child_region, || unsafe {
+ Any::from_raw_ffi_any(Self::call_visitor_raw(runtime,
*borrowed.as_raw_ffi_any()))
+ })
}
/// Dispatch borrowed `value` through the runtime's vtable.
@@ -1031,17 +1022,6 @@ unsafe fn take_error(value: Any) -> Error {
unsafe { Error::move_from_any_after_check(&mut raw) }
}
-#[inline(always)]
-fn with_field_error_context(result: Any, name: &str) -> Any {
- if result.type_index() == TVMFFITypeIndex::kTVMFFIError as i32 {
- core::hint::cold_path();
- // SAFETY: direct tag check establishes the Error representation.
- with_structural_field_error_context(unsafe { take_error(result) },
"visit", name).into()
- } else {
- result
- }
-}
-
// Columns are process-lifetime registry entries; a missing per-type cell still
// falls back to reflected traversal. A missing required column is an error.
static STRUCTURAL_VISIT_COLUMN: ColdInitLazyLock<TypeAttrColumn> =
ColdInitLazyLock::new(|| {
@@ -1074,20 +1054,3 @@ fn invalid_visit_hook_result_error() -> Error {
"structural visit hook must return None or ffi.VisitInterrupt",
)
}
-
-/// Consume `error` and annotate it from borrowed input scalar fields. Internal
-/// callers keep that input live; the Error remains owned if formatting
unwinds.
-// Perf note: Keep error formatting and context-allocation scratch out of
visit frames.
-#[cold]
-#[inline(never)]
-fn annotate_visit_error(error: Error, value: TVMFFIAny) -> Error {
- let error = update_visit_error_context(error, value);
- with_structural_error_context(
- error,
- "visit",
- &format!(
- "object `{}`",
- crate::object::type_key_of_index(value.type_index)
- ),
- )
-}
diff --git a/rust/tvm-ffi/src/extra/visit_error_context.rs
b/rust/tvm-ffi/src/extra/visit_error_context.rs
index 1b44395e..662bb140 100644
--- a/rust/tvm-ffi/src/extra/visit_error_context.rs
+++ b/rust/tvm-ffi/src/extra/visit_error_context.rs
@@ -25,9 +25,12 @@
use crate::any::AnyView;
use crate::collections::array::Array;
use crate::derive::Object;
-use crate::error::Error;
+use crate::error::{Error, Result};
+use crate::function::Function;
use crate::object::{self, Object, ObjectArc, ObjectRef, ObjectRefCore};
+use crate::reflection::{AccessPath, TypeAttrColumn};
use crate::tvm_ffi_sys::{TVMFFIAny, TVMFFIObject, TVMFFITypeIndex};
+use crate::type_traits::AnyCompatible;
/// The payload that `Error::extra_context` carries to name the nodes a failed
/// visit unwound through, laid out as the C++ `VisitErrorContextObj`.
@@ -42,6 +45,72 @@ pub(crate) struct VisitErrorContextObj {
prev_error_context: Option<ObjectRef>,
}
+/// The type attribute holding a root type's visit-error enrichment hook,
+/// `(Error, ObjectRef) -> Error`, as C++ `type_attr::kEnrichVisitError`.
+const ENRICH_VISIT_ERROR_ATTR: &str = "__ffi_enrich_visit_error__";
+
+/// `context` as a visit error context, if it is one.
+fn as_context(context: &ObjectRef) -> Option<&VisitErrorContextObj> {
+ let object = ObjectArc::as_ptr(ObjectRef::data(context));
+ // SAFETY: `context` keeps its object alive; the type is checked first.
+ unsafe {
+
object::is_instance_of::<VisitErrorContextObj>((*object.cast::<TVMFFIObject>()).type_index)
+ .then(|| &*object.cast::<VisitErrorContextObj>())
+ }
+}
+
+/// The access paths from `root` to where a failed structural traversal of
`root`
+/// stopped, as recorded in `error`'s visit context; empty if `error` has none.
+// As C++ `VisitErrorContext::TryGetFromError` then `FindAccessPaths`, full
matches only.
+pub fn find_access_paths<R>(root: &R, error: &Error) ->
Result<Array<AccessPath>>
+where
+ R: ObjectRefCore + AnyCompatible,
+{
+ let Some(context) = error
+ .extra_context()
+ .filter(|context| as_context(context).is_some())
+ else {
+ return Ok(Array::default());
+ };
+ Function::get_global("ffi.VisitErrorContext.FindAccessPaths")?
+ .call_packed(&[
+ AnyView::from(root),
+ AnyView::from(context),
+ AnyView::from(&false),
+ ])?
+ .try_into()
+}
+
+/// Pass `error` from a traversal of the borrowed `root` to the hook that
+/// `root`'s exact type registers, if `root` is an object and `error` carries a
+/// visit context. A missing or failing hook keeps the original error.
+// As C++ `VisitErrorContext::EnrichVisitError`, called once per public
traversal.
+#[cold]
+#[inline(never)]
+pub(crate) fn enrich_visit_error(error: Error, root: TVMFFIAny) -> Error {
+ if root.type_index < TVMFFITypeIndex::kTVMFFIStaticObjectBegin as i32
+ || error.extra_context().and_then(as_context).is_none()
+ {
+ return error;
+ }
+ // Looked up on each error, so a column or hook registered later is found.
+ let Some(hook) = TypeAttrColumn::lookup(ENRICH_VISIT_ERROR_ATTR)
+ .and_then(|column| column.get(root.type_index))
+ .and_then(|hook| Function::try_from(hook).ok())
+ else {
+ return error;
+ };
+ // SAFETY: the traversal's caller keeps `root` live through this call.
+ let root = unsafe { AnyView::from_raw_ffi_any(root) };
+ match hook
+ .call_packed(&[AnyView::from(&error), root])
+ .and_then(Error::try_from)
+ {
+ Ok(enriched) => enriched,
+ Err(_) => error,
+ }
+}
+
/// Name the visited object in `raw` in the error's visit context.
// As C++ `detail::UpdateVisitErrorContext`.
#[cold]
@@ -57,16 +126,6 @@ pub(crate) fn update_visit_error_context(mut error: Error,
raw: TVMFFIAny) -> Er
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 {
- object::is_instance_of::<VisitErrorContextObj>(
- (*object.cast::<TVMFFIObject>()).type_index,
- )
- .then(|| &*object.cast::<VisitErrorContextObj>())
- }
- };
// Callback, policy and default descent can report the same frame.
if let Some(context) = error.extra_context().and_then(as_context)
&& context
diff --git a/rust/tvm-ffi/tests/test_structural_mutate.rs
b/rust/tvm-ffi/tests/test_structural_mutate.rs
index d0fefe4d..685054be 100644
--- a/rust/tvm-ffi/tests/test_structural_mutate.rs
+++ b/rust/tvm-ffi/tests/test_structural_mutate.rs
@@ -1193,11 +1193,6 @@ fn
callback_errors_preserve_message_and_add_object_context() {
for error in errors {
assert_eq!(error.message(), "callback failed");
assert!(error.backtrace().contains("origin"));
- assert!(
- error
- .backtrace()
- .contains("object `testing.TestObjectBase`")
- );
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));
@@ -1364,7 +1359,12 @@ fn
shared_map_callback_error_preserves_source_and_reports_object_context() {
assert_eq!(source.get(2).unwrap(), Some(20));
assert_eq!(error.message(), "map mapper failed");
assert!(error.backtrace().contains("origin"));
- assert!(error.backtrace().contains("object `ffi.Map`"));
+ let context = Any::from(error.extra_context().unwrap().clone());
+ let records: Pattern = reflected_field(&context, "reverse_visit_pattern");
+ assert_eq!(
+ pattern_pointer(&records, records.len() - 1),
+ any_object_pointer(&Any::from(source))
+ );
}
#[test]
diff --git a/rust/tvm-ffi/tests/test_structural_visit.rs
b/rust/tvm-ffi/tests/test_structural_visit.rs
index 106c1d74..819bc5ca 100644
--- a/rust/tvm-ffi/tests/test_structural_visit.rs
+++ b/rust/tvm-ffi/tests/test_structural_visit.rs
@@ -32,6 +32,18 @@ fn runtime_error(message: &str) -> Error {
Error::new(RUNTIME_ERROR, message)
}
+/// Assert that `root` is the outermost node in `error`'s visit context.
+fn assert_visit_context_root(error: &Error, root: impl Into<Any>) {
+ let context = error.extra_context().unwrap();
+ let type_index = Any::from(context.clone()).type_index();
+ let records = FieldGetter::new(type_index, "reverse_visit_pattern")
+ .unwrap()
+ .get::<_, Array<Option<ObjectRef>>>(&**ObjectRef::data(context))
+ .unwrap();
+ let outermost = records.get(records.len() - 1).unwrap().unwrap();
+ assert!(outermost.same_as(&ObjectRef::try_from(root.into()).unwrap()));
+}
+
// Nested recursion restores regions after normal, interrupt, error and panic
exits.
#[test]
fn default_descent_scopes_regions_and_restores_after_halts() {
@@ -670,7 +682,7 @@ fn handler_error_crosses_map_traversal() {
Ok(_) => panic!("map handler unexpectedly succeeded"),
};
assert_eq!(error.message(), "map handler failed");
- assert!(error.backtrace().contains("object `ffi.Map`"));
+ assert_visit_context_root(&error, root.clone());
}
#[test]
@@ -1113,7 +1125,7 @@ fn handler_errors_include_native_visit_path() {
Ok(_) => panic!("handler unexpectedly succeeded"),
};
assert_eq!(error.message(), "handler failed");
- assert!(error.backtrace().contains("object `ffi.Array`"));
+ assert_visit_context_root(&error, root.clone());
}
#[test]
@@ -1147,7 +1159,7 @@ fn visitor_errors_include_native_visit_path() {
Ok(_) => panic!("visitor unexpectedly succeeded"),
};
assert_eq!(error.message(), "visitor failed");
- assert!(error.backtrace().contains("object `ffi.Array`"));
+ assert_visit_context_root(&error, root.clone());
struct FailingLeaf;
#[dispatch(
@@ -1169,7 +1181,7 @@ fn visitor_errors_include_native_visit_path() {
Ok(_) => panic!("callback visitor unexpectedly succeeded"),
};
assert_eq!(error.message(), "callback visitor failed");
- assert!(error.backtrace().contains("object `ffi.Array`"));
+ assert_visit_context_root(&error, root.clone());
}
#[test]
@@ -1372,7 +1384,7 @@ fn chain_link_errors_include_native_visit_path() {
Ok(_) => panic!("link unexpectedly succeeded"),
};
assert_eq!(error.message(), "link failed");
- assert!(error.backtrace().contains("object `ffi.Array`"));
+ assert_visit_context_root(&error, root.clone());
}
#[test]
@@ -2382,6 +2394,6 @@ fn
nested_walk_error_keeps_context_and_releases_callback_owner() {
.err()
.unwrap();
assert_eq!(error.message(), "nested owned failure");
- assert!(error.backtrace().contains("ffi.Array"));
+ assert_visit_context_root(&error, root.clone());
assert_eq!(owner.debug_strong_count(), Some(1));
}
diff --git a/tests/python/test_stubgen_rust.py
b/tests/python/test_stubgen_rust.py
index 68db1b8d..b257de53 100644
--- a/tests/python/test_stubgen_rust.py
+++ b/tests/python/test_stubgen_rust.py
@@ -182,7 +182,7 @@ def _uses(imports: RustImports) -> set[str]:
return {item.path for item in imports.items}
-NO_METADATA = "/// Opaque: no metadata of its own: total_size is unknown.
Fields are read through the C ABI getters."
+NO_METADATA = "// Opaque: no metadata of its own: total_size is unknown.
Fields are read through the C ABI getters."
# ---------------------------------------------------------------------------
@@ -270,7 +270,7 @@ def test_rust_ident() -> None:
def test_check_tokens_ignores_spelling() -> None:
generated = [
"use super::super::ir::ExprObj;",
- "/// Complete: every field is mirrored.",
+ "// Complete: every field is mirrored.",
"#[derive(tvm_ffi::derive::Object)]",
"pub struct AddObj { base_: ExprObj, pub a: i64, pub b: i64 }",
"const _: () = assert!(::core::mem::size_of::<AddObj>() == 40);",
@@ -352,6 +352,7 @@ def
test_generator_declares_its_directives_and_records_imports() -> None:
"opaque",
"upcast",
"custom-new",
+ "no-new",
"missing-docs",
}
imports = RUST.new_imports()
@@ -373,7 +374,6 @@ pub struct Pair {{
}}
/// The object behind [`Pair`].
-///
{NO_METADATA}
#[repr(C)]
#[derive(tvm_ffi::derive::Object)]
@@ -585,7 +585,6 @@ impl TryFrom<i64> for IterVarType {{
}}
/// The object behind [`IterVar`].
-///
{NO_METADATA}
#[repr(C)]
#[derive(tvm_ffi::derive::Object)]
@@ -731,8 +730,7 @@ pub struct Expr {
}
/// The object behind [`Expr`].
-///
-/// Complete: reflected fields fill [24, 40) exactly.
+// Complete: reflected fields fill [24, 40) exactly.
#[repr(C)]
#[derive(tvm_ffi::derive::Object)]
#[object(type_key = "ir.Expr", rs_parent)]
@@ -755,7 +753,7 @@ impl ExprObj {
}
impl Expr {
- /// Lossless complete-field allocation.
+ /// Create a `Expr` from its field values.
pub fn new(span: impl Into<Option<Span>>, ty: impl IntoArg<Type>) -> Self {
let obj = ExprObj::new(span.into(), ty.into_owned());
Self { data_: ObjectArc::new(obj) }
@@ -770,8 +768,7 @@ pub struct Add {
}
/// The object behind [`Add`].
-///
-/// Complete: reflected fields fill [40, 56) exactly.
+// Complete: reflected fields fill [40, 56) exactly.
#[repr(C)]
#[derive(tvm_ffi::derive::Object)]
#[object(type_key = "tirx.Add", final)]
@@ -801,7 +798,7 @@ impl AddObj {
}
impl Add {
- /// Lossless complete-field allocation.
+ /// Create a `Add` from its field values.
pub fn new(
span: impl Into<Option<Span>>,
ty: impl IntoArg<PrimType>,
@@ -855,7 +852,7 @@ def test_render_complete_add_golden() -> None:
def test_render_complete_span_and_prim_type() -> None:
"""Scalars take their reflected width; alignment padding is reported, not
mirrored."""
text, _ = _render(_span())
- assert "/// Opaque: field 'line' (int) has no native mirror." in text
+ assert "// Opaque: field 'line' (int) has no native mirror." in text
text, _ = _render(_span(), _span_imports())
assert (
"pub struct SpanObj {\n"
@@ -867,7 +864,7 @@ def test_render_complete_span_and_prim_type() -> None:
" pub end_column: i32,\n"
"}"
) in text
- assert "/// Complete: reflected fields fill [24, 48) exactly." in text
+ assert "// Complete: reflected fields fill [24, 48) exactly." in text
_register(_info("ir.Type", (_field("span", "ir.Span", 24, 8),),
total_size=32))
prim_type = _info(
@@ -879,7 +876,7 @@ def test_render_complete_span_and_prim_type() -> None:
)
text, imports = _render(prim_type)
assert (
- "/// Complete: reflected fields fill [32, 40) exactly (alignment
padding [36, 40))." in text
+ "// Complete: reflected fields fill [32, 40) exactly (alignment
padding [36, 40))." in text
)
assert "pub struct PrimTypeObj {\n base_: TypeObj,\n pub dtype:
DLDataType,\n}" in text
assert "assert!(::core::mem::size_of::<PrimTypeObj>() == 40);" in text
@@ -1145,7 +1142,7 @@ def test_unrenderable_field_keeps_the_type_opaque(field:
NamedTypeSchema) -> Non
assert field.size is not None
info = _info("demo.Holder", (field,), total_size=HEADER + field.size)
text, _ = _render(info)
- assert "/// Opaque: field 'x'" in text
+ assert "// Opaque: field 'x'" in text
assert "has no native mirror" in text
assert "impl HolderObj {\n pub fn x(&self) -> Result<" in text
assert "const _: () =" not in text
@@ -1162,7 +1159,7 @@ def test_custom_new_renames_the_wrapper_allocator() ->
None:
in text
)
assert (
- "impl Add {\n /// Lossless complete-field allocation.\n"
+ "impl Add {\n /// Create a `Add` from its field values.\n"
" pub fn from_fields(\n"
" span: impl IntoArg<Span>,\n"
" ty: impl IntoArg<Type>,\n"
@@ -1303,7 +1300,7 @@ def test_opaque_directive_vetoes_a_complete_type() ->
None:
imports = _span_imports()
RUST.add_directive(imports, "opaque", "ir.Span", 5)
text, _ = _render(_span(), imports)
- assert "/// Opaque: vetoed by directive although the layout is
reproducible." in text
+ assert "// Opaque: vetoed by directive although the layout is
reproducible." in text
assert "pub fn line(&self) -> Result<i32> {" in text
@@ -1311,7 +1308,7 @@ def test_builtin_parent_keeps_the_type_opaque() -> None:
"""The crate's `EnumObj` is a complete parent; a builtin stand-in cannot
be one."""
# An enum type embeds `EnumObj`; its registry allocates the values, so
there is no `new`.
text, _ = _render(object_info_from_type_key("testing.TestEnumVariant"))
- assert "/// Complete: reflected fields fill [48, 48) exactly." in text
+ assert "// Complete: reflected fields fill [48, 48) exactly." in text
assert " base_: EnumObj," in text
assert "size_of::<TestEnumVariantObj>() == 48" in text
assert "fn new(" not in text
@@ -1322,7 +1319,7 @@ def test_builtin_parent_keeps_the_type_opaque() -> None:
("testing.TestCxxStrEnum", "ffi.StrEnum"),
):
text, _ = _render(object_info_from_type_key(type_key))
- assert f"/// Opaque: parent '{parent}' is opaque (no-mirror)." in text
+ assert f"// Opaque: parent '{parent}' is opaque (no-mirror)." in text
assert "const _: () =" not in text
assert "fn new(" not in text
assert f"impl tvm_ffi::EnumRef for {type_key.rpartition('.')[2]} {{}}"
in text
@@ -1332,7 +1329,7 @@ def test_opaque_parent_keeps_the_child_opaque() -> None:
"""A child of an opaque type cannot embed a mirror of its parent: it stays
opaque."""
_register(_info("ir.Expr", (_field("span", "ir.Span", 24, 8),),
total_size=40)) # hole
text, _ = _render(_add())
- assert "/// Opaque: parent 'ir.Expr' is opaque (uncovered-bytes)." in text
+ assert "// Opaque: parent 'ir.Expr' is opaque (uncovered-bytes)." in text
assert " base_: ExprObj," in text
assert "pub fn a(&self) -> Result<Expr> {" in text
@@ -1418,7 +1415,7 @@ def _cxx_class_imports() -> RustImports:
def test_registry_complete_chain_is_mirrored() -> None:
base, _ = _render(object_info_from_type_key("testing.TestCxxClassBase"),
_cxx_class_imports())
assert (
- "/// Complete: reflected fields fill [24, 40) exactly (alignment
padding [36, 40))." in base
+ "// Complete: reflected fields fill [24, 40) exactly (alignment
padding [36, 40))." in base
)
assert (
"pub struct TestCxxClassBaseObj {\n base_: Object,\n pub v_i64:
i64,\n pub v_i32: i32,\n}"
@@ -1470,12 +1467,12 @@ def test_registry_complete_chain_is_mirrored() -> None:
def test_registry_hidden_field_and_vptr_stay_opaque() -> None:
hidden, _ =
_render(object_info_from_type_key("testing.TestCxxClassHiddenField"))
assert (
- "/// Opaque: bytes [32, 40) of [24, 48) are not accounted for by
reflected fields. "
+ "// Opaque: bytes [32, 40) of [24, 48) are not accounted for by
reflected fields. "
"Fields are read through the C ABI getters."
) in hidden
assert "pub fn v_i32(&self) -> Result<i64> {" in hidden
poly, _ =
_render(object_info_from_type_key("testing.TestCxxClassPolymorphic"))
- assert "/// Opaque: bytes [32, 40) of [24, 40)" in poly
+ assert "// Opaque: bytes [32, 40) of [24, 40)" in poly
def test_stage_3_applies_directives_to_a_registered_type(tmp_path: Path) ->
None:
@@ -1617,7 +1614,7 @@ def test_cli_check_ignores_how_a_binding_is_spelled(
("use tvm_ffi::String;\n", ""),
("use std::ops::Deref;\n", "use std::ops::Deref;\nuse
crate::testing::TestCompare;\n"),
(" base_: Object,\n", " base_: tvm_ffi::Object,\n"),
- ("/// Complete: ", "/// Mirrored: "),
+ ("// Complete: ", "// Mirrored: "),
(
" pub fn new(\n"
" p_required: i64,\n"
