-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Conversation
The Gold linker is not maintained any more by Google and it starts to have problems with linking code produced by newer compilers like clang 8.
@@ -113,10 +113,8 @@ that users do not need to install language packs for their OS. | |||
void setDefaultOrCLocale() | |||
{ | |||
#if __unix__ | |||
if (!std::setlocale(LC_ALL, "")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In libc++ the setlocale
is not in the std
namespace.
@@ -41,15 +41,6 @@ if (("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU") OR ("${CMAKE_CXX_COMPILER_ID}" MA | |||
set(CMAKE_CXX_FLAGS_RELEASE "-O3 -DNDEBUG") | |||
set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O2 -g") | |||
|
|||
option(USE_LD_GOLD "Use GNU gold linker" ON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, which linker will we use for clang builds if we don't use the gold linker?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GNU ld
. Also lld
from LLVM can be used (is much faster, I'm using it for building LLVM itself where linking time is huge). In the same way: -fuse-ld=lld
. But only clang supports it (GCC not).
As I explained in the commit message, Google has switched to use LLD, and they are not interested in maintaining ld.gold. Fedora is considering moving ld.gold out of the main package repo. And I have linking error when using clang-8. There must be new "linking features" that clang-8 depends on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go, it looks like the AppVeyor run timed out but that's probably not related to your changes since yours only impact Linux
No description provided.