[PATCH] D85309: [Driver] Support GNU ld on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro marked an inline comment as done.
ro added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:305
 
+bool tools::isLinkerGnuLd(const ToolChain &TC, const ArgList &Args) {
+  // Only used if targetting Solaris.
----------------
MaskRay wrote:
> I suppose that this should be in a Solaris specific file to indicate that it's not for other systems.
> 
> GNU ld is almost ubiquitous on Linux and is almost always available at /usr/bin/ld (with very few distributions using others linkers by default or providing an option).
> 
> Detecting linker to affect driver decisions is we Linux are very wary of. We are nervous even trying to do some stuff only related to lld.
> 
> We likely don't want this function to be in CommonArgs to lure other contributors to use.
> I suppose that this should be in a Solaris specific file to indicate that it's not for other systems.

Indeed.  I've moved it to `Solaris.{h,cpp}` now.  In fact, initially i'd tried to assert a Solaris target in `isLinkerGnuLd`, but that cannot work because in some cases the function is called in common code, even though the result is only used on Solaris.

I briefly thought about generalizing both `IsLinkerGnuLd` and `IsLinkerLLD` into a common
```
enum LinkerFlavour getLinkerFlavor();
```
but doing this for all possible configs seemed like a can of worms I'd rather not open.

> GNU ld is almost ubiquitous on Linux and is almost always available at /usr/bin/ld (with very few distributions using others linkers by default or providing an option).
> 
> Detecting linker to affect driver decisions is we Linux are very wary of. We are nervous even trying to do some stuff only related to lld.

I saw: that code is only used on Darwin, it seems, and I'd like to avoid touching that.

> We likely don't want this function to be in CommonArgs to lure other contributors to use.

Apart from the move to `Solaris.h`, I've also moved it into the `solaris` namespace to make things completely obvious.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85309/new/

https://reviews.llvm.org/D85309