[PATCH] D93987: [ASTMatchers] Fix build when no targets are enabled

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 5 05:51:46 PST 2021


aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/cmake/modules/LLVM-Config.cmake:31
     set(omitted_targets ${LLVM_ALL_TARGETS})
-    list(REMOVE_ITEM omitted_targets ${LLVM_TARGETS_TO_BUILD})
+    if (LLVM_TARGETS_TO_BUILD)
+      list(REMOVE_ITEM omitted_targets ${LLVM_TARGETS_TO_BUILD})
----------------
steveire wrote:
> aaron.ballman wrote:
> > I'm not a CMake expert, so can you explain what effect this change has? From the documentation (and my own limited understanding of cmake), I would have expected this to be a noop -- if the list of elements is empty, nothing will get removed from `omitted_targets`, right?
> 
> You can reproduce the issue in a small testcase with this file:
> 
> ```
> set(omitted_targets one two three)
> 
> # Comment this to reproduce the failure
> set(LLVM_TARGETS_TO_BUILD X86)
> 
> list(REMOVE_ITEM omitted_targets ${LLVM_TARGETS_TO_BUILD})
> 
> ```
> 
> Run this after commenting the line:
> 
> ```
> $ cmake -P cmaketest.cmake
> CMake Error at cmaketest.cmake:6 (list):
>   list sub-command REMOVE_ITEM requires two or more arguments.
> ```
> 
Huh, wow, how unintuitive! Thank you for the explanation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93987/new/

https://reviews.llvm.org/D93987



More information about the llvm-commits mailing list