[cmake] Stop using add_definitions

[foad]

Summary

Since CMake 3.12 this has been superseded by add_compile_definitions and
other commands.

[foad]

llvm/docs/CMake.rst also mentions add_definitions:

include_directories(${LLVM_INCLUDE_DIRS})
separate_arguments(LLVM_DEFINITIONS_LIST NATIVE_COMMAND ${LLVM_DEFINITIONS})
add_definitions(${LLVM_DEFINITIONS_LIST})

I'm not sure what this should be changed to. @sepavloff do you know?

[barannikov88]

llvm/cmake/modules/HandleLLVMOptions.cmake

71

I wonder if remove_definitions($<$<OR:$<COMPILE_LANGUAGE:C>,$<COMPILE_LANGUAGE:CXX>>:NDEBUG>) works?
It seems it is able to remove previously added DDEBUG from the command line, but otherwise does not pass "-UNDEBUG" to the compiler.

[foad]

llvm/cmake/modules/HandleLLVMOptions.cmake

71

Interesting. I did not know about remove_definitions. But I'd prefer not to do that in this patch, since I have already tested it as-is.

[beanz]

llvm/cmake/modules/HandleLLVMOptions.cmake

71

Generator expressions may be a bit convoluted to read, but I think it is better to use add_compile_options with a conditional generator expression than to add the option, then need to remove it from places where it shouldn't be.

I think the code as written is preferable and easier to maintain in the long run.

llvm/CMakeLists.txt

 
 option(LLVM_BUILD_RUNTIME
   "Build the LLVM runtime libraries." ON)
 option(LLVM_BUILD_EXAMPLES
   "Build the LLVM example programs. If OFF, just generate build targets." OFF)
 option(LLVM_INCLUDE_EXAMPLES "Generate build targets for the LLVM examples" ON)
 
 if(LLVM_BUILD_EXAMPLES)
-  add_definitions(-DBUILD_EXAMPLES)
+  add_compile_definitions(BUILD_EXAMPLES)
 endif(LLVM_BUILD_EXAMPLES)
 
 option(LLVM_BUILD_TESTS
   "Build LLVM unit tests. If OFF, just generate build targets." OFF)
 option(LLVM_INCLUDE_TESTS "Generate build targets for the LLVM unit tests." ON)
 
 option(LLVM_BUILD_BENCHMARKS "Add LLVM benchmark targets to the list of default
 targets. If OFF, benchmarks still could be built using Benchmarks target." OFF)
set(CMAKE_SHARED_LINKER_FLAGS
     "${CMAKE_SHARED_LINKER_FLAGS} -Wl,-lto_library -Wl,${DARWIN_LTO_LIBRARY}")
   set(CMAKE_MODULE_LINKER_FLAGS
     "${CMAKE_MODULE_LINKER_FLAGS} -Wl,-lto_library -Wl,${DARWIN_LTO_LIBRARY}")
 endif()
 
 # Build with _XOPEN_SOURCE on AIX, as stray macros in _ALL_SOURCE mode tend to
 # break things. In this case we need to enable the large-file API as well.
 if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
-          add_definitions("-D_XOPEN_SOURCE=700")
-          add_definitions("-D_LARGE_FILE_API")
+          add_compile_definitions(_XOPEN_SOURCE=700)
+          add_compile_definitions(_LARGE_FILE_API)
 
   # CMake versions less than 3.16 set default linker flags to include -brtl, as
   # well as setting -G when building libraries, so clear them out. Note we only
   # try to clear the form that CMake will set as part of its initial
   # configuration, it is still possible the user may force it as part of a
   # compound option.
   if(CMAKE_VERSION VERSION_LESS 3.16)
     string(REGEX REPLACE "(^|[ \t]+)-Wl,-brtl([ \t]+|$)" " " CMAKE_EXE_LINKER_FLAGS  "${CMAKE_EXE_LINKER_FLAGS}")
if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
   string(APPEND CMAKE_MODULE_LINKER_FLAGS " -shared -Wl,-G")
 
   # Also set the correct flags for building shared libraries.
   string(APPEND CMAKE_SHARED_LINKER_FLAGS " -shared")
 endif()
 
 # Build with _XOPEN_SOURCE on z/OS.
 if (CMAKE_SYSTEM_NAME MATCHES "OS390")
-  add_definitions("-D_XOPEN_SOURCE=600")
-  add_definitions("-D_OPEN_SYS") # Needed for process information.
-  add_definitions("-D_OPEN_SYS_FILE_EXT") # Needed for EBCDIC I/O.
+  add_compile_definitions(_XOPEN_SOURCE=600)
+  add_compile_definitions(_OPEN_SYS) # Needed for process information.
+  add_compile_definitions(_OPEN_SYS_FILE_EXT) # Needed for EBCDIC I/O.
 endif()
 
 # Build with _FILE_OFFSET_BITS=64 on Solaris to match g++ >= 9.
 if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "SunOS")
