[PATCH] D13739: [libcxx] Make libc++.so a linker script by default on most platforms.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 14 14:57:06 PDT 2015


EricWF added inline comments.

================
Comment at: CMakeLists.txt:71
@@ -70,1 +70,3 @@
+# Use a static copy of the ABI library when linking libc++. This option
+# cannot be used with LIBCXX_ENABLE_ABI_LINKER_SCRIPT.
 option(LIBCXX_ENABLE_STATIC_ABI_LIBRARY "Statically link the ABI library" OFF)
----------------
jroelofs wrote:
> Should probably make this a cmake error, not just have it do something other than what you wanted.
It already is :-). Theres some logic near line #179.

================
Comment at: CMakeLists.txt:78
@@ +77,3 @@
+# is ON.
+set(DEFAULT_VALUE OFF)
+if (LLVM_HAVE_LINK_VERSION_SCRIPT AND NOT LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
----------------
jroelofs wrote:
> `DEFAULT_VALUE` seems too generic a name for this... or is this a common CMake pattern that I'm unaware of?
Nope, I was just being lazy with the name because it's not meant to be a global name but instead a scratch variable. I'll fix it anyway.

================
Comment at: cmake/Modules/HandleOutOfTreeLLVM.cmake:116
@@ +115,3 @@
+    if(CYGWIN)
+      set(LLVM_ON_WIN32 0)
+      set(LLVM_ON_UNIX 1)
----------------
jroelofs wrote:
> Where are `LLVM_ON_WIN32` and `LLVM_ON_UNIX` used?
Nowhere yet but they will be used to validate sanitizer flags and other UNIX specific behavior. The whole chunk is copy and pasted from HandleLLVMOptions.cmake


http://reviews.llvm.org/D13739





More information about the cfe-commits mailing list