[PATCH] D152090: [clang][Driver] Add -fcaret-diagnostics-max-lines as a driver option

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 9 13:10:00 PDT 2023


MaskRay added a comment.

In D152090#4407632 <https://reviews.llvm.org/D152090#4407632>, @dyung wrote:

> If we do not accept the form "`-fcaret-diagnostics-max-lines n`", can we add a negative for that? Or if we do a positive test for it as well?

I think this is not very useful. This is a new option. Separate-form driver options are unusual and typically legacy GCC options (before 2000 or early 2000s).
If we have tested `-fcaret-diagnostics-max-lines=N`, it is unnecessary to test `-fcaret-diagnostics-max-lines N`.



================
Comment at: clang/test/Driver/caret-diagnostics-max-lines.cpp:1
+//RUN: not %clang++ -fsyntax-only -fcaret-diagnostics-max-lines=2 %s 2>&1 | FileCheck %s -strict-whitespace
+
----------------
tbaeder wrote:
> MaskRay wrote:
> > The `test/Driver` test just tests that `-fcaret-diagnostics-max-lines=2` is forwarded to CC1. The semantic tests are performed in other test directories, `Misc/caret-diags-multiline.cpp` in this case.
> Do you want me to remove the semantic part in this file or add a `RUN` line to `caret-diags-multiline.cpp` using `%clang`?
`clang/test/Driver/caret-diagnostics-max-lines.cpp` should just check that the driver option `-fcaret-diagnostics-max-lines=N` is translated to `-cc1 {{.*}}"-fcaret-diagnostics-max-lines=N"`.

The functionality feature is tested in other test directories, and should not be duplicated in `test/Driver/`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152090/new/

https://reviews.llvm.org/D152090



More information about the cfe-commits mailing list