[PATCH] D57535: [CMake] Use LLVM_ENABLE_PROJECTS as the "single source" of truth when used.

[Dan Liew via Phabricator via llvm-commits]

delcypher marked 2 inline comments as done.
delcypher added inline comments.


================
Comment at: llvm/CMakeLists.txt:136-137
+    string(REGEX REPLACE "-" "_" upper_proj ${upper_proj})
+    list(FIND LLVM_ENABLE_PROJECTS "${proj}" PROJECT_INDEX)
+    if (${PROJECT_INDEX} EQUAL -1)
+      message(STATUS "${proj} project is disabled")
----------------
smeenai wrote:
> You can use `IN_LIST` instead.
Looks like `IN_LIST` landed in CMake 3.3. I hadn't realised that we moved to CMake 3.4.3 as the minimum version so this change should be fine.


================
Comment at: llvm/CMakeLists.txt:167-168
+endif()
+unset(SHOULD_ENABLE_PROJECT)
+unset(PROJECT_INDEX)
 
----------------
smeenai wrote:
> It's not really common in LLVM to unset CMake variables after use, though of course it doesn't hurt either.
I find CMake's variable scoping here to be really unhelpful. Because the variables that were created in loop persist outside it there's a risk of accidentally using them later on (e.g. autocomplete typo). By unsetting them as soon as were done with them we increase the chance  that using them accidentally will be noticed.


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

https://reviews.llvm.org/D57535