[PATCH] D28869: [CMake] Fix `is_llvm_target_library` and support out-of-order components

[bryant via Phabricator via llvm-commits]

bryant added inline comments.


================
Comment at: cmake/modules/LLVM-Config.cmake:33
   endforeach()
 endfunction(is_llvm_target_library)
 
----------------
(See the inline comment after this one for justication.)

```
@@ -32,6 +32,28 @@ function(is_llvm_target_library library return_var)
   endforeach()
 endfunction(is_llvm_target_library)

+function(is_omitted_target_lib library return_var)
+  set(${return_var} OFF PARENT_SCOPE)
+  string(TOUPPER "${library}" capitalized_lib)
+  set(omitted_targets ${LLVM_ALL_TARGETS})
+  list(REMOVE_ITEM omitted_targets ${LLVM_TARGETS_TO_BUILD})
+  string(TOUPPER "${omitted_targets}" targets)
+  foreach(t ${targets})
+    if( capitalized_lib STREQUAL t OR
+        capitalized_lib STREQUAL "${t}" OR
+        capitalized_lib STREQUAL "${t}DESC" OR
+        capitalized_lib STREQUAL "${t}CODEGEN" OR
+        capitalized_lib STREQUAL "${t}ASMPARSER" OR
+        capitalized_lib STREQUAL "${t}ASMPRINTER" OR
+        capitalized_lib STREQUAL "${t}DISASSEMBLER" OR
+        capitalized_lib STREQUAL "${t}INFO" OR
+        capitalized_lib STREQUAL "${t}UTILS" OR
+        capitalized_lib STREQUAL "${t}INSTPRINTER" )
+      set(${return_var} ON PARENT_SCOPE)
+      break()
+    endif()
+  endforeach()
+endfunction(is_omitted_target_lib)

 macro(llvm_config executable)
   cmake_parse_arguments(ARG "USE_SHARED" "" "" ${ARGN})
```


================
Comment at: cmake/modules/LLVM-Config.cmake:211
+          list(APPEND expanded_components LLVM${c})
         endif()
       else( lib_idx LESS 0 )
----------------
I might be reading this wrong, but the logic here seems to be:

- Check if $c is a target lib (i.e., LLVMAArch64Utils) regardless of whether its corresponding target will be built;
  - If it is, then ignore it.
  - Otherwise, assume that it's a yet-to-be-scanned library.

This seems a bit off, since I recall you mentioning that "adding target libraries that are configured out is a bad idea." I think what we need is:

- Check if $c is a target lib that will not be built.
  - If it is, ignore it.
  - Otherwise, it's either a yet-to-be-scanned {normal,target} lib.

```
@@ -201,12 +223,12 @@ function(llvm_map_components_to_libnames out_libs)
       list(FIND capitalized_libs LLVM${capitalized} lib_idx)
       if( lib_idx LESS 0 )
         # The component is unknown. Maybe is an omitted target?
-        is_llvm_target_library(${c} iltl_result)
-        if( NOT iltl_result )
-          # If it is not an omitted target we should assume it is a component
-          # that hasn't yet been processed by CMake. Missing components will
-          # cause errors later in the configuration, so we can safely assume
-          # that this is valid here.
+        is_omitted_target_lib(${c} iltl_result)    # see above inline comment
+        if(iltl_result)
+          message(FATAL_ERROR "Depended on ${c}, whose target isn't builts.")
+        else()
+          # either target lib or a normal lib that will be built but has yet to
+          # be scanned.
           list(APPEND expanded_components "$<LINK_ONLY:LLVM${c}>")
         endif()
       else( lib_idx LESS 0 )
```

This seems to generate the right build.ninja modulo reordered dependencies.


https://reviews.llvm.org/D28869