Page MenuHomePhabricator

[Driver] Fix driver support for color diagnostics
ClosedPublic

Authored by bruno on May 18 2016, 4:59 PM.

Details

Summary

Diagnostics that happen during driver time do not have color output
support unless -fcolor-diagonostic is explicitly passed into the driver.
OTOH, it works great for cc1 mode since dianostic arguments are properly
handled and color is enabled by default if the terminal supports it.

This patch fix this behavior by using the same logic present in -cc1
argument construction to decide whether it should use colors. I tried to
find a way to refactor this logic into a common place that could be used
from both lib/Driver/Tools.cpp and lib/Frontend/CompilerInvocation.cpp,
but could not see any options besides libBasic, which also seems wrong
because it's not the ideal place to put options parsing code either. So
I duplicated part of the logic and changed some driver unrelated logic.
(look for ShowColors in lib/Driver/Tools.cpp for details). Ideas welcome.

I do not see a way to write a testcase for this.

Diff Detail

Event Timeline

bruno updated this revision to Diff 57713.May 18 2016, 4:59 PM
bruno retitled this revision from to [Driver] Fix driver support for color diagnostics.
bruno updated this object.
bruno added a reviewer: rsmith.
bruno added subscribers: cfe-commits, dexonsmith.
silvas added a subscriber: silvas.May 18 2016, 6:54 PM

I don't see an issue with putthing this as a helper in libBasic. We may need to add a libOption dependency to it but that sounds fine.

I don't think we even need a helper.

Clang::ConstructJob takes a Compilation, which has a Driver, which has a DiagnosticsEngine, which has a DiagnosticsOptions.

In other words, I think you can delete the code in Clang::ConstructJob (change it to a lookup) now that you've changed clang::ParseDiagnosticArgs to update DiagnosticsOptions.

bruno updated this revision to Diff 57993.May 20 2016, 2:37 PM
bruno added reviewers: dexonsmith, silvas.
bruno removed subscribers: silvas, dexonsmith.

Great idea Duncan, attached a new patch!

dexonsmith edited edge metadata.May 26 2016, 2:27 PM
dexonsmith added a subscriber: dexonsmith.

LGTM.

bruno accepted this revision.May 31 2016, 11:59 AM
bruno added a reviewer: bruno.
This revision is now accepted and ready to land.May 31 2016, 11:59 AM
bruno closed this revision.May 31 2016, 12:00 PM

Committed in r271042