-          add_definitions("-D_FILE_OFFSET_BITS=64")
+          add_compile_definitions(_FILE_OFFSET_BITS=64)
 endif()
 
 set(CMAKE_INCLUDE_CURRENT_DIR ON)
 
 include_directories( ${LLVM_INCLUDE_DIR} ${LLVM_MAIN_INCLUDE_DIR})
 
 # when crosscompiling import the executable targets from a file
 if(LLVM_USE_HOST_TOOLS)

llvm/cmake/modules/HandleLLVMOptions.cmake

if(CMAKE_GENERATOR MATCHES "Ninja")
   endif()
 elseif(LLVM_PARALLEL_LINK_JOBS)
   message(WARNING "Job pooling is only available with Ninja generators.")
 endif()
 
 if( LLVM_ENABLE_ASSERTIONS )
   # MSVC doesn't like _DEBUG on release builds. See PR 4379.
   if( NOT MSVC )
-    add_definitions( -D_DEBUG )
+    add_compile_definitions(_DEBUG)
   endif()
   # On non-Debug builds cmake automatically defines NDEBUG, so we
   # explicitly undefine it:
   if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
-    # NOTE: use `add_compile_options` rather than `add_definitions` since
-    # `add_definitions` does not support generator expressions.
     add_compile_options($<$<OR:$<COMPILE_LANGUAGE:C>,$<COMPILE_LANGUAGE:CXX>>:-UNDEBUG>)
     if (MSVC)
       # Also remove /D NDEBUG to avoid MSVC warnings about conflicting defines.
       foreach (flags_var_to_scrub
           CMAKE_CXX_FLAGS_RELEASE
           CMAKE_CXX_FLAGS_RELWITHDEBINFO
           CMAKE_CXX_FLAGS_MINSIZEREL
           CMAKE_C_FLAGS_RELEASE
           CMAKE_C_FLAGS_RELWITHDEBINFO
           CMAKE_C_FLAGS_MINSIZEREL)
         string (REGEX REPLACE "(^| )[/-]D *NDEBUG($| )" " "
           "${flags_var_to_scrub}" "${${flags_var_to_scrub}}")
       endforeach()
     endif()
   endif()
 endif()
 
 if(LLVM_ENABLE_EXPENSIVE_CHECKS)
