Skip to content

Conversation

nico
Copy link
Contributor

@nico nico commented Sep 26, 2023

GCC added support for the flag in gcc 13.1.

No behavior change.

GCC added support for the flag in gcc 13.1.

No behavior change.
@nico nico requested a review from a team as a code owner September 26, 2023 13:31
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 26, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 26, 2023

@llvm/pr-subscribers-libcxx

Changes

GCC added support for the flag in gcc 13.1.

No behavior change.


Full diff: https://github.com/llvm/llvm-project/pull/67429.diff

1 Files Affected:

  • (modified) libcxx/CMakeLists.txt (+4-2)
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index b9d0ed51be26033..c42337fa83062f7 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -624,9 +624,11 @@ function(cxx_link_system_libraries target)
 
 # In order to remove just libc++ from the link step
 # we need to use -nostdlib++ whenever it is supported.
-# Unfortunately this cannot be used universally because for example g++ supports
-# only -nodefaultlibs in which case all libraries will be removed and
+# Unfortunately this cannot be used universally because g++ does not support
+# -nostdlib++ before gcc 13 (https://gcc.gnu.org/gcc-13/changes.html) and needs
+# to use -nodefaultlibs in which case all libraries will be removed and
 # all libraries but c++ have to be added in manually.
+# Once libc++ requires gcc 13+, this can be simplified.
   if (CXX_SUPPORTS_NOSTDLIBXX_FLAG)
     target_add_link_flags_if_supported(${target} PRIVATE "-nostdlib++")
   else()

@philnik777
Copy link
Contributor

libc++ already requires GCC13, so this can be simplified now.

@ldionne
Copy link
Member

ldionne commented Sep 27, 2023

libc++ already requires GCC13, so this can be simplified now.

I think the real reason why we're still checking for whether -nostdlib++ is supported is to handle Clang-cl, which doesn't support it IIUC. I think we could probably simplify that to be something like

if (MSVC)
  target_add_link_flags(${target} PRIVATE "/nodefaultlibs")
else()
  target_add_link_flags(${target} PRIVATE "-nostdlib++")
endif()

but then the rest of the function where we link against various system libraries will have to change too (we could drop a bunch of complexity on non-MSVC envs). @nico Do you want to go for that or do you want us to pick that up?

@nico
Copy link
Contributor Author

nico commented Oct 11, 2023

Feel free to pick it up, thanks!

@nico nico closed this Oct 11, 2023
@nico nico deleted the libcxx-gcc-13-comment branch October 11, 2023 02:11
@ldionne
Copy link
Member

ldionne commented Oct 11, 2023

@nico Here's my attempt to clean this up a bit further: #68832

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants