[PATCH] D48116: [libclang] Allow skipping warnings from all included files

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.
Herald added a subscriber: arphaman.

In D48116#1144732 <https://reviews.llvm.org/D48116#1144732>, @ilya-biryukov wrote:

> Have you considered doing the same filtering in ASTUnit's `StoredDiagnosticConsumer`? It should not be more difficult and allows to avoid changing the clang's diagnostic interfaces. That's what we do in clangd.


Will check.

> I wonder if you want to handle notes and remarks in a special manner? They can be seen as part of the original diagnostic, rather than the separate diagnostic. E.g. showing a note in the main file, but not showing the diagnostic from the headers file that this note comes from, might be confusing to the users.

Looks like this case is handled fine.

WIthout the new option, a note is printed as expected/always (shortened output):

  $ bin/c-index-test -test-load-source function ignore-warnings-from-headers.cpp -Wunused-parameter
  ignore-warnings-from-headers.h:1:12: warning: unused parameter 'unusedInHeader' [-Wunused-parameter]
  ignore-warnings-from-headers.cpp:1:10: note: in file included from ignore-warnings-from-headers.cpp:1:
  ignore-warnings-from-headers.cpp:3:12: warning: unused parameter 'unusedInMainFile' [-Wunused-parameter]

With the new option, note is not printed and thus no confusion should arise:

  $ CINDEXTEST_IGNORE_NONERRORS_FROM_INCLUDED_FILES=1 bin/c-index-test -test-load-source function ignore-warnings-from-headers.cpp -Wunused-parameter
  ignore-warnings-from-headers.cpp:3:12: warning: unused parameter 'unusedInMainFile' [-Wunused-parameter]

I will add "// CHECK-NOT: note: in file included from" to the test.

> Maybe also add tests for diagnostics in the main file with notes/remarks in the header files and vice versa?

"Vice versa" is covered as shown above. If the diagnostic is in main file and a note of that one refers to the header, then the note should be shown/included. This case seems also fine - I've added a test for this.

In D48116#1144838 <https://reviews.llvm.org/D48116#1144838>, @yvvan wrote:

> But this one misses a way to set this flag for everything except libclang. We can probably change that (Nikolai is in vacation till the end of summer so it's probably my part now) and add some tests outside of Index for it (probably Frontend)


What's the use case of this?


Repository:
  rC Clang

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

https://reviews.llvm.org/D48116