[PATCH] D23856: [libc++] Perform configuration checks with -nodefaultlibs

Shoaib Meenai via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 29 13:00:47 PDT 2016


smeenai added inline comments.

================
Comment at: cmake/config-ix.cmake:18
@@ +17,3 @@
+if (LIBCXX_SUPPORTS_NODEFAULTLIBS_FLAG)
+  list(APPEND CMAKE_REQUIRED_LIBRARIES -nodefaultlibs)
+  if (LIBCXX_HAS_C_LIB)
----------------
EricWF wrote:
> smeenai wrote:
> > smeenai wrote:
> > > compnerd wrote:
> > > > Can we not use `CMAKE_SHARED_LINKER_FLAGS` instead of `CMAKE_REQUIRED_LIBRARIES`?  It is slightly misleading.
> > > That would pollute everything building a shared library, whereas this limits it to only configuration checks. I know it's misleading, but I view it similar to how `target_link_libraries` is also used for specifying linker flags.
> > > 
> > > Speaking of pollution though, my `REQUIRED_*` changes in this file will end up affecting other projects for non-standalone builds, correct? What's the best way to limit this to be libc++-only?
> > Never mind. cmake automatically limits variables to the scope of the current directory, so this shouldn't pollute anything outside of libc++.
> > 
> > This should be good to review.
> `CMAKE_REQUIRED_LIBRARIES` should still only really be used for libraries. I think `CMAKE_REQUIRED_FLAGS` would be a better place to put this, if not `CMAKE_SHARED_LINKER_FLAGS`.
I'm fine moving it to `CMAKE_REQUIRED_FLAGS`. Lemme make that change and test it out and then re-upload it.


https://reviews.llvm.org/D23856





More information about the cfe-commits mailing list