guan404ming commented on code in PR #399: URL: https://github.com/apache/tvm-ffi/pull/399#discussion_r2732889922
##########
include/tvm/ffi/function_details.h:
##########
@@ -219,6 +237,14 @@ TVM_FFI_INLINE void
unpack_call(std::index_sequence<Is...>, const std::string* o
// 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>) {
Review Comment:
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<T>). Here's how I see the two possible
directions:
Option A: Keep current behavior (auto-unwrap)
``` c
// 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<T>() and gets the value directly
- Cons: Cannot distinguish "returned error" from "raised error"
Option B: No unwrap, return Expected directly
```c
// Proposed change
*rv = f(...); // return Expected as-is
```
- Pros: Clear distinction between returned vs raised errors
- Cons: Caller must use Call<Expected<T>>() 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
