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

2016-12-12 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu added a comment. Nico suggested I make change to CMake instead of Clang so that CMake adds -Wl,-color-diagnostics if it detects the linker can accept that option. I think that makes sense, so I'll look into it. https://reviews.llvm.org/D27603

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

2016-12-09 Thread 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

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

2016-12-09 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu added inline comments. Comment at: lib/Driver/Tools.cpp:284 + // propagate -fdiagnostics-color. + if (StringRef(TC.GetLinkerPath()).endswith("ld.lld") && + D.getDiags().getShowColors()) rsmith wrote: > I don't think this will work for

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

2016-12-09 Thread 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()) I don't think this will work for `-fuse-ld=$BINDIR/lld` and

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

2016-12-09 Thread Hans Wennborg via Phabricator via cfe-commits
hans added inline comments. Comment at: lib/Driver/Tools.cpp:234 const ArgList , ArgStringList , const JobAction ) { const Driver = TC.getDriver(); ruiu wrote: > hans wrote: > > Yes, this doesn't seem

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

2016-12-09 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu added inline comments. Comment at: lib/Driver/Tools.cpp:234 const ArgList , ArgStringList , const JobAction ) { const Driver = TC.getDriver(); hans wrote: > Yes, this doesn't seem like exactly