feat: add ffi::Expected<T> for exception-free error handling #399

guan404ming · 2026-01-11T15:38:35Z

Why

Enable exception-free C++ API similar to Rust's Result or C++23's std::expected, as requested in #234.

How

Add Expected<T> class holding either success value T or Error
Add Function::CallExpected<T>() for exception-free function calls
Add is_expected type trait and Expected handling in unpack_call
Support registering functions that return Expected<T>"

gemini-code-assist · 2026-01-11T15:38:52Z

Summary of Changes

Hello @guan404ming, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves the error handling capabilities within the FFI layer by introducing a new Expected type. This type provides a robust, exception-free mechanism for FFI functions to communicate either a successful result or a detailed error, aligning with modern C++ practices and similar patterns in other languages. The changes ensure that FFI calls can explicitly manage potential failures without relying on exceptions for control flow, leading to more predictable and maintainable code.

Highlights

Introduced ffi::Expected: A new type for exception-free error handling, similar to Rust's Result or C++23's std::expected, allowing functions to return either a success value T or an Error.
Added Function::CallExpected(): A new method for FFI function calls that returns an Expected, converting any caught exceptions into Error objects within the Expected type.
Enhanced FFI Type System: Integrated Expected into the FFI type traits (is_expected and RetSupported) and the unpack_call mechanism, enabling seamless handling of functions returning Expected.
Support for Registering Expected-Returning Functions: Functions that return Expected can now be registered with the FFI system, allowing them to be called from other languages or contexts while preserving the explicit error handling.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces ffi::Expected<T> for exception-free error handling, a valuable addition for creating more robust FFI functions. The implementation is comprehensive, covering the Expected<T> class, integration with tvm::ffi::Function via CallExpected, and support for packed functions returning Expected<T>. The tests are thorough and cover many important use cases.

The overall approach of using tvm::ffi::Any for storage within Expected<T> is a clever way to leverage the existing FFI infrastructure. I have a few suggestions to simplify the TypeTraits specialization and to add move semantics for value/error accessors, which would improve code clarity and performance. Overall, this is a well-executed feature.

gemini-code-assist · 2026-01-11T15:41:47Z

include/tvm/ffi/expected.h

+  TVM_FFI_INLINE T value() const {
+    if (is_err()) {
+      TVM_FFI_THROW(RuntimeError) << "Bad expected access: contains error";
+    }
+    return data_.cast<T>();
+  }


The value() method is const-qualified, which means it always returns a copy of the contained value. This can be inefficient when the Expected object is an rvalue (e.g., a temporary returned from a function). Consider adding an rvalue-qualified overload to allow moving the value out. This would improve performance in scenarios like std::move(expected).value().

You can change the existing method to be const&-qualified and add a &&-qualified overload:

TVM_FFI_INLINE T value() const& { if (is_err()) { TVM_FFI_THROW(RuntimeError) << "Bad expected access: contains error"; } return data_.cast<T>(); } TVM_FFI_INLINE T value() && { if (is_err()) { TVM_FFI_THROW(RuntimeError) << "Bad expected access: contains error"; } return std::move(data_).cast<T>(); }

gemini-code-assist · 2026-01-11T15:41:47Z

include/tvm/ffi/expected.h

+  TVM_FFI_INLINE Error error() const {
+    TVM_FFI_ICHECK(is_err()) << "Expected does not contain an error";
+    return data_.cast<Error>();
+  }


Similar to value(), the error() method is const-qualified and always returns a copy. You can add an rvalue-qualified overload to allow moving the Error object out, which is more efficient.

TVM_FFI_INLINE Error error() const& { TVM_FFI_ICHECK(is_err()) << "Expected does not contain an error"; return data_.cast<Error>(); } TVM_FFI_INLINE Error error() && { TVM_FFI_ICHECK(is_err()) << "Expected does not contain an error"; return std::move(data_).cast<Error>(); }

gemini-code-assist · 2026-01-11T15:41:47Z

include/tvm/ffi/expected.h

+  TVM_FFI_INLINE static void CopyToAnyView(const Expected<T>& src, TVMFFIAny* result) {
+    // Extract value from src.data_ and copy it properly
+    const TVMFFIAny* src_any = reinterpret_cast<const TVMFFIAny*>(&src.data_);
+
+    if (TypeTraits<T>::CheckAnyStrict(src_any)) {
+      // It contains T, copy it out and move to result
+      T value = TypeTraits<T>::CopyFromAnyViewAfterCheck(src_any);
+      TypeTraits<T>::MoveToAny(std::move(value), result);
+    } else {
+      // It contains Error, copy it out and move to result
+      Error err = TypeTraits<Error>::CopyFromAnyViewAfterCheck(src_any);
+      TypeTraits<Error>::MoveToAny(std::move(err), result);
+    }
+  }


