gemini-code-assist[bot] commented on code in PR #354: URL: https://github.com/apache/tvm-ffi/pull/354#discussion_r2638595173
########## examples/quickstart/CMakeLists.txt: ########## @@ -61,22 +61,23 @@ find_package(tvm_ffi CONFIG REQUIRED) if (EXAMPLE_NAME STREQUAL "compile_cpu") # Example 1. C++ `add_one` add_library(add_one_cpu SHARED compile/add_one_cpu.cc) - target_link_libraries(add_one_cpu PRIVATE tvm_ffi_header) - target_link_libraries(add_one_cpu PRIVATE tvm_ffi_shared) + target_link_libraries(add_one_cpu PRIVATE tvm_ffi::header) + target_link_libraries(add_one_cpu PRIVATE tvm_ffi::shared) set_target_properties(add_one_cpu PROPERTIES PREFIX "" SUFFIX ".so") set_flat_output_dirs(add_one_cpu) elseif (EXAMPLE_NAME STREQUAL "compile_cuda") # Example 2. CUDA `add_one` enable_language(CUDA) add_library(add_one_cuda SHARED compile/add_one_cuda.cu) - target_link_libraries(add_one_cuda PRIVATE tvm_ffi_shared) + target_link_libraries(add_one_cuda PRIVATE tvm_ffi::header) + target_link_libraries(add_one_cuda PRIVATE tvm_ffi::shared) Review Comment:  For conciseness and better readability, you can combine these two `target_link_libraries` calls into a single one. ``` target_link_libraries(add_one_cuda PRIVATE tvm_ffi::header tvm_ffi::shared) ``` ########## cmake/Utils/Library.cmake: ########## @@ -257,24 +258,24 @@ function (tvm_ffi_configure_target target) # LINK_HEADER if (tvm_ffi_arg__LINK_HEADER) - if (TARGET tvm_ffi_header) - target_link_libraries("${target}" PRIVATE tvm_ffi_header) + if (TARGET tvm_ffi::header) + target_link_libraries("${target}" PRIVATE tvm_ffi::header) else () message( FATAL_ERROR - "tvm_ffi_configure_target(${target}): LINK_HEADER requested but target 'tvm_ffi_header' does not exist." + "tvm_ffi_configure_target(${target}): LINK_HEADER requested but targets 'tvm_ffi::header' do not exist." ) endif () endif () # LINK_SHARED if (tvm_ffi_arg__LINK_SHARED) - if (TARGET tvm_ffi_shared) - target_link_libraries("${target}" PRIVATE tvm_ffi_shared) + if (TARGET tvm_ffi::shared) + target_link_libraries("${target}" PRIVATE tvm_ffi::shared) else () message( FATAL_ERROR - "tvm_ffi_configure_target(${target}): LINK_SHARED requested but target 'tvm_ffi_shared' does not exist." + "tvm_ffi_configure_target(${target}): LINK_SHARED requested but targets 'tvm_ffi::shared' do not exist." ) endif () endif () Review Comment:  For consistency with `cmake/Utils/EmbedCubin.cmake` and to improve backward compatibility, it would be beneficial to check for the old target names (`tvm_ffi_header`, `tvm_ffi_shared`) as a fallback if the new namespaced targets are not found. This would make the transition smoother for users who might have older configurations. The error messages should also be updated to reflect both possible target names. ``` # LINK_HEADER if (tvm_ffi_arg__LINK_HEADER) if (TARGET tvm_ffi::header) target_link_libraries("${target}" PRIVATE tvm_ffi::header) elseif (TARGET tvm_ffi_header) target_link_libraries("${target}" PRIVATE tvm_ffi_header) else () message( FATAL_ERROR "tvm_ffi_configure_target(${target}): LINK_HEADER requested but target 'tvm_ffi::header' or 'tvm_ffi_header' does not exist." ) endif () endif () # LINK_SHARED if (tvm_ffi_arg__LINK_SHARED) if (TARGET tvm_ffi::shared) target_link_libraries("${target}" PRIVATE tvm_ffi::shared) elseif (TARGET tvm_ffi_shared) target_link_libraries("${target}" PRIVATE tvm_ffi_shared) else () message( FATAL_ERROR "tvm_ffi_configure_target(${target}): LINK_SHARED requested but target 'tvm_ffi::shared' or 'tvm_ffi_shared' does not exist." ) endif () endif () ``` ########## examples/quickstart/CMakeLists.txt: ########## @@ -61,22 +61,23 @@ find_package(tvm_ffi CONFIG REQUIRED) if (EXAMPLE_NAME STREQUAL "compile_cpu") # Example 1. C++ `add_one` add_library(add_one_cpu SHARED compile/add_one_cpu.cc) - target_link_libraries(add_one_cpu PRIVATE tvm_ffi_header) - target_link_libraries(add_one_cpu PRIVATE tvm_ffi_shared) + target_link_libraries(add_one_cpu PRIVATE tvm_ffi::header) + target_link_libraries(add_one_cpu PRIVATE tvm_ffi::shared) set_target_properties(add_one_cpu PROPERTIES PREFIX "" SUFFIX ".so") set_flat_output_dirs(add_one_cpu) elseif (EXAMPLE_NAME STREQUAL "compile_cuda") # Example 2. CUDA `add_one` enable_language(CUDA) add_library(add_one_cuda SHARED compile/add_one_cuda.cu) - target_link_libraries(add_one_cuda PRIVATE tvm_ffi_shared) + target_link_libraries(add_one_cuda PRIVATE tvm_ffi::header) + target_link_libraries(add_one_cuda PRIVATE tvm_ffi::shared) set_target_properties(add_one_cuda PROPERTIES PREFIX "" SUFFIX ".so") set_flat_output_dirs(add_one_cuda) elseif (EXAMPLE_NAME STREQUAL "load_cpp") # Example 3. Load C++ shared library add_executable(load_cpp load/load_cpp.cc) - target_link_libraries(load_cpp PRIVATE tvm_ffi_header) - target_link_libraries(load_cpp PRIVATE tvm_ffi_shared) + target_link_libraries(load_cpp PRIVATE tvm_ffi::header) + target_link_libraries(load_cpp PRIVATE tvm_ffi::shared) Review Comment:  For conciseness and better readability, you can combine these two `target_link_libraries` calls into a single one. ``` target_link_libraries(load_cpp PRIVATE tvm_ffi::header tvm_ffi::shared) ``` ########## examples/quickstart/CMakeLists.txt: ########## @@ -61,22 +61,23 @@ find_package(tvm_ffi CONFIG REQUIRED) if (EXAMPLE_NAME STREQUAL "compile_cpu") # Example 1. C++ `add_one` add_library(add_one_cpu SHARED compile/add_one_cpu.cc) - target_link_libraries(add_one_cpu PRIVATE tvm_ffi_header) - target_link_libraries(add_one_cpu PRIVATE tvm_ffi_shared) + target_link_libraries(add_one_cpu PRIVATE tvm_ffi::header) + target_link_libraries(add_one_cpu PRIVATE tvm_ffi::shared) Review Comment:  For conciseness and better readability, you can combine these two `target_link_libraries` calls into a single one. ``` target_link_libraries(add_one_cpu PRIVATE tvm_ffi::header tvm_ffi::shared) ``` ########## examples/stable_c_abi/CMakeLists.txt: ########## @@ -73,8 +73,8 @@ if (EXAMPLE_NAME STREQUAL "kernel") elseif (EXAMPLE_NAME STREQUAL "load") # Example 2. Load `add_one_cpu` shared library in C add_executable(load src/load.c) - target_link_libraries(load PRIVATE tvm_ffi_header) - target_link_libraries(load PRIVATE tvm_ffi_shared) + target_link_libraries(load PRIVATE tvm_ffi::header) + target_link_libraries(load PRIVATE tvm_ffi::shared) Review Comment:  For conciseness and better readability, you can combine these two `target_link_libraries` calls into a single one. ``` target_link_libraries(load PRIVATE tvm_ffi::header tvm_ffi::shared) ``` ########## examples/stable_c_abi/CMakeLists.txt: ########## @@ -58,8 +58,8 @@ find_package(tvm_ffi CONFIG REQUIRED) if (EXAMPLE_NAME STREQUAL "kernel") # Example 1. `add_one_cpu` in C add_library(add_one_cpu SHARED src/add_one_cpu.c) - target_link_libraries(add_one_cpu PRIVATE tvm_ffi_header) - target_link_libraries(add_one_cpu PRIVATE tvm_ffi_shared) + target_link_libraries(add_one_cpu PRIVATE tvm_ffi::header) + target_link_libraries(add_one_cpu PRIVATE tvm_ffi::shared) Review Comment:  For conciseness and better readability, you can combine these two `target_link_libraries` calls into a single one. ``` target_link_libraries(add_one_cpu PRIVATE tvm_ffi::header tvm_ffi::shared) ``` -- 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]
