[CMake] Replace list(FIND) by if(IN_LIST) where index isn't used

[aaronpuchert]

Summary

If we don't use the index otherwise, if(IN_LIST) is more readable and
doesn't clutter the local scope with index variables.

This was pointed out by @beanz in D96670.

[beanz]

LGTM. Thank you for the nice cleanup :D

[aaronpuchert]

llvm/cmake/modules/TableGen.cmake

45–54

I hope we're not running into https://cmake.org/cmake/help/latest/command/macro.html#argument-caveats here.

[beanz]

llvm/cmake/modules/TableGen.cmake

45–54

You're inside a function here, so that should be fine...

[aaronpuchert]

llvm/cmake/modules/TableGen.cmake

45–54

Right, this isn't a macro. Thanks for clarifying that.

[aaronpuchert]

@beanz, a configuration error caused by this in a third-party project has been reported to me:

CMake Warning (dev) at /usr/lib/llvm-16/lib/cmake/llvm/LLVM-Config.cmake:230 (if):
  Policy CMP0057 is not set: Support new IN_LIST if() operator.  Run "cmake
  --help-policy CMP0057" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  IN_LIST will be interpreted as an operator when the policy is set to NEW.
  Since the policy is not set the OLD behavior will be used.
Call Stack (most recent call first):
  clang_delta/CMakeLists.txt:465 (llvm_map_components_to_libnames)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Error at /usr/lib/llvm-16/lib/cmake/llvm/LLVM-Config.cmake:230 (if):
  if given arguments:

    "engine" "IN_LIST" "link_components"

The project has cmake_minimum_required(VERSION 2.8.12) and is now running afoul CMP0057:

CMake 3.3 adds support for the new IN_LIST operator.

The OLD behavior for this policy is to ignore the IN_LIST operator. The NEW behavior is to interpret the IN_LIST operator.

This policy was introduced in CMake version 3.3. CMake version 3.25.2 warns when the policy is not set and uses OLD behavior. Use the cmake_policy() command to set it to OLD or NEW explicitly.

This doesn't affect us internally because we have cmake_minimum_required(VERSION 3.13.4), but should we add cmake_policy(SET CMP0057 NEW) to the exported files? Perhaps in a PUSH/POP?

[beanz]

In D142405#4081473, @aaronpuchert wrote:

This doesn't affect us internally because we have cmake_minimum_required(VERSION 3.13.4), but should we add cmake_policy(SET CMP0057 NEW) to the exported files? Perhaps in a PUSH/POP?

IMO, this is a problem for the downstream project.

We require CMake 3.13.4 for building LLVM which allows us to use CMake 3.13 features in our CMake modules. Any project using LLVM’s CMake modules _should_ have the same requirement. If we were going to do anything to address this, the approach I would take would be to make importing the LLVM CMake modules a fatal error if CMAKE_MINIMUM_REQUIRED_VERSION is less than 3.13.4.

Supporting some parts of LLVM’s CMake on older required versions is not going to be trivial, and I don’t think we should do it.

Not sure if @phosek, @smeenai, @compnerd, or @ldionne have different thoughts.

[phosek]

In D142405#4081563, @beanz wrote:

In D142405#4081473, @aaronpuchert wrote:

This doesn't affect us internally because we have cmake_minimum_required(VERSION 3.13.4), but should we add cmake_policy(SET CMP0057 NEW) to the exported files? Perhaps in a PUSH/POP?

IMO, this is a problem for the downstream project.

We require CMake 3.13.4 for building LLVM which allows us to use CMake 3.13 features in our CMake modules. Any project using LLVM’s CMake modules _should_ have the same requirement. If we were going to do anything to address this, the approach I would take would be to make importing the LLVM CMake modules a fatal error if CMAKE_MINIMUM_REQUIRED_VERSION is less than 3.13.4.

Supporting some parts of LLVM’s CMake on older required versions is not going to be trivial, and I don’t think we should do it.

Not sure if @phosek, @smeenai, @compnerd, or @ldionne have different thoughts.

I agree, supporting older CMake versions has never been our goal, the fact that it works today is just a coincidence. I also support including an explicit version check in exported files.

llvm/cmake/config-ix.cmake

