From d402ce4ce86e502ba2e6b185d123113a368237b2 Mon Sep 17 00:00:00 2001 From: Bingran Hu Date: Mon, 24 Mar 2025 14:20:58 -0400 Subject: [PATCH 1/7] Improve --- CMake/ystdlib-cpp-helpers.cmake | 81 ++++++++++++++++++++++++--------- 1 file changed, 59 insertions(+), 22 deletions(-) diff --git a/CMake/ystdlib-cpp-helpers.cmake b/CMake/ystdlib-cpp-helpers.cmake index 21e1cd93..3164b779 100644 --- a/CMake/ystdlib-cpp-helpers.cmake +++ b/CMake/ystdlib-cpp-helpers.cmake @@ -1,3 +1,14 @@ +# @param REQUIRED_ARGS The list of arguments to check. +# @param ARG_KEYWORDS_MISSING_VALUES The list of arguments with missing values. +# @param LIB_NAME The library target on which to perform the check. +function(require_argument_values REQUIRED_ARGS ARG_KEYWORDS_MISSING_VALUES LIB_NAME) + foreach(arg IN LISTS REQUIRED_ARGS) + if("${arg}" IN_LIST ARG_KEYWORDS_MISSING_VALUES) + message(FATAL_ERROR "Missing values for argument '${arg}' in target '${LIB_NAME}'.") + endif() + endforeach() +endfunction() + # @param SOURCE_LIST The list of source files to check. # @param IS_HEADER_ONLY Returns whether the list only contains header files. # @param NON_HEADER_FILE Returns the name of the first, if any, non-header file. @@ -20,16 +31,17 @@ endfunction() # If `YSTDLIB_CPP_ENABLE_TESTS` is ON, builds the unit tests specific to the current library, and # links this library against the unified unit test target for the entire `ystdlib-cpp` project. # -# @param NAME -# @param NAMESPACE -# @param PUBLIC_HEADERS -# @param PRIVATE_SOURCES -# @param PUBLIC_LINK_LIBRARIES -# @param PRIVATE_LINK_LIBRARIES -# @parms TESTS_SOURCES -# @param [BUILD_INCLUDE_DIR="${PROJECT_SOURCE_DIR}/src"] The list of include paths for building the -# library and for external projects that builds `ystdlib-cpp` as a CMAKE subproject via the -# add_subdirectory() function. +# @param {string} NAME +# @param {string} NAMESPACE +# @param {string[]} PUBLIC_HEADERS +# @parms {string[]} TESTS_SOURCES +# @param {string[]} [PRIVATE_SOURCES] +# @param {string[]} [PUBLIC_LINK_LIBRARIES] +# @param {string[]} [PRIVATE_LINK_LIBRARIES] +# @parms {string[]} [TESTS_LINK_LIBRARIES] +# @param {string[]} [BUILD_INCLUDE_DIR="${PROJECT_SOURCE_DIR}/src"] The list of include paths for +# building the library and for external projects that builds `ystdlib-cpp` as a CMAKE subproject via +# the add_subdirectory() function. function(cpp_library) set(options "") set(oneValueArgs @@ -42,16 +54,28 @@ function(cpp_library) PUBLIC_LINK_LIBRARIES PRIVATE_LINK_LIBRARIES TESTS_SOURCES + TESTS_LINK_LIBRARIES + BUILD_INCLUDE_DIR + ) + set(requireValueArgs + NAME + NAMESPACE + PUBLIC_HEADERS + TESTS_SOURCES BUILD_INCLUDE_DIR ) + cmake_parse_arguments(arg_cpp_lib "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) set(_ALIAS_TARGET_NAME "${arg_cpp_lib_NAMESPACE}::${arg_cpp_lib_NAME}") - # TODO: Turn this into a function for handling other optional params that have default values. - if("BUILD_INCLUDE_DIR" IN_LIST arg_cpp_lib_KEYWORDS_MISSING_VALUES) - message(FATAL_ERROR "Missing build interface list for ${_ALIAS_TARGET_NAME}.") - elseif(NOT DEFINED arg_cpp_lib_BUILD_INCLUDE_DIR) + require_argument_values( + "${requireValueArgs}" + "${arg_cpp_lib_KEYWORDS_MISSING_VALUES}" + "${_ALIAS_TARGET_NAME}" + ) + + if(NOT DEFINED arg_cpp_lib_BUILD_INCLUDE_DIR) set(arg_cpp_lib_BUILD_INCLUDE_DIR "${PROJECT_SOURCE_DIR}/src") endif() @@ -65,12 +89,19 @@ function(cpp_library) check_if_header_only(arg_cpp_lib_PRIVATE_SOURCES _IS_INTERFACE_LIB _) if(_IS_INTERFACE_LIB) + if(arg_cpp_lib_PRIVATE_LINK_LIBRARIES) + message( + FATAL_ERROR + "Private dependency specifications disabled for interface library target ${_ALIAS_TARGET_NAME}." + ) + endif() add_library(${arg_cpp_lib_NAME} INTERFACE) target_include_directories( ${arg_cpp_lib_NAME} INTERFACE "$" ) + target_link_libraries(${arg_cpp_lib_NAME} INTERFACE ${arg_cpp_lib_PUBLIC_LINK_LIBRARIES}) target_compile_features(${arg_cpp_lib_NAME} INTERFACE cxx_std_20) else() # The library type is specified by `BUILD_SHARED_LIBS` if it is defined. Otherwise, the type @@ -87,16 +118,16 @@ function(cpp_library) PUBLIC "$" ) + target_link_libraries( + ${arg_cpp_lib_NAME} + PUBLIC + ${arg_cpp_lib_PUBLIC_LINK_LIBRARIES} + PRIVATE + ${arg_cpp_lib_PRIVATE_LINK_LIBRARIES} + ) target_compile_features(${arg_cpp_lib_NAME} PUBLIC cxx_std_20) endif() - target_link_libraries( - ${arg_cpp_lib_NAME} - PUBLIC - ${arg_cpp_lib_PUBLIC_LINK_LIBRARIES} - PRIVATE - ${arg_cpp_lib_PRIVATE_LINK_LIBRARIES} - ) add_library(${_ALIAS_TARGET_NAME} ALIAS ${arg_cpp_lib_NAME}) if(YSTDLIB_CPP_ENABLE_TESTS) @@ -109,6 +140,7 @@ function(cpp_library) PRIVATE Catch2::Catch2WithMain ${_ALIAS_TARGET_NAME} + ${arg_cpp_lib_TESTS_LINK_LIBRARIES} ) target_compile_features(${_UNIT_TEST_TARGET} PRIVATE cxx_std_20) set_property( @@ -121,6 +153,11 @@ function(cpp_library) # Link against unified unit test target_sources(${UNIFIED_UNIT_TEST_TARGET} PRIVATE ${arg_cpp_lib_TESTS_SOURCES}) - target_link_libraries(${UNIFIED_UNIT_TEST_TARGET} PRIVATE ${_ALIAS_TARGET_NAME}) + target_link_libraries( + ${UNIFIED_UNIT_TEST_TARGET} + PRIVATE + ${_ALIAS_TARGET_NAME} + ${arg_cpp_lib_TESTS_LINK_LIBRARIES} + ) endif() endfunction() From 6dd560dec8be52896e809d3b92114a1e63e854e3 Mon Sep 17 00:00:00 2001 From: Bingran Hu Date: Mon, 24 Mar 2025 14:27:12 -0400 Subject: [PATCH 2/7] Improve docstring with types --- CMake/ystdlib-cpp-helpers.cmake | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CMake/ystdlib-cpp-helpers.cmake b/CMake/ystdlib-cpp-helpers.cmake index 3164b779..7ae624ca 100644 --- a/CMake/ystdlib-cpp-helpers.cmake +++ b/CMake/ystdlib-cpp-helpers.cmake @@ -1,6 +1,6 @@ -# @param REQUIRED_ARGS The list of arguments to check. -# @param ARG_KEYWORDS_MISSING_VALUES The list of arguments with missing values. -# @param LIB_NAME The library target on which to perform the check. +# @param {string[]} REQUIRED_ARGS The list of arguments to check. +# @param {string[]} ARG_KEYWORDS_MISSING_VALUES The list of arguments with missing values. +# @param {string} LIB_NAME The library target on which to perform the check. function(require_argument_values REQUIRED_ARGS ARG_KEYWORDS_MISSING_VALUES LIB_NAME) foreach(arg IN LISTS REQUIRED_ARGS) if("${arg}" IN_LIST ARG_KEYWORDS_MISSING_VALUES) @@ -9,9 +9,9 @@ function(require_argument_values REQUIRED_ARGS ARG_KEYWORDS_MISSING_VALUES LIB_N endforeach() endfunction() -# @param SOURCE_LIST The list of source files to check. -# @param IS_HEADER_ONLY Returns whether the list only contains header files. -# @param NON_HEADER_FILE Returns the name of the first, if any, non-header file. +# @param {string[]} SOURCE_LIST The list of source files to check. +# @param {bool} IS_HEADER_ONLY Returns whether the list only contains header files. +# @param {string} NON_HEADER_FILE Returns the name of the first, if any, non-header file. function(check_if_header_only SOURCE_LIST IS_HEADER_ONLY NON_HEADER_FILE) set(_LOCAL_SOURCE_LIST "${${SOURCE_LIST}}") foreach(src_file IN LISTS _LOCAL_SOURCE_LIST) From 24df44d5f40c71be49587b9132e9e929771b41b5 Mon Sep 17 00:00:00 2001 From: Bingran Hu Date: Tue, 25 Mar 2025 02:00:23 -0400 Subject: [PATCH 3/7] revert changes that are moved to another PR --- CMake/ystdlib-cpp-helpers.cmake | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/CMake/ystdlib-cpp-helpers.cmake b/CMake/ystdlib-cpp-helpers.cmake index 7ae624ca..85e9c3bf 100644 --- a/CMake/ystdlib-cpp-helpers.cmake +++ b/CMake/ystdlib-cpp-helpers.cmake @@ -89,12 +89,6 @@ function(cpp_library) check_if_header_only(arg_cpp_lib_PRIVATE_SOURCES _IS_INTERFACE_LIB _) if(_IS_INTERFACE_LIB) - if(arg_cpp_lib_PRIVATE_LINK_LIBRARIES) - message( - FATAL_ERROR - "Private dependency specifications disabled for interface library target ${_ALIAS_TARGET_NAME}." - ) - endif() add_library(${arg_cpp_lib_NAME} INTERFACE) target_include_directories( ${arg_cpp_lib_NAME} @@ -118,16 +112,17 @@ function(cpp_library) PUBLIC "$" ) - target_link_libraries( - ${arg_cpp_lib_NAME} - PUBLIC - ${arg_cpp_lib_PUBLIC_LINK_LIBRARIES} - PRIVATE - ${arg_cpp_lib_PRIVATE_LINK_LIBRARIES} - ) target_compile_features(${arg_cpp_lib_NAME} PUBLIC cxx_std_20) endif() + target_link_libraries( + ${arg_cpp_lib_NAME} + PUBLIC + ${arg_cpp_lib_PUBLIC_LINK_LIBRARIES} + PRIVATE + ${arg_cpp_lib_PRIVATE_LINK_LIBRARIES} + ) + add_library(${_ALIAS_TARGET_NAME} ALIAS ${arg_cpp_lib_NAME}) if(YSTDLIB_CPP_ENABLE_TESTS) From 596c4ebc30170443ca833cb74bc8af40b6a4932c Mon Sep 17 00:00:00 2001 From: Bingran Hu Date: Tue, 25 Mar 2025 02:01:59 -0400 Subject: [PATCH 4/7] lint fix --- CMake/ystdlib-cpp-helpers.cmake | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/CMake/ystdlib-cpp-helpers.cmake b/CMake/ystdlib-cpp-helpers.cmake index 85e9c3bf..173eed41 100644 --- a/CMake/ystdlib-cpp-helpers.cmake +++ b/CMake/ystdlib-cpp-helpers.cmake @@ -64,7 +64,6 @@ function(cpp_library) TESTS_SOURCES BUILD_INCLUDE_DIR ) - cmake_parse_arguments(arg_cpp_lib "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) set(_ALIAS_TARGET_NAME "${arg_cpp_lib_NAMESPACE}::${arg_cpp_lib_NAME}") @@ -95,7 +94,6 @@ function(cpp_library) INTERFACE "$" ) - target_link_libraries(${arg_cpp_lib_NAME} INTERFACE ${arg_cpp_lib_PUBLIC_LINK_LIBRARIES}) target_compile_features(${arg_cpp_lib_NAME} INTERFACE cxx_std_20) else() # The library type is specified by `BUILD_SHARED_LIBS` if it is defined. Otherwise, the type @@ -115,13 +113,13 @@ function(cpp_library) target_compile_features(${arg_cpp_lib_NAME} PUBLIC cxx_std_20) endif() - target_link_libraries( - ${arg_cpp_lib_NAME} - PUBLIC - ${arg_cpp_lib_PUBLIC_LINK_LIBRARIES} - PRIVATE - ${arg_cpp_lib_PRIVATE_LINK_LIBRARIES} - ) + target_link_libraries( + ${arg_cpp_lib_NAME} + PUBLIC + ${arg_cpp_lib_PUBLIC_LINK_LIBRARIES} + PRIVATE + ${arg_cpp_lib_PRIVATE_LINK_LIBRARIES} + ) add_library(${_ALIAS_TARGET_NAME} ALIAS ${arg_cpp_lib_NAME}) From 8dd5a14ae81c8f9d8d68af7fc7940c85049e5c5c Mon Sep 17 00:00:00 2001 From: Bingran Hu Date: Tue, 25 Mar 2025 02:03:16 -0400 Subject: [PATCH 5/7] Remove extra line --- CMake/ystdlib-cpp-helpers.cmake | 1 - 1 file changed, 1 deletion(-) diff --git a/CMake/ystdlib-cpp-helpers.cmake b/CMake/ystdlib-cpp-helpers.cmake index 173eed41..e61e4083 100644 --- a/CMake/ystdlib-cpp-helpers.cmake +++ b/CMake/ystdlib-cpp-helpers.cmake @@ -120,7 +120,6 @@ function(cpp_library) PRIVATE ${arg_cpp_lib_PRIVATE_LINK_LIBRARIES} ) - add_library(${_ALIAS_TARGET_NAME} ALIAS ${arg_cpp_lib_NAME}) if(YSTDLIB_CPP_ENABLE_TESTS) From 6c5bda777cd42641b54a5ba113f4e3fe6cd4664c Mon Sep 17 00:00:00 2001 From: Bingran Hu Date: Tue, 25 Mar 2025 16:37:45 -0400 Subject: [PATCH 6/7] Apply suggestions from code review Co-authored-by: davidlion --- CMake/ystdlib-cpp-helpers.cmake | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CMake/ystdlib-cpp-helpers.cmake b/CMake/ystdlib-cpp-helpers.cmake index e61e4083..49d45bb8 100644 --- a/CMake/ystdlib-cpp-helpers.cmake +++ b/CMake/ystdlib-cpp-helpers.cmake @@ -1,16 +1,16 @@ # @param {string[]} REQUIRED_ARGS The list of arguments to check. # @param {string[]} ARG_KEYWORDS_MISSING_VALUES The list of arguments with missing values. -# @param {string} LIB_NAME The library target on which to perform the check. -function(require_argument_values REQUIRED_ARGS ARG_KEYWORDS_MISSING_VALUES LIB_NAME) +# @param {string} TARGET_NAME The target on which to perform the check. +function(require_argument_values REQUIRED_ARGS ARG_KEYWORDS_MISSING_VALUES TARGET_NAME) foreach(arg IN LISTS REQUIRED_ARGS) if("${arg}" IN_LIST ARG_KEYWORDS_MISSING_VALUES) - message(FATAL_ERROR "Missing values for argument '${arg}' in target '${LIB_NAME}'.") + message(FATAL_ERROR "Missing values for argument '${arg}' in target '${TARGET_NAME}'.") endif() endforeach() endfunction() # @param {string[]} SOURCE_LIST The list of source files to check. -# @param {bool} IS_HEADER_ONLY Returns whether the list only contains header files. +# @param {bool} IS_HEADER_ONLY Returns TRUE if list only contains header files, FALSE otherwise. # @param {string} NON_HEADER_FILE Returns the name of the first, if any, non-header file. function(check_if_header_only SOURCE_LIST IS_HEADER_ONLY NON_HEADER_FILE) set(_LOCAL_SOURCE_LIST "${${SOURCE_LIST}}") From 5724932dc6e006adeec9c6cf980f5d1248919ceb Mon Sep 17 00:00:00 2001 From: Bingran Hu Date: Tue, 25 Mar 2025 16:47:54 -0400 Subject: [PATCH 7/7] Typo fix --- CMake/ystdlib-cpp-helpers.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMake/ystdlib-cpp-helpers.cmake b/CMake/ystdlib-cpp-helpers.cmake index 58f2255a..0d780c5d 100644 --- a/CMake/ystdlib-cpp-helpers.cmake +++ b/CMake/ystdlib-cpp-helpers.cmake @@ -34,11 +34,11 @@ endfunction() # @param {string} NAME # @param {string} NAMESPACE # @param {string[]} PUBLIC_HEADERS -# @parms {string[]} TESTS_SOURCES +# @param {string[]} TESTS_SOURCES # @param {string[]} [PRIVATE_SOURCES] # @param {string[]} [PUBLIC_LINK_LIBRARIES] # @param {string[]} [PRIVATE_LINK_LIBRARIES] -# @parms {string[]} [TESTS_LINK_LIBRARIES] +# @param {string[]} [TESTS_LINK_LIBRARIES] # @param {string[]} [BUILD_INCLUDE_DIR="${PROJECT_SOURCE_DIR}/src"] The list of include paths for # building the library and for external projects that builds `ystdlib-cpp` as a CMAKE subproject via # the add_subdirectory() function.