[PATCH] D13739: [libcxx] Make libc++.so a linker script by default on most platforms.
Jonathan Roelofs via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 14 14:42:04 PDT 2015
jroelofs 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)
----------------
Should probably make this a cmake error, not just have it do something other than what you wanted.
================
Comment at: CMakeLists.txt:76
@@ +75,3 @@
+# will link libc++ to the correct ABI library. This option is ON by default
+# On UNIX platforms other than Apple unless 'LIBCXX_ENABLE_STATIC_ABI_LIBRARY'
+# is ON.
----------------
s/On/on/ ?
================
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)
----------------
`DEFAULT_VALUE` seems too generic a name for this... or is this a common CMake pattern that I'm unaware of?
================
Comment at: cmake/Modules/HandleOutOfTreeLLVM.cmake:116
@@ +115,3 @@
+ if(CYGWIN)
+ set(LLVM_ON_WIN32 0)
+ set(LLVM_ON_UNIX 1)
----------------
Where are `LLVM_ON_WIN32` and `LLVM_ON_UNIX` used?
================
Comment at: docs/BuildingLibcxx.rst:176
@@ +175,3 @@
+ **Default**: ``OFF`` On Windows and Apple or when
+ ``LIBCXX_ENABLE_STATIC_ABI_LIBRARY`` is specified and ``ON`` otherwise.
+
----------------
I find this sentence hard to parse. Maybe phrase it the same way you did in the comment:
"ON by default on UNIX platforms other than Apple unless 'LIBCXX_ENABLE_STATIC_ABI_LIBRARY' is ON"
================
Comment at: docs/UsingLibcxx.rst:59
@@ +58,3 @@
+some libc++ installations require the user manually link libc++abi themselves.
+If you are running into linker errors when using libc++ try adding "-lc++abi"
+to the link line. For example:
----------------
Looks like an inconsistent use of `"` vs `'` for `'-stdlib=libc++'` and `"-lc++abi"`. I don't have good RST-fu, but that seems fishy.
http://reviews.llvm.org/D13739
More information about the cfe-commits
mailing list