[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 llvm-commits at lists.llvm.org
Tue Jun 26 06:53:27 PDT 2018


delcypher added a comment.

In https://reviews.llvm.org/D48422#1142634, @phosek wrote:

> Personally, I'd prefer to spell out all the headers explicitly rather than using globing which is more error-prone to future changes. I also agree with @kubamracek's comment about using `crt` (which may be also mistaken for `crt*.o`) to `compiler_rt`.


My patch does allow for this (use `ADDITIONAL_HEADERS` argument). In fact I used that for `clang_rt.tsan` because there was already a list of TSan header files (previously unused) already there.

I already mentioned (in the commit message) that globbing for sources is generally a CMake anti-pattern. However this is the approach used the in the LLVM CMake code, which is why I adopted it.

I have no problem listing the header files explicitly, but I don't know how the rest of the compiler-rt developers feel about this. I'll add a few more reviewers to see if we can get a consensus.



================
Comment at: cmake/Modules/AddCompilerRT.cmake:61
+  # Add headers to LIB_SOURCES for IDEs
+  crt_process_sources(LIB_SOURCES
+    ${LIB_SOURCES}
----------------
kubamracek wrote:
> Please rename "crt" to "compiler_rt" (throughout the patch). I don't think we're using "crt" to mean "compiler-rt" anywhere in the codebase.
Sure. I'll fix that.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D48422





More information about the llvm-commits mailing list