foreach (NATIVE_KEYWORD host Native)
   list(FIND LLVM_TARGETS_TO_BUILD ${NATIVE_KEYWORD} idx)
   if( NOT idx LESS 0 )
     list(REMOVE_AT LLVM_TARGETS_TO_BUILD ${idx})
     list(APPEND LLVM_TARGETS_TO_BUILD ${LLVM_NATIVE_ARCH})
     list(REMOVE_DUPLICATES LLVM_TARGETS_TO_BUILD)
   endif()
 endforeach()
 
-list(FIND LLVM_TARGETS_TO_BUILD ${LLVM_NATIVE_ARCH} NATIVE_ARCH_IDX)
-if (NATIVE_ARCH_IDX EQUAL -1)
+if (NOT ${LLVM_NATIVE_ARCH} IN_LIST LLVM_TARGETS_TO_BUILD)
   message(STATUS
     "Native target ${LLVM_NATIVE_ARCH} is not selected; lli will not JIT code")
 else ()
   message(STATUS "Native target architecture is ${LLVM_NATIVE_ARCH}")
   set(LLVM_NATIVE_TARGET LLVMInitialize${LLVM_NATIVE_ARCH}Target)
   set(LLVM_NATIVE_TARGETINFO LLVMInitialize${LLVM_NATIVE_ARCH}TargetInfo)
   set(LLVM_NATIVE_TARGETMC LLVMInitialize${LLVM_NATIVE_ARCH}TargetMC)
   set(LLVM_NATIVE_ASMPRINTER LLVMInitialize${LLVM_NATIVE_ARCH}AsmPrinter)

llvm/cmake/modules/AddLLVM.cmake

while(NOT "${new_libs}" STREQUAL "")
           get_target_property(lib_type ${lib} TYPE)
           if("${lib_type}" STREQUAL "STATIC_LIBRARY")
             list(APPEND static_libs ${lib})
           else()
             list(APPEND other_libs ${lib})
           endif()
           get_target_property(transitive_libs ${lib} INTERFACE_LINK_LIBRARIES)
           foreach(transitive_lib ${transitive_libs})
-            list(FIND link_libs ${transitive_lib} idx)
-            if(TARGET ${transitive_lib} AND idx EQUAL -1)
+            if(TARGET ${transitive_lib} AND NOT ${transitive_lib} IN_LIST link_libs)
               list(APPEND newer_libs ${transitive_lib})
               list(APPEND link_libs ${transitive_lib})
             endif()
           endforeach(transitive_lib)
         endif()
       endforeach(lib)
       set(new_libs ${newer_libs})
       set(newer_libs "")

llvm/cmake/modules/AddSphinxTarget.cmake

function (add_sphinx_target builder project)
   set_property(DIRECTORY APPEND PROPERTY
                ADDITIONAL_MAKE_CLEAN_FILES
                "${SPHINX_BUILD_DIR}")
 
   # We need to remove ${SPHINX_DOC_TREE_DIR} when make clean is run
   # but we should only add this path once
   get_property(_CURRENT_MAKE_CLEAN_FILES
                DIRECTORY PROPERTY ADDITIONAL_MAKE_CLEAN_FILES)
-  list(FIND _CURRENT_MAKE_CLEAN_FILES "${SPHINX_DOC_TREE_DIR}" _INDEX)
-  if (_INDEX EQUAL -1)
+  if (NOT "${SPHINX_DOC_TREE_DIR}" IN_LIST _CURRENT_MAKE_CLEAN_FILES)
     set_property(DIRECTORY APPEND PROPERTY
                  ADDITIONAL_MAKE_CLEAN_FILES
                  "${SPHINX_DOC_TREE_DIR}")
   endif()
 
   if (LLVM_BUILD_DOCS)
     add_dependencies(sphinx ${SPHINX_TARGET_NAME})
 

llvm/cmake/modules/ChooseMSVCCRT.cmake

variables (LLVM_USE_CRT_DEBUG, etc) instead.")
   endforeach(build_type)
 
   foreach(build_type ${CMAKE_CONFIGURATION_TYPES} ${CMAKE_BUILD_TYPE})
     string(TOUPPER "${build_type}" build)
     if ("${LLVM_USE_CRT_${build}}" STREQUAL "")
       set(flag_string " ")
     else()
       set(flag_string " /${LLVM_USE_CRT_${build}} ")
