gemini-code-assist[bot] commented on code in PR #676:
URL: https://github.com/apache/tvm-ffi/pull/676#discussion_r3613760032
##########
python/tvm_ffi/dataclasses/gen_abi_cpp.py:
##########
@@ -319,19 +319,35 @@ def __init__(self, emitted_infos: list[TypeInfo]) -> None:
def _lower_object(self, schema: TypeSchema) -> _Carrier:
type_index = schema.origin_type_index
if type_index == _OBJECT_TYPE_INDEX or schema.origin == "Object":
- return _Carrier("::tvm::ffi::ObjectPtr<::tvm::ffi::Object>", 8, 8)
+ return _Carrier("::tvm::ffi::Arc<::tvm::ffi::Object>", 8, 8)
builtin = self.builtins.get(schema.origin_type_index)
if builtin is not None:
if builtin.value_type is None:
raise ValueError(
f"Static TVM-FFI type {schema.origin!r} has no supported
C++ value wrapper"
)
+ if builtin.value_type.startswith("::tvm::ffi::ObjectPtr<"):
+ return _Carrier(f"::tvm::ffi::Arc<{builtin.object_type}>", 8,
8)
return _Carrier(builtin.value_type, builtin.size,
builtin.alignment)
if schema.origin_type_index < _DYNAMIC_OBJECT_TYPE_INDEX_BEGIN:
raise ValueError(f"Schema {schema!r} is not a registered object
type")
info = _lookup_or_register_type_info_from_type_key(schema.origin)
self.dependencies[info.type_index] = info
object_type = _cpp_name(info.type_key).qualified
+ return _Carrier(f"::tvm::ffi::Arc<{object_type}>", 8, 8)
+
+ def _lower_nullable_object(self, schema: TypeSchema) -> _Carrier:
+ type_index = schema.origin_type_index
+ if type_index == _OBJECT_TYPE_INDEX or schema.origin == "Object":
+ object_type = "::tvm::ffi::Object"
+ elif (builtin := self.builtins.get(type_index)) is not None:
+ object_type = builtin.object_type
+ else:
+ if type_index < _DYNAMIC_OBJECT_TYPE_INDEX_BEGIN:
+ raise ValueError(f"Schema {schema!r} is not a registered
object type")
+ info = _lookup_or_register_type_info_from_type_key(schema.origin)
+ self.dependencies[info.type_index] = info
+ object_type = _cpp_name(info.type_key).qualified
return _Carrier(f"::tvm::ffi::ObjectPtr<{object_type}>", 8, 8)
Review Comment:
There's some duplicated logic between `_lower_object` and
`_lower_nullable_object` for resolving the C++ type of dynamic objects. This
can be factored out into a helper method to improve maintainability and reduce
redundancy.
For example, you could introduce a helper like
`_get_dynamic_object_cpp_type`:
```python
def _get_dynamic_object_cpp_type(self, schema: TypeSchema) -> str:
if schema.origin_type_index < _DYNAMIC_OBJECT_TYPE_INDEX_BEGIN:
raise ValueError(f"Schema {schema!r} is not a registered object
type")
info = _lookup_or_register_type_info_from_type_key(schema.origin)
self.dependencies[info.type_index] = info
return _cpp_name(info.type_key).qualified
```
Then, you can simplify `_lower_object` and `_lower_nullable_object` to use
this helper for the dynamic object case.
--
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]
