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 · 2026-01-25T13:58:12Z

The ci passed after the upgrade!

tqchen · 2026-01-26T18:57:38Z

include/tvm/ffi/function_details.h

  // use index sequence to do recursive-less unpacking
  if constexpr (std::is_same_v<R, void>) {
    f(ArgValueWithContext<std::tuple_element_t<Is, PackedArgs>>{args, Is, optional_name, f_sig}...);
+  } else if constexpr (is_expected_v<R>) {


This behavior can be a bit confusing. If the ffi.Function is explicitly returning Expected Value (instead of throw using the error handling mechanism, then the function should successully return instead of implicitly throw when error is found?

Mayb need a regression testcase for this

i think the current design unwraps Expected<T> at the FFI boundary. CallExpected use safe_call to catch the error and return Expected<T>; on the contrary, the normal call path would throw an error. This might seem a bit convoluted. Do you have better suggested ideas?

There are two ways that Expected get returned

W0: Expected get "raised" internally by setting the TLS value via TVMFFISetRaised

W1: Expected get returned as a normal return value

And there are two ways to call a function now

C0: Normal call

C1: CallExpected

Would be good to discuss the overall relation in the mix of four cases

C0+ W0: we need to throw the error to caller

C0 + W1: no error should be thrown, the Expected should be returned to the caller

C1 + W0: we should return Expected with the error "raised" set to the returned Expected value

C1 + W1: we need to return the Expected, note that in this case it is harder to distinguish error being returned versus error being raised if the desirable logic is to return an error, but this is more of a rare case.

In any case, it would be good first to make sure behavior of C0 is correct, and the particular context seems to suggest C0+W1, in such case, we should return the value to the caller.

Thanks for the detailed breakdown of the four scenarios (C0/C1 + W0/W1) which really helped clarify the design considerations. Based on my understanding, the key question is about the C0 + W1 scenario (normal Call() on a function returning Expected). Here's how I see the two possible directions:

Option A: Keep current behavior (auto-unwrap)

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

Pros: Caller uses Call() and gets the value directly

Cons: Cannot distinguish "returned error" from "raised error"

Option B: No unwrap, return Expected directly

// Proposed change *rv = f(...); // return Expected as-is

Pros: Clear distinction between returned vs raised errors

Cons: Caller must use Call<Expected>() and handle it explicitly

Personally, I'd slightly lean toward Option B after @tqchen's breakdown. If a function explicitly chooses to return Expected, I think we should respect that and let the caller receive it directly.

I'd like to confirm your preference before updating the implementation. Happy to add regression tests or update implementation for whichever approach we go with.

agree option B is better

Updated with option B. Please take another look, thanks!

tqchen · 2026-01-26T18:58:41Z

include/tvm/ffi/expected.h

+ * \return Expected<T> containing the success value.
+ */
+template <typename T>
+TVM_FFI_INLINE Expected<T> ExpectedOk(T value) {


I think it is good to do round of API review, can you list all the APIs that are not conforming to std::expected, list their names, and discuss choice(i know some comes from rust API style, but good to be explicit). Would be good to list the APIs in the comment

tqchen · 2026-01-26T19:00:43Z

Thanks @guan404ming , i think we are close, i have one behavior comment. Besides that, given this is new public API, i think it would be helpful to do a round of quick API review, can you provide a list of APIs(including methods), would be good to categorize them as:

C0: strictly follows c++ standard
C1: others and list rationale(e.g. we follows rust API)

Would also be good to discuss move and value copy behavior.

guan404ming · 2026-01-27T16:43:01Z

Here is the API review

| API                | Category | Notes                                                              |                                                                         
|--------------------|----------|--------------------------------------------------------------------|                                                                         
| is_ok()            | C0       | Follows std::expected::has_value() semantics                       |                                                                         
| is_err()           | C1       | Rust-style naming; equivalent to !has_value()                      |                                                                         
| has_value()        | C0       | Alias for is_ok(), follows C++ standard                            |                                                                         
| value()            | C0       | Follows std::expected::value()                                     |                                                                         
| value() &&         | C1       | Move-qualified overload for efficiency (not in C++23 standard)     |                                                                         
| error()            | C0       | Follows std::expected::error()                                     |                                                                         
| error() &&         | C1       | Move-qualified overload for efficiency                             |                                                                         
| value_or(default)  | C0       | Follows std::expected::value_or()                                  |                                                                         
| ExpectedOk(value)  | C1       | Rust-style helper; C++ standard has no equivalent factory function |                                                                         
| ExpectedErr(error) | C1       | Rust-style helper; C++ standard has no equivalent factory function |

Rationale for C1 APIs:

Move-qualified overloads (&&): Added for performance when the Expected is a temporary, avoiding unnecessary copies of large objects.
ExpectedOk() / ExpectedErr(): Rust-style factory functions for cleaner syntax with type deduction. C++23 std::expected relies on constructors instead.
is_err(): Rust naming convention; provides symmetry with is_ok().

APIs not implemented (compared to C++23 std::expected):

operator bool(): can use has_value() or is_ok() instead
operator*() / operator->(): unchecked access, omitted for safety
Monadic operations (and_then, transform, or_else, transform_error): may add in future if needed

tqchen · 2026-01-27T18:22:53Z

thanks @guan404ming , cc @Ubospica @Seven-Streams @junrushao @DarkSharpness for quick check on APIs. the ask

please comment on the API naming/choice, especially C1 APIs.
help cross check needs in possible downstream (@Ubospica would be good to confirm xgrammar needs)

DarkSharpness · 2026-01-27T19:12:03Z

Here is the API review

| API                | Category | Notes                                                              |                                                                         
|--------------------|----------|--------------------------------------------------------------------|                                                                         
| is_ok()            | C0       | Follows std::expected::has_value() semantics                       |                                                                         
| is_err()           | C1       | Rust-style naming; equivalent to !has_value()                      |                                                                         
| has_value()        | C0       | Alias for is_ok(), follows C++ standard                            |                                                                         
| value()            | C0       | Follows std::expected::value()                                     |                                                                         
| value() &&         | C1       | Move-qualified overload for efficiency (not in C++23 standard)     |                                                                         
| error()            | C0       | Follows std::expected::error()                                     |                                                                         
| error() &&         | C1       | Move-qualified overload for efficiency                             |                                                                         
| value_or(default)  | C0       | Follows std::expected::value_or()                                  |                                                                         
| ExpectedOk(value)  | C1       | Rust-style helper; C++ standard has no equivalent factory function |                                                                         
| ExpectedErr(error) | C1       | Rust-style helper; C++ standard has no equivalent factory function |

Rationale for C1 APIs:

Move-qualified overloads (&&): Added for performance when the Expected is a temporary, avoiding unnecessary copies of large objects.
ExpectedOk() / ExpectedErr(): Rust-style factory functions for cleaner syntax with type deduction. C++23 std::expected relies on constructors instead.
is_err(): Rust naming convention; provides symmetry with is_ok().

APIs not implemented (compared to C++23 std::expected):

operator bool(): can use has_value() or is_ok() instead
operator*() / operator->(): unchecked access, omitted for safety
Monadic operations (and_then, transform, or_else, transform_error): may add in future if needed

Thanks for the detailed API review.

One small difference worth calling out for ExpectedOk / ExpectedErr is a C++-specific type deduction behavior compared to Rust. In Rust, generic parameters can be inferred from the return type, while in C++ template argument deduction only considers function arguments, not the return type. This means that with factory-style helpers like ExpectedOk, the deduced type may sometimes differ from the desired return type. For example:

template <typename T>
Expect <T> ExpectOk(T value);

Expect<long> function(int x) {
  return ExpectOk(x); // <- in many cases, we want [T = long], which deduced from return type.
                      // but in this case T will deduced as int, which may lead to compile error.
}

In such cases, C++ cannot deduce T = long from the return type, which may lead to a compilation error.

I'm not sure whether this will cause some inconvenience in real usage, but the current design is reasonable for simplicity.

In XGrammar we implement something like PartialExpected, which can be implicitly converted to any Expected<T> and solves the return type inference problem. However, this actually introduces much complexity https://github.com/mlc-ai/xgrammar/blob/aa44ded202fdaa2cb55f48deb31d17c9822a8a1f/cpp/support/utils.h#L130-L212

guan404ming · 2026-01-28T05:09:14Z

Thanks for bringing up the discussion! I agree with keeping the design simple. While return type deduction is nice, explicit type specification is a fair trade-off to keep the API clean.

I think type deduction works for most cases. For exceptions, users can simply use like Expected<long>::Ok(x). For a foundational library like tvm-ffi, the complexity of PartialExpected isn't worth the marginal benefit in my opinion. This strikes the right balance between simplicity and usability.

tqchen · 2026-01-28T13:48:51Z

just want to make sure xgrammar can switch to make use of the new impl here. once you think the remaining apis are good please explicitly approve, @Ubospica @Seven-Streams @junrushao @DarkSharpness

tqchen · 2026-01-28T13:57:15Z

Thinking a bit more about partial deduction, it might be useful to think about the usages.

The main principle we like to follow to deliberate the API design is mainly because of stability, so we want to have one consistent behavior once introduced(less suprising to users), or not introducing the API so users can do their own wrapping. We want to avoid the case where api causes inconsistency with standard usages, or find need to update the api spec later, that is why aligning with standard is good, and having people to chime in is always better.

Thanks a lot for working through this deliberation process. Specifically, it is good to think about what are the expected usage pattern of ExpectedOk/ExpectedErr, ideally we want to see real code snippest from examples like xgrammar or simply ask genai to provide some.

The main thing to consider is consistency with std::optional, the following code is generally a common pattern:

std::optional<long> func_optional(bool valid, int x) {
   if (valid) return x;
   return std::nullopt;
}

We can find that return value conversion from x to std::optional<long> is automatically deduced by the return type.

So what @DarkSharpness mentioned is that it is desirable to have similar syntax. In which case ExpectedOk and ExpectedErr follows the similar rationale of targetted usage.

Expected<long> func_optional_rvalue_deduce(bool valid, int x) {
   if (value) return ExpectedOk(x);
   return ExpectedErr(Error("ValueError", "xx"));
}

In the current impl, we need to write it as

Expected<long> func_optional_param_deduce(bool valid, int x) {
   if (value) return Expected<long>(x);
   return Expected<long>(Error("ValueError", "xx"));
}

And for ExpectedErr seems we always have to write ExpectedErr because T cannot be deduced from input type

The main question we want to ask is the common usage scenarios the sugar ExpectedOk/ExpectedErr, e.g. are they mostly used in the case similar to func_optional where we want to simplify by deducing from return values, if so, then I think the deferred deduction is wortwhile, alternatively we should left out the ExpectedOk/ExpectedErr sugar since for most common case we expect people to explicitly write Expected<T>

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

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

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

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

guan404ming added 3 commits January 25, 2026 21:38

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

292ee84

refactor: apply review comments

8508ce3

refactor: apply review comments

ad1c187

guan404ming force-pushed the feat/expected-type branch from e40b425 to ad1c187 Compare January 25, 2026 13:38

tqchen requested changes Jan 26, 2026

View reviewed changes

Streamline error handling in FFI function calls

58b9af6

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

Uh oh!

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!

guan404ming Jan 24, 2026 •

edited

Loading

Kathryn-cat left a comment •

edited

Loading

guan404ming Jan 24, 2026 •

edited

Loading

Kathryn-cat Jan 26, 2026 •

edited

Loading