gemini-code-assist[bot] commented on code in PR #394:
URL: https://github.com/apache/tvm-ffi/pull/394#discussion_r2677987105


##########
docs/concepts/func_module.rst:
##########
@@ -0,0 +1,427 @@
+..  Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+
+..    http://www.apache.org/licenses/LICENSE-2.0
+
+..  Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied.  See the License for the
+    specific language governing permissions and limitations
+    under the License.
+
+Function and Module
+===================
+
+TVM-FFI provides a unified and ABI-stable calling convention that enables
+cross-language function calls between C++, Python, Rust and other languages.
+Functions are first-class :doc:`TVM-FFI objects <object_and_class>` that can 
be:
+
+- **Invoked with type-erased arguments** using a stable C ABI
+- **Registered globally** and accessed by name across languages
+- **Exported as C symbols** for efficient codegen and linking
+- **Wrapped in modules** for dynamic loading and namespace isolation
+
+This tutorial covers everything you need to know about defining, registering, 
and calling
+TVM-FFI functions, as well as working with modules.
+
+Glossary
+--------
+
+TVM-FFI ABI. :cpp:type:`TVMFFISafeCallType`
+  A stable C calling convention where every function is represented by a 
single signature:
+
+  .. code-block:: cpp
+
+    int func(void* handle, const TVMFFIAny* args, int num_args, TVMFFIAny* 
result);
+
+  Arguments are passed in as :cpp:class:`~tvm::ffi::AnyView`
+  and return value is passed in as :cpp:class:`~tvm::ffi::Any`,
+  enabling type-erased, cross-language function calls.
+  This calling convention is used across all TVM-FFI function calls at the ABI 
boundary.
+  See :ref:`Stable C ABI <tvm_ffi_c_abi>` for a quick introduction.
+
+TVM-FFI Function. :py:class:`tvm_ffi.Function`, 
:cpp:class:`tvm::ffi::FunctionObj`, :cpp:class:`tvm::ffi::Function`
+  A reference-counted function object and its managed reference, which wraps 
any callable,
+  including language-agnostic functions and lambdas (C++, Python, Rust, etc.),
+  member functions, external C symbols, and other callable objects,
+  all sharing the same calling convention.
+
+TVM-FFI Module. :py:class:`tvm_ffi.Module`, :cpp:class:`tvm::ffi::ModuleObj`, 
:cpp:class:`tvm::ffi::Module`
+  A namespace for a collection of functions, loaded from a shared library via 
``dlopen`` (Linux, macOS) or ``LoadLibraryW`` (Windows),
+  or statically linked to the current executable.
+
+Global Functions and Registry. :py:func:`tvm_ffi.get_global_func` and 
:py:func:`tvm_ffi.register_global_func`
+  A registry is a table that maps string names to 
:cpp:class:`~tvm::ffi::Function` objects
+  and their metadata (name, docs, signatures, etc.) for cross-language access.
+  Functions in the registry are called **global functions**.
+
+Common Usage
+------------
+
+TVM-FFI C Symbols
+~~~~~~~~~~~~~~~~~
+
+**Shared library**. Use :c:macro:`TVM_FFI_DLL_EXPORT_TYPED_FUNC` to export
+a function as a C symbol that follows the TVM-FFI ABI:
+
+.. code-block:: cpp
+
+   static int AddTwo(int x) { return x + 2; }
+
+   TVM_FFI_DLL_EXPORT_TYPED_FUNC(/*ExportName=*/add_two, /*Function=*/AddTwo)
+
+This creates a C symbol ``__tvm_ffi_${ExportName}`` if the symbol is exported 
in a shared library,
+and later loaded and called via :py:func:`tvm_ffi.load_module`:
+
+.. code-block:: python
+
+   import tvm_ffi
+
+   mod = tvm_ffi.load_module("path/to/library.so")
+   result = mod.add_two(40)  # -> 42
+
+**System library**. If the symbol is bundled in the same executable, 
:cpp:func:`TVMFFIEnvModRegisterSystemLibSymbol`
+is required to register the symbol during static initialization in 
:c:macro:`TVM_FFI_STATIC_INIT_BLOCK`.
+See examples in :py:func:`tvm_ffi.system_lib` for a complete workflow.
+
+
+Global Functions
+~~~~~~~~~~~~~~~~
+
+**Register a global function**. In C++, use 
:cpp:class:`tvm::ffi::reflection::GlobalDef` to
+register a function:
+
+.. code-block:: cpp
+
+   #include <tvm/ffi/tvm_ffi.h>
+
+   static int AddOne(int x) { return x + 1; }
+
+   TVM_FFI_STATIC_INIT_BLOCK() {
+     namespace refl = tvm::ffi::reflection;
+     refl::GlobalDef()
+         .def("my_ext.add_one", AddOne, "Add one to the input");
+   }
+
+:c:macro:`TVM_FFI_STATIC_INIT_BLOCK` macro ensures the registration logic 
happens
+during library initialization. The registered function is then accessible from
+Python under the name ``my_ext.add_one``.
+
+In Python, use the decorator :py:func:`tvm_ffi.register_global_func` to 
register a global function:
+
+.. code-block:: python
+
+   import tvm_ffi
+
+   @tvm_ffi.register_global_func("my_ext.add_one")
+   def add_one(x: int) -> int:
+       return x + 1
+
+**Retrieve a global function**. After registration, functions are accessible 
by name.
+
+In Python, use :py:func:`tvm_ffi.get_global_func` to retrieve a global 
function:
+
+.. code-block:: python
+
+   import tvm_ffi
+
+   # Get a function from the global registry
+   add_one = tvm_ffi.get_global_func("my_ext.add_one")
+   result = add_one(41)  # -> 42
+
+In C++, use :cpp:func:`tvm::ffi::Function::GetGlobal` to retrieve a global 
function:
+
+.. code-block:: cpp
+
+   ffi::Function func = ffi::Function::GetGlobal("my_ext.add_one");
+   int result = func(41)  // -> 42
+
+
+Create Functions
+~~~~~~~~~~~~~~~~
+
+**From C++**. An :cpp:class:`tvm::ffi::Function` can be created via 
:cpp:func:`tvm::ffi::Function::FromTyped`
+or :cpp:class:`tvm::ffi::TypedFunction`'s constructor.
+
+.. code-block:: cpp
+
+   // Create type-erased function: add_type_erased
+   ffi::Function add_type_erased = ffi::Function::FromTyped([](int x, int y) {
+     return x + y;
+   });
+
+   // Create a typed function: add_typed
+   ffi::TypedFunction<int(int, int)> add_typed = [](int x, int y) {
+     return x + y;
+   };
+
+   // Converts typed function to type-erased function
+   ffi::Function generic = add;

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The variable `add` is not defined in this code snippet. It seems you 
intended to use `add_typed`, which was defined in the preceding lines.
   
   ```suggestion
      ffi::Function generic = add_typed;
   ```



