[Clang-CL][DXC] Should we expose -fdiagnostics-color= to clang-cl / clang-dxc?

[Maetveis]

Apologies if the HLSL tag is not appropriate I did not find one for the dxc driver.

Recently while exposing more options to the Fortran flang driver, some options were accidentally exposed to clang-cl / clang-dxc too. Among them is -fdiagnostics-color=. Note that -fcolor-diagnostics and -fno-color-diagnostics are already allowed for both.
-fdiagnostics-color= allows one additional value auto which roughly means only use colors if the output is a terminal. This is also the default value of nothing is specified therefore, -fdiagnostics-color= is mainly useful to override an earlier -f(no)?-color-diagnostics / -fdiagnostics-color= option.

In #118640 I am looking to disable to flag again, mostly to keep the PR a clean fix and not introduce new features, but
personally I don't see a reason why this option shouldn't be exposed to clang-cl and clang-dxc. If we do we should definitely add test however.

[zmodem]

Supporting -fdiagnostics-color= in clang-cl sounds good to me. You can test the flag with the existing ones in clang/test/Driver/cl-options.c

[Maetveis]

So what needs to be done is enabling CLOption and DXCOption for Visibility in clang/include/clang/Driver/Options.td:1993, and adding a test for clang-cl at clang/test/Driver/cl-options.c and finding where to add a test for dxc.

Probably somewhere there is already testing for -fdiagnostics-color and -fno-diagnostics-color for clang-dxc adding -fdiagnostics-color= tests there would make sense.

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
3. Fix the issue locally.
4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
5. Create a Git commit.
6. Run git clang-format HEAD~1 to format your changes.
7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: Mészáros Gergely (Maetveis)

Apologies if the `HLSL` tag is not appropriate I did not find one for the `dxc` driver.

Recently while exposing more options to the Fortran flang driver, some options were accidentally exposed to clang-cl / clang-dxc too. Among them is -fdiagnostics-color=. Note that -fcolor-diagnostics and -fno-color-diagnostics are already allowed for both.
-fdiagnostics-color= allows one additional value auto which roughly means only use colors if the output is a terminal. This is also the default value of nothing is specified therefore, -fdiagnostics-color= is mainly useful to override an earlier -f(no)?-color-diagnostics / -fdiagnostics-color= option.

In #118640 I am looking to disable to flag again, mostly to keep the PR a clean fix and not introduce new features, but
personally I don't see a reason why this option shouldn't be exposed to clang-cl and clang-dxc. If we do we should definitely add test however.

[whiteio]

Hey! I'd like to work on this, could it be assigned to me please?