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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