##########
docs/concepts/func_module.rst:
##########
@@ -0,0 +1,427 @@
+..  Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+
+..    http://www.apache.org/licenses/LICENSE-2.0
+
+..  Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied.  See the License for the
+    specific language governing permissions and limitations
+    under the License.
+
+Function and Module
+===================
+
+TVM-FFI provides a unified and ABI-stable calling convention that enables
+cross-language function calls between C++, Python, Rust and other languages.
+Functions are first-class :doc:`TVM-FFI objects <object_and_class>` that can 
be:
+
+- **Invoked with type-erased arguments** using a stable C ABI
+- **Registered globally** and accessed by name across languages
+- **Exported as C symbols** for efficient codegen and linking
+- **Wrapped in modules** for dynamic loading and namespace isolation
+
+This tutorial covers everything you need to know about defining, registering, 
and calling
+TVM-FFI functions, as well as working with modules.
+
+Glossary
+--------
+
+TVM-FFI ABI. :cpp:type:`TVMFFISafeCallType`
+  A stable C calling convention where every function is represented by a 
single signature:
+
+  .. code-block:: cpp
+
+    int func(void* handle, const TVMFFIAny* args, int num_args, TVMFFIAny* 
result);
+
+  Arguments are passed in as :cpp:class:`~tvm::ffi::AnyView`
+  and return value is passed in as :cpp:class:`~tvm::ffi::Any`,
+  enabling type-erased, cross-language function calls.
+  This calling convention is used across all TVM-FFI function calls at the ABI 
boundary.
+  See :ref:`Stable C ABI <tvm_ffi_c_abi>` for a quick introduction.
+
+TVM-FFI Function. :py:class:`tvm_ffi.Function`, 
:cpp:class:`tvm::ffi::FunctionObj`, :cpp:class:`tvm::ffi::Function`
+  A reference-counted function object and its managed reference, which wraps 
any callable,
+  including language-agnostic functions and lambdas (C++, Python, Rust, etc.),
+  member functions, external C symbols, and other callable objects,
+  all sharing the same calling convention.
+
+TVM-FFI Module. :py:class:`tvm_ffi.Module`, :cpp:class:`tvm::ffi::ModuleObj`, 
:cpp:class:`tvm::ffi::Module`
+  A namespace for a collection of functions, loaded from a shared library via 
``dlopen`` (Linux, macOS) or ``LoadLibraryW`` (Windows),
+  or statically linked to the current executable.
+
+Global Functions and Registry. :py:func:`tvm_ffi.get_global_func` and 
:py:func:`tvm_ffi.register_global_func`
+  A registry is a table that maps string names to 
:cpp:class:`~tvm::ffi::Function` objects
+  and their metadata (name, docs, signatures, etc.) for cross-language access.
+  Functions in the registry are called **global functions**.
+
+Common Usage
+------------
+
+TVM-FFI C Symbols
+~~~~~~~~~~~~~~~~~
+
+**Shared library**. Use :c:macro:`TVM_FFI_DLL_EXPORT_TYPED_FUNC` to export
+a function as a C symbol that follows the TVM-FFI ABI:
+
+.. code-block:: cpp
+
+   static int AddTwo(int x) { return x + 2; }
+
+   TVM_FFI_DLL_EXPORT_TYPED_FUNC(/*ExportName=*/add_two, /*Function=*/AddTwo)
+
+This creates a C symbol ``__tvm_ffi_${ExportName}`` if the symbol is exported 
in a shared library,

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The placeholder `${ExportName}` might be confusing as it resembles shell 
variable expansion. Using a more conventional placeholder like `<ExportName>` 
would be clearer for readers.
   
   ```suggestion
   This creates a C symbol ``__tvm_ffi_<ExportName>`` if the symbol is exported 
in a shared library,
   ```



