FYI: [C++] Use CMake target instead of add_definitions() for xxHash

Author: kou

cpp/CMakeLists.txt

@@ -351,12 +351,6 @@ endif()
 
 include(SetupCxxFlags)
 
-if(${CMAKE_CXX_FLAGS_DEBUG} MATCHES "-Og")
-  # GH-47475: xxhash fails inlining when -Og is used.
-  # See: https://github.com/Cyan4973/xxHash/issues/943
-  add_definitions(-DXXH_NO_INLINE_HINTS)
-endif()
-
 #
 # Linker flags
 #

cpp/cmake_modules/ThirdpartyToolchain.cmake

@@ -371,6 +371,13 @@ add_library(arrow::flatbuffers INTERFACE IMPORTED)
 target_include_directories(arrow::flatbuffers
                            INTERFACE "${THIRDPARTY_DIR}/flatbuffers/include")
 
+add_library(arrow::xxhash INTERFACE IMPORTED)
+# GH-47475: xxhash fails inlining when -Og is
+# used. -DCMAKE_BUILD_TYPE=Debug may use -Og.
+# See: https://github.com/Cyan4973/xxHash/issues/943
+target_compile_definitions(arrow::xxhash
+                           INTERFACE "$<$<CONFIG:Debug>:XXH_NO_INLINE_HINTS>")
+
 # ----------------------------------------------------------------------
 # Some EP's require other EP's
 

cpp/src/arrow/CMakeLists.txt

@@ -402,6 +402,9 @@ arrow_add_object_library(ARROW_ARRAY
                          array/statistics.cc
                          array/util.cc
                          array/validate.cc)
+foreach(target ${ARROW_ARRAY_TARGETS})
+  target_link_libraries(${target} PRIVATE arrow::xxhash)
+endforeach()
 
 arrow_add_object_library(ARROW_IO
                          io/buffered.cc
@@ -865,6 +868,7 @@ if(ARROW_COMPUTE)
                 ${ARROW_VERSION_SCRIPT_FLAGS} # Defined in cpp/arrow/CMakeLists.txt
   )
   foreach(LIB_TARGET ${ARROW_COMPUTE_LIBRARIES})
+    target_link_libraries(${LIB_TARGET} PRIVATE arrow::xxhash)
     target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_COMPUTE_EXPORTING)
   endforeach()
 
@@ -874,6 +878,9 @@ if(ARROW_COMPUTE)
 endif()
 
 arrow_add_object_library(ARROW_COMPUTE_CORE ${ARROW_COMPUTE_SRCS})
+foreach(target ${ARROW_COMPUTE_CORE_TARGETS})
+  target_link_libraries(${target} PRIVATE arrow::xxhash)
+endforeach()
 
 if(ARROW_USE_XSIMD)
   foreach(ARROW_COMPUTE_CORE_TARGET ${ARROW_COMPUTE_CORE_TARGETS})
@@ -1167,6 +1174,7 @@ if(ARROW_BUILD_STATIC AND WIN32)
 endif()
 
 foreach(LIB_TARGET ${ARROW_LIBRARIES})
+  target_link_libraries(${LIB_TARGET} PRIVATE arrow::xxhash)
   target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_EXPORTING)
   # C++17 is required to compile against Arrow C++ headers and libraries
   target_compile_features(${LIB_TARGET} PUBLIC cxx_std_17)

cpp/src/arrow/engine/CMakeLists.txt

@@ -67,6 +67,7 @@ add_arrow_lib(arrow_substrait
               ${SUBSTRAIT_INCLUDES})
 
 foreach(LIB_TARGET ${ARROW_SUBSTRAIT_LIBRARIES})
+  target_link_libraries(${LIB_TARGET} PRIVATE arrow::xxhash)
   target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_ENGINE_EXPORTING)
 endforeach()
 

cpp/src/arrow/util/CMakeLists.txt

@@ -38,7 +38,7 @@ else()
   set(IO_UTIL_TEST_SOURCES io_util_test.cc)
 endif()
 
-set(ARROW_UTILITY_TEST_LINK_LIBS Boost::headers)
+set(ARROW_UTILITY_TEST_LINK_LIBS Boost::headers arrow::xxhash)
 if(ARROW_USE_XSIMD)
   list(APPEND ARROW_UTILITY_TEST_LINK_LIBS ${ARROW_XSIMD})
 endif()

cpp/src/gandiva/CMakeLists.txt

@@ -174,6 +174,7 @@ add_arrow_lib(gandiva
               LLVM::LLVM_LIBS)
 
 foreach(LIB_TARGET ${GANDIVA_LIBRARIES})
+  target_link_libraries(${LIB_TARGET} PRIVATE arrow::xxhash)
   target_compile_definitions(${LIB_TARGET} PRIVATE GANDIVA_EXPORTING)
 endforeach()
 

cpp/src/parquet/CMakeLists.txt

@@ -309,7 +309,7 @@ add_arrow_lib(parquet
 if(WIN32 AND NOT (ARROW_TEST_LINKAGE STREQUAL "static"))
   add_library(parquet_test_support STATIC
               "${PARQUET_THRIFT_SOURCE_DIR}/parquet_types.cpp")
-  target_link_libraries(parquet_test_support thrift::thrift)
+  target_link_libraries(parquet_test_support PRIVATE thrift::thrift)
   list(PREPEND PARQUET_TEST_LINK_LIBS parquet_test_support)
   list(APPEND PARQUET_LIBRARIES parquet_test_support)
 endif()
@@ -339,8 +339,9 @@ endif()
 add_definitions(-DPARQUET_THRIFT_VERSION_MAJOR=${Thrift_VERSION_MAJOR})
 add_definitions(-DPARQUET_THRIFT_VERSION_MINOR=${Thrift_VERSION_MINOR})
 
-# Thrift requires these definitions for some types that we use
 foreach(LIB_TARGET ${PARQUET_LIBRARIES})
+  target_link_libraries(${LIB_TARGET} PRIVATE arrow::xxhash)
+  # Thrift requires these definitions for some types that we use
   target_compile_definitions(${LIB_TARGET}
                              PRIVATE PARQUET_EXPORTING
                              PRIVATE HAVE_INTTYPES_H