[PATCH] D27603: Propagate -fdiagnostics-color to LLD.

[Richard Smith via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Driver/Tools.cpp:284
+  // propagate -fdiagnostics-color.
+  if (StringRef(TC.GetLinkerPath()).endswith("ld.lld") &&
+      D.getDiags().getShowColors())
----------------
ruiu wrote:
> rsmith wrote:
> > I don't think this will work for `-fuse-ld=$BINDIR/lld` and the like. We also have code around Tools.cpp doing things like this:
> > 
> >     const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
> >     if (llvm::sys::path::filename(Exec) == "lld") {
> > 
> > and
> > 
> >     const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
> >     if (llvm::sys::path::stem(Exec).equals_lower("lld")) {
> > 
> > so detecting `ld.lld` here seems inconsistent. (Meanwhile, the above checks won't fire for `-fuse-ld=lld`, since that sets the linker path to .../ld.lld.)
> > 
> > I think it's time to factor out an `isLinkerLLD` function and use that to detect whether we're using lld.
> What if a linker is just `/usr/bin/ld`? I want to detect if it's LLD or not, but it's not doable without running that command. (If we pass it via the environment, we can always set LLD_COLOR_DIAGNOSTICS=1 though.)
Maybe the best we can do is have each target tell us what it expects `/usr/bin/ld` to be, and/or `readlink` it and see if it is a symlink to a binary named `lld`. This doesn't seem like a very satisfying answer, though :(

We already pass some flags if we think we're running `lld` (for instance, `-flavor gnu -target $TARGET`); passing those through the environment doesn't seem ideal.


https://reviews.llvm.org/D27603