zaks.anna added a comment.

> I tried to come up with some advice on where checks should go in 
> http://clang.llvm.org/extra/clang-tidy/#choosing-the-right-place-for-your-check

The guidelines stated on the clang-tidy page seem reasonable to me.

> For example, IMO, AST-based analyses make more sense in clang-tidy for a few 
> reasons:
> 
> They usually are easier expressible in terms of AST matchers, and clang-tidy 
> allows to use AST matchers with less boilerplate.
>  Clang Static Analyzer is linked into clang, where AST matchers are 
> undesired, since they tend to blow the size of binary significantly.
>  It's more consistent to keep all similar analyses together, it simplifies 
> search for already implemented similar functionality and code reviews.

I agree that there is a gray area specifically in the AST-matching-based bug 
finding, which unfortunately leads to duplication of effort. However, we 
currently cannot ban/move all AST-based checks out of the analyzer. The main 
problem I see with restricting the AST-based analysis to clang-tidy is impact 
on the end users. While clang-tidy does export the clang static analyzer 
results, the reverse does not hold. There are many end users who do not use 
clang-tidy but do use the clang static analyzer (either directly or through 
scan-build). Until that is the case, I do not think we should disallow AST 
based checks from the analyzer, especially if they come from contributors who 
are interested in contributing to this tool. Note that the format in which the 
results are presented from these tools is not uniform, which makes transition 
more complicated.

The AST matchers can be used from the analyzer and if the code size becomes a 
problem, we could consider moving the analyzer out of the clang codebase.

Generally, I am not a big fan of the split between the checks based on the 
technology used to implement them since it does not lead to a great user 
interface - the end users do not think in terms of 
path-sensitive/flow-sensitive/AST-matching, they just want to enable specific 
functionality such as find bugs or ensure they follow coding guidelines. This 
is why I like the existing guidelines with their focus on what clang-tidy is 
from user perspective:

> clang-tidy check is a good choice for linter-style checks, checks that are 
> related to a certain coding style, checks that address code readability, etc.

Another thing that could be worth adding here is that clang-tidy finds bugs 
that tend to be easy to fix and, in most cases, provide the fixits. (I believe, 
this is one of the major strengths of the clang-tidy tool!)

> Flow-sensitive analyses (that don't need any path-sensitivity) seem to be 
> equally suitable for SA and clang-tidy (unless I'm missing something), so I 
> don't feel strongly as to where they should go.

As far as I know, currently, all flow-sensitive analysis are in the Analysis 
library in clang. These are analysis for compiler warnings and analysis used by 
the static analyzer. I believe this is where future analysis should go as well, 
especially, for analysis that could have multiple clients. If clang code size 
impact turns out to be a serious problem, we could also have that library 
provide APIs that could be used from other tools/checks. (Note, the analysis 
could be implemented in the library, but the users of the analysis (checks) can 
be elsewhere.)

Regarding the points about "heuristics" and "flexibility", the analyzer is full 
of heuristics and fuzzy API matching. These techniques are very common in 
static analysis in general as we are trying to solve undecidable problems and 
heuristics is the only way to go.


Repository:
  rL LLVM

https://reviews.llvm.org/D29151



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

Reply via email to