gemini-code-assist[bot] commented on code in PR #147:
URL: https://github.com/apache/tvm-ffi/pull/147#discussion_r2436890244
##########
python/tvm_ffi/cython/function.pxi:
##########
@@ -423,6 +423,19 @@ cdef int TVMFFIPyArgSetterCtypesVoidPtr_(
return 0
+cdef int TVMFFIPyArgSetterFFIOpaquePtrCompatible_(
+ TVMFFIPyArgSetter* handle, TVMFFIPyCallContext* ctx,
+ PyObject* py_arg, TVMFFIAny* out
+) except -1:
+ """Setter for objects that implement the `__tvm_ffi_opaque_ptr__`
protocol."""
+ cdef object arg = <object>py_arg
+ cdef TVMFFIObjectHandle temp_chandle
+ cdef long long long_ptr = <long long>arg.__tvm_ffi_opaque_ptr__()
Review Comment:
The variable `temp_chandle` is declared but never used. It can be removed to
improve code clarity.
```
cdef long long long_ptr = <long long>arg.__tvm_ffi_opaque_ptr__()
```
##########
tests/python/test_function.py:
##########
@@ -297,3 +297,18 @@ def __init__(self, metadata: Any) -> None:
assert isinstance(fechoed, tvm_ffi.Function)
assert fechoed.__chandle__() == f_sub.__chandle__()
assert fechoed(10) == 10
+
+
+def test_function_with_opaque_ptr_protocol() -> None:
+ class MyObject:
+ def __init__(self, value: Any) -> None:
+ self.value = value
+
+ def __tvm_ffi_opaque_ptr__(self) -> Any:
+ return self.value
+
+ fecho = tvm_ffi.get_global_func("testing.echo")
+ x = MyObject(10)
+ y = fecho(x)
+ assert isinstance(y, ctypes.c_void_p)
+ assert y.value == 10
Review Comment:
The current test case uses an integer `10` as a mock pointer value. While
this works for the `echo` function, it can be a bit misleading as it doesn't
demonstrate passing an actual memory address.
To make the test more representative of the feature's intended use case
(passing pointers to C structs), I suggest modifying it to use a real `ctypes`
object and its address. This would make the test clearer and provide a better
example of how to use this new protocol.
```python
def test_function_with_opaque_ptr_protocol() -> None:
class MyObject:
def __init__(self) -> None:
self._c_data = ctypes.c_int(123)
def __tvm_ffi_opaque_ptr__(self) -> int:
return ctypes.addressof(self._c_data)
@property
def value(self) -> int:
return self._c_data.value
fecho = tvm_ffi.get_global_func("testing.echo")
x = MyObject()
y = fecho(x)
assert isinstance(y, ctypes.c_void_p)
assert y.value == x.__tvm_ffi_opaque_ptr__()
# To show the pointer is valid, we can cast it back and check.
ret_ptr = ctypes.cast(y, ctypes.POINTER(ctypes.c_int))
assert ret_ptr.contents.value == x.value
```
--
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]