-      list(FIND ${MSVC_CRT} ${LLVM_USE_CRT_${build}} idx)
-      if (idx LESS 0)
+      if (NOT ${LLVM_USE_CRT_${build}} IN_LIST ${MSVC_CRT})
         message(FATAL_ERROR
           "Invalid value for LLVM_USE_CRT_${build}: ${LLVM_USE_CRT_${build}}. Valid options are one of: ${${MSVC_CRT}}")
-      endif (idx LESS 0)
+      endif()
       message(STATUS "Using ${build_type} VC++ CRT: ${LLVM_USE_CRT_${build}}")
     endif()
     foreach(lang C CXX)
       set_flag_in_var(CMAKE_${lang}_FLAGS_${build} MSVC_CRT_REGEX flag_string)
     endforeach(lang)
   endforeach(build_type)
 endmacro(choose_msvc_crt MSVC_CRT)
 

llvm/cmake/modules/LLVM-Config.cmake

 function(llvm_expand_pseudo_components out_components)
   set( link_components ${ARGN} )
   if(NOT LLVM_AVAILABLE_LIBS)
     # Inside LLVM itself available libs are in a global property.
     get_property(LLVM_AVAILABLE_LIBS GLOBAL PROPERTY LLVM_LIBS)
   endif()
   foreach(c ${link_components})
     # add codegen, asmprinter, asmparser, disassembler
-    list(FIND LLVM_TARGETS_TO_BUILD ${c} idx)
-    if( NOT idx LESS 0 )
-      list(FIND LLVM_AVAILABLE_LIBS LLVM${c}CodeGen lib_idx)
-      if( lib_idx GREATER_EQUAL 0 )
+    if(${c} IN_LIST LLVM_TARGETS_TO_BUILD)
+      if(LLVM${c}CodeGen IN_LIST LLVM_AVAILABLE_LIBS)
         list(APPEND expanded_components "${c}CodeGen")
       else()
-        list(FIND LLVM_AVAILABLE_LIBS LLVM${c} lib_idx)
-        if( lib_idx GREATER_EQUAL 0 )
+        if(LLVM${c} IN_LIST LLVM_AVAILABLE_LIBS)
           list(APPEND expanded_components "${c}")
         else()
           message(FATAL_ERROR "Target ${c} is not in the set of libraries.")
         endif()
       endif()
       foreach(subcomponent IN ITEMS AsmPrinter AsmParser Desc Disassembler Info Utils)
-        list(FIND LLVM_AVAILABLE_LIBS LLVM${c}${subcomponent} lib_idx)
-        if( lib_idx GREATER_EQUAL 0 )
+        if(LLVM${c}${subcomponent} IN_LIST LLVM_AVAILABLE_LIBS)
           list(APPEND expanded_components "${c}${subcomponent}")
         endif()
       endforeach()
     elseif( c STREQUAL "nativecodegen" )
       foreach(subcomponent IN ITEMS CodeGen Desc Info)
-        list(FIND LLVM_AVAILABLE_LIBS LLVM${LLVM_NATIVE_ARCH}${subcomponent} lib_idx)
-        if( lib_idx GREATER_EQUAL 0 )
+        if(LLVM${LLVM_NATIVE_ARCH}${subcomponent} IN_LIST LLVM_AVAILABLE_LIBS)
           list(APPEND expanded_components "${LLVM_NATIVE_ARCH}${subcomponent}")
         endif()
       endforeach()
     elseif( c STREQUAL "AllTargetsCodeGens" )
       # Link all the codegens from all the targets
       foreach(t ${LLVM_TARGETS_TO_BUILD})
         if( TARGET LLVM${t}CodeGen)
           list(APPEND expanded_components "${t}CodeGen")
         endif()
       endforeach(t)
     elseif( c STREQUAL "AllTargetsAsmParsers" )
       # Link all the asm parsers from all the targets
       foreach(t ${LLVM_TARGETS_TO_BUILD})
-        list(FIND LLVM_AVAILABLE_LIBS LLVM${t}AsmParser lib_idx)
-        if( lib_idx GREATER_EQUAL 0 )
+        if(LLVM${t}AsmParser IN_LIST LLVM_AVAILABLE_LIBS)
           list(APPEND expanded_components "${t}AsmParser")
         endif()
       endforeach(t)
     elseif( c STREQUAL "AllTargetsDescs" )
       # Link all the descs from all the targets
       foreach(t ${LLVM_TARGETS_TO_BUILD})
