[Clang-CL][DXC] Expose -fdiagnostic-color= to clang-cl and clang-dxc

[whiteio]

Exposing -fdiagnostic-color= to clang-cl and clang-dxc. -fcolor-diagnostics and -fno-color-diagnostics are already allowed in both of these and -fdiagnostics-color= allows one additional value, auto.

I've added the tests for clang-cl to cl-options.c as per the comments in the issue linked below. I couldn't finding a suitable existing file to add the clang-dxc tests to so I've created a new one.

Resolves #119184

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Chris White (whiteio)

Changes

Exposing -fdiagnostic-color= to clang-cl and clang-dxc. -fcolor-diagnostics and -fno-color-diagnostics are already allowed in both of these and -fdiagnostics-color= allows one additional value, auto.

I've added the tests for clang-cl to cl-options.c as per the comments in the issue linked below. I couldn't finding a suitable existing file to add the clang-dxc tests to so I've created a new one.

Resolves #119184

---

Full diff: https://github.com/llvm/llvm-project/pull/120644.diff

3 Files Affected:

• (modified) clang/include/clang/Driver/Options.td (+1-1)
• (modified) clang/test/Driver/cl-options.c (+9)
• (added) clang/test/Driver/dxc_options.hlsl (+8)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 638f8c52053ec5..ad64626d960d43 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1990,7 +1990,7 @@ def : Flag<["-"], "fno-diagnostics-color">, Group<f_Group>,
   Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>,
   Alias<fno_color_diagnostics>;
 def fdiagnostics_color_EQ : Joined<["-"], "fdiagnostics-color=">, Group<f_Group>,
-  Visibility<[ClangOption, FlangOption]>,
+  Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>,
   Values<"auto,always,never">,
   HelpText<"When to use colors in diagnostics">;
 def fansi_escape_codes : Flag<["-"], "fansi-escape-codes">, Group<f_Group>,
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 477e8489e74280..fe57a5232b2fdc 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -664,6 +664,15 @@
 // RUN: not %clang_cl /guard:foo -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARDINVALID %s
 // CFGUARDINVALID: invalid value 'foo' in '/guard:'
 
+// The test doesn't run in a PTY, so "auto" defaults to off.
+// RUN: %clang_cl -fdiagnostics-color=auto -### -- %s 2>&1 | FileCheck -check-prefix=NO_COLOR %s
+
+// RUN: %clang_cl -fdiagnostics-color -### -- %s 2>&1 | FileCheck -check-prefix=COLOR %s
+// RUN: %clang_cl -fdiagnostics-color=always -### -- %s 2>&1 | FileCheck -check-prefix=COLOR %s
+// RUN: %clang_cl -fdiagnostics-color=never -### -- %s 2>&1 | FileCheck -check-prefix=NO_COLOR %s
+// COLOR: "-fcolor-diagnostics"
+// NO_COLOR-NOT: "-fcolor-diagnostics"
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
diff --git a/clang/test/Driver/dxc_options.hlsl b/clang/test/Driver/dxc_options.hlsl
new file mode 100644
index 00000000000000..9bd4b61cdcf19d
--- /dev/null
+++ b/clang/test/Driver/dxc_options.hlsl
@@ -0,0 +1,8 @@
+// The test doesn't run in a PTY, so "auto" defaults to off.
+// RUN: %clang_dxc -Tlib_6_7 -fdiagnostics-color=auto -### -- %s 2>&1 | FileCheck -check-prefix=NO_COLOR %s
+
+// RUN: %clang_dxc -Tlib_6_7 -fdiagnostics-color -### %s 2>&1 | FileCheck -check-prefix=COLOR %s
+// RUN: %clang_dxc -Tlib_6_7 -fdiagnostics-color=always -### %s 2>&1 | FileCheck -check-prefix=COLOR %s
+// RUN: %clang_dxc -Tlib_6_7 -fdiagnostics-color=never -### %s 2>&1 | FileCheck -check-prefix=NO_COLOR %s
+// COLOR: "-fcolor-diagnostics"
+// NO_COLOR-NOT: "-fcolor-diagnostics"
\ No newline at end of file

[Maetveis]

Thank you, mostly looks good, I think the tests can be made simpler. And I think adding release notes would benefit users.
You can add a new point to "New Compiler Flags" in the clang release notes that clang-cl and clang-dxc now also accept-fdiagnostics-color=[auto|never|always] in addition to -f[no-]color-diagnostics.

[whiteio]

Thanks @Maetveis, I've addressed the comments.

[Maetveis]

Great work, Thank you very much!