[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