[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

Before clang-tidy came into existence the guidelines were very clear. One should write a clang warning if the diagnostic:

- can be implemented without path-insensitive analysis (including flow-sensitive)
- is checking for language rules (not specific API misuse)

In my view, this should still be the rule going forward because compiler warnings are most effective in reaching users.

The Clang Static Analyzer used to be the place for all other diagnostics. Most of the checkers it contains rely on path-sensitive analysis. Note that one might catch the same bug with flow-sensitive as well as path-sensitive analysis. So in some of those cases, we have both warnings as well as analyzer checkers finding the same type of user error. However, the checkers can catch more bugs since they are path-sensitive. The analyzer also has several flow-sensitive/ AST matching checkers. Those checks could not have been written as warnings mainly because they check for APIs, which are not part of the language.

My understanding is that clang-tidy supports fixits, which the clang static analyzer currently does not support. However, there could be other benefits to placing not path-sensitive checks there as well. I am not sure. I've also heard opinions that it's more of a linter tool, so the user expectations could be different.

> Even right now there are clang-tidy checks that finds subset of alpha checks, but probably having lower false positive rate.

Again, alpha checks are unfinished work, so we should not use them as examples of checkers that have false positives. Some of them are research projects and should probably be deleted.


Repository:
  rL LLVM

https://reviews.llvm.org/D29151