tqchen commented on code in PR #230:
URL: https://github.com/apache/tvm-ffi/pull/230#discussion_r2539628029


##########
python/tvm_ffi/cpp/extension.py:
##########
@@ -366,8 +371,14 @@ def _decorate_with_tvm_ffi(source: str, functions: 
Mapping[str, str]) -> str:
     ]
 
     for func_name, func_doc in functions.items():
-        sources.append(f"TVM_FFI_DLL_EXPORT_TYPED_FUNC({func_name}, 
{func_name});")
-        _ = func_doc  # todo: add support to embed function docstring to the 
tvm ffi functions.
+        if func_doc:
+            # Escape the docstring for C++ string literal
+            escaped_doc = func_doc.replace("\\", "\\\\").replace('"', 
'\\"').replace("\n", "\\n")
+            sources.append(

Review Comment:
   not too sure if we need to export docs here atm, perhaps @yaoyaoding can 
also comment, we can skip first and update it as a followup



##########
python/tvm_ffi/cpp/extension.py:
##########
@@ -366,8 +371,14 @@ def _decorate_with_tvm_ffi(source: str, functions: 
Mapping[str, str]) -> str:
     ]
 
     for func_name, func_doc in functions.items():
-        sources.append(f"TVM_FFI_DLL_EXPORT_TYPED_FUNC({func_name}, 
{func_name});")
-        _ = func_doc  # todo: add support to embed function docstring to the 
tvm ffi functions.
+        if func_doc:
+            # Escape the docstring for C++ string literal
+            escaped_doc = func_doc.replace("\\", "\\\\").replace('"', 
'\\"').replace("\n", "\\n")

Review Comment:
   Leave it out for now since not too sure if we need doc attachment here. Also 
replace is less robust, maybe we should have a function 
_escape_cpp_string_literal, but this is not needed here



##########
python/tvm_ffi/cpp/extension.py:
##########
@@ -40,7 +40,7 @@ def _hash_sources(
     cuda_source: str | None,
     cpp_files: Sequence[str] | None,
     cuda_files: Sequence[str] | None,
-    functions: Sequence[str] | Mapping[str, str],
+    functions: Sequence[str] | Mapping[str, str | None],

Review Comment:
   it is strange to have Optional None here, do we know why?



##########
include/tvm/ffi/function.h:
##########
@@ -881,22 +932,66 @@ inline int32_t TypeKeyToIndex(std::string_view type_key) {
  * });
  * \endcode
  *
- * \note The final symbol name is `__tvm_ffi_<ExportName>`.
+ * \note The final symbol names are:
+ *       - `__tvm_ffi_<ExportName>` (function)
+ *       - `__tvm_ffi__metadata_<ExportName>` (metadata - only when
+ * TVM_FFI_DLL_EXPORT_INCLUDE_METADATA is defined)
  */
-#define TVM_FFI_DLL_EXPORT_TYPED_FUNC(ExportName, Function)                    
        \
-  extern "C" {                                                                 
        \
-  TVM_FFI_DLL_EXPORT int __tvm_ffi_##ExportName(void* self, const TVMFFIAny* 
args,     \
-                                                int32_t num_args, TVMFFIAny* 
result) { \
-    TVM_FFI_SAFE_CALL_BEGIN();                                                 
        \
-    using FuncInfo = ::tvm::ffi::details::FunctionInfo<decltype(Function)>;    
        \
-    static std::string name = #ExportName;                                     
        \
-    ::tvm::ffi::details::unpack_call<typename FuncInfo::RetType>(              
        \
-        std::make_index_sequence<FuncInfo::num_args>{}, &name, Function,       
        \
-        reinterpret_cast<const ::tvm::ffi::AnyView*>(args), num_args,          
        \
-        reinterpret_cast<::tvm::ffi::Any*>(result));                           
        \
-    TVM_FFI_SAFE_CALL_END();                                                   
        \
-  }                                                                            
        \
-  }
+#define TVM_FFI_DLL_EXPORT_TYPED_FUNC(ExportName, Function) \
+  TVM_FFI_DLL_EXPORT_TYPED_FUNC_IMPL_(ExportName, Function) \
+  TVM_FFI_DLL_EXPORT_TYPED_FUNC_METADATA_IMPL_(ExportName, Function)
+
+/*!
+ * \brief Export typed function with documentation as SafeCallType symbols.
+ *
+ * This is an extended version of TVM_FFI_DLL_EXPORT_TYPED_FUNC that also 
exports
+ * a documentation string. The docstring can be used by stub generators and
+ * documentation tools.
+ *
+ * \param ExportName The symbol name to be exported.
+ * \param Function The typed function.
+ * \param DocString The documentation string (C string literal).
+ *
+ * \sa ffi::TypedFunction, TVM_FFI_DLL_EXPORT_TYPED_FUNC
+ *
+ * \code
+ *
+ * int Add(int a, int b) {
+ *   return a + b;
+ * }
+ *
+ * TVM_FFI_DLL_EXPORT_TYPED_FUNC_WITH_DOC(
+ *     add,

Review Comment:
   somehow my previous comment get missing somewhere, I think we can have 
separate macro 
   
   TVM_FFI_DLL_EXPORT_TYPED_FUNC_DOC(ExportName, DocString) independent from 
the normal export. TVM_FFI_DLL_EXPORT_INCLUDE_METADATA can also togle this



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