##########
docs/concepts/func_module.rst:
##########
@@ -0,0 +1,427 @@
+..  Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+
+..    http://www.apache.org/licenses/LICENSE-2.0
+
+..  Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied.  See the License for the
+    specific language governing permissions and limitations
+    under the License.
+
+Function and Module
+===================
+
+TVM-FFI provides a unified and ABI-stable calling convention that enables
+cross-language function calls between C++, Python, Rust and other languages.
+Functions are first-class :doc:`TVM-FFI objects <object_and_class>` that can 
be:
+
+- **Invoked with type-erased arguments** using a stable C ABI
+- **Registered globally** and accessed by name across languages
+- **Exported as C symbols** for efficient codegen and linking
+- **Wrapped in modules** for dynamic loading and namespace isolation
+
+This tutorial covers everything you need to know about defining, registering, 
and calling
+TVM-FFI functions, as well as working with modules.
+
+Glossary
+--------
+
+TVM-FFI ABI. :cpp:type:`TVMFFISafeCallType`
+  A stable C calling convention where every function is represented by a 
single signature:
+
+  .. code-block:: cpp
+
+    int func(void* handle, const TVMFFIAny* args, int num_args, TVMFFIAny* 
result);
+
+  Arguments are passed in as :cpp:class:`~tvm::ffi::AnyView`
+  and return value is passed in as :cpp:class:`~tvm::ffi::Any`,
+  enabling type-erased, cross-language function calls.
+  This calling convention is used across all TVM-FFI function calls at the ABI 
boundary.
+  See :ref:`Stable C ABI <tvm_ffi_c_abi>` for a quick introduction.
+
+TVM-FFI Function. :py:class:`tvm_ffi.Function`, 
:cpp:class:`tvm::ffi::FunctionObj`, :cpp:class:`tvm::ffi::Function`
+  A reference-counted function object and its managed reference, which wraps 
any callable,
+  including language-agnostic functions and lambdas (C++, Python, Rust, etc.),
+  member functions, external C symbols, and other callable objects,
+  all sharing the same calling convention.
+
+TVM-FFI Module. :py:class:`tvm_ffi.Module`, :cpp:class:`tvm::ffi::ModuleObj`, 
:cpp:class:`tvm::ffi::Module`
+  A namespace for a collection of functions, loaded from a shared library via 
``dlopen`` (Linux, macOS) or ``LoadLibraryW`` (Windows),
+  or statically linked to the current executable.
+
+Global Functions and Registry. :py:func:`tvm_ffi.get_global_func` and 
:py:func:`tvm_ffi.register_global_func`
+  A registry is a table that maps string names to 
:cpp:class:`~tvm::ffi::Function` objects
+  and their metadata (name, docs, signatures, etc.) for cross-language access.
+  Functions in the registry are called **global functions**.
+
+Common Usage
+------------
+
+TVM-FFI C Symbols
+~~~~~~~~~~~~~~~~~
+
+**Shared library**. Use :c:macro:`TVM_FFI_DLL_EXPORT_TYPED_FUNC` to export
+a function as a C symbol that follows the TVM-FFI ABI:
+
+.. code-block:: cpp
+
+   static int AddTwo(int x) { return x + 2; }
+
+   TVM_FFI_DLL_EXPORT_TYPED_FUNC(/*ExportName=*/add_two, /*Function=*/AddTwo)
+
+This creates a C symbol ``__tvm_ffi_${ExportName}`` if the symbol is exported 
in a shared library,
+and later loaded and called via :py:func:`tvm_ffi.load_module`:
+
+.. code-block:: python
+
+   import tvm_ffi
+
+   mod = tvm_ffi.load_module("path/to/library.so")
+   result = mod.add_two(40)  # -> 42
+
+**System library**. If the symbol is bundled in the same executable, 
:cpp:func:`TVMFFIEnvModRegisterSystemLibSymbol`
+is required to register the symbol during static initialization in 
:c:macro:`TVM_FFI_STATIC_INIT_BLOCK`.
+See examples in :py:func:`tvm_ffi.system_lib` for a complete workflow.
+
+
+Global Functions
+~~~~~~~~~~~~~~~~
+
+**Register a global function**. In C++, use 
:cpp:class:`tvm::ffi::reflection::GlobalDef` to
+register a function:
+
+.. code-block:: cpp
+
+   #include <tvm/ffi/tvm_ffi.h>
+
+   static int AddOne(int x) { return x + 1; }
+
+   TVM_FFI_STATIC_INIT_BLOCK() {
+     namespace refl = tvm::ffi::reflection;
+     refl::GlobalDef()
+         .def("my_ext.add_one", AddOne, "Add one to the input");
+   }
+
+:c:macro:`TVM_FFI_STATIC_INIT_BLOCK` macro ensures the registration logic 
happens
+during library initialization. The registered function is then accessible from
+Python under the name ``my_ext.add_one``.
+
+In Python, use the decorator :py:func:`tvm_ffi.register_global_func` to 
register a global function:
+
+.. code-block:: python
+
+   import tvm_ffi
+
+   @tvm_ffi.register_global_func("my_ext.add_one")
+   def add_one(x: int) -> int:
+       return x + 1
+
+**Retrieve a global function**. After registration, functions are accessible 
by name.
+
+In Python, use :py:func:`tvm_ffi.get_global_func` to retrieve a global 
function:
+
+.. code-block:: python
+
+   import tvm_ffi
+
+   # Get a function from the global registry
+   add_one = tvm_ffi.get_global_func("my_ext.add_one")
+   result = add_one(41)  # -> 42
+
+In C++, use :cpp:func:`tvm::ffi::Function::GetGlobal` to retrieve a global 
function:
+
+.. code-block:: cpp
+
+   ffi::Function func = ffi::Function::GetGlobal("my_ext.add_one");
+   int result = func(41)  // -> 42

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   `tvm::ffi::Function::GetGlobal` returns a 
`std::optional<tvm::ffi::Function>`, not a `tvm::ffi::Function` directly. The 
code example as written would not compile. To fix this, you should either 
handle the optional (e.g., by dereferencing it) or use `GetGlobalRequired` for 
a more concise example that throws on failure.
   
   ```suggestion
      ffi::Function func = ffi::Function::GetGlobalRequired("my_ext.add_one");
      int result = func(41);  // -> 42
   ```



