gemini-code-assist[bot] commented on code in PR #438:
URL: https://github.com/apache/tvm-ffi/pull/438#discussion_r2780989838
##########
include/tvm/ffi/reflection/registry.h:
##########
@@ -587,6 +630,42 @@ class ObjectDef : public ReflectionDefBase {
return *this;
}
+ /*!
+ * \brief Enable copy support for this object type.
+ *
+ * Registers `__ffi_shallow_copy__` as an instance method that performs a
shallow
+ * copy via the C++ copy constructor: `make_object<Class>(*self)`.
+ * Also registers it as a type attribute so that the generic deep copy
function
+ * can look it up by type index.
+ *
+ * \param ec An instance of `enable_copy`.
+ * \param extra Optional additional metadata such as docstring.
+ *
+ * \return Reference to this `ObjectDef` for method chaining.
+ *
+ * Example:
+ *
+ * \code{.cpp}
+ * refl::ObjectDef<MyObject>()
+ * .def(refl::enable_copy())
+ * .def(refl::init<int64_t, std::string>());
+ * \endcode
+ */
+ template <typename... Extra>
+ TVM_FFI_INLINE ObjectDef& def([[maybe_unused]] enable_copy ec, Extra&&...
extra) {
+ // Register __ffi_shallow_copy__ as an instance method
+ RegisterMethod(kShallowCopyMethodName, false, &enable_copy::template
shallow_copy<Class>,
+ std::forward<Extra>(extra)...);
+ // Also register as a type attribute for generic deep copy lookup
+ Function copy_fn = GetMethod(std::string(type_key_) + "." +
kShallowCopyMethodName,
+ &enable_copy::template shallow_copy<Class>);
+ TVMFFIByteArray attr_name = {kShallowCopyMethodName,
+
std::char_traits<char>::length(kShallowCopyMethodName)};
+ TVMFFIAny attr_value = AnyView(copy_fn).CopyToTVMFFIAny();
+ TVM_FFI_CHECK_SAFE_CALL(TVMFFITypeRegisterAttr(type_index_, &attr_name,
&attr_value));
+ return *this;
+ }
Review Comment:
The current implementation creates the `Function` object for the shallow
copy method twice: once within `RegisterMethod` on line 657 and again
explicitly via `GetMethod` on line 660. This is slightly inefficient as it
involves redundant work during type registration.
You can avoid this by retrieving the method that was just registered using
`TVMFFITypeGetMethod` and then using that to register the type attribute. This
makes the code more efficient and avoids the redundant object creation.
```c
TVM_FFI_INLINE ObjectDef& def([[maybe_unused]] enable_copy ec, Extra&&...
extra) {
// Register __ffi_shallow_copy__ as an instance method
RegisterMethod(kShallowCopyMethodName, false, &enable_copy::template
shallow_copy<Class>,
std::forward<Extra>(extra)...);
// Also register as a type attribute for generic deep copy lookup
TVMFFIMethodInfo method_info;
TVMFFIByteArray method_name = {kShallowCopyMethodName,
std::char_traits<char>::length(kShallowCopyMethodName)};
TVM_FFI_CHECK_SAFE_CALL(TVMFFITypeGetMethod(type_index_, &method_name,
&method_info));
TVM_FFI_CHECK_SAFE_CALL(TVMFFITypeRegisterAttr(type_index_,
&method_name, &method_info.method));
return *this;
}
```
--
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]
