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


##########
include/tvm/ffi/any.h:
##########
@@ -824,6 +870,69 @@ struct AnyEqual {
   }
 };
 
+// Defer this definition until any.h so the throwing path can depend on
+// TVM_FFI_THROW(TypeError), while object.h stays below the error layer.
+template <typename ObjectRefType, typename>
+TVM_FFI_INLINE ObjectRefType ObjectRef::as_or_throw() const& {
+  if (data_ != nullptr) {
+    // Piggy back to Any TypeTraits for rich ObjectRef check, temp any_data 
will optimize away.
+    TVMFFIAny any_data;
+    any_data.type_index = data_->type_index();
+    TVM_FFI_UNSAFE_ASSUME(any_data.type_index >= 
TypeIndex::kTVMFFIStaticObjectBegin);
+    any_data.zero_padding = 0;
+    TVM_FFI_CLEAR_PTR_PADDING_IN_FFI_ANY(&any_data);
+    any_data.v_obj = 
reinterpret_cast<TVMFFIObject*>(const_cast<Object*>(data_.get()));
+    if (TypeTraits<ObjectRefType>::CheckAnyStrict(&any_data)) {
+      ObjectRefType result(UnsafeInit{});
+      result.data_ = data_;
+      return result;
+    } else {
+      TVM_FFI_THROW(TypeError) << "Cannot treat type `"
+                               << 
TypeTraits<ObjectRefType>::GetMismatchTypeInfo(&any_data)
+                               << "` as type `" << 
TypeTraits<ObjectRefType>::TypeStr() << "`";
+    }
+  } else {
+    if constexpr (ObjectRefType::_type_is_nullable) {
+      ObjectRefType result(UnsafeInit{});
+      return result;
+    } else {
+      TVM_FFI_THROW(TypeError) << "Cannot treat type `" << 
StaticTypeKey::kTVMFFINone
+                               << "` as type `" << 
TypeTraits<ObjectRefType>::TypeStr() << "`";
+    }
+  }
+}
+
+template <typename ObjectRefType, typename>
+TVM_FFI_INLINE ObjectRefType ObjectRef::as_or_throw() && {
+  if (data_ != nullptr) {
+    // Piggy back to Any TypeTraits for rich ObjectRef check, temp any_data 
will optimize away.
+    TVMFFIAny any_data;
+    any_data.type_index = data_->type_index();
+    TVM_FFI_UNSAFE_ASSUME(any_data.type_index >= 
TypeIndex::kTVMFFIStaticObjectBegin);
+    any_data.zero_padding = 0;
+    TVM_FFI_CLEAR_PTR_PADDING_IN_FFI_ANY(&any_data);
+    any_data.v_obj = 
reinterpret_cast<TVMFFIObject*>(const_cast<Object*>(data_.get()));
+    if (TypeTraits<ObjectRefType>::CheckAnyStrict(&any_data)) {
+      ObjectRefType result(UnsafeInit{});
+      result.data_ = std::move(data_);
+      data_ = nullptr;
+      return result;
+    } else {
+      TVM_FFI_THROW(TypeError) << "Cannot treat type `"
+                               << 
TypeTraits<ObjectRefType>::GetMismatchTypeInfo(&any_data)
+                               << "` as type `" << 
TypeTraits<ObjectRefType>::TypeStr() << "`";
+    }
+  } else {
+    if constexpr (ObjectRefType::_type_is_nullable) {
+      ObjectRefType result(UnsafeInit{});
+      return result;
+    } else {
+      TVM_FFI_THROW(TypeError) << "Cannot treat type `" << 
StaticTypeKey::kTVMFFINone
+                               << "` as type `" << 
TypeTraits<ObjectRefType>::TypeStr() << "`";
+    }
+  }
+}

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The implementations of `ObjectRef::as_or_throw() const&` and 
`ObjectRef::as_or_throw() &&` duplicate a significant amount of low-level logic 
(such as setting up `any_data`, checking strict types, and handling null 
pointers) that is already fully implemented in `ObjectRef::as() const&` and 
`ObjectRef::as() &&`.
   
   We can simplify these definitions by delegating directly to `as()`, matching 
the elegant pattern already used for `Any::as_or_throw`. This significantly 
improves maintainability and readability while preserving all performance 
optimizations (like avoiding atomic reference count changes on rvalue moves).
   
   ```c
   template <typename ObjectRefType, typename>
   TVM_FFI_INLINE ObjectRefType ObjectRef::as_or_throw() const& {
     std::optional<ObjectRefType> result = this->as<ObjectRefType>();
     if (TVM_FFI_PREDICT_FALSE(!result.has_value())) {
       TVM_FFI_THROW(TypeError) << "Cannot treat type '" << GetTypeKey()
                                << "' as type '" << 
TypeTraits<ObjectRefType>::TypeStr() << "'";
     }
     return *std::move(result);
   }
   
   template <typename ObjectRefType, typename>
   TVM_FFI_INLINE ObjectRefType ObjectRef::as_or_throw() && {
     std::optional<ObjectRefType> result = std::move(*this).as<ObjectRefType>();
     if (TVM_FFI_PREDICT_FALSE(!result.has_value())) {
       TVM_FFI_THROW(TypeError) << "Cannot treat type '" << GetTypeKey()
                                << "' as type '" << 
TypeTraits<ObjectRefType>::TypeStr() << "'";
     }
     return *std::move(result);
   }
   ```



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