gemini-code-assist[bot] commented on code in PR #399:
URL: https://github.com/apache/tvm-ffi/pull/399#discussion_r2755330429


##########
include/tvm/ffi/function.h:
##########
@@ -643,6 +643,64 @@ 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 = std::move(result).template as<T>()) {
+          return std::move(*val);
+        }
+        return Error("TypeError",
+                     "CallExpected: result type mismatch, expected " + 
TypeTraits<T>::TypeStr(),
+                     "");

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The current implementation uses `as<T>()` to extract the result, which 
performs a strict type check without conversions. This is inconsistent with the 
typical behavior of `any.cast<T>()`, which allows for conversions (e.g., from 
an `int` result to a `float` expectation). Using `try_cast<T>()` would align 
`CallExpected` with the more flexible casting semantics of the FFI, improving 
user experience.
   
   Additionally, the current `TypeError` message doesn't include the actual 
type received, which can make debugging difficult. Including the received type 
key in the error message would be very helpful.
   
   I suggest the following change to use `try_cast` and improve the error 
message.
   
   ```c
           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(),
                        "");
   ```



-- 
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]

Reply via email to