-        list(FIND LLVM_AVAILABLE_LIBS LLVM${t}Desc lib_idx)
-        if( lib_idx GREATER_EQUAL 0 )
+        if(LLVM${t}Desc IN_LIST LLVM_AVAILABLE_LIBS)
           list(APPEND expanded_components "${t}Desc")
         endif()
       endforeach(t)
     elseif( c STREQUAL "AllTargetsDisassemblers" )
       # Link all the disassemblers from all the targets
       foreach(t ${LLVM_TARGETS_TO_BUILD})
-        list(FIND LLVM_AVAILABLE_LIBS LLVM${t}Disassembler lib_idx)
-        if( lib_idx GREATER_EQUAL 0 )
+        if(LLVM${t}Disassembler IN_LIST LLVM_AVAILABLE_LIBS)
           list(APPEND expanded_components "${t}Disassembler")
         endif()
       endforeach(t)
     elseif( c STREQUAL "AllTargetsInfos" )
       # Link all the infos from all the targets
       foreach(t ${LLVM_TARGETS_TO_BUILD})
-        list(FIND LLVM_AVAILABLE_LIBS LLVM${t}Info lib_idx)
-        if( lib_idx GREATER_EQUAL 0 )
+        if(LLVM${t}Info IN_LIST LLVM_AVAILABLE_LIBS)
           list(APPEND expanded_components "${t}Info")
         endif()
       endforeach(t)
     elseif( c STREQUAL "AllTargetsMCAs" )
       # Link all the TargetMCAs from all the targets
       foreach(t ${LLVM_TARGETS_TO_BUILD})
         if( TARGET LLVM${t}TargetMCA )
           list(APPEND expanded_components "${t}TargetMCA")
foreach(c ${link_components})
       is_llvm_target_specifier(${c} iltl_result ALL_TARGETS)
       if(iltl_result)
         message(FATAL_ERROR "Specified target library before target registration is complete.")
       endif()
     endforeach()
   endif()
 
   # Expand some keywords:
-  list(FIND LLVM_TARGETS_TO_BUILD "${LLVM_NATIVE_ARCH}" have_native_backend)
-  list(FIND link_components "engine" engine_required)
-  if( NOT engine_required EQUAL -1 )
-    list(FIND LLVM_TARGETS_WITH_JIT "${LLVM_NATIVE_ARCH}" have_jit)
-    if( NOT have_native_backend EQUAL -1 AND NOT have_jit EQUAL -1 )
+  if(engine IN_LIST link_components)
+    if(${LLVM_NATIVE_ARCH} IN_LIST LLVM_TARGETS_TO_BUILD AND
+       ${LLVM_NATIVE_ARCH} IN_LIST LLVM_TARGETS_WITH_JIT)
       list(APPEND link_components "jit")
       list(APPEND link_components "native")
     else()
       list(APPEND link_components "interpreter")
     endif()
   endif()
-  list(FIND link_components "native" native_required)
-  if( NOT native_required EQUAL -1 )
-    if( NOT have_native_backend EQUAL -1 )
-      list(APPEND link_components ${LLVM_NATIVE_ARCH})
-    endif()
-  endif()
+  if(native IN_LIST link_components AND ${LLVM_NATIVE_ARCH} IN_LIST LLVM_TARGETS_TO_BUILD)
 
   # Translate symbolic component names to real libraries:
   llvm_expand_pseudo_components(link_components ${link_components})
   foreach(c ${link_components})
     get_property(c_rename GLOBAL PROPERTY LLVM_COMPONENT_NAME_${c})
     if(c_rename)
         set(c ${c_rename})
     endif()
function(llvm_map_components_to_libnames out_libs)
 
   set(${out_libs} ${expanded_components} PARENT_SCOPE)
 endfunction()
 
 # Perform a post-order traversal of the dependency graph.
 # This duplicates the algorithm used by llvm-config, originally
 # in tools/llvm-config/llvm-config.cpp, function ComputeLibsForComponents.
 function(expand_topologically name required_libs visited_libs)
-  list(FIND visited_libs ${name} found)
-  if( found LESS 0 )
+  if(NOT ${name} IN_LIST visited_libs)
     list(APPEND visited_libs ${name})
     set(visited_libs ${visited_libs} PARENT_SCOPE)
 
     #
     get_property(libname GLOBAL PROPERTY LLVM_COMPONENT_NAME_${name})
     if(libname)
       set(cname LLVM${libname})
     elseif(TARGET ${name})

