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


##########
cmake/Utils/Library.cmake:
##########
@@ -137,3 +136,223 @@ function (tvm_ffi_add_target_from_obj target_name 
obj_target_name)
   tvm_ffi_add_apple_dsymutil(${target_name}_shared)
   tvm_ffi_add_apple_dsymutil(${target_name}_testing)
 endfunction ()
+
+# cmake-lint: disable=C0301,R0912,R0915
+# ~~~
+# tvm_ffi_configure_target(
+#   target_name
+#   [LINK_SHARED ON|OFF] [LINK_HEADER ON|OFF] [DEBUG_SYMBOL ON|OFF] 
[MSVC_FLAGS ON|OFF]
+#   [STUB_DIR <dir>] [STUB_PKG <pkg>] [STUB_PREFIX <prefix>]
+# )
+# Configure a target to integrate with TVM-FFI CMake utilities:
+#   - Optionally link against tvm_ffi_header and/or tvm_ffi_shared
+#   - Always apply tvm_ffi_add_prefix_map(target_name <current source dir>)
+#   - Optionally enable Apple dSYM generation via 
tvm_ffi_add_apple_dsymutil(target_name)
+#   - Optionally apply MSVC-specific flags via 
tvm_ffi_add_msvc_flags(target_name)
+#
+# Parameters:
+#   target_name: Existing CMake target to modify (positional, required)
+#
+# Keyword parameters:
+#   LINK_SHARED:  Whether to link tvm_ffi_shared into the target (default: ON; 
ON/OFF-style)
+#   LINK_HEADER:  Whether to link tvm_ffi_header into the target (default: ON; 
ON/OFF-style)
+#   DEBUG_SYMBOL: Whether to enable debug symbol post-processing hooks.
+#                 On Apple this calls tvm_ffi_add_apple_dsymutil(target_name) 
(default: ON; ON/OFF-style)
+#                 On non-Apple platforms this is currently a no-op unless you 
extend it. (default: ON)
+#   MSVC_FLAGS:   Whether to call tvm_ffi_add_msvc_flags(target_name) to apply 
MSVC-specific flags (default: ON; ON/OFF-style)
+#   STUB_DIR:     Optional directory to generate Python stubs. If unset, no 
stub generation runs. (string; relative paths resolved against 
CMAKE_CURRENT_SOURCE_DIR)
+#   STUB_PKG:     Package name passed to stub generator (default: 
${SKBUILD_PROJECT_NAME} if set, otherwise target name)
+#   STUB_PREFIX:  Module prefix passed to stub generator (default: 
"<STUB_PKG>.")
+#
+# Notes:
+#   - Installation is intentionally split out. Use tvm_ffi_install(target_name 
DESTINATION <dir>) to install artifacts.
+#   - This function requires tvm_ffi_add_prefix_map() to be defined/included.
+#   - If LINK_SHARED/LINK_HEADER are ON, the corresponding targets 
(tvm_ffi_shared/tvm_ffi_header) must exist.
+# ~~~
+function (tvm_ffi_configure_target target)
+  if (NOT target)
+    message(
+      FATAL_ERROR
+        "tvm_ffi_configure_target: missing target name. "
+        "Usage: tvm_ffi_configure_target(<target> [LINK_SHARED ON|OFF] 
[LINK_HEADER ON|OFF] [DEBUG_SYMBOL ON|OFF] [MSVC_FLAGS ON|OFF] [STUB_DIR <dir>] 
[STUB_PKG <pkg>] [STUB_PREFIX <prefix>])"
+    )
+  endif ()
+
+  if (NOT TARGET "${target}")
+    message(FATAL_ERROR "tvm_ffi_configure_target: '${target}' is not an 
existing CMake target.")
+  endif ()
+
+  # Parse keyword args after the positional target name.
+  set(tvm_ffi_arg_options) # none; require explicit ON/OFF style values
+  set(tvm_ffi_arg_oneValueArgs
+      LINK_SHARED
+      LINK_HEADER
+      DEBUG_SYMBOL
+      MSVC_FLAGS
+      STUB_DIR
+      STUB_PKG
+      STUB_PREFIX
+  )
+  set(tvm_ffi_arg_multiValueArgs)
+
+  cmake_parse_arguments(
+    tvm_ffi_arg_ "${tvm_ffi_arg_options}" "${tvm_ffi_arg_oneValueArgs}"
+    "${tvm_ffi_arg_multiValueArgs}" ${ARGN}
+  )
+
+  # Defaults
+  foreach (arg IN ITEMS LINK_SHARED LINK_HEADER DEBUG_SYMBOL MSVC_FLAGS)
+    if (NOT DEFINED tvm_ffi_arg__${arg})
+      set(tvm_ffi_arg__${arg} ON)
+    endif ()
+  endforeach ()
+
+  if (NOT DEFINED tvm_ffi_arg__STUB_PKG)
+    if (DEFINED SKBUILD_PROJECT_NAME)

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The use of `if (DEFINED SKBUILD_PROJECT_NAME)` could be problematic if 
`SKBUILD_PROJECT_NAME` is defined as an empty string. In that scenario, 
`tvm_ffi_arg__STUB_PKG` would also become an empty string, which might lead to 
unexpected behavior in the stub generation script.
   
   To make this check more robust, I suggest using `if (SKBUILD_PROJECT_NAME)`. 
This will evaluate to true only if the variable is defined and contains a 
non-empty, non-false value, which is the desired behavior here.
   
   ```
       if (SKBUILD_PROJECT_NAME)
   ```



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