ctetreau created this revision.
Herald added subscribers: llvm-commits, libcxx-commits, lldb-commits, 
Sanitizers, lebedev.ri, hiraditya, arichardson, mgorny.
Herald added a reviewer: lebedev.ri.
Herald added projects: Sanitizers, LLDB, libc++, libc++abi, libunwind, LLVM.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.
Herald added a reviewer: libunwind.
ctetreau requested review of this revision.
Herald added a subscriber: JDevlieghere.

uppercase_CMAKE_BUILD_TYPE is often defined to be
toupper(CMAKE_BUILD_TYPE). This is sometimes neccesary when trying to
programatically inspect the values of builtin cmake variables that have
the build type as part of the name. However, this variable is also often
used to just check the build type.

The use of this variable is error prone because it is not a builtin variable.
Users may add a usage of it without defining it. Since CMake treats undefined
variables as being defined to the empty string, this will not cause any sort
of error. It may even be the case that it appears to work for the author
and bad code makes it into master. (D89807 <https://reviews.llvm.org/D89807>)

This patch removes unneccesary usages of uppercase_CMAKE_BUILD_TYPE.
Remaining usages of it are needed to construct variable names such as
`CMAKE_CXX_FLAGS_DEBUG`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89813

Files:
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  libcxx/CMakeLists.txt
  libcxx/utils/google-benchmark/test/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/AddOCaml.cmake
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/cmake/modules/TableGen.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/utils/benchmark/test/CMakeLists.txt

Index: llvm/utils/benchmark/test/CMakeLists.txt
===================================================================
--- llvm/utils/benchmark/test/CMakeLists.txt
+++ llvm/utils/benchmark/test/CMakeLists.txt
@@ -5,8 +5,7 @@
 
 # NOTE: Some tests use `<cassert>` to perform the test. Therefore we must
 # strip -DNDEBUG from the default CMake flags in DEBUG mode.
-string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
-if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
+if( NOT CMAKE_BUILD_TYPE STREQUAL "Debug" )
   add_definitions( -UNDEBUG )
   add_definitions(-DTEST_BENCHMARK_LIBRARY_HAS_NO_ASSERTIONS)
   # Also remove /D NDEBUG to avoid MSVC warnings about conflicting defines.
Index: llvm/lib/Support/CMakeLists.txt
===================================================================
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -50,6 +50,7 @@
 endif()
 
 # Override the C runtime allocator on Windows and embed it into LLVM tools & libraries
+string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
 if(LLVM_INTEGRATED_CRT_ALLOC)
   if (CMAKE_BUILD_TYPE AND NOT ${LLVM_USE_CRT_${uppercase_CMAKE_BUILD_TYPE}} MATCHES "^(MT|MTd)$")
     message(FATAL_ERROR "LLVM_INTEGRATED_CRT_ALLOC only works with /MT or /MTd. Use LLVM_USE_CRT_${uppercase_CMAKE_BUILD_TYPE} to set the appropriate option.")
Index: llvm/cmake/modules/TableGen.cmake
===================================================================
--- llvm/cmake/modules/TableGen.cmake
+++ llvm/cmake/modules/TableGen.cmake
@@ -54,8 +54,8 @@
   endif()
   # Comments are only useful for Debug builds. Omit them if the backend
   # supports it.
-  if (NOT (uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" OR
-           uppercase_CMAKE_BUILD_TYPE STREQUAL "RELWITHDEBINFO"))
+  if (NOT (CMAKE_BUILD_TYPE STREQUAL "Debug" OR
+           CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo"))
     list(FIND ARGN "-gen-dag-isel" idx)
     if (NOT idx EQUAL -1)
       list(APPEND LLVM_TABLEGEN_FLAGS "-omit-comments")
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===================================================================
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -2,10 +2,6 @@
 # options and executing the appropriate CMake commands to realize the users'
 # selections.
 
-# This is commonly needed so make sure it's defined before we include anything
-# else.
-string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
-
 include(CheckCompilerVersion)
 include(HandleLLVMStdlib)
 include(CheckCCompilerFlag)
@@ -58,7 +54,7 @@
   endif()
   # On non-Debug builds cmake automatically defines NDEBUG, so we
   # explicitly undefine it:
-  if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
+  if( NOT 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>)
@@ -298,7 +294,7 @@
   endif()
   # GCC for MIPS can miscompile LLVM due to PR37701.
   if(CMAKE_COMPILER_IS_GNUCXX AND LLVM_NATIVE_ARCH STREQUAL "Mips" AND
-         NOT Uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
+         NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
     add_flag_or_print_warning("-fno-shrink-wrap" FNO_SHRINK_WRAP)
   endif()
   # gcc with -O3 -fPIC generates TLS sequences that violate the spec on
@@ -448,7 +444,7 @@
   # (/Ob1 vs /Ob2 or -O2 vs -O3). LLVM provides this flag so that users can get
   # PDBs without changing codegen.
   option(LLVM_ENABLE_PDB OFF)
-  if (LLVM_ENABLE_PDB AND uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
+  if (LLVM_ENABLE_PDB AND CMAKE_BUILD_TYPE STREQUAL "Release")
     append("/Zi" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
     # /DEBUG disables linker GC and ICF, but we want those in Release mode.
     append("/DEBUG /OPT:REF /OPT:ICF"
@@ -525,8 +521,8 @@
     set(module_flags "${module_flags} -Xclang -fmodules-local-submodule-visibility")
   endif()
   if (LLVM_ENABLE_MODULE_DEBUGGING AND
-      ((uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG") OR
-       (uppercase_CMAKE_BUILD_TYPE STREQUAL "RELWITHDEBINFO")))
+      ((CMAKE_BUILD_TYPE STREQUAL "Debug") OR
+       (CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo")))
     set(module_flags "${module_flags} -gmodules")
   endif()
   set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${module_flags}")
@@ -742,12 +738,12 @@
     # Append -fno-omit-frame-pointer and turn on debug info to get better
     # stack traces.
     add_flag_if_supported("-fno-omit-frame-pointer" FNO_OMIT_FRAME_POINTER)
-    if (NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" AND
-        NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "RELWITHDEBINFO")
+    if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug" AND
+        NOT CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo")
       add_flag_if_supported("-gline-tables-only" GLINE_TABLES_ONLY)
     endif()
     # Use -O1 even in debug mode, otherwise sanitizers slowdown is too large.
-    if (uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" AND LLVM_OPTIMIZE_SANITIZED_BUILDS)
+    if (CMAKE_BUILD_TYPE STREQUAL "Debug" AND LLVM_OPTIMIZE_SANITIZED_BUILDS)
       add_flag_if_supported("-O1" O1)
     endif()
   elseif (CLANG_CL)
@@ -822,8 +818,8 @@
 
 # Turn on -gsplit-dwarf if requested in debug builds.
 if (LLVM_USE_SPLIT_DWARF AND
-    ((uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG") OR
-     (uppercase_CMAKE_BUILD_TYPE STREQUAL "RELWITHDEBINFO")))
+    ((CMAKE_BUILD_TYPE STREQUAL "Debug") OR
+     (CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo")))
   # Limit to clang and gcc so far. Add compilers supporting this option.
   if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR
       CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
@@ -858,7 +854,7 @@
 # flags instead if LLVM_NO_DEAD_STRIP is set.
 if(NOT CYGWIN AND NOT WIN32)
   if(NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin" AND
-     NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
+     NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
     check_c_compiler_flag("-Werror -fno-function-sections" C_SUPPORTS_FNO_FUNCTION_SECTIONS)
     if (C_SUPPORTS_FNO_FUNCTION_SECTIONS)
       # Don't add -ffunction-sections if it can't be disabled with -fno-function-sections.
@@ -870,7 +866,7 @@
     add_flag_if_supported("-fdata-sections" FDATA_SECTIONS)
   endif()
 elseif(MSVC)
-  if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
+  if( NOT CMAKE_BUILD_TYPE STREQUAL "Debug" )
     append("/Gw" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
   endif()
 endif()
Index: llvm/cmake/modules/AddOCaml.cmake
===================================================================
--- llvm/cmake/modules/AddOCaml.cmake
+++ llvm/cmake/modules/AddOCaml.cmake
@@ -93,6 +93,7 @@
   endforeach()
   # include -D/-UNDEBUG to match dump function visibility
   # regex from HandleLLVMOptions.cmake
+  string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
   string(REGEX MATCH "(^| )[/-][UD] *NDEBUG($| )" flag_matches
          "${CMAKE_C_FLAGS_${uppercase_CMAKE_BUILD_TYPE}} ${CMAKE_C_FLAGS}")
   set(c_flags "${c_flags} ${flag_matches}")
Index: llvm/cmake/modules/AddLLVM.cmake
===================================================================
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -217,7 +217,7 @@
 function(add_link_opts target_name)
   # Don't use linker optimizations in debug builds since it slows down the
   # linker in a context where the optimizations are not important.
-  if (NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
+  if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
 
     # Pass -O3 to the linker. This enabled different optimizations on different
     # linkers.
@@ -1968,6 +1968,7 @@
       set(output_path "-o=${output_name}")
     endif()
 
+    string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
     if(CMAKE_CXX_FLAGS MATCHES "-flto"
       OR CMAKE_CXX_FLAGS_${uppercase_CMAKE_BUILD_TYPE} MATCHES "-flto")
 
Index: llvm/CMakeLists.txt
===================================================================
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -254,10 +254,8 @@
 Please delete them.")
 endif()
 
-string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
-
 if (CMAKE_BUILD_TYPE AND
-    NOT uppercase_CMAKE_BUILD_TYPE MATCHES "^(DEBUG|RELEASE|RELWITHDEBINFO|MINSIZEREL)$")
+    NOT CMAKE_BUILD_TYPE MATCHES "^(Debug|Release|RelWithDebInfo|MinSizeRel)$")
   message(FATAL_ERROR "Invalid value for CMAKE_BUILD_TYPE: ${CMAKE_BUILD_TYPE}")
 endif()
 
@@ -414,7 +412,7 @@
 
 option(LLVM_ENABLE_DUMP "Enable dump functions even when assertions are disabled" OFF)
 
-if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
+if( NOT CMAKE_BUILD_TYPE STREQUAL "Debug" )
   option(LLVM_ENABLE_ASSERTIONS "Enable assertions" OFF)
 else()
   option(LLVM_ENABLE_ASSERTIONS "Enable assertions" ON)
@@ -521,7 +519,7 @@
   if(LLVM_USE_SANITIZER)
     message(FATAL_ERROR "LLVM_INTEGRATED_CRT_ALLOC cannot be used along with LLVM_USE_SANITIZER!")
   endif()
-  if(CMAKE_BUILD_TYPE AND uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
+  if(CMAKE_BUILD_TYPE STREQUAL "Debug")
     message(FATAL_ERROR "The Debug target isn't supported along with LLVM_INTEGRATED_CRT_ALLOC!")
   endif()
 endif()
Index: lldb/CMakeLists.txt
===================================================================
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -30,7 +30,7 @@
 include(AddLLDB)
 
 # Define the LLDB_CONFIGURATION_xxx matching the build type.
-if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
+if(CMAKE_BUILD_TYPE STREQUAL "Debug" )
   add_definitions(-DLLDB_CONFIGURATION_DEBUG)
 endif()
 
Index: libunwind/CMakeLists.txt
===================================================================
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -259,7 +259,6 @@
 endif()
 
 # Assert
-string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
 if (LIBUNWIND_ENABLE_ASSERTIONS)
   # MSVC doesn't like _DEBUG on release builds. See PR 4379.
   if (NOT MSVC)
@@ -268,11 +267,11 @@
 
   # On Release builds cmake automatically defines NDEBUG, so we
   # explicitly undefine it:
-  if (uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
+  if (CMAKE_BUILD_TYPE STREQUAL "Release")
     add_compile_flags(-UNDEBUG)
   endif()
 else()
-  if (NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
+  if (NOT CMAKE_BUILD_TYPE STREQUAL "Release")
     add_compile_flags(-DNDEBUG)
   endif()
 endif()
Index: libcxxabi/CMakeLists.txt
===================================================================
--- libcxxabi/CMakeLists.txt
+++ libcxxabi/CMakeLists.txt
@@ -322,7 +322,6 @@
 endif()
 
 # Assert
-string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
 if (LIBCXXABI_ENABLE_ASSERTIONS)
   # MSVC doesn't like _DEBUG on release builds. See PR 4379.
   if (NOT MSVC)
@@ -330,11 +329,11 @@
   endif()
   # On Release builds cmake automatically defines NDEBUG, so we
   # explicitly undefine it:
-  if (uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
+  if (CMAKE_BUILD_TYPE STREQUAL "Release")
     list(APPEND LIBCXXABI_COMPILE_FLAGS -UNDEBUG)
   endif()
 else()
-  if (NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
+  if (NOT CMAKE_BUILD_TYPE STREQUAL "Release")
     list(APPEND LIBCXXABI_COMPILE_FLAGS -DNDEBUG)
   endif()
 endif()
Index: libcxx/utils/google-benchmark/test/CMakeLists.txt
===================================================================
--- libcxx/utils/google-benchmark/test/CMakeLists.txt
+++ libcxx/utils/google-benchmark/test/CMakeLists.txt
@@ -5,8 +5,7 @@
 
 # NOTE: Some tests use `<cassert>` to perform the test. Therefore we must
 # strip -DNDEBUG from the default CMake flags in DEBUG mode.
-string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
-if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
+if( NOT CMAKE_BUILD_TYPE STREQUAL "Debug" )
   add_definitions( -UNDEBUG )
   add_definitions(-DTEST_BENCHMARK_LIBRARY_HAS_NO_ASSERTIONS)
   # Also remove /D NDEBUG to avoid MSVC warnings about conflicting defines.
Index: libcxx/CMakeLists.txt
===================================================================
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -472,8 +472,7 @@
   set(CMAKE_BUILD_TYPE "COVERAGE" CACHE STRING "" FORCE)
 endif()
 
-string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
-if (uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
+if (CMAKE_BUILD_TYPE STREQUAL "Debug")
   set(LIBCXX_DEBUG_BUILD ON)
 else()
   set(LIBCXX_DEBUG_BUILD OFF)
@@ -677,8 +676,8 @@
     append_flags_if_supported(SANITIZER_FLAGS "-fno-omit-frame-pointer")
     append_flags_if_supported(SANITIZER_FLAGS "-gline-tables-only")
 
-    if (NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" AND
-            NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "RELWITHDEBINFO")
+    if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug" AND
+            NOT CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo")
       append_flags_if_supported(SANITIZER_FLAGS "-gline-tables-only")
     endif()
     if (USE_SANITIZER STREQUAL "Address")
Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===================================================================
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -697,6 +697,7 @@
   endif()
 
   if(APPLE)
+    string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
     if(CMAKE_CXX_FLAGS MATCHES "-flto"
       OR CMAKE_CXX_FLAGS_${uppercase_CMAKE_BUILD_TYPE} MATCHES "-flto")
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to