[FFI] Back Optional and Variant by TVMFFIAny for a stable ABI layout#657
Conversation
There was a problem hiding this comment.
Code Review
This pull request unifies the ABI layout of the Variant and Optional containers in the TVM FFI. Both Variant<...> and Optional<T> (for storage-enabled types) are now uniformly backed by a single 16-byte TVMFFIAny (Any) cell, making their memory layouts independent of the contained types. This simplifies the implementation, removes several specializations (such as those for String, Bytes, and ObjectRef types), and ensures ABI stability. The Rust bindings have been updated accordingly, replacing the previous per-type mirrors (OptionPod, OptionStr, OptionObjRef) with a single unified Optional<T> wrapper. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
NOTE: this PR do not break the C ABI so it. But it does impact the extra json Stringfy ABI, will followup on downstream tvm update. This is one step forward as previously there is no guarantee on the layout but now we have one |
Give Optional<T> and Variant<...> a TVMFFIAny-backed representation so the in-memory layout no longer depends on the contained type, keeping the ABI stable across types. C++: - Variant<...> is always backed by a single Any (TVMFFIAny); it is no longer ObjectRef-derived even when all alternatives are ObjectRef. The former VariantBase (with its ObjectRef-derived specialization) is folded directly into Variant. - Optional<T> is a hybrid: for types that enable Any storage (TypeTraits<T>::storage_enabled) it is backed by a single Any with nullopt represented as kTVMFFINone (sizeof(Optional<T>) == sizeof(Any)); for non-storage types (e.g. non-owning views) it falls back to std::optional<T>. None/null reads back as nullopt, giving one stable cross-language representation consistent with Python's None. - TypeTraits<Optional<T>>::storage_enabled propagates from TypeTraits<T>::storage_enabled, so a nested Optional<Optional<T>> and Optional<T> used inside Variant<...>/containers are Any-backed exactly when T is storage-enabled. - json::Stringify takes its Optional<int> indent by const reference. - User-facing semantics are preserved (value/value_or/has_value, comparisons, move/copy, container conversion, TypeTraits error text/schema, and the Variant equivalents). Rust: - Replace the three per-layout Optional mirrors (OptionPod / OptionStr / OptionObjRef) with a single Optional<T> in optional.rs: repr(transparent) over the 16-byte Any/TVMFFIAny cell, nullopt == kTVMFFINone, named Optional to distinguish it from Rust's Option.
bb95052 to
b5996a9
Compare
This PR makes
tvm::ffi::Optional<T>andtvm::ffi::Variant<...>layout stable across the contained typeTby backing them withAny. Note thatOptional<T>still falls back tostd::optional<T>whenTdoes not support Any storage.This helps stabilize the future object-layout ABI of
OptionalandVariant.