##########
docs/concepts/func_module.rst:
##########
@@ -0,0 +1,427 @@
+..  Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+
+..    http://www.apache.org/licenses/LICENSE-2.0
+
+..  Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied.  See the License for the
+    specific language governing permissions and limitations
+    under the License.
+
+Function and Module
+===================
+
+TVM-FFI provides a unified and ABI-stable calling convention that enables
+cross-language function calls between C++, Python, Rust and other languages.
+Functions are first-class :doc:`TVM-FFI objects <object_and_class>` that can 
be:
+
+- **Invoked with type-erased arguments** using a stable C ABI
+- **Registered globally** and accessed by name across languages
+- **Exported as C symbols** for efficient codegen and linking
+- **Wrapped in modules** for dynamic loading and namespace isolation
+
+This tutorial covers everything you need to know about defining, registering, 
and calling
+TVM-FFI functions, as well as working with modules.
+
+Glossary
+--------
+
+TVM-FFI ABI. :cpp:type:`TVMFFISafeCallType`
+  A stable C calling convention where every function is represented by a 
single signature:
+
+  .. code-block:: cpp
+
+    int func(void* handle, const TVMFFIAny* args, int num_args, TVMFFIAny* 
result);

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The type of `num_args` is specified as `int`, but the `TVMFFISafeCallType` 
typedef uses `int32_t` for this parameter. To maintain consistency with the 
actual ABI and other examples in this document (e.g., line 263), it should be 
`int32_t`.
   
   ```suggestion
       int func(void* handle, const TVMFFIAny* args, int32_t num_args, 
TVMFFIAny* result);
   ```



##########
python/tvm_ffi/cython/function.pxi:
##########
@@ -889,6 +889,26 @@ cdef class Function(Object):
     def __cinit__(self) -> None:
         self.c_release_gil = _RELEASE_GIL_BY_DEFAULT
 
+    def __init__(self, func: Callable[..., Any]) -> None:
+        """Initialize a Function.
+
+        Parameters
+        ----------
+        func : Callable[..., Any]
+            Optional Python callable to wrap. If provided, creates an FFI
+            function that wraps this callable. If None, the Function is
+            expected to be initialized via static factory methods.

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The docstring for the `func` parameter is a bit confusing. It's described as 
"Optional", but the type hint `Callable[..., Any]` and the implementation `if 
not callable(func): ...` indicate it's a required argument. The part about `If 
None, ...` is also misleading, as passing `None` would raise a `TypeError`.
   
   To improve clarity, I suggest rephrasing the docstring to reflect that 
`func` is a required callable.
   
   ```
               The Python callable to wrap.
   ```



-- 
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]

Reply via email to