-  add_definitions(-DEXPENSIVE_CHECKS)
+  add_compile_definitions(EXPENSIVE_CHECKS)
 
   # In some libstdc++ versions, std::min_element is not constexpr when
   # _GLIBCXX_DEBUG is enabled.
   CHECK_CXX_SOURCE_COMPILES("
     #define _GLIBCXX_DEBUG
     #include <algorithm>
     int main(int argc, char** argv) {
       static constexpr int data[] = {0, 1};
       constexpr const int* min_elt = std::min_element(&data[0], &data[2]);
       return 0;
     }" CXX_SUPPORTS_GLIBCXX_DEBUG)
   if(CXX_SUPPORTS_GLIBCXX_DEBUG)
-    add_definitions(-D_GLIBCXX_DEBUG)
+    add_compile_definitions(_GLIBCXX_DEBUG)
   else()
-    add_definitions(-D_GLIBCXX_ASSERTIONS)
+    add_compile_definitions(_GLIBCXX_ASSERTIONS)
   endif()
 endif()
 
 if (LLVM_ENABLE_STRICT_FIXED_SIZE_VECTORS)
-  add_definitions(-DSTRICT_FIXED_SIZE_VECTORS)
+  add_compile_definitions(STRICT_FIXED_SIZE_VECTORS)
 endif()
 
 string(TOUPPER "${LLVM_ABI_BREAKING_CHECKS}" uppercase_LLVM_ABI_BREAKING_CHECKS)
 
 if( uppercase_LLVM_ABI_BREAKING_CHECKS STREQUAL "WITH_ASSERTS" )
   if( LLVM_ENABLE_ASSERTIONS )
     set( LLVM_ENABLE_ABI_BREAKING_CHECKS 1 )
   endif()
if((NOT (${CMAKE_SYSTEM_NAME} MATCHES "AIX")) AND
   # about use of the attributes. As long as we don't use the attributes (to
   # override the default) we shouldn't set the command line options either.
   # GCC on AIX warns if -fvisibility-inlines-hidden is used and Clang on AIX doesn't currently support visibility.
   check_cxx_compiler_flag("-fvisibility-inlines-hidden" SUPPORTS_FVISIBILITY_INLINES_HIDDEN_FLAG)
   append_if(SUPPORTS_FVISIBILITY_INLINES_HIDDEN_FLAG "-fvisibility-inlines-hidden" CMAKE_CXX_FLAGS)
 endif()
 
 if(CMAKE_SIZEOF_VOID_P EQUAL 8 AND MINGW)
-  add_definitions( -D_FILE_OFFSET_BITS=64 )
+  add_compile_definitions(_FILE_OFFSET_BITS=64)
 endif()
 
 if( CMAKE_SIZEOF_VOID_P EQUAL 8 AND NOT WIN32 )
   # TODO: support other platforms and toolchains.
   if( LLVM_BUILD_32_BITS )
     message(STATUS "Building 32 bits executables and libraries.")
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -m32")
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -m32")
     set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -m32")
     set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -m32")
     set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} -m32")
 
     # FIXME: CMAKE_SIZEOF_VOID_P is still 8
-    add_definitions(-D_LARGEFILE_SOURCE)
-    add_definitions(-D_FILE_OFFSET_BITS=64)
+    add_compile_definitions(_LARGEFILE_SOURCE)
+    add_compile_definitions(_FILE_OFFSET_BITS=64)
   endif( LLVM_BUILD_32_BITS )
 endif( CMAKE_SIZEOF_VOID_P EQUAL 8 AND NOT WIN32 )
 
 # If building on a GNU specific 32-bit system, make sure off_t is 64 bits
 # so that off_t can stored offset > 2GB.
 # Android until version N (API 24) doesn't support it.
 if (ANDROID AND (ANDROID_NATIVE_API_LEVEL LESS 24))
   set(LLVM_FORCE_SMALLFILE_FOR_ANDROID TRUE)
 endif()
 if( CMAKE_SIZEOF_VOID_P EQUAL 4 AND NOT LLVM_FORCE_SMALLFILE_FOR_ANDROID)
   # FIXME: It isn't handled in LLVM_BUILD_32_BITS.
-  add_definitions( -D_LARGEFILE_SOURCE )
-  add_definitions( -D_FILE_OFFSET_BITS=64 )
+  add_compile_definitions(_LARGEFILE_SOURCE)
+  add_compile_definitions(_FILE_OFFSET_BITS=64)
 endif()
 
 if( XCODE )
   # For Xcode enable several build settings that correspond to
   # many warnings that are on by default in Clang but are
   # not enabled for historical reasons.  For versions of Xcode
   # that do not support these options they will simply
   # be ignored.

 
 # On Win32 using MS tools, provide an option to set the number of parallel jobs
 # to use.
 if( MSVC_IDE )
   set(LLVM_COMPILER_JOBS "0" CACHE STRING
     "Number of parallel compiler jobs. 0 means use all processors. Default is 0.")
   if( NOT LLVM_COMPILER_JOBS STREQUAL "1" )
     if( LLVM_COMPILER_JOBS STREQUAL "0" )
-      add_definitions( /MP )
+      add_compile_options(/MP)
     else()
       message(STATUS "Number of parallel compiler jobs set to " ${LLVM_COMPILER_JOBS})
-      add_definitions( /MP${LLVM_COMPILER_JOBS} )
+      add_compile_options(/MP${LLVM_COMPILER_JOBS})
     endif()
   else()
     message(STATUS "Parallel compilation disabled")
   endif()
 endif()
 
 # set stack reserved size to ~10MB
 if(MSVC)

 endif()
 
 option(LLVM_ENABLE_WARNINGS "Enable compiler warnings." ON)
 
 if( MSVC )
   include(ChooseMSVCCRT)
 
   # Add definitions that make MSVC much less annoying.
-  add_definitions(
+  add_compile_definitions(
     # For some reason MS wants to deprecate a bunch of standard functions...
-    -D_CRT_SECURE_NO_DEPRECATE
-    -D_CRT_SECURE_NO_WARNINGS
-    -D_CRT_NONSTDC_NO_DEPRECATE
-    -D_CRT_NONSTDC_NO_WARNINGS
-    -D_SCL_SECURE_NO_DEPRECATE
-    -D_SCL_SECURE_NO_WARNINGS
+    _CRT_SECURE_NO_DEPRECATE
+    _CRT_SECURE_NO_WARNINGS
+    _CRT_NONSTDC_NO_DEPRECATE
+    _CRT_NONSTDC_NO_WARNINGS
+    _SCL_SECURE_NO_DEPRECATE
+    _SCL_SECURE_NO_WARNINGS
     )
 
   # Tell MSVC to use the Unicode version of the Win32 APIs instead of ANSI.
-  add_definitions(
-    -DUNICODE
-    -D_UNICODE
+  add_compile_definitions(
+    UNICODE
+    _UNICODE
   )
 
   if (LLVM_WINSYSROOT)
     if (NOT CLANG_CL)
       message(ERROR "LLVM_WINSYSROOT requires clang-cl")
     endif()
     append("/winsysroot${LLVM_WINSYSROOT}" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
   endif()
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR
     add_compile_options(-gsplit-dwarf)
     include(LLVMCheckLinkerFlag)
     llvm_check_linker_flag(CXX "-Wl,--gdb-index" LINKER_SUPPORTS_GDB_INDEX)
     append_if(LINKER_SUPPORTS_GDB_INDEX "-Wl,--gdb-index"
       CMAKE_EXE_LINKER_FLAGS CMAKE_MODULE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
   endif()
 endif()
 
-add_definitions( -D__STDC_CONSTANT_MACROS )
-add_definitions( -D__STDC_FORMAT_MACROS )
-add_definitions( -D__STDC_LIMIT_MACROS )
+add_compile_definitions(__STDC_CONSTANT_MACROS)
+add_compile_definitions(__STDC_FORMAT_MACROS)
+add_compile_definitions(__STDC_LIMIT_MACROS)
 
 # clang and gcc don't default-print colored diagnostics when invoked from Ninja.
 if (UNIX AND
     CMAKE_GENERATOR MATCHES "Ninja" AND
     (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR
      (CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND
       NOT (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.9))))
   append("-fdiagnostics-color" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)

llvm/cmake/modules/TensorFlowCompile.cmake

else()
 
     tf_get_model(${model} LLVM_ML_MODELS_ABSOLUTE)
     tf_compile(${LLVM_ML_MODELS_ABSOLUTE} ${tag_set} ${signature_def_key} ${fname} ${cpp_class} ${hdr_file} ${obj_file})
   endif()
 
   set(GeneratedMLSources ${GeneratedMLSources} ${GENERATED_OBJS} ${GENERATED_HEADERS} PARENT_SCOPE)
   set(MLDeps ${MLDeps} tf_xla_runtime PARENT_SCOPE)
   set(MLLinkDeps ${MLLinkDeps} tf_xla_runtime PARENT_SCOPE)
-  add_definitions(-DLLVM_HAVE_TF_AOT_${fname_allcaps})
+  add_compile_definitions(LLVM_HAVE_TF_AOT_${fname_allcaps})
 endfunction()

llvm/lib/Support/BLAKE3/CMakeLists.txt

 set(LLVM_BLAKE3_FILES
   blake3.c
   blake3_dispatch.c
   blake3_portable.c
   blake3_neon.c
 )
 
 if (LLVM_DISABLE_ASSEMBLY_FILES)
   set(CAN_USE_ASSEMBLER FALSE)
 else()
   set(CAN_USE_ASSEMBLER TRUE)
 endif()
 
 macro(disable_blake3_x86_simd)
-  add_definitions(-DBLAKE3_NO_AVX512 -DBLAKE3_NO_AVX2 -DBLAKE3_NO_SSE41 -DBLAKE3_NO_SSE2)
+  add_compile_definitions(BLAKE3_NO_AVX512 BLAKE3_NO_AVX2 BLAKE3_NO_SSE41 BLAKE3_NO_SSE2)
 endmacro()
 
 # The BLAKE3 team recommends using the assembly versions, from the README:
 #
 # "For each of the x86 SIMD instruction sets, four versions are available:
 # three flavors of assembly (Unix, Windows MSVC, and Windows GNU) and one
 # version using C intrinsics. The assembly versions are generally
 # preferred. They perform better, they perform more consistently across

llvm/lib/Support/CMakeLists.txt

elseif( CMAKE_HOST_UNIX )
   endif()
   if( UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "SunOS" )
     set(system_libs ${system_libs} kstat)
   endif()
   if( FUCHSIA )
     set(system_libs ${system_libs} zircon)
   endif()
   if ( HAIKU )
-    add_definitions(-D_BSD_SOURCE)
+    add_compile_definitions(_BSD_SOURCE)
     set(system_libs ${system_libs} bsd)
   endif()
 endif( MSVC OR MINGW )
 
 # Delay load shell32.dll if possible to speed up process startup.
 set (delayload_flags)
 if (MSVC)
   set (delayload_flags delayimp -delayload:shell32.dll -delayload:ole32.dll)
if(LLVM_INTEGRATED_CRT_ALLOC)
 
   string(REGEX REPLACE "(/|\\\\)$" "" LLVM_INTEGRATED_CRT_ALLOC "${LLVM_INTEGRATED_CRT_ALLOC}")
 
   if(NOT EXISTS "${LLVM_INTEGRATED_CRT_ALLOC}")
     message(FATAL_ERROR "Cannot find the path to `git clone` for the CRT allocator! (${LLVM_INTEGRATED_CRT_ALLOC}). Currently, rpmalloc, snmalloc and mimalloc are supported.")
   endif()
 
   if(LLVM_INTEGRATED_CRT_ALLOC MATCHES "rpmalloc$")
-    add_definitions(-DENABLE_OVERRIDE -DENABLE_PRELOAD)
+    add_compile_definitions(ENABLE_OVERRIDE ENABLE_PRELOAD)
     set(ALLOCATOR_FILES "${LLVM_INTEGRATED_CRT_ALLOC}/rpmalloc/rpmalloc.c")
   elseif(LLVM_INTEGRATED_CRT_ALLOC MATCHES "snmalloc$")
     set(ALLOCATOR_FILES "${LLVM_INTEGRATED_CRT_ALLOC}/src/snmalloc/override/new.cc")
     set(system_libs ${system_libs} "mincore.lib" "-INCLUDE:malloc")
   elseif(LLVM_INTEGRATED_CRT_ALLOC MATCHES "mimalloc$")
     set(MIMALLOC_LIB "${LLVM_INTEGRATED_CRT_ALLOC}/out/msvc-x64/Release/mimalloc-static.lib")
     if(NOT EXISTS "${MIMALLOC_LIB}")
     message(FATAL_ERROR "Cannot find the mimalloc static library. To build it, first apply the patch from https://github.com/microsoft/mimalloc/issues/268 then build the Release x64 target through ${LLVM_INTEGRATED_CRT_ALLOC}\\ide\\vs2019\\mimalloc.sln")

 endif()
 
 set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${llvm_system_libs}")
 
 
 if(LLVM_INTEGRATED_CRT_ALLOC)
   if(LLVM_INTEGRATED_CRT_ALLOC MATCHES "snmalloc$")
     set_property(TARGET LLVMSupport PROPERTY CXX_STANDARD 17)
-    add_definitions(-D_SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING)
+    add_compile_definitions(_SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING)
     if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang" AND
         "${CMAKE_SYSTEM_PROCESSOR}" MATCHES "x86_64")
       set_property(TARGET LLVMSupport PROPERTY COMPILE_FLAGS "-mcx16")
     endif()
   endif()
 endif()
 
 if(LLVM_WITH_Z3)
   target_include_directories(LLVMSupport SYSTEM
     PRIVATE
     ${Z3_INCLUDE_DIR}
     )
 endif()

llvm/tools/llvm-config/CMakeLists.txt

llvm_canonicalize_cmake_booleans(
   LLVM_BUILD_LLVM_DYLIB
   LLVM_LINK_LLVM_DYLIB
   LLVM_HAS_RTTI
   BUILD_SHARED_LIBS)
 llvm_expand_pseudo_components(LLVM_DYLIB_COMPONENTS_expanded "${LLVM_DYLIB_COMPONENTS}")
 configure_file(${BUILDVARIABLES_SRCPATH} ${BUILDVARIABLES_OBJPATH} @ONLY)
 
 # Set build-time environment(s).
-add_definitions(-DCMAKE_CFG_INTDIR="${CMAKE_CFG_INTDIR}")
+add_compile_definitions(CMAKE_CFG_INTDIR="${CMAKE_CFG_INTDIR}")
 
 if(LLVM_ENABLE_MODULES)
   target_compile_options(llvm-config PUBLIC
     "-fmodules-ignore-macro=CMAKE_CFG_INTDIR"
     )
 endif()
 
 # Add the dependency on the generation step.