gemini-code-assist[bot] commented on code in PR #486:
URL: https://github.com/apache/tvm-ffi/pull/486#discussion_r2866825027
##########
python/tvm_ffi/registry.py:
##########
@@ -391,8 +457,46 @@ def _setup_copy_methods(
setattr(type_cls, "__replace__", _replace_unsupported)
-def __init__invalid(self: Any, *args: Any, **kwargs: Any) -> None:
- raise RuntimeError("The __init__ method of this class is not implemented.")
+def _install_init(cls: type, *, enabled: bool) -> None:
+ """Install ``__init__`` from C++ reflection metadata, or a guard."""
+ if "__init__" in cls.__dict__:
+ return
+ type_info: TypeInfo | None = getattr(cls, "__tvm_ffi_type_info__", None)
+ if type_info is None:
+ return
+ if enabled:
+ has_c_init = False
+ is_auto_init = False
+ for method in type_info.methods:
+ if method.name == "__ffi_init__":
+ has_c_init = True
+ is_auto_init = bool(method.metadata.get("auto_init", False))
+ break
Review Comment:
The loop to find the `__ffi_init__` method and its metadata can be written
more concisely and Pythonically using a generator expression with `next()`.
This improves readability and maintainability.
```python
ffi_init_method = next((m for m in type_info.methods if m.name ==
"__ffi_init__"), None)
has_c_init = ffi_init_method is not None
is_auto_init = has_c_init and
bool(ffi_init_method.metadata.get("auto_init", False))
```
##########
python/tvm_ffi/registry.py:
##########
@@ -329,32 +330,97 @@ def init_ffi_api(namespace: str, target_module_name: str
| None = None) -> None:
setattr(target_module, fname, f)
+__SENTINEL = object()
+
+
+def _make_init(type_cls: type, type_info: TypeInfo) -> Callable[..., None]:
+ """Build a Python ``__init__`` that delegates to the C++ auto-generated
``__ffi_init__``."""
+ sig = _make_init_signature(type_info)
+ kwargs_obj = core.KWARGS
+
+ def __init__(self: Any, *args: Any, **kwargs: Any) -> None:
+ ffi_args: list[Any] = list(args)
+ ffi_args.append(kwargs_obj)
+ for key, val in kwargs.items():
+ ffi_args.append(key)
+ ffi_args.append(val)
+ self.__ffi_init__(*ffi_args)
+
+ __init__.__signature__ = sig # ty: ignore[unresolved-attribute]
+ __init__.__qualname__ = f"{type_cls.__qualname__}.__init__"
+ __init__.__module__ = type_cls.__module__
+ return __init__
+
+
+def _make_init_signature(type_info: TypeInfo) -> inspect.Signature:
+ """Build an ``inspect.Signature`` from reflection field metadata."""
+ positional: list[tuple[str, bool]] = [] # (name, has_default)
+ kw_only: list[tuple[str, bool]] = [] # (name, has_default)
+
+ # Walk the parent chain to collect all fields (parent-first order).
+ all_fields: list[Any] = []
+ ti: TypeInfo | None = type_info
+ chain: list[TypeInfo] = []
+ while ti is not None:
+ chain.append(ti)
+ ti = ti.parent_type_info
+ for ancestor_info in reversed(chain):
+ all_fields.extend(ancestor_info.fields)
+
+ for field in all_fields:
+ if not field.c_init:
+ continue
+ if field.c_kw_only:
+ kw_only.append((field.name, field.c_has_default))
+ else:
+ positional.append((field.name, field.c_has_default))
+
+ # Required params must come before optional ones within each group.
+ pos_required = [(n, d) for n, d in positional if not d]
+ pos_default = [(n, d) for n, d in positional if d]
+ kw_required = [(n, d) for n, d in kw_only if not d]
+ kw_default = [(n, d) for n, d in kw_only if d]
+
+ params: list[inspect.Parameter] = []
+ params.append(inspect.Parameter("self",
inspect.Parameter.POSITIONAL_OR_KEYWORD))
+
+ for name, _has_default in pos_required:
+ params.append(inspect.Parameter(name,
inspect.Parameter.POSITIONAL_OR_KEYWORD))
+
+ for name, _has_default in pos_default:
+ params.append(
+ inspect.Parameter(name, inspect.Parameter.POSITIONAL_OR_KEYWORD,
default=__SENTINEL)
+ )
+
+ for name, _has_default in kw_required:
+ params.append(inspect.Parameter(name, inspect.Parameter.KEYWORD_ONLY))
+
+ for name, _has_default in kw_default:
+ params.append(inspect.Parameter(name, inspect.Parameter.KEYWORD_ONLY,
default=__SENTINEL))
+
+ return inspect.Signature(params)
+
+
def _add_class_attrs(type_cls: type, type_info: TypeInfo) -> type:
for field in type_info.fields:
name = field.name
if not hasattr(type_cls, name): # skip already defined attributes
setattr(type_cls, name, field.as_property(type_cls))
- has_c_init = False
has_shallow_copy = False
for method in type_info.methods:
name = method.name
if name == "__ffi_init__":
name = "__c_ffi_init__"
- has_c_init = True
if name == "__ffi_shallow_copy__":
has_shallow_copy = True
# Always override: shallow copy is type-specific and must not be
inherited
setattr(type_cls, name, method.as_callable(type_cls))
elif name == "__c_ffi_init__":
- # Always override: each type has its own constructor signature
+ # Always override: init is type-specific and must not be inherited
setattr(type_cls, name, method.as_callable(type_cls))
elif not hasattr(type_cls, name):
setattr(type_cls, name, method.as_callable(type_cls))
Review Comment:
The logic for handling `__ffi_init__` in this loop is a bit convoluted due
to reassigning the `name` variable and relying on the `if/elif` chain.
Refactoring this to handle `__ffi_init__` in its own `if` block would make the
code clearer and easier to understand.
```suggestion
for method in type_info.methods:
name = method.name
if name == "__ffi_init__":
# Always override: init is type-specific and must not be
inherited
setattr(type_cls, "__c_ffi_init__", method.as_callable(type_cls))
elif name == "__ffi_shallow_copy__":
has_shallow_copy = True
# Always override: shallow copy is type-specific and must not be
inherited
setattr(type_cls, name, method.as_callable(type_cls))
elif not hasattr(type_cls, name):
setattr(type_cls, name, method.as_callable(type_cls))
```
--
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]