The implementation of CopyToAnyView is correct but can be simplified. Instead of checking the type and manually performing a copy-then-move for each case, you can leverage the Any type's copy constructor to handle the logic. This makes the code more concise and less error-prone, as it relies on the already-tested Any copy semantics.

TVM_FFI_INLINE static void CopyToAnyView(const Expected<T>& src, TVMFFIAny* result) { // An Expected<T> is represented by its underlying value (T or Error) // at the FFI boundary. We can simply copy the contained Any. Any copied_any = src.data_; *result = details::AnyUnsafe::MoveAnyToTVMFFIAny(std::move(copied_any)); }

gemini-code-assist · 2026-01-11T15:41:47Z

include/tvm/ffi/expected.h

+  TVM_FFI_INLINE static void MoveToAny(Expected<T> src, TVMFFIAny* result) {
+    // Extract value from src.data_ and move it properly
+    TVMFFIAny* src_any = reinterpret_cast<TVMFFIAny*>(&src.data_);
+
+    if (TypeTraits<T>::CheckAnyStrict(src_any)) {
+      // It contains T, move it out and move to result
+      T value = TypeTraits<T>::MoveFromAnyAfterCheck(src_any);
+      TypeTraits<T>::MoveToAny(std::move(value), result);
+    } else {
+      // It contains Error, move it out and move to result
+      Error err = TypeTraits<Error>::MoveFromAnyAfterCheck(src_any);
+      TypeTraits<Error>::MoveToAny(std::move(err), result);
+    }
+  }


Similar to CopyToAnyView, the MoveToAny implementation can be greatly simplified. Since src is passed by value (effectively a move), you can move the underlying Any object directly. This is more readable and directly expresses the intent of moving the contained value.

TVM_FFI_INLINE static void MoveToAny(Expected<T> src, TVMFFIAny* result) { // An Expected<T> is represented by its underlying value (T or Error) // at the FFI boundary. We can simply move the contained Any. *result = details::AnyUnsafe::MoveAnyToTVMFFIAny(std::move(src.data_)); }

tqchen · 2026-01-12T14:15:11Z

Thanks a lot for the contribution, this would benefit from careful reviews, @junrushao @DarkSharpness @Ubospica can you help

Kathryn-cat · 2026-01-14T16:24:22Z

@guan404ming Thanks for the contribution! I will take a closer look sometime this week to provide a round of review.

Kathryn-cat · 2026-01-20T07:58:09Z

include/tvm/ffi/expected.h

+   */
+  TVM_FFI_INLINE T value() const {
+    if (is_err()) {
+      TVM_FFI_THROW(RuntimeError) << "Bad expected access: contains error";


If value() failed, it appears that it throws an error here but doesn't preserve the original error. Is it possible to retain the original error information?

Done. Changed value() to throw the contained error directly instead of a generic RuntimeError:

if (is_err()) { throw data_.cast<Error>(); }

Kathryn-cat · 2026-01-20T08:20:17Z

include/tvm/ffi/expected.h

+    if (is_err()) {
+      TVM_FFI_THROW(RuntimeError) << "Bad expected access: contains error";
+    }
+    return data_.cast<T>();


For large data, we can use move instead of copy, so I agree with Gemini that we can add an overload function here:

TVM_FFI_INLINE T value() && { if (is_err()) { throw error(); } return std::move(data_).cast<T>(); }

Agree as well. I've added both const& and && qualified overloads for value():

Actually we might not need both here, @tqchen would like to hear your opinion if we wanna keep one of them

Kathryn-cat · 2026-01-20T08:29:25Z

include/tvm/ffi/expected.h

+   * \brief Check if the Expected contains an error.
+   * \return True if contains error, false if contains success value.
+   */
+  TVM_FFI_INLINE bool is_err() const { return data_.as<Error>().has_value(); }


It is likely that ObjectRef as a base class of Error can have both is_ok()=True and is_err()=True, since CheckAnyStrict relies on inheritance checking, not type matching. I would suggest returning !is_ok() here.

Also it would be helpful to enhance the test cases on this.

Changed is_err() to return !is_ok() and also fixed is_ok() to check for Error first.

Kathryn-cat · 2026-01-20T08:41:38Z

include/tvm/ffi/expected.h

+  /*!
+   * \brief Access the error value.
+   * \return The error value.
+   * \note Behavior is undefined if the Expected contains a success value.


Let's instead of saying "behavior is undefined", say it throws a RuntimeError on bad access.

Perhaps change the function body into

if (!is_err()) { TVM_FFI_THROW(RuntimeError) << "Bad expected access: contains value, not error"; }

to mimic value() impl.

Sure, I've updated the docstring and changed from TVM_FFI_ICHECK to TVM_FFI_THROW(RuntimeError)

Kathryn-cat · 2026-01-23T08:09:21Z

include/tvm/ffi/expected.h

+/*!
+ * \brief Helper function to create Expected::Err.
+ * \param error The error value.
+ * \return Expected<Any> containing the error.


Let's fix the comment here, should return Expected<T> same as the ExpectedOk case.

Kathryn-cat · 2026-01-23T08:12:38Z

include/tvm/ffi/expected.h

+ * \return Expected<Any> containing the error.
+ * \note Returns Expected<Any> to allow usage in contexts where T is inferred.
+ */
+template <typename T = Any>


Perhaps template <typename T> is cleaner? Do we have to default to Any here

Yes, that would be better. Thanks.

Kathryn-cat · 2026-01-23T08:19:00Z

include/tvm/ffi/expected.h

+   * \return The success value if present, otherwise the default value.
+   */
+  template <typename U = std::remove_cv_t<T>>
+  TVM_FFI_INLINE T value_or(U&& default_value) const {


I think this function might be rarely used, because the philosophy of Expected<T> is handling error explicitly. Let's leave it for discussion here.

I think maybe we should keep this since that is kind of standard practice std::optional and C++23 std::expected both have value_or.

ref std::expected (C++23) and std::optional (C++17)

Kathryn-cat · 2026-01-23T08:43:32Z

include/tvm/ffi/expected.h

+  TVM_FFI_INLINE static void CopyToAnyView(const Expected<T>& src, TVMFFIAny* result) {
+    const TVMFFIAny* src_any = reinterpret_cast<const TVMFFIAny*>(&src.data_);
+    if (TypeTraits<T>::CheckAnyStrict(src_any)) {
+      TypeTraits<T>::MoveToAny(TypeTraits<T>::CopyFromAnyViewAfterCheck(src_any), result);


We possibly don't need to create a copy of T then move it.

Possible simplification:

if (src.is_err()) { TypeTraits<Error>::CopyToAnyView(src.error(), result); } else { TypeTraits<T>::CopyToAnyView(src.value(), result); }

Thanks, this is more clearer.

Kathryn-cat · 2026-01-23T08:46:11Z

include/tvm/ffi/expected.h

+  TVM_FFI_INLINE static void MoveToAny(Expected<T> src, TVMFFIAny* result) {
+    TVMFFIAny* src_any = reinterpret_cast<TVMFFIAny*>(&src.data_);
+    if (TypeTraits<T>::CheckAnyStrict(src_any)) {
+      TypeTraits<T>::MoveToAny(TypeTraits<T>::MoveFromAnyAfterCheck(src_any), result);


ditto,

if (src.is_err()) { TypeTraits<Error>::MoveToAny(std::move(src).error(), result); } else { TypeTraits<T>::MoveToAny(std::move(src).value(), result); }

Kathryn-cat · 2026-01-23T09:03:46Z

include/tvm/ffi/expected.h

+  }
+
+  TVM_FFI_INLINE static std::string TypeSchema() {
+    return R"({"type":"Expected","args":[)" + details::TypeSchema<T>::v() + "]}";


Maybe also mention {"type":"ffi.Error"} in type schema?

Kathryn-cat · 2026-01-23T09:06:29Z

include/tvm/ffi/expected.h

+    return std::nullopt;
+  }
+
+  TVM_FFI_INLINE static std::string GetMismatchTypeInfo(const TVMFFIAny* src) {


Let's remove this

Kathryn-cat · 2026-01-23T23:13:31Z

include/tvm/ffi/function.h

+
+    if (ret_code == 0) {
+      // Success - cast result to T and return Ok
+      return Expected<T>::Ok(std::move(result).cast<T>());


Here's a tricky part. If the result does not match the type T, cast<T>() will throw, and this error will escape CallExpected.

Some proposed change: (feel free to brainstorm better ones)

if (ret_code == 0) { if (auto val = std::move(result).template as<T>()) { return Expected<T>::Ok(std::move(*val)); } else { return Expected<T>::Err(Error("TypeError", "CallExpected: result type mismatch, expected " + TypeTraits<T>::TypeStr(), "")); } }

Nice catch. The implementation looks good. I think there is a potential edge case that we probably don't want ot try casting if T = Any. Here is updated implementation

if (ret_code == 0) { if constexpr (std::is_same_v<T, Any>) { return Expected<T>::Ok(std::move(result)); } else { if (auto val = std::move(result).template as<T>()) { return Expected<T>::Ok(std::move(*val)); } else { return Expected<T>::Err(Error( "TypeError", "CallExpected: result type mismatch, expected " + TypeTraits<T>::TypeStr(), "")); } } }

Kathryn-cat

Nice work @guan404ming, thanks for addressing the comments! The PR implements the proposal cleanly and the functionalities are in good state. My last advice are below, cc' @tqchen for a final round of discussion.

Since it introduces new function calling API to the users, @guan404ming could you document the usage? I think your test case RegisterExpectedReturning shows a good example on registering and calling Expected functions, and when to use the throw exception vs Expected flows. (cc' @junrushao if you could provide a pointer to where to add the docs)

Kathryn-cat · 2026-01-24T00:18:35Z

include/tvm/ffi/function_details.h

-static constexpr bool RetSupported =
-    (std::is_same_v<T, Any> || std::is_void_v<T> || TypeTraits<T>::convert_enabled);
+static constexpr bool RetSupported = (std::is_same_v<T, Any> || std::is_void_v<T> ||
+                                      TypeTraits<T>::convert_enabled || is_expected_v<T>);


I think convert_enabled is already True here, is_expected_v might be redundant

You're right, let me remove it.

Kathryn-cat · 2026-01-24T00:21:16Z

include/tvm/ffi/function_details.h

+    if (expected_result.is_ok()) {
+      *rv = expected_result.value();
+    } else {
+      throw expected_result.error();


Perhaps use move?

if (expected_result.is_ok()) { *rv = std::move(expected_result).value(); } else { throw std::move(expected_result).error(); }

Sure, updated!

guan404ming · 2026-01-24T04:46:23Z

Since it introduces new function calling API to the users, @guan404ming could you document the usage?

Sure, I think I could do it as a follow up after this one!

Kathryn-cat · 2026-01-24T05:19:26Z

@tqchen the CI fails because tvm-ffi's DLPack version is 1.0.2, while dlpack exchange api is tested against PyTorch's native impl (DLPack 1.0.3), perhaps we need to upgrade tvm-ffi's DLPack version

Kathryn-cat · 2026-01-24T05:36:56Z

Since it introduces new function calling API to the users, @guan404ming could you document the usage?

Sure, I think I could do it as a follow up after this one!

Thanks @guan404ming , I think the PR is in good state, let's wait for CI to be fixed and get it in!

guan404ming · 2026-01-24T06:52:40Z

Here is the upgrade pr #420. I've checked there is no any compat issue need to be updated in our src code.

guan404ming changed the title ~~feat: Add ffi::Expected<T> for exception-free error handling~~ feat: add ffi::Expected<T> for exception-free error handling Jan 11, 2026

gemini-code-assist bot reviewed Jan 11, 2026

View reviewed changes

guan404ming force-pushed the feat/expected-type branch from 2f70b38 to f1a4dc9 Compare January 11, 2026 15:46

guan404ming marked this pull request as ready for review January 12, 2026 02:56

tqchen assigned Kathryn-cat Jan 14, 2026

Kathryn-cat suggested changes Jan 20, 2026

View reviewed changes

feat: Add ffi::Expected<T> for exception-free error handling

b510df6

guan404ming force-pushed the feat/expected-type branch from f1a4dc9 to 486f68e Compare January 20, 2026 11:20

guan404ming requested a review from Kathryn-cat January 20, 2026 11:21

refactor: apply review comments

ad48e1b

guan404ming force-pushed the feat/expected-type branch from 486f68e to ad48e1b Compare January 20, 2026 11:27

Kathryn-cat reviewed Jan 23, 2026

View reviewed changes

guan404ming requested a review from Kathryn-cat January 23, 2026 16:35

Kathryn-cat reviewed Jan 23, 2026

View reviewed changes

Kathryn-cat suggested changes Jan 24, 2026

View reviewed changes

refactor: apply review comments

e40b425

guan404ming force-pushed the feat/expected-type branch from 806fe56 to e40b425 Compare January 24, 2026 04:47

Kathryn-cat approved these changes Jan 24, 2026

View reviewed changes

feat: add ffi::Expected<T> for exception-free error handling #399

Are you sure you want to change the base?

feat: add ffi::Expected<T> for exception-free error handling #399

Conversation

guan404ming commented Jan 11, 2026

Why

How

Uh oh!

gemini-code-assist bot commented Jan 11, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

tqchen commented Jan 12, 2026

Uh oh!

Kathryn-cat commented Jan 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guan404ming Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kathryn-cat left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

guan404ming Jan 24, 2026 •

edited

Loading

Kathryn-cat left a comment •

edited

Loading

guan404ming Jan 24, 2026 •

edited

Loading