llvm/cmake/modules/LLVMExternalProjectUtils.cmake

if(TARGET ${lib})
       list(APPEND RUNTIME_LIBRARIES ${lib})
     endif()
   endforeach()
 
   if(ARG_ALWAYS_CLEAN)
     set(always_clean clean)
   endif()
 
-  list(FIND TOOLCHAIN_TOOLS clang FOUND_CLANG)
-  if(FOUND_CLANG GREATER -1)
+  if(clang IN_LIST TOOLCHAIN_TOOLS)
     set(CLANG_IN_TOOLCHAIN On)
   endif()
 
   if(RUNTIME_LIBRARIES AND CLANG_IN_TOOLCHAIN)
     list(APPEND TOOLCHAIN_BINS ${RUNTIME_LIBRARIES})
   endif()
 
   set(STAMP_DIR ${CMAKE_CURRENT_BINARY_DIR}/${name}-stamps/)

llvm/cmake/modules/LLVMProcessSources.cmake

foreach(g ${globbed})
     else()
       set(entry "${fn}")
     endif()
     get_filename_component(gp ${g} REALPATH)
 
     # Don't reject hidden files. Some editors create backups in the
     # same directory as the file.
     if (NOT "${fn}" MATCHES "^\\.")
-      list(FIND LLVM_OPTIONAL_SOURCES ${entry} idx)
-      if( idx LESS 0 )
-        list(FIND listed ${gp} idx)
-        if( idx LESS 0 )
+      if(NOT ${entry} IN_LIST LLVM_OPTIONAL_SOURCES)
+        if(NOT ${gp} IN_LIST listed)
           if(ARG_SOURCE_DIR)
               set(fn_relative "${ARG_SOURCE_DIR}/${fn}")
           else()
               set(fn_relative "${fn}")
           endif()
           message(SEND_ERROR "Found unknown source file ${fn_relative}
 Please update ${CMAKE_CURRENT_LIST_FILE}\n")
         endif()
       endif()
     endif()
   endforeach()
 endfunction(llvm_check_source_file_list)

llvm/cmake/modules/TableGen.cmake

function(tablegen project ofn)
   endif()
 
   if (IS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
     set(LLVM_TARGET_DEFINITIONS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
   else()
     set(LLVM_TARGET_DEFINITIONS_ABSOLUTE
       ${CMAKE_CURRENT_SOURCE_DIR}/${LLVM_TARGET_DEFINITIONS})
   endif()
-  if (LLVM_ENABLE_DAGISEL_COV)
-    list(FIND ARGN "-gen-dag-isel" idx)
-    if( NOT idx EQUAL -1 )
-      list(APPEND LLVM_TABLEGEN_FLAGS "-instrument-coverage")
-    endif()
-  endif()
-  if (LLVM_ENABLE_GISEL_COV)
-    list(FIND ARGN "-gen-global-isel" idx)
-    if( NOT idx EQUAL -1 )
-      list(APPEND LLVM_TABLEGEN_FLAGS "-instrument-gisel-coverage")
-      list(APPEND LLVM_TABLEGEN_FLAGS "-gisel-coverage-file=${LLVM_GISEL_COV_PREFIX}all")
-    endif()
-  endif()
-  if (LLVM_OMIT_DAGISEL_COMMENTS)
-    list(FIND ARGN "-gen-dag-isel" idx)
-    if (NOT idx EQUAL -1)
-      list(APPEND LLVM_TABLEGEN_FLAGS "-omit-comments")
-    endif()
-  endif()
+  if (LLVM_ENABLE_DAGISEL_COV AND "-gen-dag-isel" IN_LIST ARGN)
+  if (LLVM_ENABLE_GISEL_COV AND "-gen-global-isel" IN_LIST ARGN)
+  if (LLVM_OMIT_DAGISEL_COMMENTS AND "-gen-dag-isel" IN_LIST ARGN)
 
   # MSVC can't support long string literals ("long" > 65534 bytes)[1], so if there's
   # a possibility of generated tables being consumed by MSVC, generate arrays of
   # char literals, instead. If we're cross-compiling, then conservatively assume
   # that the source might be consumed by MSVC.
   # [1] https://docs.microsoft.com/en-us/cpp/cpp/compiler-limits?view=vs-2017
   if (MSVC AND project STREQUAL LLVM)
     list(APPEND LLVM_TABLEGEN_FLAGS "--long-string-literals=0")