tqchen commented on code in PR #399:
URL: https://github.com/apache/tvm-ffi/pull/399#discussion_r2731999783
##########
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:
There are two ways that Expected<T> get returned
- W0: Expected<T> get "raised" internally by setting the TLS value via
TVMFFISetRaised
- W1: Expected<T> 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<T> should be returned to
the caller
- C1 + W0: we should return Expected<Any> with the error "raised" set to the
returned Expected value
- C1 + W1: we need to return the Expected<T>, 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.
--
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]
