[PATCH] D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE

[Dan Liew via Phabricator via llvm-commits]

delcypher added inline comments.


================
Comment at: cmake/Modules/CompilerRTUtils.cmake:356
+  set(headers "")
+  if (XCODE OR MSVC_IDE OR CMAKE_EXTRA_GENERATOR)
+    # For IDEs we need to tell CMake about header files.
----------------
george.karpenkov wrote:
> Since we are not testing on MSVC should we really handle it here?
This conditional is taken from `cmake/modules/HandleLLVMOptions.cmake` (LLVM repo) and is the expression used to initialize the default value of `LLVM_ENABLE_IDE`. Given that support for IDE folders was already added to compiler-rt for an in-tree build for Visual Studio (r275111) at least in one point in time Visual Studio was used to build compiler-rt.

I'd rather keep the `MSVC_IDE` condition and only remove it if the bots break.


================
Comment at: cmake/Modules/CompilerRTUtils.cmake:361
+    foreach (header_dir ${ARG_ADDITIONAL_HEADER_DIRS})
+      compiler_rt_find_headers_in_dir(headers_in_dir "${header_dir}")
+      list(APPEND headers ${headers_in_dir})
----------------
george.karpenkov wrote:
> I'm not sure what do we need here. Could we just ask Clang on each source file which headers it requires? I believe that's what CMake does behind the scenes to support incremental compilation, it's just harder for compiler-rt since we are not using the default compiler.
> Could we just ask Clang on each source file which headers it requires? I believe that's what CMake does behind the scenes to support incremental compilation,

No. First we can't assume we are using clang. Compiler-rt is also built by gcc (and possibly by MSVC). Also incremental compilation is actually dealt with (at least in the case of the Makefile and Ninja generators) by the generated build system by having the first invocation of a target generate dependency files that are later consumed by re-compilations of the same target.

What I'm doing here is simply what the LLVM project does (apart from TSan which already had a list lying around that I could just use). I would actually prefer to do what @phosek suggests and list all header files explicitly, however I'm not sure if people will agree with that because that's not what is done in the LLVM project.


https://reviews.llvm.org/D48422