Fix "Unknown arguments specified" to if in lldb

[JDevlieghere]

What's the motivation for this?

[ctetreau]

In D89807#2342192, @JDevlieghere wrote:

What's the motivation for this?

This is preventing me from generating the build system:

-- SWIG 2 or later is required for Python support in LLDB but could not be found
CMake Error at [redacted]/lldb/cmake/modules/FindPythonAndSwig.cmake:49 (if):
  if given arguments:

    "3.8.0" "VERSION_GREATER_EQUAL" "3.7" "AND" "" "VERSION_LESS" "4.0" "AND" "WIN32" "AND" "(" "1" "OR" "(" "STREQUAL" "Debug" ")" ")"

  Unknown arguments specified
Call Stack (most recent call first):
  [redacted]/lldb/cmake/modules/LLDBConfig.cmake:48 (find_package)
  [redacted]/lldb/cmake/modules/LLDBConfig.cmake:59 (add_optional_dependency)
  [redacted]/lldb/CMakeLists.txt:29 (include)


-- Configuring incomplete, errors occurred!

There are two issues:

STREQUAL accepts variables on the LHS, so the braces are unneccesary. The real issue though is that uppercase_CMAKE_BUILD_TYPE is not defined at this point. I could define it, but the case of the possible values of CMAKE_BUILD_TYPE are well defined so there's no reason to.

[JDevlieghere]

In D89807#2342351, @ctetreau wrote:

The real issue though is that uppercase_CMAKE_BUILD_TYPE is not defined at this point. I could define it, but the case of the possible values of CMAKE_BUILD_TYPE are well defined so there's no reason to.

I thought it was CMake that provided uppercase_CMAKE_BUILD_TYPE but it's llvm itself. I'm not sure how we end up here without that variable set, wiht in-tree builds we inherit all the LLVM variables and in standalone builds AddLLVM should take care of it. Given that the rest of LLVM uses the uppercase variant and thus accepts values like "DEBUG" I'm not a fan of being more strict here if only for the sake of consistency.

[ctetreau]

After experimenting a bit, it seems that if you pass -DCMAKE_BUILD_TYPE=RELEASE, then the value of CMAKE_BUILD_TYPE will also be uppercase. This might represent a "good reason" to use the uppercase_CMAKE_BUILD_TYPE version. I need to think on this a bit, but I may end up just making sure that this variable is defined.

[ctetreau]

restore use of uppercase_CMAKE_BUILD_TYPE

[ctetreau]

OK, for now, I've restored the use of uppercase_CMAKE_BUILD_TYPE. It looks like this does get set, and it's just the empty string on my machine because windows.

I'd like to see it get removed, but this commit is about fixing the build.

[labath]

Sorry about the breakage.

lldb/cmake/modules/FindPythonAndSwig.cmake

51

Maybe add quotes here instead of dropping the ${}. I'm not really sure which one is more idiomatic, but that's what I did for the other variables (SWIG_VERSION, etc.) -- it seems I just forgot about this one.

lldb/cmake/modules/FindPythonAndSwig.cmake

if (SWIG_FOUND)
       FindPython3()
   else()
     message(STATUS "SWIG 2 or later is required for Python support in LLDB but could not be found")
   endif()
 
   get_property(MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
   if ("${Python3_VERSION}" VERSION_GREATER_EQUAL "3.7" AND
       "${SWIG_VERSION}" VERSION_LESS "4.0" AND WIN32 AND (
-      ${MULTI_CONFIG} OR (${uppercase_CMAKE_BUILD_TYPE} STREQUAL "DEBUG")))
+      ${MULTI_CONFIG} OR (uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")))
     # Technically this can happen with non-Windows builds too, but we are not
     # able to detect whether Python was built with assertions, and only Windows
     # has the requirement that Debug LLDB must use Debug Python.
     message(WARNING "Debug builds of LLDB are likely to be unusable due to "
       "<https://github.com/swig/swig/issues/1321>. Please use SWIG >= 4.0.")
   endif()
 
   include(FindPackageHandleStandardArgs)