gemini-code-assist[bot] commented on code in PR #137:
URL: https://github.com/apache/tvm-ffi/pull/137#discussion_r2433867483
##########
include/tvm/ffi/reflection/registry.h:
##########
@@ -576,8 +576,8 @@ class ObjectDef : public ReflectionDefBase {
* \endcode
*/
template <typename... Args, typename... Extra>
- TVM_FFI_INLINE ObjectDef& def(init<Args...> init_func, Extra&&... extra) {
- RegisterMethod(INIT_METHOD_NAME, true, &init<Args...>::template
execute<Class>,
+ TVM_FFI_INLINE ObjectDef& def([[maybe_unused]] init<Args...> init_func,
Extra&&... extra) {
+ RegisterMethod(kInitMethodName, true, &init<Args...>::template
execute<Class>,
std::forward<Extra>(extra)...);
return *this;
Review Comment:
The changes to use `kInitMethodName` and `[[maybe_unused]]` are great
improvements. While reviewing this, I noticed that the documentation for this
function seems to have a small inaccuracy. The comment on line 561 states that
this method registers a `__init__` method, but the actual method name used is
`__ffi_init__` (as defined by `kInitMethodName`). To avoid confusion, it would
be great to update the documentation to reflect the correct method name.
For example:
```diff
- * This method registers a static `__init__` method that constructs an
instance
+ * This method registers a static `__ffi_init__` method that constructs an
instance
```
Since this change is outside the current diff, I'm adding this comment here
for visibility. Given the PR's goal to fix comments related to the
initialization method, this seems like a relevant and easy fix to include.
--
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]
