alexfh added a comment.

My apologies again for the delay. There are a few things I'm concerned about:

1. Suppression list being a command-line option is going to be inconvenient to 
completely useless in many setups. The `-line-filter` option is special in this 
regard, since it was added for a rather specific use case - clang-tidy-diff.py. 
It's impractical to expect a user to specify a long blacklist on the command 
line, and it seems hacky to introduce a separate wrapper script with an 
explicit goal to pass this argument to clang-tidy.
2. There are already three mechanisms to filter warnings, introducing another 
one is going to add more complexity both in the code and in the usage of the 
tool. We should carefully consider all alternatives before doing so. In 
particular, if there are important use cases w.r.t. filtering of results 
clang-tidy doesn't handle well (but should), we could either improve existing 
suppression mechanisms or delegate warning suppression to an external tool like 
Code Checker (https://github.com/Ericsson/codechecker).

Going back to the use cases you have:

1. Header files need a different set of checks:

  library_a/a.cc:
  #include "library_b/b.h"
  // <some code that should be clean w.r.t. check1, but may have warnings from 
check2>
    
  library_b/b.h:
  // <some code that is not clean w.r.t. check1, but needs to be clean from 
check2 warnings>

This seems like a natural use case for the `.clang-tidy` configuration file: 
one should be able to turn on `check1` in `library_a/.clang_tidy` and `check2` 
in `library_b/.clang_tidy` and expect to only get corresponding warnings in 
each of these files. The problem is that it's not implemented that way. 
Currently the configuration for the main file will be used for all diagnostics 
generated while analysing the whole translation unit. The checks filter in the 
configuration file is used for two separate purposes: creating the list of 
checks to run on each translation unit and filtering diagnostics. We can't use 
different list of checks for analyzing each header in a single translation 
unit, but it seems possible to use proper configuration for each header when 
filtering diagnostics. This way in the example above we would run `check1` for 
the whole TU, but filter out all `check1` warnings from `library_b/` files. It 
might even be possible to further improve that to create a union of sets of 
checks from all headers used by the TU and then filter out irrelevant ones, but 
it would be a different performance / precision trade-off and it doesn't seem 
to be necessary at first.

2. Header files should be clean w.r.t. a certain check, but there is a small 
number of false positives that need to be suppressed without modifying the 
source code. This seems like a use case of an external suppression list that is 
best implemented by a stateful tool like Code Checker that stores a database of 
findings and a suppression list. Alternatively, we could implement a way to 
read a suppression list from a file (command line doesn't seem to be a good 
medium for such information) that would ideally be stored and versioned with 
the code. I believe, it's the same kind of a last resort solution as `// 
NOLINT` suppression comments. It's better when each check provides a sane 
check-specific way to suppress the diagnostic (e.g. like Clang's 
`-Wparentheses` warning - https://godbolt.org/g/nVl7we).

3. Warnings in headers that are caused by notes in a different library (what 
you discuss in https://reviews.llvm.org/D26418#590417). I thought quite a bit 
about notes in user code unsuppressing warnings in third-party library code 
back in the time when implementing the corresponding clang-tidy feature, but it 
turned out to be rather infrequent situation in my setup, so I don't have a 
strong opinion on how this should be handled. Do you have more motivating 
examples showing when one would want to disable this feature? (Or am I missing 
some examples already mentioned in this thread?)


https://reviews.llvm.org/D26418



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to