tqchen commented on code in PR #399:
URL: https://github.com/apache/tvm-ffi/pull/399#discussion_r2755677357
##########
include/tvm/ffi/function.h:
##########
@@ -643,6 +643,65 @@ class Function : public ObjectRef {
static_cast<FunctionObj*>(data_.get())->CallPacked(args.data(),
args.size(), result);
}
+ /*!
+ * \brief Call the function and return Expected<T> for exception-free error
handling.
+ * \tparam T The expected return type (default: Any).
+ * \param args The arguments to pass to the function.
+ * \return Expected<T> containing either the result or an error.
+ *
+ * This method provides exception-free calling by catching all exceptions
+ * and returning them as Error values in the Expected type.
+ *
+ * \code
+ * Function func = Function::GetGlobal("risky_function");
+ * Expected<int> result = func.CallExpected<int>(arg1, arg2);
+ * if (result.is_ok()) {
+ * int value = result.value();
+ * } else {
+ * Error err = result.error();
+ * }
+ * \endcode
+ */
+ template <typename T = Any, typename... Args>
+ TVM_FFI_INLINE Expected<T> CallExpected(Args&&... args) const {
+ constexpr size_t kNumArgs = sizeof...(Args);
+ AnyView args_pack[kNumArgs > 0 ? kNumArgs : 1];
+ PackedArgs::Fill(args_pack, std::forward<Args>(args)...);
+
+ Any result;
+ FunctionObj* func_obj = static_cast<FunctionObj*>(data_.get());
+
+ // Use safe_call path to catch exceptions
+ int ret_code = func_obj->safe_call(func_obj, reinterpret_cast<const
TVMFFIAny*>(args_pack),
+ kNumArgs,
reinterpret_cast<TVMFFIAny*>(&result));
+
+ if (ret_code == 0) {
+ if constexpr (std::is_same_v<T, Any>) {
+ return std::move(result);
+ } else {
+ // Check if result is Error (from Expected-returning function that
returned Err)
+ if (result.template as<Error>().has_value()) {
Review Comment:
T should take higher priority than Error as that is the fast path, always
hold the value as optional first and move that value, avoid double has value
checking
##########
include/tvm/ffi/function.h:
##########
@@ -643,6 +643,65 @@ class Function : public ObjectRef {
static_cast<FunctionObj*>(data_.get())->CallPacked(args.data(),
args.size(), result);
}
+ /*!
+ * \brief Call the function and return Expected<T> for exception-free error
handling.
+ * \tparam T The expected return type (default: Any).
+ * \param args The arguments to pass to the function.
+ * \return Expected<T> containing either the result or an error.
+ *
+ * This method provides exception-free calling by catching all exceptions
+ * and returning them as Error values in the Expected type.
+ *
+ * \code
+ * Function func = Function::GetGlobal("risky_function");
+ * Expected<int> result = func.CallExpected<int>(arg1, arg2);
+ * if (result.is_ok()) {
+ * int value = result.value();
+ * } else {
+ * Error err = result.error();
+ * }
+ * \endcode
+ */
+ template <typename T = Any, typename... Args>
+ TVM_FFI_INLINE Expected<T> CallExpected(Args&&... args) const {
+ constexpr size_t kNumArgs = sizeof...(Args);
+ AnyView args_pack[kNumArgs > 0 ? kNumArgs : 1];
+ PackedArgs::Fill(args_pack, std::forward<Args>(args)...);
+
+ Any result;
+ FunctionObj* func_obj = static_cast<FunctionObj*>(data_.get());
+
+ // Use safe_call path to catch exceptions
+ int ret_code = func_obj->safe_call(func_obj, reinterpret_cast<const
TVMFFIAny*>(args_pack),
+ kNumArgs,
reinterpret_cast<TVMFFIAny*>(&result));
+
+ if (ret_code == 0) {
+ if constexpr (std::is_same_v<T, Any>) {
+ return std::move(result);
+ } else {
+ // Check if result is Error (from Expected-returning function that
returned Err)
+ if (result.template as<Error>().has_value()) {
+ return std::move(result).template cast<Error>();
+ }
+ // Try to extract as T
+ if (auto val = result.template try_cast<T>()) {
+ return std::move(*val);
+ }
+ return Error("TypeError",
+ "CallExpected: result type mismatch, expected " +
TypeTraits<T>::TypeStr() +
+ ", but got " + result.GetTypeKey(),
+ "");
+ }
+ } else if (ret_code == -2) {
+ // Environment error already set (e.g., Python KeyboardInterrupt)
+ // We still throw this since it's a signal, not a normal error
+ throw ::tvm::ffi::EnvErrorAlreadySet();
+ } else {
+ // Error occurred - retrieve from safe call context and return Err
+ return details::MoveFromSafeCallRaised();
Review Comment:
Explicitly use Unexpected
##########
include/tvm/ffi/function_details.h:
##########
@@ -219,6 +237,9 @@ 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>) {
+ *rv = f(ArgValueWithContext<std::tuple_element_t<Is, PackedArgs>>{args,
Is, optional_name,
Review Comment:
looks like this is not necessary, please cross check
##########
include/tvm/ffi/function.h:
##########
@@ -643,6 +643,65 @@ class Function : public ObjectRef {
static_cast<FunctionObj*>(data_.get())->CallPacked(args.data(),
args.size(), result);
}
+ /*!
+ * \brief Call the function and return Expected<T> for exception-free error
handling.
+ * \tparam T The expected return type (default: Any).
+ * \param args The arguments to pass to the function.
+ * \return Expected<T> containing either the result or an error.
+ *
+ * This method provides exception-free calling by catching all exceptions
+ * and returning them as Error values in the Expected type.
+ *
+ * \code
+ * Function func = Function::GetGlobal("risky_function");
+ * Expected<int> result = func.CallExpected<int>(arg1, arg2);
+ * if (result.is_ok()) {
+ * int value = result.value();
+ * } else {
+ * Error err = result.error();
+ * }
+ * \endcode
+ */
+ template <typename T = Any, typename... Args>
+ TVM_FFI_INLINE Expected<T> CallExpected(Args&&... args) const {
+ constexpr size_t kNumArgs = sizeof...(Args);
+ AnyView args_pack[kNumArgs > 0 ? kNumArgs : 1];
+ PackedArgs::Fill(args_pack, std::forward<Args>(args)...);
+
+ Any result;
+ FunctionObj* func_obj = static_cast<FunctionObj*>(data_.get());
+
+ // Use safe_call path to catch exceptions
+ int ret_code = func_obj->safe_call(func_obj, reinterpret_cast<const
TVMFFIAny*>(args_pack),
+ kNumArgs,
reinterpret_cast<TVMFFIAny*>(&result));
+
+ if (ret_code == 0) {
+ if constexpr (std::is_same_v<T, Any>) {
+ return std::move(result);
+ } else {
+ // Check if result is Error (from Expected-returning function that
returned Err)
+ if (result.template as<Error>().has_value()) {
+ return std::move(result).template cast<Error>();
+ }
+ // Try to extract as T
+ if (auto val = result.template try_cast<T>()) {
+ return std::move(*val);
+ }
+ return Error("TypeError",
+ "CallExpected: result type mismatch, expected " +
TypeTraits<T>::TypeStr() +
+ ", but got " + result.GetTypeKey(),
+ "");
+ }
+ } else if (ret_code == -2) {
+ // Environment error already set (e.g., Python KeyboardInterrupt)
+ // We still throw this since it's a signal, not a normal error
Review Comment:
i think explicitly throw here is not as good since it breaks the assumption
of no exception. Actually we find it is better to further unify things a bit, i
made https://github.com/apache/tvm-ffi/pull/425 so we don't have to handle -2
##########
include/tvm/ffi/function_details.h:
##########
@@ -67,6 +72,19 @@ static constexpr bool ArgSupported =
std::is_same_v<std::remove_const_t<std::remove_reference_t<T>>, AnyView>
||
TypeTraitsNoCR<T>::convert_enabled));
+template <typename T>
+struct is_expected : std::false_type {
+ using value_type = void;
+};
+
+template <typename T>
+struct is_expected<Expected<T>> : std::true_type {
+ using value_type = T;
+};
+
+template <typename T>
+inline constexpr bool is_expected_v = is_expected<T>::value;
Review Comment:
not sure if this is needed anymore given the new convention
##########
include/tvm/ffi/function.h:
##########
@@ -643,6 +643,65 @@ class Function : public ObjectRef {
static_cast<FunctionObj*>(data_.get())->CallPacked(args.data(),
args.size(), result);
}
+ /*!
+ * \brief Call the function and return Expected<T> for exception-free error
handling.
+ * \tparam T The expected return type (default: Any).
+ * \param args The arguments to pass to the function.
+ * \return Expected<T> containing either the result or an error.
+ *
+ * This method provides exception-free calling by catching all exceptions
+ * and returning them as Error values in the Expected type.
+ *
+ * \code
+ * Function func = Function::GetGlobal("risky_function");
+ * Expected<int> result = func.CallExpected<int>(arg1, arg2);
+ * if (result.is_ok()) {
+ * int value = result.value();
+ * } else {
+ * Error err = result.error();
+ * }
+ * \endcode
+ */
+ template <typename T = Any, typename... Args>
+ TVM_FFI_INLINE Expected<T> CallExpected(Args&&... args) const {
+ constexpr size_t kNumArgs = sizeof...(Args);
+ AnyView args_pack[kNumArgs > 0 ? kNumArgs : 1];
+ PackedArgs::Fill(args_pack, std::forward<Args>(args)...);
+
+ Any result;
+ FunctionObj* func_obj = static_cast<FunctionObj*>(data_.get());
+
+ // Use safe_call path to catch exceptions
+ int ret_code = func_obj->safe_call(func_obj, reinterpret_cast<const
TVMFFIAny*>(args_pack),
+ kNumArgs,
reinterpret_cast<TVMFFIAny*>(&result));
+
+ if (ret_code == 0) {
+ if constexpr (std::is_same_v<T, Any>) {
+ return std::move(result);
+ } else {
+ // Check if result is Error (from Expected-returning function that
returned Err)
+ if (result.template as<Error>().has_value()) {
+ return std::move(result).template cast<Error>();
+ }
+ // Try to extract as T
+ if (auto val = result.template try_cast<T>()) {
+ return std::move(*val);
Review Comment:
move optional bvalue should be `*std::move(val)`, would be good to confirm
with genai
##########
include/tvm/ffi/function.h:
##########
@@ -643,6 +643,65 @@ class Function : public ObjectRef {
static_cast<FunctionObj*>(data_.get())->CallPacked(args.data(),
args.size(), result);
}
+ /*!
+ * \brief Call the function and return Expected<T> for exception-free error
handling.
+ * \tparam T The expected return type (default: Any).
+ * \param args The arguments to pass to the function.
+ * \return Expected<T> containing either the result or an error.
+ *
+ * This method provides exception-free calling by catching all exceptions
+ * and returning them as Error values in the Expected type.
+ *
+ * \code
+ * Function func = Function::GetGlobal("risky_function");
+ * Expected<int> result = func.CallExpected<int>(arg1, arg2);
+ * if (result.is_ok()) {
+ * int value = result.value();
+ * } else {
+ * Error err = result.error();
+ * }
+ * \endcode
+ */
+ template <typename T = Any, typename... Args>
+ TVM_FFI_INLINE Expected<T> CallExpected(Args&&... args) const {
+ constexpr size_t kNumArgs = sizeof...(Args);
+ AnyView args_pack[kNumArgs > 0 ? kNumArgs : 1];
+ PackedArgs::Fill(args_pack, std::forward<Args>(args)...);
+
+ Any result;
+ FunctionObj* func_obj = static_cast<FunctionObj*>(data_.get());
+
+ // Use safe_call path to catch exceptions
+ int ret_code = func_obj->safe_call(func_obj, reinterpret_cast<const
TVMFFIAny*>(args_pack),
+ kNumArgs,
reinterpret_cast<TVMFFIAny*>(&result));
+
+ if (ret_code == 0) {
+ if constexpr (std::is_same_v<T, Any>) {
+ return std::move(result);
+ } else {
+ // Check if result is Error (from Expected-returning function that
returned Err)
+ if (result.template as<Error>().has_value()) {
+ return std::move(result).template cast<Error>();
+ }
+ // Try to extract as T
+ if (auto val = result.template try_cast<T>()) {
+ return std::move(*val);
+ }
+ return Error("TypeError",
Review Comment:
I know it is a bit more verbose, but i think it is helpful to always use
Unexpected(Errror()), and ban implicit conversion from Error
--
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]
