alexfh added a comment. In http://reviews.llvm.org/D12761#304823, @Alexander_Droste wrote:
> Ah ok, I wasn't aware that clang-tidy is not restricted to checks which > verify stylistic issues. > What makes it more convenient to integrate the checks in clang-tidy? Is it > how the AST-Matcher > functionality can be accessed? Yes, less boilerplate code needed to work with AST matchers, powerful clang diagnostic infrastructure is used to report errors, also the add_new_check.py script ;) > > I'm not an expert in the static analyzer code, so I'm not the one to review > > even AST-based checks there. > > > The AST-based checks basically use zero functionality from the static > analyzer codebase > except the entry callback and the functionality to generate bug reports. Well, that's one of the ways to say that I have little experience and interest in reviewing patches to the static analyzer ;) > > There's nothing that prevents adding more error-detecting checks to > > clang-tidy. > > > Wrt. to the checks being part of this patch I have some concerns: > > The first problem I see is that some functionlity is shared between the > AST-based > and path-sensitive checks. But I think this should be restricted to the > `MPIFunctionClassifier` class. If this class stays in the static analyzer, the clang-tidy check code could probably just depend on the relevant library and use this class? > Another question I have is how this would affect the usability. At the moment > all detected bugs (AST-based and path-sensitive) appear in a single report, > generated by `scan-build`. Is there still a way to get the results in a > single report > if the checks get moved to `clang-tidy`? clang-tidy can run static analyzer checks. However, it only supports the plain text output format, which is sub-optimal for representing the results of the path-based checks. So the single report scenario is not easy to implement right now. Gábor Horváth proposed a solution for this a few months ago (PList output for clang-tidy), but a proper implementation of this seemed to require some changes in the clang's diagnostic subsystem. > Would the implementation need to change to AST-Matcher based? That's not a 100% necessary step (you can always add a matcher for translationUnitDecl() and then traverse the AST using a RecursiveASTVisitor, for example), but this usually results in a more compact and easy to understand code. > The point about the current implementation is that the AST-based checks > are also already backed by a range of tests defined in > `tools/clang/test/Analysis/MPIChecker.cpp`. Clang-tidy checks should also be backed by tests ;) http://reviews.llvm.org/D12761 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
