This is an archive of the discontinued LLVM Phabricator instance.

Strip preceeding -Xclang when stripping -fcolor-diagnostics or -fdiagnostics-color

Authored by DaanDeMeyer on Feb 23 2020, 7:07 AM.



Fixes We were removing the color options but not the preceeding -Xclang which causes errors since the -Xclang would now apply to the next option in the list of options. Now, when removing a color option, we check if there was a preceeding -Xclang and remove it as well.

Diff Detail

Event Timeline

DaanDeMeyer created this revision.Feb 23 2020, 7:07 AM

Thanks, LGTM. But please upload the diff with full context, see and

We've also discussed this offline and think that a more principled solution is necessary in clangd side to switch from "string manipulations" to a more semantic approach.

In the meantime this fix sounds like a good short term solution, maybe even a long term one for clang-tools that will keep using these adjusters. WDYT @sammccall ?

sammccall accepted this revision.Feb 25 2020, 2:10 AM

LGTM too, though the overall solution space here is a bit sad:

  • obviously there's lots of other instances of this pattern that should really be fixed as long as the arg-adjuster API is supported
  • driver is such a mess that patching the CompilerInvocation afterwards is also really hard in many cases, though not this one (I prototyped this)

Maybe add a FIXME: this should apply to most arg adjusters!

This revision is now accepted and ready to land.Feb 25 2020, 2:10 AM

Added FIXME.

I do not have commit access. It would be great if someone could merge this for me.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 26 2020, 12:15 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript