[PATCH] D12488: Enable linking tools, shared libraries against libLLVM

Andrew Wilkins via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 31 23:07:08 PDT 2015


axw added inline comments.

================
Comment at: cmake/modules/AddLLVM.cmake:44
@@ -43,3 +43,3 @@
   #   - PROPERTY COMPILE_FLAGS is string.
-  string(REPLACE ";" " " target_compile_flags "${LLVM_COMPILE_FLAGS}")
+  string(REPLACE ";" " " target_compile_flags " ${LLVM_COMPILE_FLAGS}")
 
----------------
chapuni wrote:
> Is it relevant to your changes?
> FYI, it triggers recompilation of whole files (on ninja).
> Is it relevant to your changes?

This was to fix a bug I found while making my changes. In some cases, COMPILE_FLAGS did not end with whitespace, so argument lists were being concatenated badly.

> FYI, it triggers recompilation of whole files (on ninja).

Once only, right? Is that a problem?

================
Comment at: cmake/modules/TableGen.cmake:74
@@ -73,3 +73,3 @@
 macro(add_tablegen target project)
   set(${target}_OLD_LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS})
   set(LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS} TableGen)
----------------
chapuni wrote:
> I thought it'd be just a variable, not an option, IMO.
> 
> The name "DISABLE_LLVM_LINK_LLVM_DYLIB" may be shorten.
I'm happy to change the name, given suggestions. It's likely only ever going to be used in two places, so I don't think worth spending much energy on.


http://reviews.llvm.org/D12488





More information about the llvm-commits mailing list