[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

Thorsten via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 19 16:35:38 PDT 2019


thorsten-klein added a comment.

Hello, 
can you please support with this Pull-Request? You really have better knowledge about source code. 
For our case this solution was working fine and I wanted to share with you. Maybe it will help you if I create an examplary project which reproduces the mentioned issue?



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:454
+  StringRef FileName(Loc.printToString(Loc.getManager()));
+  if(getHeaderFilter()->match(FileName))
+  {
----------------
alexfh wrote:
> aaron.ballman wrote:
> > Formatting is incorrect here -- you should run the patch through clang-format (https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting), and elide the braces.
> I believe, this will break the handling of notes. If the notes attached to the diagnostic are in the "interesting" part of the code (see the checkFilters method below), the whole diagnostic should be treated as such.
Hello,
Unfortunately the emitDiagnostic method is called also for files, which should be actually excluded (since the FileName does not match the HeaderFilter regex). This results in diagnostic output, also if there should not be any. For this purpose I added this check here at this point. If the FileName is matching the HeaderFilter regex, then all diagnostic is emitted as before. 

Did you know about any issue regarding this? If this is not a working solution for you: do you have any other proposal?

PS: I have also read about this issue in another project: https://fuchsia.googlesource.com/peridot/+/HEAD/docs/lint.md
--> They say that they disable some clang-tidy checks as workaround


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

https://reviews.llvm.org/D59135





More information about the cfe-commits mailing list